FPGA Intellectual Property
PCI Express*, Networking and Connectivity, Memory Interfaces, DSP IP, and Video IP
6335 Discussions

PCIe MSI interrupt sharing problem

Altera_Forum
Honored Contributor II
1,263 Views

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, 

 

Tiger
0 Kudos
6 Replies
Altera_Forum
Honored Contributor II
499 Views

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?
0 Kudos
Altera_Forum
Honored Contributor II
499 Views

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,
0 Kudos
Altera_Forum
Honored Contributor II
499 Views

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! }
0 Kudos
Altera_Forum
Honored Contributor II
500 Views

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
0 Kudos
Altera_Forum
Honored Contributor II
500 Views

Bingo. Thank you, TCWORLD!

0 Kudos
Altera_Forum
Honored Contributor II
500 Views

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,
0 Kudos
Reply