Does NDIS spinlock serves as a memory barrier for DMA?

user_17348498 0
Viewed 64

In an NDIS driver I need to write some data to a shared memory and then notify the HW to fetch this data. Writing to the shared memory is protected by an NDIS spinlock. There is a possible race between writing to the shared memory and notifying the HW that the data was written.

Does the spinlock serve as an implicit memory barrier to prevent this race? Or should a memory barrier be explicitly added?

NdisAcquireSpinLock();
writeDataToSharedMem();
NdisReleaseSpinLock();

// MemoryBarrier(); // Is an explicit memory barrier needed?
NdisWriteRegisterUlong(); // Notify the HW that data was written
ndis spinlock barrier dma memory
1 Answers

NDIS spinlocks provide a full memory barrier for synchronization between processors. This barrier is indeed the equivalent to the MemoryBarrier() routine that your code alludes to. The code example you gave would be adequate synchronization if you were merely multithreading.

(In fact, NdisWriteRegisterUlong() also provides a memory barrier, so there is a redundant memory barrier in your code example. If there's no other code between releasing the spinlock and the register access, you could eliminate one of the memory barriers by switching to WRITE_REGISTER_NOFENCE_ULONG. But in just a moment, we'll add some code there, so that barrier will become meaningful.)

However, since you are sharing data between a processor and a device, in theory you also need a primitive that synchronizes device access to memory. KeFlushIoBuffers() is just that primitive. Note that some of the DMA routines (BuildScatterGatherList etc) will internally flush buffers, so you don't always have to invoke KeFlushIoBuffers() directly. But if you're just writing commands into a ring buffer without calling any DMA routine after each write, then DMA can't possibly know when to flush the IO buffer on your behalf. In that case, you must flush it yourself.

It might help to indulge in a thought experiment: if you were to (somehow) find a single-processor system, MemoryBarrier() could be implemented as a no-op. But you would still need something else to synchronize between the processor and a PCIe-connected network adapter. KeFlushIoBuffers() is that something else.

KeFlushIoBuffers() is a no-op when the PCIe bus is cache coherent, e.g., as it is architecturally required to be on most x86 and x64 platforms. So it's actually pretty easy to forget to include an IO flush, if you only ever test your code on x64. This is why you might not see other drivers using this routine as much as they theoretically ought to.

As a sidenote, I recommend using NT synchronization primitives (KeAcquireSpinLock etc) instead of the NDIS ones. The NDIS locks were designed as an OS abstraction layer so the same source code could be recompiled for Windows 9x, Windows CE, OS/2, and Windows NT. These days, most developers only need to worry about NT and can safely use the NT routines directly. The NDIS routines don't add value, and your code will be understood by more people if you just use the NT routines.

Your final code might look like:

KeAcquireSpinLock(&command_buffer_lock);
write_to_command_buffer();
KeReleaseSpinLock(&command_buffer_lock);

KeFlushIoBuffers(command_buffer_mdl, FALSE, TRUE);

WRITE_REGISTER_ULONG(&bar->command_buffer_doorbell);

If there are never other processors racing to access the shared memory, you don't need the spinlock at all! For example, if you only ever touch the shared memory from a dedicated thread, then you could update the code to simply be:

write_to_command_buffer();

KeFlushIoBuffers(command_buffer_mdl, FALSE, TRUE);

WRITE_REGISTER_ULONG(&bar->command_buffer_doorbell);
user_7941280
Related