Forum Discussion
Altera_Forum
Honored Contributor
11 years agoPCIe MSI interrupt sharing problem
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, Tiger6 Replies
- Altera_Forum
Honored Contributor
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? - Altera_Forum
Honored Contributor
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,
- Altera_Forum
Honored Contributor
Stripping the above down, you have this:
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:PFDO_DATA FdoData = NULL; if (ulStatusW != 0) { //Write Complete ... } else { if (ulStatusR != 0) { //Read Complete ... else { ... WdfInterruptQueueDpcForIsr(Interrupt); return FALSE; } } return TRUE;
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: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)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! } - Altera_Forum
Honored Contributor
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 - Altera_Forum
Honored Contributor
Bingo. Thank you, TCWORLD!
- Altera_Forum
Honored Contributor
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,