- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Hi,
MSI shared for DMA Read and Write. In ISR() routine, checks status to determine read or write completion. Either there is missing MSI or Read completion MSI can be mistaking as Write completion, and vice versa. If the same routine, runs ok for read only dma (loop 10000 times) and for write only Dma(10000 times). But sometimes it fails to do simultaneously R/W. How to resolve this issue? Thanks, TigerLink Copied
6 Replies
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
My crystal ball says there is an issue with line 42 of your code.... That's about as helpful as I can be with the information you have provided.
Is this Windows? Linux? Something Else? What is in your interrupt routine?- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Host: Windows 7 KDMF device driver
FPGA: Cyclone V GT PCIe hard IP with DMA interface The ISR(): BOOLEAN PCIeEvtInterruptIsr(IN WDFINTERRUPT Interrupt, IN ULONG MessageID) { PFDO_DATA FdoData = NULL; ULONG ulStatusR = 1; ULONG ulStatusW = 1; for (i = 0; i < bNumOfWriteDesc; i++) {//check All Write Status ulStatusW &= pWriteReg->ulStatus;}
if (ulstatusw != 0) { //write complete
fdodata->ulirqcntw++;
if (fdodata->m_ulloopsw > 0 && fdodata->m_ulloopsw < 0x7fffffff) {
fdodata->m_ulloopsw--;
bdowrite = true; //more to go
}
else {//done
rtlzeromemory(pwritereg, 0x200);//clears all write status
fdodata->ulwritedone = 1;
}
}
else {
for (i = 0; i < bnumofreaddesc; i++) {//check all read status
ulstatusr &= preadreg->ulstatus; } if (ulStatusR != 0) { //Read Complete FdoData->ulIRQCntR++; if (FdoData->m_ulLoopsR > 0 && FdoData->m_ulLoopsR < 0x7FFFFFFF) { FdoData->m_ulLoopsR--; bDoRead = TRUE;//more to go } else {//Done RtlZeroMemory(pReadReg, 0x200);//Clears all Read Status FdoData->ulReadDone = 1; } } else { FdoData->ulErrors++; WdfInterruptQueueDpcForIsr(Interrupt); return FALSE; } } if (bDoWrite) { RtlZeroMemory((PUCHAR)pStatusRegW, 0x200);//Clears all Write Status LastPtr = bNumOfReadDesc; } if (bDoRead) { RtlZeroMemory((PUCHAR)pStatusRegR, 0x200);//Clears all Read Status LastPtr = bNumOfWriteDesc; } if (FdoData->ulReadDone == 1 || FdoData->ulWriteDone == 1) { WdfInterruptQueueDpcForIsr(Interrupt); //processing } return TRUE; } Thanks,
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Stripping the above down, you have this:
PFDO_DATA FdoData = NULL;
if (ulStatusW != 0) { //Write Complete
...
}
else {
if (ulStatusR != 0) { //Read Complete
...
else {
...
WdfInterruptQueueDpcForIsr(Interrupt);
return FALSE;
}
}
return TRUE;
There are a few issues here. First of all, if two interrupts occur very near to each other time wise (like may happen if you are running read and write at the same time), Windows may queue the occurrences into a single ISR call. What this can mean is that your code could miss the fact that the 'read' is complete as you are checking 'write' first and only checking 'read' if it wasn't 'write'. What you need to do is check *both* in each interrupt call. Secondly, you pointer FdoData which is initialised to point to NULL. Then in later lines you start writing to this without updating it to point to some place - this is VERY VERY VERY bad, and I'm surprised it didn't cause a blue screen. You are basically trying to read/write values from uninitialized memory. You are also writing to a variable LastPtr which doesn't appear to exist? In which case the only conclusion I can draw is that this is declared as a global variable, which should *not* be done in a KMDF driver. You should be storing everything in the device extension. Now while a pointer to your device extension isn't passed as a parameter in the ISR, you can get it by other means, you simply add these two lines to the top of your ISR:
PDEVICE_EXTENSION devExt;
devExt = DriverGetDeviceContext(WdfInterruptGetDevice(Interrupt));
//Note: DriverGetDeviceContext should be replaced by whatever you put in your:
//WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(DEVICE_EXTENSION, DriverGetDeviceContext)
The third issue with your code is more a misunderstanding of KMDF. If it wasn't read or write, you call WdfInterruptQueueDpcForIsr(), and then return FALSE. Returning FALSE instructs the KDMF framework that the interrupt is either not recognised, or not handled by the ISR - in which case you should *not* be handling it in a DPC - because you don't recognise it. Basically your ISR should be as succinct as possible, something along these lines:
BOOLEAN PCIeEvtInterruptIsr(IN WDFINTERRUPT Interrupt, IN ULONG MessageID)
{
PDEVICE_EXTENSION devExt; //DO NOT USE GLOBAL VARIABLES, put anything you need to be global in your DEVICE_EXTENSION...
devExt = DriverGetDeviceContext(WdfInterruptGetDevice(Interrupt));
//Note: DriverGetDeviceContext should be replaced by whatever you put in your:
//WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(DEVICE_EXTENSION, DriverGetDeviceContext)
//(1) Check if this is a message ID you are expecting. If you only have one IRQ source (one MSI interrupt), this will *always* be 0
if (MessageID != 0) {
return FALSE; //Unknown source. This ISR will not handle it so let KMDF deal with the problem...
}
//(2) Do the *bare minimum* required to check the Write status flags
BOOL writeDone = ...;
...;
//(3) Do the *bear minimum* required to check the Read status flags
BOOL readDone = ...;
...;
//(4) Queue DPC to do the rest.
if (writeDone || readDone) {
WdfInterruptQueueDpcForIsr(Interrupt); //processing
}
return TRUE; //We recognised the message ID and did everything we needed to handle it - even if that didn't involve queuing a DPC!
}
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Thank TCWorld,
First, this is a edited version of ISR() for easy to read. So I removed some unnecessary lines of code. FDO obj is obtained on the first line of the routine: FdoData = FdoGetData(WdfInterruptGetDevice(Interrupt)); sorry for misleading. Let me modify as you instructed. let you know the results. Thank you- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
Bingo. Thank you, TCWORLD!
- Mark as New
- Bookmark
- Subscribe
- Mute
- Subscribe to RSS Feed
- Permalink
- Report Inappropriate Content
By the way, TCWORLD - PICe guru, please let me ask another question:
In the PCIe Hard IP, (Cyclone V - with DMA interface), there are max 16 lines for MSI. How these 16 lines to be associated with ISRs? In other words, how do I know which MSI line is sending the MSI? Or how to specify a line for particular function? Thank you,
Reply
Topic Options
- Subscribe to RSS Feed
- Mark Topic as New
- Mark Topic as Read
- Float this Topic for Current User
- Bookmark
- Subscribe
- Printer Friendly Page