ISR and I/O synchronization on multiple-processor....

Hello,

I have a problem with my driver. My problem only occurs when running on a multi-core processor. I have two process accessing my driver at the same time. When one process triggers an interrupt, the ISR routine modifies the PCI I/O adress 0x8, used to decide witch module to reroute to.

I implemented a spin lock in the IOControl function to prevent calling of two I/O at same time. This seems to prevent the double I/O access.

I implemented another spin lock in the ISR and in the Read/Write functions I use to prevent this adress to being changed but I end up having a deadlock and system crashing after a while.

My main problem here is that I want to prevent the ISR running on one processor when a Read or Write is being done on another processor. Basicly a system to say: *Stop all instructions on the processors and wait till I finish with the ISR before doing anything else*

Any suggestion?

bool OnIsr(PKINTERRUPT Interrupt, PVOID ServContext)
{
ISR_DATA * ptr = (ISR_DATA *)ServContext;
PUCHAR RealOffset = (PUCHAR) (ptr->pBaseAddress);
UCHAR ucSource = 0;
bool ReturnValue;
KIRQL Irql;

KeAcquireSpinLock(ptr->pLock,&Irql);

if( RealInterrupt)
{
// Do READ_PORT here… and WRITE_PORT here…
KeInsertQueueDpc((PRKDPC)ptr->pDBC,ptr,NULL);
ReturnValue = true;
}
else
{
ReturnValue = false;
}

KeReleaseSpinLock(ptr->pLock,Irql);
return ReturnValue;
}

Here is an example of my function that is called WriteByte:

void CIoRange::AcquireIsrSafe()
{
// Raise IRQ to Interrupt IRQL so I don’t have deadlock when ISR pops
KeRaiseIrql(m_IntIrql,&m_OldIrql);

KeAcquireSpinLock(&m_InISRLock,&m_OldIrql);
}
void CIoRange::ReleaseIsrSafe()
{
// Let Lock go
KeReleaseSpinLock(&m_InISRLock,m_OldIrql);

// Lower IRQL
KeLowerIrql(m_OldIrql);
}

void CIoRange::WriteByte(ULONG Offset, UCHAR Value)
{
if(Offset < m_Size && m_BaseAddress != NULL)
{
// Prevent ISR to change something on PCI card when I will do my Write.
AcquireIsrSafe();
PUCHAR RealOffset;
if(m_Type == MEMORY)
{
RealOffset = (PUCHAR) ((PUCHAR) (m_BaseAddress) + Offset);
WRITE_REGISTER_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,*RealOffset));
}
else
{
RealOffset = (PUCHAR) ((PUCHAR) (m_PhysicalBaseAddress.LowPart) + Offset);
WRITE_PORT_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,READ_PORT_UCHAR(RealOffset)));
}
// Critiical Section Over…
ReleaseIsrSafe();
}
}

=====================
In case you are wondering what my Connect Interrupt looks like:

KeInitializeDpc( m_pDPC,(PKDEFERRED_ROUTINE)OnDpc, this);

IsrData = new(NonPagedPool)ISR_DATA;
IsrData->pBaseAddress = (PUCHAR)m_Io.GetPhysicalBaseAddress().LowPart;
IsrData->pDBC= m_pDPC;
IsrData->pLock = m_Io.GetLock(); // Returns IoRange::m_InISRLock address

status = IoConnectInterrupt(&m_pInterrupt,
(PKSERVICE_ROUTINE)OnIsr,
(PVOID)IsrData,
NULL,
pPartialDescriptor->u.Interrupt.Vector,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(pPartialDescriptor->Flags == CM_RESOURCE_INTERRUPT_LATCHED) ? Latched : LevelSensitive,
(pPartialDescriptor->ShareDisposition == CmResourceShareShared),
pPartialDescriptor->u.Interrupt.Affinity,
FALSE);


