[PATCH] ntoskrnl.exe: Implement Ke(Initialize|Insert|Remove)DeviceQueue.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- MSDN says that the cancel spinlock has to be held whenever these device queues are used, when setting cancel routines, but it's not clear to me why, as IRP cancellation has apparently no effect on these functions behavior (I initially thought they should check and not queue cancelled IRPs, but it looks like they don't). dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 6 +- dlls/ntoskrnl.exe/sync.c | 41 ++++++++++++ dlls/ntoskrnl.exe/tests/driver.c | 96 +++++++++++++++++++++++++++++ include/ddk/wdm.h | 4 ++ 4 files changed, 144 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 5fdaa922f45..c1244a42146 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -567,7 +567,7 @@ @ stub KeI386SetGdtSelector @ stub KeIcacheFlushCount @ stdcall KeInitializeApc(ptr ptr long ptr ptr ptr long ptr) -@ stub KeInitializeDeviceQueue +@ stdcall KeInitializeDeviceQueue(ptr) @ stdcall KeInitializeDpc(ptr ptr ptr) @ stdcall KeInitializeEvent(ptr long long) @ stub KeInitializeInterrupt @@ -579,7 +579,7 @@ @ stdcall KeInitializeTimer(ptr) @ stdcall KeInitializeTimerEx(ptr long) @ stub KeInsertByKeyDeviceQueue -@ stub KeInsertDeviceQueue +@ stdcall KeInsertDeviceQueue(ptr ptr) @ stub KeInsertHeadQueue @ stdcall KeInsertQueue(ptr ptr) @ stub KeInsertQueueApc @@ -617,7 +617,7 @@ @ stdcall KeReleaseSpinLockFromDpcLevel(ptr) @ stub KeRemoveByKeyDeviceQueue @ stub KeRemoveByKeyDeviceQueueIfBusy -@ stub KeRemoveDeviceQueue +@ stdcall KeRemoveDeviceQueue(ptr) @ stub KeRemoveEntryDeviceQueue @ stub KeRemoveQueue @ stub KeRemoveQueueDpc diff --git a/dlls/ntoskrnl.exe/sync.c b/dlls/ntoskrnl.exe/sync.c index 445c8d890ab..9063608be4a 100644 --- a/dlls/ntoskrnl.exe/sync.c +++ b/dlls/ntoskrnl.exe/sync.c @@ -1362,3 +1362,44 @@ BOOLEAN WINAPI KeSetTimer(KTIMER *timer, LARGE_INTEGER duetime, KDPC *dpc) return KeSetTimerEx(timer, duetime, 0, dpc); } + +void WINAPI KeInitializeDeviceQueue( KDEVICE_QUEUE *queue ) +{ + TRACE( "queue %p.\n", queue ); + + KeInitializeSpinLock( &queue->Lock ); + InitializeListHead( &queue->DeviceListHead ); + queue->Busy = FALSE; + queue->Type = IO_TYPE_DEVICE_QUEUE; + queue->Size = sizeof(*queue); +} + +BOOLEAN WINAPI KeInsertDeviceQueue( KDEVICE_QUEUE *queue, KDEVICE_QUEUE_ENTRY *entry ) +{ + KIRQL irql; + + TRACE( "queue %p, entry %p.\n", queue, entry ); + + KeAcquireSpinLock( &queue->Lock, &irql ); + if ((entry->Inserted = queue->Busy)) InsertTailList( &queue->DeviceListHead, &entry->DeviceListEntry ); + queue->Busy = TRUE; + KeReleaseSpinLock( &queue->Lock, irql ); + + return entry->Inserted; +} + +KDEVICE_QUEUE_ENTRY * WINAPI KeRemoveDeviceQueue( KDEVICE_QUEUE *queue ) +{ + LIST_ENTRY *entry = NULL; + KIRQL irql; + + TRACE( "queue %p.\n", queue ); + + KeAcquireSpinLock( &queue->Lock, &irql ); + if (IsListEmpty( &queue->DeviceListHead )) queue->Busy = FALSE; + else entry = RemoveHeadList( &queue->DeviceListHead ); + KeReleaseSpinLock( &queue->Lock, irql ); + + if (!entry) return NULL; + return CONTAINING_RECORD( entry, KDEVICE_QUEUE_ENTRY, DeviceListEntry ); +} diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index f1c72d8fdaf..3446dc8e529 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -141,6 +141,101 @@ static void test_irp_struct(IRP *irp, DEVICE_OBJECT *device) IoFreeIrp(irp); } +static int cancel_queue_cnt; + +static void WINAPI cancel_queued_irp(DEVICE_OBJECT *device, IRP *irp) +{ + IoReleaseCancelSpinLock(irp->CancelIrql); + ok(irp->Cancel == TRUE, "Cancel = %x\n", irp->Cancel); + ok(!irp->CancelRoutine, "CancelRoutine = %p\n", irp->CancelRoutine); + irp->IoStatus.Status = STATUS_CANCELLED; + irp->IoStatus.Information = 0; + cancel_queue_cnt++; +} + +static void test_queue(void) +{ + KDEVICE_QUEUE_ENTRY *entry; + KDEVICE_QUEUE queue; + BOOLEAN ret; + KIRQL irql; + IRP *irp; + + irp = IoAllocateIrp(1, FALSE); + + memset(&queue, 0xcd, sizeof(queue)); + KeInitializeDeviceQueue(&queue); + ok(!queue.Busy, "unexpected Busy state\n"); + ok(queue.Size == sizeof(queue), "unexpected Size %x\n", queue.Size); + ok(queue.Type == IO_TYPE_DEVICE_QUEUE, "unexpected Type %x\n", queue.Type); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(!entry, "expected KeRemoveDeviceQueue to return NULL\n"); + ok(!queue.Busy, "unexpected Busy state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry == NULL, "expected KeRemoveDeviceQueue to return NULL\n"); + ok(!queue.Busy, "unexpected Busy state\n"); + + IoCancelIrp(irp); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(queue.Busy, "expected Busy state\n"); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + + IoFreeIrp(irp); + + irp = IoAllocateIrp(1, FALSE); + + IoAcquireCancelSpinLock(&irql); + IoSetCancelRoutine(irp, cancel_queued_irp); + IoReleaseCancelSpinLock(irql); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + IoCancelIrp(irp); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + ok(cancel_queue_cnt, "expected cancel routine to be called\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + ok(cancel_queue_cnt, "expected cancel routine to be called\n"); + ok(queue.Busy, "expected Busy state\n"); + + IoFreeIrp(irp); +} + static void test_mdl_map(void) { char buffer[20] = "test buffer"; @@ -2126,6 +2221,7 @@ static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *st test_init_funcs(); test_load_driver(); test_sync(); + test_queue(); test_version(); test_stack_callout(); test_lookaside_list(); diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h index 447833c4bc9..9bc3305471c 100644 --- a/include/ddk/wdm.h +++ b/include/ddk/wdm.h @@ -397,6 +397,7 @@ typedef struct _WAIT_CONTEXT_BLOCK { #define IO_TYPE_ERROR_LOG 0x0b #define IO_TYPE_ERROR_MESSAGE 0x0c #define IO_TYPE_DEVICE_OBJECT_EXTENSION 0x0d +#define IO_TYPE_DEVICE_QUEUE 0x14 typedef struct _DEVICE_OBJECT { CSHORT Type; @@ -1750,6 +1751,7 @@ void WINAPI KeEnterCriticalRegion(void); void WINAPI KeGenericCallDpc(PKDEFERRED_ROUTINE,PVOID); ULONG WINAPI KeGetCurrentProcessorNumber(void); PKTHREAD WINAPI KeGetCurrentThread(void); +void WINAPI KeInitializeDeviceQueue(KDEVICE_QUEUE*); void WINAPI KeInitializeDpc(KDPC*,PKDEFERRED_ROUTINE,void*); void WINAPI KeInitializeEvent(PRKEVENT,EVENT_TYPE,BOOLEAN); void WINAPI KeInitializeMutex(PRKMUTEX,ULONG); @@ -1757,6 +1759,7 @@ void WINAPI KeInitializeSemaphore(PRKSEMAPHORE,LONG,LONG); void WINAPI KeInitializeSpinLock(KSPIN_LOCK*); void WINAPI KeInitializeTimerEx(PKTIMER,TIMER_TYPE); void WINAPI KeInitializeTimer(KTIMER*); +BOOLEAN WINAPI KeInsertDeviceQueue(KDEVICE_QUEUE*,KDEVICE_QUEUE_ENTRY*); void WINAPI KeLeaveCriticalRegion(void); ULONG WINAPI KeQueryActiveProcessorCountEx(USHORT); KAFFINITY WINAPI KeQueryActiveProcessors(void); @@ -1769,6 +1772,7 @@ LONG WINAPI KeReleaseMutex(PRKMUTEX,BOOLEAN); LONG WINAPI KeReleaseSemaphore(PRKSEMAPHORE,KPRIORITY,LONG,BOOLEAN); void WINAPI KeReleaseSpinLock(KSPIN_LOCK*,KIRQL); void WINAPI KeReleaseSpinLockFromDpcLevel(KSPIN_LOCK*); +KDEVICE_QUEUE_ENTRY * WINAPI KeRemoveDeviceQueue(KDEVICE_QUEUE*); LONG WINAPI KeResetEvent(PRKEVENT); void WINAPI KeRevertToUserAffinityThread(void); void WINAPI KeRevertToUserAffinityThreadEx(KAFFINITY affinity); -- 2.31.0
On 6/22/21 7:02 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
MSDN says that the cancel spinlock has to be held whenever these device queues are used, when setting cancel routines, but it's not clear to me why, as IRP cancellation has apparently no effect on these functions behavior (I initially thought they should check and not queue cancelled IRPs, but it looks like they don't).
Where do you see this? I couldn't find any mention of it from a brief search.
dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 6 +- dlls/ntoskrnl.exe/sync.c | 41 ++++++++++++ dlls/ntoskrnl.exe/tests/driver.c | 96 +++++++++++++++++++++++++++++ include/ddk/wdm.h | 4 ++ 4 files changed, 144 insertions(+), 3 deletions(-)
diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 5fdaa922f45..c1244a42146 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -567,7 +567,7 @@ @ stub KeI386SetGdtSelector @ stub KeIcacheFlushCount @ stdcall KeInitializeApc(ptr ptr long ptr ptr ptr long ptr) -@ stub KeInitializeDeviceQueue +@ stdcall KeInitializeDeviceQueue(ptr) @ stdcall KeInitializeDpc(ptr ptr ptr) @ stdcall KeInitializeEvent(ptr long long) @ stub KeInitializeInterrupt @@ -579,7 +579,7 @@ @ stdcall KeInitializeTimer(ptr) @ stdcall KeInitializeTimerEx(ptr long) @ stub KeInsertByKeyDeviceQueue -@ stub KeInsertDeviceQueue +@ stdcall KeInsertDeviceQueue(ptr ptr) @ stub KeInsertHeadQueue @ stdcall KeInsertQueue(ptr ptr) @ stub KeInsertQueueApc @@ -617,7 +617,7 @@ @ stdcall KeReleaseSpinLockFromDpcLevel(ptr) @ stub KeRemoveByKeyDeviceQueue @ stub KeRemoveByKeyDeviceQueueIfBusy -@ stub KeRemoveDeviceQueue +@ stdcall KeRemoveDeviceQueue(ptr) @ stub KeRemoveEntryDeviceQueue @ stub KeRemoveQueue @ stub KeRemoveQueueDpc diff --git a/dlls/ntoskrnl.exe/sync.c b/dlls/ntoskrnl.exe/sync.c index 445c8d890ab..9063608be4a 100644 --- a/dlls/ntoskrnl.exe/sync.c +++ b/dlls/ntoskrnl.exe/sync.c @@ -1362,3 +1362,44 @@ BOOLEAN WINAPI KeSetTimer(KTIMER *timer, LARGE_INTEGER duetime, KDPC *dpc)
return KeSetTimerEx(timer, duetime, 0, dpc); } + +void WINAPI KeInitializeDeviceQueue( KDEVICE_QUEUE *queue ) +{ + TRACE( "queue %p.\n", queue ); + + KeInitializeSpinLock( &queue->Lock ); + InitializeListHead( &queue->DeviceListHead ); + queue->Busy = FALSE; + queue->Type = IO_TYPE_DEVICE_QUEUE; + queue->Size = sizeof(*queue); +} + +BOOLEAN WINAPI KeInsertDeviceQueue( KDEVICE_QUEUE *queue, KDEVICE_QUEUE_ENTRY *entry ) +{ + KIRQL irql; + + TRACE( "queue %p, entry %p.\n", queue, entry ); + + KeAcquireSpinLock( &queue->Lock, &irql ); + if ((entry->Inserted = queue->Busy)) InsertTailList( &queue->DeviceListHead, &entry->DeviceListEntry ); + queue->Busy = TRUE; + KeReleaseSpinLock( &queue->Lock, irql ); + + return entry->Inserted; +} + +KDEVICE_QUEUE_ENTRY * WINAPI KeRemoveDeviceQueue( KDEVICE_QUEUE *queue ) +{ + LIST_ENTRY *entry = NULL; + KIRQL irql; + + TRACE( "queue %p.\n", queue ); + + KeAcquireSpinLock( &queue->Lock, &irql ); + if (IsListEmpty( &queue->DeviceListHead )) queue->Busy = FALSE; + else entry = RemoveHeadList( &queue->DeviceListHead ); + KeReleaseSpinLock( &queue->Lock, irql ); + + if (!entry) return NULL; + return CONTAINING_RECORD( entry, KDEVICE_QUEUE_ENTRY, DeviceListEntry ); +} diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index f1c72d8fdaf..3446dc8e529 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -141,6 +141,101 @@ static void test_irp_struct(IRP *irp, DEVICE_OBJECT *device) IoFreeIrp(irp); }
+static int cancel_queue_cnt; + +static void WINAPI cancel_queued_irp(DEVICE_OBJECT *device, IRP *irp) +{ + IoReleaseCancelSpinLock(irp->CancelIrql); + ok(irp->Cancel == TRUE, "Cancel = %x\n", irp->Cancel); + ok(!irp->CancelRoutine, "CancelRoutine = %p\n", irp->CancelRoutine); + irp->IoStatus.Status = STATUS_CANCELLED; + irp->IoStatus.Information = 0; + cancel_queue_cnt++; +} + +static void test_queue(void) +{ + KDEVICE_QUEUE_ENTRY *entry; + KDEVICE_QUEUE queue; + BOOLEAN ret; + KIRQL irql; + IRP *irp; + + irp = IoAllocateIrp(1, FALSE); + + memset(&queue, 0xcd, sizeof(queue)); + KeInitializeDeviceQueue(&queue); + ok(!queue.Busy, "unexpected Busy state\n"); + ok(queue.Size == sizeof(queue), "unexpected Size %x\n", queue.Size); + ok(queue.Type == IO_TYPE_DEVICE_QUEUE, "unexpected Type %x\n", queue.Type);
Could we test the actual contents of the queue list after all these calls?
+ + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(!entry, "expected KeRemoveDeviceQueue to return NULL\n"); + ok(!queue.Busy, "unexpected Busy state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry == NULL, "expected KeRemoveDeviceQueue to return NULL\n"); + ok(!queue.Busy, "unexpected Busy state\n"); + + IoCancelIrp(irp); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(!ret, "expected KeInsertDeviceQueue to not insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(queue.Busy, "expected Busy state\n"); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + + IoFreeIrp(irp); + + irp = IoAllocateIrp(1, FALSE); + + IoAcquireCancelSpinLock(&irql); + IoSetCancelRoutine(irp, cancel_queued_irp); + IoReleaseCancelSpinLock(irql); + + ret = KeInsertDeviceQueue(&queue, &irp->Tail.Overlay.DeviceQueueEntry); + ok(ret, "expected KeInsertDeviceQueue to insert IRP\n"); + ok(queue.Busy, "expected Busy state\n"); + + IoCancelIrp(irp); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + ok(cancel_queue_cnt, "expected cancel routine to be called\n"); + + entry = KeRemoveDeviceQueue(&queue); + ok(entry != NULL, "expected KeRemoveDeviceQueue to return non-NULL\n"); + ok(CONTAINING_RECORD(entry, IRP, Tail.Overlay.DeviceQueueEntry) == irp, "unexpected IRP returned\n"); + ok(irp->Cancel, "unexpected non-cancelled state\n"); + ok(cancel_queue_cnt, "expected cancel routine to be called\n"); + ok(queue.Busy, "expected Busy state\n"); + + IoFreeIrp(irp); +} + static void test_mdl_map(void) { char buffer[20] = "test buffer"; @@ -2126,6 +2221,7 @@ static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *st test_init_funcs(); test_load_driver(); test_sync(); + test_queue(); test_version(); test_stack_callout(); test_lookaside_list(); diff --git a/include/ddk/wdm.h b/include/ddk/wdm.h index 447833c4bc9..9bc3305471c 100644 --- a/include/ddk/wdm.h +++ b/include/ddk/wdm.h @@ -397,6 +397,7 @@ typedef struct _WAIT_CONTEXT_BLOCK { #define IO_TYPE_ERROR_LOG 0x0b #define IO_TYPE_ERROR_MESSAGE 0x0c #define IO_TYPE_DEVICE_OBJECT_EXTENSION 0x0d +#define IO_TYPE_DEVICE_QUEUE 0x14
typedef struct _DEVICE_OBJECT { CSHORT Type; @@ -1750,6 +1751,7 @@ void WINAPI KeEnterCriticalRegion(void); void WINAPI KeGenericCallDpc(PKDEFERRED_ROUTINE,PVOID); ULONG WINAPI KeGetCurrentProcessorNumber(void); PKTHREAD WINAPI KeGetCurrentThread(void); +void WINAPI KeInitializeDeviceQueue(KDEVICE_QUEUE*); void WINAPI KeInitializeDpc(KDPC*,PKDEFERRED_ROUTINE,void*); void WINAPI KeInitializeEvent(PRKEVENT,EVENT_TYPE,BOOLEAN); void WINAPI KeInitializeMutex(PRKMUTEX,ULONG); @@ -1757,6 +1759,7 @@ void WINAPI KeInitializeSemaphore(PRKSEMAPHORE,LONG,LONG); void WINAPI KeInitializeSpinLock(KSPIN_LOCK*); void WINAPI KeInitializeTimerEx(PKTIMER,TIMER_TYPE); void WINAPI KeInitializeTimer(KTIMER*); +BOOLEAN WINAPI KeInsertDeviceQueue(KDEVICE_QUEUE*,KDEVICE_QUEUE_ENTRY*); void WINAPI KeLeaveCriticalRegion(void); ULONG WINAPI KeQueryActiveProcessorCountEx(USHORT); KAFFINITY WINAPI KeQueryActiveProcessors(void); @@ -1769,6 +1772,7 @@ LONG WINAPI KeReleaseMutex(PRKMUTEX,BOOLEAN); LONG WINAPI KeReleaseSemaphore(PRKSEMAPHORE,KPRIORITY,LONG,BOOLEAN); void WINAPI KeReleaseSpinLock(KSPIN_LOCK*,KIRQL); void WINAPI KeReleaseSpinLockFromDpcLevel(KSPIN_LOCK*); +KDEVICE_QUEUE_ENTRY * WINAPI KeRemoveDeviceQueue(KDEVICE_QUEUE*); LONG WINAPI KeResetEvent(PRKEVENT); void WINAPI KeRevertToUserAffinityThread(void); void WINAPI KeRevertToUserAffinityThreadEx(KAFFINITY affinity);
On 6/22/21 5:20 PM, Zebediah Figura (she/her) wrote:
On 6/22/21 7:02 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
MSDN says that the cancel spinlock has to be held whenever these device queues are used, when setting cancel routines, but it's not clear to me why, as IRP cancellation has apparently no effect on these functions behavior (I initially thought they should check and not queue cancelled IRPs, but it looks like they don't).
Where do you see this? I couldn't find any mention of it from a brief search.
For instance [1]: "A driver must hold the system cancel spin lock when calling this routine if the driver uses the I/O manager-supplied device queue in the device object. The driver runs at IRQL = DISPATCH_LEVEL after calling IoAcquireCancelSpinLock until it releases the cancel spin lock with IoReleaseCancelSpinLock." [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-ios... I understand the "I/O manager-supplied device queue in the device object" is the "KDEVICE_QUEUE DeviceQueue" member that I wanted to use with these functions instead of re-implementing a queue and a lock again.
+ +static void test_queue(void) +{ + KDEVICE_QUEUE_ENTRY *entry; + KDEVICE_QUEUE queue; + BOOLEAN ret; + KIRQL irql; + IRP *irp; + + irp = IoAllocateIrp(1, FALSE); + + memset(&queue, 0xcd, sizeof(queue)); + KeInitializeDeviceQueue(&queue); + ok(!queue.Busy, "unexpected Busy state\n"); + ok(queue.Size == sizeof(queue), "unexpected Size %x\n", queue.Size); + ok(queue.Type == IO_TYPE_DEVICE_QUEUE, "unexpected Type %x\n", queue.Type);
Could we test the actual contents of the queue list after all these calls?
I guess. It's hopefully going to be empty? -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/22/21 10:29 AM, Rémi Bernon wrote:
On 6/22/21 5:20 PM, Zebediah Figura (she/her) wrote:
On 6/22/21 7:02 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
MSDN says that the cancel spinlock has to be held whenever these device queues are used, when setting cancel routines, but it's not clear to me why, as IRP cancellation has apparently no effect on these functions behavior (I initially thought they should check and not queue cancelled IRPs, but it looks like they don't).
Where do you see this? I couldn't find any mention of it from a brief search.
For instance [1]:
"A driver must hold the system cancel spin lock when calling this routine if the driver uses the I/O manager-supplied device queue in the device object. The driver runs at IRQL = DISPATCH_LEVEL after calling IoAcquireCancelSpinLock until it releases the cancel spin lock with IoReleaseCancelSpinLock."
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-ios...
I understand the "I/O manager-supplied device queue in the device object" is the "KDEVICE_QUEUE DeviceQueue" member that I wanted to use with these functions instead of re-implementing a queue and a lock again.
Based on [2] and [3] I'm guessing the device queue functions never acquire the lock, but rather the point is to synchronize with dequeuing and cancelling. I can't figure out why it makes a difference, though... It doesn't seem obvious to me that using KDEVICE_QUEUEs in Wine makes our job any easier, fwiw. Especially the part where you can't actually queue the first pending IRP. I'm not sure it's meant to handle those kinds of pending IRPs, but then it's not really clear how these things are supposed to be used; there are no code samples anywhere. [2] https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/cancel-rout... [3] https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/synchronizi...
+ +static void test_queue(void) +{ + KDEVICE_QUEUE_ENTRY *entry; + KDEVICE_QUEUE queue; + BOOLEAN ret; + KIRQL irql; + IRP *irp; + + irp = IoAllocateIrp(1, FALSE); + + memset(&queue, 0xcd, sizeof(queue)); + KeInitializeDeviceQueue(&queue); + ok(!queue.Busy, "unexpected Busy state\n"); + ok(queue.Size == sizeof(queue), "unexpected Size %x\n", queue.Size); + ok(queue.Type == IO_TYPE_DEVICE_QUEUE, "unexpected Type %x\n", queue.Type);
Could we test the actual contents of the queue list after all these calls?
I guess. It's hopefully going to be empty?
Empty after this call, but not after an eventual KeInsertDeviceQueue call; I think it's worth testing the state after all of the Ke* calls below.
On 6/22/21 6:14 PM, Zebediah Figura (she/her) wrote:
On 6/22/21 10:29 AM, Rémi Bernon wrote:
On 6/22/21 5:20 PM, Zebediah Figura (she/her) wrote:
On 6/22/21 7:02 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
MSDN says that the cancel spinlock has to be held whenever these device queues are used, when setting cancel routines, but it's not clear to me why, as IRP cancellation has apparently no effect on these functions behavior (I initially thought they should check and not queue cancelled IRPs, but it looks like they don't).
Where do you see this? I couldn't find any mention of it from a brief search.
For instance [1]:
"A driver must hold the system cancel spin lock when calling this routine if the driver uses the I/O manager-supplied device queue in the device object. The driver runs at IRQL = DISPATCH_LEVEL after calling IoAcquireCancelSpinLock until it releases the cancel spin lock with IoReleaseCancelSpinLock."
[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-ios...
I understand the "I/O manager-supplied device queue in the device object" is the "KDEVICE_QUEUE DeviceQueue" member that I wanted to use with these functions instead of re-implementing a queue and a lock again.
Based on [2] and [3] I'm guessing the device queue functions never acquire the lock, but rather the point is to synchronize with dequeuing and cancelling. I can't figure out why it makes a difference, though...
It doesn't seem obvious to me that using KDEVICE_QUEUEs in Wine makes our job any easier, fwiw. Especially the part where you can't actually queue the first pending IRP. I'm not sure it's meant to handle those kinds of pending IRPs, but then it's not really clear how these things are supposed to be used; there are no code samples anywhere.
It probably also has something to do with IoStart(Next)Packet which need to handle cancellation. Even when not using these, and even if queue-ing an IRP needs a loop I think using the device queue directly makes the code slightly simpler as it doesn't need locking. It also saves a dedicated irp_queue and irp_queue_lock members which are already in the device object, and currently unused.
+ +static void test_queue(void) +{ + KDEVICE_QUEUE_ENTRY *entry; + KDEVICE_QUEUE queue; + BOOLEAN ret; + KIRQL irql; + IRP *irp; + + irp = IoAllocateIrp(1, FALSE); + + memset(&queue, 0xcd, sizeof(queue)); + KeInitializeDeviceQueue(&queue); + ok(!queue.Busy, "unexpected Busy state\n"); + ok(queue.Size == sizeof(queue), "unexpected Size %x\n", queue.Size); + ok(queue.Type == IO_TYPE_DEVICE_QUEUE, "unexpected Type %x\n", queue.Type);
Could we test the actual contents of the queue list after all these calls?
I guess. It's hopefully going to be empty?
Empty after this call, but not after an eventual KeInsertDeviceQueue call; I think it's worth testing the state after all of the Ke* calls below.
Yeah I figured, I'll add more tests. -- Rémi Bernon <rbernon(a)codeweavers.com>
participants (2)
-
Rémi Bernon -
Zebediah Figura (she/her)