Express yourself with free Messenger emoticons. Get them today!
http://www.freemessengeremoticons.ca/?icid=EMENCA122

You have a number of problems in the code you supplied. You should not try
to acquire a spinlock in an ISR, this does not work. The ISR has a spinlock
protecting it and you should use it via KeSynchronizeExecution or
KeAcquireInterruptSpinLock calls in the other code that uses the same
resource.

Raising things to the interrupt level is worthless, unless you do it via the
above mechanisms.


Don Burn (MVP, Windows DDK)
Windows 2k/XP/2k3 Filesystem and Driver Consulting
Website: http://www.windrvr.com
Blog: http://msmvps.com/blogs/WinDrvr
Remove StopSpam to reply

“Frederick Guillemot” wrote in message
news:xxxxx@ntdev…

Hello,

I have a problem with my driver. My problem only occurs when running on a
multi-core processor. I have two process accessing my driver at the same
time. When one process triggers an interrupt, the ISR routine modifies the
PCI I/O adress 0x8, used to decide witch module to reroute to.

I implemented a spin lock in the IOControl function to prevent calling of
two I/O at same time. This seems to prevent the double I/O access.

I implemented another spin lock in the ISR and in the Read/Write functions I
use to prevent this adress to being changed but I end up having a deadlock
and system crashing after a while.

My main problem here is that I want to prevent the ISR running on one
processor when a Read or Write is being done on another processor. Basicly a
system to say: Stop all instructions on the processors and wait till I
finish with the ISR before doing anything else


Any suggestion?
=====================

bool OnIsr(PKINTERRUPT Interrupt, PVOID ServContext)
{
ISR_DATA * ptr = (ISR_DATA *)ServContext;
PUCHAR RealOffset = (PUCHAR) (ptr->pBaseAddress);
UCHAR ucSource = 0;
bool ReturnValue;
KIRQL Irql;

KeAcquireSpinLock(ptr->pLock,&Irql);

if( RealInterrupt)
{
// Do READ_PORT here… and WRITE_PORT here…
KeInsertQueueDpc((PRKDPC)ptr->pDBC,ptr,NULL);
ReturnValue = true;
}
else
{
ReturnValue = false;
}

KeReleaseSpinLock(ptr->pLock,Irql);
return ReturnValue;
}
=====================
Here is an example of my function that is called WriteByte:
=====================

void CIoRange::AcquireIsrSafe()
{
// Raise IRQ to Interrupt IRQL so I don’t have deadlock when ISR pops
KeRaiseIrql(m_IntIrql,&m_OldIrql);

KeAcquireSpinLock(&m_InISRLock,&m_OldIrql);
}
void CIoRange::ReleaseIsrSafe()
{
// Let Lock go
KeReleaseSpinLock(&m_InISRLock,m_OldIrql);

// Lower IRQL
KeLowerIrql(m_OldIrql);
}

void CIoRange::WriteByte(ULONG Offset, UCHAR Value)
{
if(Offset < m_Size && m_BaseAddress != NULL)
{
// Prevent ISR to change something on PCI card when I will do my Write.
AcquireIsrSafe();
PUCHAR RealOffset;
if(m_Type == MEMORY)
{
RealOffset = (PUCHAR) ((PUCHAR) (m_BaseAddress) + Offset);
WRITE_REGISTER_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,*RealOffset));
}
else
{
RealOffset = (PUCHAR) ((PUCHAR) (m_PhysicalBaseAddress.LowPart) + Offset);
WRITE_PORT_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X
(0x%X)”,RealOffset,Value,READ_PORT_UCHAR(RealOffset)));
}
// Critiical Section Over…
ReleaseIsrSafe();
}
}

=====================
In case you are wondering what my Connect Interrupt looks like:
=====================

KeInitializeDpc( m_pDPC,(PKDEFERRED_ROUTINE)OnDpc, this);

IsrData = new(NonPagedPool)ISR_DATA;
IsrData->pBaseAddress = (PUCHAR)m_Io.GetPhysicalBaseAddress().LowPart;
IsrData->pDBC= m_pDPC;
IsrData->pLock = m_Io.GetLock(); // Returns IoRange::m_InISRLock address

status = IoConnectInterrupt(&m_pInterrupt,
(PKSERVICE_ROUTINE)OnIsr,
(PVOID)IsrData,
NULL,
pPartialDescriptor->u.Interrupt.Vector,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(pPartialDescriptor->Flags == CM_RESOURCE_INTERRUPT_LATCHED) ? Latched :
LevelSensitive,
(pPartialDescriptor->ShareDisposition == CmResourceShareShared),
pPartialDescriptor->u.Interrupt.Affinity,
FALSE);
_________________________________________________________________
Express yourself with free Messenger emoticons. Get them today!
http://www.freemessengeremoticons.ca/?icid=EMENCA122

You should not be using a spinlock in this fashion. Once a KSPIN_LOCK is associated with a PKINTERRUPT object you must use the interrupt routines (KeXxx) to acquire/release it. So for what you want to do in the IOCTL handler, call KeAcquire/ReleaseInterruptSpinLock(m_pInterrupt), which will synchronize access to the register done by the ISR. This works on XP and later. If you are targeting win2k you need to use KeSynchronizeExecution and implement a callback that will run synchronized against the ISR. The CIoRange::AcquireIsrSafe is flawed since KeAcquireSpinLock changes the IRQL to dispatch_level which could easily be below m_IntIrql (there is no guarantee that the function does an IRQL check to see if current irql is > DISPATCH_LEVEL before changing the IRQL).

d

From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of Frederick Guillemot
Sent: Monday, November 26, 2007 9:11 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] ISR and I/O synchronization on multiple-processor…

Hello,

I have a problem with my driver. My problem only occurs when running on a multi-core processor. I have two process accessing my driver at the same time. When one process triggers an interrupt, the ISR routine modifies the PCI I/O adress 0x8, used to decide witch module to reroute to.

I implemented a spin lock in the IOControl function to prevent calling of two I/O at same time. This seems to prevent the double I/O access.

I implemented another spin lock in the ISR and in the Read/Write functions I use to prevent this adress to being changed but I end up having a deadlock and system crashing after a while.

My main problem here is that I want to prevent the ISR running on one processor when a Read or Write is being done on another processor. Basicly a system to say: *Stop all instructions on the processors and wait till I finish with the ISR before doing anything else*

Any suggestion?

=====================
bool OnIsr(PKINTERRUPT Interrupt, PVOID ServContext)
{
ISR_DATA * ptr = (ISR_DATA *)ServContext;
PUCHAR RealOffset = (PUCHAR) (ptr->pBaseAddress);
UCHAR ucSource = 0;
bool ReturnValue;
KIRQL Irql;

KeAcquireSpinLock(ptr->pLock,&Irql);

if( RealInterrupt)
{
// Do READ_PORT here… and WRITE_PORT here…
KeInsertQueueDpc((PRKDPC)ptr->pDBC,ptr,NULL);
ReturnValue = true;
}
else
{
ReturnValue = false;
}

KeReleaseSpinLock(ptr->pLock,Irql);
return ReturnValue;
}

Here is an example of my function that is called WriteByte:

void CIoRange::AcquireIsrSafe()
{
// Raise IRQ to Interrupt IRQL so I don’t have deadlock when ISR pops
KeRaiseIrql(m_IntIrql,&m_OldIrql);

KeAcquireSpinLock(&m_InISRLock,&m_OldIrql);
}
void CIoRange::ReleaseIsrSafe()
{
// Let Lock go
KeReleaseSpinLock(&m_InISRLock,m_OldIrql);

// Lower IRQL
KeLowerIrql(m_OldIrql);
}

void CIoRange::WriteByte(ULONG Offset, UCHAR Value)
{
if(Offset < m_Size && m_BaseAddress != NULL)
{
// Prevent ISR to change something on PCI card when I will do my Write.
AcquireIsrSafe();
PUCHAR RealOffset;
if(m_Type == MEMORY)
{
RealOffset = (PUCHAR) ((PUCHAR) (m_BaseAddress) + Offset);
WRITE_REGISTER_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,*RealOffset));
}
else
{
RealOffset = (PUCHAR) ((PUCHAR) (m_PhysicalBaseAddress.LowPart) + Offset);
WRITE_PORT_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,READ_PORT_UCHAR(RealOffset)));
}
// Critiical Section Over…
ReleaseIsrSafe();
}
}

=====================
In case you are wondering what my Connect Interrupt looks like:

KeInitializeDpc( m_pDPC,(PKDEFERRED_ROUTINE)OnDpc, this);

IsrData = new(NonPagedPool)ISR_DATA;
IsrData->pBaseAddress = (PUCHAR)m_Io.GetPhysicalBaseAddress().LowPart;
IsrData->pDBC= m_pDPC;
IsrData->pLock = m_Io.GetLock(); // Returns IoRange::m_InISRLock address

status = IoConnectInterrupt(&m_pInterrupt,
(PKSERVICE_ROUTINE)OnIsr,
(PVOID)IsrData,
NULL,
pPartialDescriptor->u.Interrupt.Vector,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(pPartialDescriptor->Flags == CM_RESOURCE_INTERRUPT_LATCHED) ? Latched : LevelSensitive,
(pPartialDescriptor->ShareDisposition == CmResourceShareShared),
pPartialDescriptor->u.Interrupt.Affinity,
FALSE);


Express yourself with free Messenger emoticons. Get them today!http:

NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer</http:>

Sorry, but your code is just incredible piece of nonsense…

Look - the very first thing KeAcquireSpinlock() does is setting IRQL to DPC level. You raise IRQL to DIRQL, and then immediately call KeAcquireSpinlock(). What is the IRQL after KeAcquireSpinlock() returns control(actually, this is the reason why you cannot call KeAcquireSpinlock() in ISR)??? Therefore, you code is just bound to fail.

Your code *could* work if you replaced KeAcquireSpinlock() with KeAcquireSpinlockAtDpcLevel(), because the latter does not access CPU’s Task Priority Register, so that IRQL stays intact when you call KeAcquireSpinlockAtDpcLevel(). However, unless you made your ISR service different KINTERRUPT objects, in practical terms it does not make sense to do it - as Don and Doron explained to you already, ISR is protected by a spinlock in its corresponding KINTERRUPT object, so that more than one instance of ISR cannot run at any particular moment. If you want to synchronize non-ISR code with ISR, you can do it with KeSynchronizeExecition() or via interrupt spinlock (only on XP and above)…

Anton Bassov

Do not use the usual spinlocks in ISRs.

Instead: if the ISR touches some register, then the other code touching
it should use KeSynchronizeExecution and KeAcquireInterruptSpinLock. No need in
any explicit spinlocks in the ISR.

KeSynchronizeExecution is:

KeAcquireInterruptSpinLock
(Routine)(Address);
KeReleaseInterruptSpinLock

  • and was here since NT 3.1.

KeAcquireInterruptSpinLock is exported from the kernel since XP.


Maxim Shatskih, Windows DDK MVP
StorageCraft Corporation
xxxxx@storagecraft.com
http://www.storagecraft.com

“Frederick Guillemot” wrote in message
news:xxxxx@ntdev…

Hello,

I have a problem with my driver. My problem only occurs when running on a
multi-core processor. I have two process accessing my driver at the same time.
When one process triggers an interrupt, the ISR routine modifies the PCI I/O
adress 0x8, used to decide witch module to reroute to.

I implemented a spin lock in the IOControl function to prevent calling of two
I/O at same time. This seems to prevent the double I/O access.

I implemented another spin lock in the ISR and in the Read/Write functions I
use to prevent this adress to being changed but I end up having a deadlock and
system crashing after a while.

My main problem here is that I want to prevent the ISR running on one processor
when a Read or Write is being done on another processor. Basicly a system to
say: Stop all instructions on the processors and wait till I finish with the
ISR before doing anything else


Any suggestion?
=====================

bool OnIsr(PKINTERRUPT Interrupt, PVOID ServContext)
{
ISR_DATA * ptr = (ISR_DATA *)ServContext;
PUCHAR RealOffset = (PUCHAR) (ptr->pBaseAddress);
UCHAR ucSource = 0;
bool ReturnValue;
KIRQL Irql;

KeAcquireSpinLock(ptr->pLock,&Irql);

if( RealInterrupt)
{
// Do READ_PORT here… and WRITE_PORT here…
KeInsertQueueDpc((PRKDPC)ptr->pDBC,ptr,NULL);
ReturnValue = true;
}
else
{
ReturnValue = false;
}

KeReleaseSpinLock(ptr->pLock,Irql);
return ReturnValue;
}
=====================
Here is an example of my function that is called WriteByte:
=====================

void CIoRange::AcquireIsrSafe()
{
// Raise IRQ to Interrupt IRQL so I don’t have deadlock when ISR pops
KeRaiseIrql(m_IntIrql,&m_OldIrql);

KeAcquireSpinLock(&m_InISRLock,&m_OldIrql);
}
void CIoRange::ReleaseIsrSafe()
{
// Let Lock go
KeReleaseSpinLock(&m_InISRLock,m_OldIrql);

// Lower IRQL
KeLowerIrql(m_OldIrql);
}

void CIoRange::WriteByte(ULONG Offset, UCHAR Value)
{
if(Offset < m_Size && m_BaseAddress != NULL)
{
// Prevent ISR to change something on PCI card when I will do my Write.
AcquireIsrSafe();
PUCHAR RealOffset;
if(m_Type == MEMORY)
{
RealOffset = (PUCHAR) ((PUCHAR) (m_BaseAddress) + Offset);
WRITE_REGISTER_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X (0x%X)”,RealOffset,Value,*RealOffset));
}
else
{
RealOffset = (PUCHAR) ((PUCHAR) (m_PhysicalBaseAddress.LowPart) + Offset);
WRITE_PORT_UCHAR(RealOffset,Value);
ERR0((“Write Byte @ 0x%X => 0x%X
(0x%X)”,RealOffset,Value,READ_PORT_UCHAR(RealOffset)));
}
// Critiical Section Over…
ReleaseIsrSafe();
}
}

=====================
In case you are wondering what my Connect Interrupt looks like:
=====================

KeInitializeDpc( m_pDPC,(PKDEFERRED_ROUTINE)OnDpc, this);

IsrData = new(NonPagedPool)ISR_DATA;
IsrData->pBaseAddress = (PUCHAR)m_Io.GetPhysicalBaseAddress().LowPart;
IsrData->pDBC= m_pDPC;
IsrData->pLock = m_Io.GetLock(); // Returns IoRange::m_InISRLock address

status = IoConnectInterrupt(&m_pInterrupt,
(PKSERVICE_ROUTINE)OnIsr,
(PVOID)IsrData,
NULL,
pPartialDescriptor->u.Interrupt.Vector,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(KIRQL)pPartialDescriptor->u.Interrupt.Level,
(pPartialDescriptor->Flags == CM_RESOURCE_INTERRUPT_LATCHED) ? Latched :
LevelSensitive,
(pPartialDescriptor->ShareDisposition == CmResourceShareShared),
pPartialDescriptor->u.Interrupt.Affinity,
FALSE);
_________________________________________________________________
Express yourself with free Messenger emoticons. Get them today!
http://www.freemessengeremoticons.ca/?icid=EMENCA122