[dpdk-dev] [kmods PATCH] windows/netuio: add interrupt support

[dpdk-dev] [kmods PATCH] windows/netuio: add interrupt support

[Dmitry Kozlyuk]

Add IOCTL for interrupt event delivery to user space.

* IOCTL_NETUIO_INTR_CONTROL:
  Enable/disable the delivery of interrupt events.
  This is a software switch to ignore received interrupts.
  Programming the HW to stop issuing interrupts is up to the user.

* IOCTL_NETUIO_INTR_QUERY:
  Wait for an interrupt event on the specified vector.
  More than one outstanding request may be issued for the same vector.
  This can be used to balance the interrupt servicing if the requests
  are issued using distinct handles waited by different threads.
  If N identical requests are issued using the same handle,
  one request will be completed each time the interrupt is triggered.

MSI-X support is advertised for the driver, but the implementation
works with any type of interrupts assigned by the kernel.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 windows/netuio/netuio.inf         |   3 +
 windows/netuio/netuio_dev.c       | 109 +++++++++++++++++++++++++++++-
 windows/netuio/netuio_dev.h       |  10 +++
 windows/netuio/netuio_interface.h |  22 ++++++
 windows/netuio/netuio_queue.c     | 102 ++++++++++++++++++++--------
 5 files changed, 214 insertions(+), 32 deletions(-)

diff --git a/windows/netuio/netuio.inf b/windows/netuio/netuio.inf
index d166868..cac378d 100644
--- a/windows/netuio/netuio.inf
+++ b/windows/netuio/netuio.inf
@@ -63,6 +63,9 @@ AddReg=Device.HW.Registry
 [Device.HW.Registry]
 ; Ensure that only administrators can access our device object.
 HKR,,Security,,"D:P(A;;GA;;;SY)(A;;GA;;;BA)"
+HKR,Interrupt Management,,0x00000010
+HKR,Interrupt Management\MessageSignaledInterruptProperties,,0x00000010
+HKR,Interrupt Management\MessageSignaledInterruptProperties,MSISupported,0x00010001,1
 
 ;-------------- Service installation
 [netuio_Device.NT.Services]
diff --git a/windows/netuio/netuio_dev.c b/windows/netuio/netuio_dev.c
index b2deb10..d4662b6 100644
--- a/windows/netuio/netuio_dev.c
+++ b/windows/netuio/netuio_dev.c
@@ -135,6 +135,109 @@ netuio_create_device(PWDFDEVICE_INIT DeviceInit)
     return status;
 }
 
+static EVT_WDF_INTERRUPT_ISR netuio_intr_service;
+static EVT_WDF_INTERRUPT_WORKITEM netuio_intr_deliver;
+
+_Use_decl_annotations_
+static BOOLEAN
+netuio_intr_service(WDFINTERRUPT intr, ULONG message_id)
+{
+	WDFDEVICE device = WdfInterruptGetDevice(intr);
+	NETUIO_CONTEXT_DATA *ctx = netuio_get_context_data(device);
+	BOOLEAN enabled = ctx->intr_enabled;
+
+	UNREFERENCED_PARAMETER(message_id);
+	if (enabled)
+		WdfInterruptQueueWorkItemForIsr(intr);
+	return enabled;
+}
+
+_Use_decl_annotations_
+static void
+netuio_intr_deliver(WDFINTERRUPT intr, WDFOBJECT device)
+{
+	NETUIO_INTR_CTX *intr_ctx = netuio_intr_ctx_get(intr);
+	WDFREQUEST request;
+	NTSTATUS status;
+
+	UNREFERENCED_PARAMETER(device);
+	PAGED_CODE();
+	status = WdfIoQueueRetrieveNextRequest(intr_ctx->queue, &request);
+	if (NT_SUCCESS(status))
+		WdfRequestComplete(request, STATUS_SUCCESS);
+}
+
+static NTSTATUS
+netuio_intr_alloc(WDFDEVICE device, WDFCMRESLIST raw, WDFCMRESLIST translated)
+{
+	NETUIO_CONTEXT_DATA *ctx;
+	CM_PARTIAL_RESOURCE_DESCRIPTOR *desc;
+	WDFINTERRUPT* intrs;
+	WDF_OBJECT_ATTRIBUTES intr_attr;
+	WDF_INTERRUPT_CONFIG intr_config;
+	WDF_OBJECT_ATTRIBUTES queue_attr;
+	WDF_IO_QUEUE_CONFIG queue_config;
+	ULONG i, resource_n, intr_n = 0, intr_created = 0;
+	NTSTATUS status;
+
+	resource_n = WdfCmResourceListGetCount(translated);
+	for (i = 0; i < resource_n; i++) {
+		desc = WdfCmResourceListGetDescriptor(translated, i);
+		if (desc->Type == CmResourceTypeInterrupt)
+			intr_n++;
+	}
+	if (intr_n == 0)
+		return STATUS_SUCCESS;
+
+	intrs = ExAllocatePoolZero(PagedPool, sizeof(intrs[0]) * intr_n, 'rtni');
+	if (intrs == NULL)
+		return STATUS_INSUFFICIENT_RESOURCES;
+
+	WDF_OBJECT_ATTRIBUTES_INIT(&queue_attr);
+	WDF_IO_QUEUE_CONFIG_INIT(&queue_config, WdfIoQueueDispatchManual);
+	WDF_OBJECT_ATTRIBUTES_INIT_CONTEXT_TYPE(&intr_attr, NETUIO_INTR_CTX);
+	WDF_INTERRUPT_CONFIG_INIT(&intr_config, netuio_intr_service, NULL);
+	intr_config.EvtInterruptWorkItem = netuio_intr_deliver;
+	for (i = 0; i < resource_n; i++) {
+		NETUIO_INTR_CTX *intr_ctx;
+		WDFINTERRUPT intr;
+		WDFQUEUE queue;
+
+		desc = WdfCmResourceListGetDescriptor(translated, i);
+		if (desc->Type != CmResourceTypeInterrupt)
+			continue;
+
+		intr_config.InterruptRaw = WdfCmResourceListGetDescriptor(raw, i);
+		intr_config.InterruptTranslated = desc;
+		status = WdfInterruptCreate(device, &intr_config, &intr_attr, &intr);
+		if (!NT_SUCCESS(status))
+			goto error;
+
+		queue_attr.ParentObject = intr;
+		status = WdfIoQueueCreate(device, &queue_config, &queue_attr, &queue);
+		if (!NT_SUCCESS(status)) {
+			WdfObjectDelete(intr);
+			goto error;
+		}
+
+		intr_ctx = netuio_intr_ctx_get(intr);
+		intr_ctx->queue = queue;
+		intrs[intr_created] = intr;
+		intr_created++;
+	}
+
+	ctx = netuio_get_context_data(device);
+	ctx->intr = intrs;
+	ctx->intr_n = intr_n;
+	return STATUS_SUCCESS;
+
+error:
+	for (i = 0; i < intr_created; i++)
+		WdfObjectDelete(intrs[i]);
+	ExFreePool(intrs);
+	return status;
+}
+
 _Use_decl_annotations_
 NTSTATUS
 netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST Resources, WDFCMRESLIST ResourcesTranslated)
@@ -208,7 +311,7 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST Resources, WDFCMRESLIST R
 
             if (descriptor == NULL) {
                 status = STATUS_DEVICE_CONFIGURATION_ERROR;
-                goto end;
+                goto end_of_loop;
             }
         } while ((descriptor->Type != CmResourceTypeMemory) ||
                  !(descriptor->Flags & CM_RESOURCE_MEMORY_BAR));
@@ -237,8 +340,9 @@ netuio_map_hw_resources(WDFDEVICE Device, WDFCMRESLIST Resources, WDFCMRESLIST R
 
         ctx->dpdk_hw[bar_index].mem.size = ctx->bar[bar_index].size;
     } // for bar_index
+end_of_loop:
 
-    status = STATUS_SUCCESS;
+    status = netuio_intr_alloc(Device, Resources, ResourcesTranslated);
 
 end:
     if (status != STATUS_SUCCESS) {
@@ -267,6 +371,7 @@ netuio_free_hw_resources(WDFDEVICE Device)
             MmUnmapIoSpace(ctx->bar[bar_index].virt_addr, ctx->bar[bar_index].size);
         }
     }
+    /* Interrupt objects are released with the device. */
 
     RtlZeroMemory(ctx->dpdk_hw, sizeof(ctx->dpdk_hw));
     RtlZeroMemory(ctx->bar, sizeof(ctx->bar));
diff --git a/windows/netuio/netuio_dev.h b/windows/netuio/netuio_dev.h
index 6285114..1c8a6fa 100644
--- a/windows/netuio/netuio_dev.h
+++ b/windows/netuio/netuio_dev.h
@@ -24,6 +24,13 @@ struct dev_addr {
     USHORT  func_num;
 };
 
+/** Interrupt vector associated data. */
+typedef struct NETUIO_INTR_CTX {
+    WDFQUEUE queue; /**< Queue of requests for this interrupt. */
+} NETUIO_INTR_CTX;
+
+WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(NETUIO_INTR_CTX, netuio_intr_ctx_get);
+
 /**
  * The device context performs the same job as a WDM device extension in the driver frameworks
  */
@@ -34,6 +41,9 @@ typedef struct _NETUIO_CONTEXT_DATA
     struct pci_bar          bar[PCI_MAX_BAR];       /**< device BARs */
     struct dev_addr         addr;                   /**< B:D:F details of device */
     struct mem_map_region   dpdk_hw[PCI_MAX_BAR];   /**< mapped region for the device's register space */
+    BOOLEAN                 intr_enabled;           /**< Whether interrupt delivery is enabled. */
+    WDFINTERRUPT            *intr;                  /**< Interrupt objects indexed by vector. */
+    size_t                  intr_n;                 /**< Number of interrupt objects. */
 } NETUIO_CONTEXT_DATA, *PNETUIO_CONTEXT_DATA;
 
 
diff --git a/windows/netuio/netuio_interface.h b/windows/netuio/netuio_interface.h
index 71f5395..b5f936f 100644
--- a/windows/netuio/netuio_interface.h
+++ b/windows/netuio/netuio_interface.h
@@ -50,6 +50,20 @@ DEFINE_GUID (GUID_DEVINTERFACE_netUIO, 0x08336f60,0x0679,0x4c6c,0x85,0xd2,0xae,0
  */
 #define IOCTL_NETUIO_PCI_CONFIG_IO        CTL_CODE(FILE_DEVICE_NETWORK, 52, METHOD_BUFFERED, FILE_READ_ACCESS | FILE_WRITE_ACCESS)
 
+/**
+ * Control whether interrupts are enabled.
+ * Input: struct netuio_intr_control
+ * Output: N/A
+ */
+#define IOCTL_NETUIO_INTR_CONTROL CTL_CODE(FILE_DEVICE_NETWORK, 53, METHOD_BUFFERED, FILE_ANY_ACCESS)
+
+/**
+ * Wait until an interrupt occurs.
+ * Input: struct netuio_intr_query
+ * Output: N/A
+ */
+#define IOCTL_NETUIO_INTR_QUERY CTL_CODE(FILE_DEVICE_NETWORK, 54, METHOD_BUFFERED, FILE_ANY_ACCESS)
+
 struct mem_region {
     UINT64           size;       /**< memory region size */
     LARGE_INTEGER    phys_addr;  /**< physical address of the memory region */
@@ -83,6 +97,14 @@ struct dpdk_pci_config_io
     } data; /**< Data to be written, in case of write operations */
 };
 
+struct netuio_intr_control {
+    UINT32 enable; /**< 1 to enable interrupts, 0 to disable */
+};
+
+struct netuio_intr_query {
+    UINT32 vector; /**< Vector of the interrupt to wait for. */
+};
+
 #pragma pack(pop)
 
 #endif // NETUIO_INTERFACE_H
diff --git a/windows/netuio/netuio_queue.c b/windows/netuio/netuio_queue.c
index 0583394..1f9eb9e 100644
--- a/windows/netuio/netuio_queue.c
+++ b/windows/netuio/netuio_queue.c
@@ -245,37 +245,49 @@ end:
     return;
 }
 
-/*
-Routine Description:
-    This event is invoked when the framework receives IRP_MJ_DEVICE_CONTROL request.
-
-Return Value:
-    None
- */
-_Use_decl_annotations_
-VOID
-netuio_evt_IO_device_control(WDFQUEUE Queue, WDFREQUEST Request,
-                             size_t OutputBufferLength, size_t InputBufferLength,
-                             ULONG IoControlCode)
+static void
+netuio_ioctl_intr_control(NETUIO_CONTEXT_DATA *ctx, WDFREQUEST request)
 {
-    UNREFERENCED_PARAMETER(OutputBufferLength);
-    UNREFERENCED_PARAMETER(InputBufferLength);
-
-    NTSTATUS status = STATUS_SUCCESS;
-    PVOID    input_buf = NULL, output_buf = NULL;
-    size_t   input_buf_size, output_buf_size;
-    size_t  bytes_returned = 0;
-
-    WDFDEVICE device = WdfIoQueueGetDevice(Queue);
+	struct netuio_intr_control *in;
+	NTSTATUS status;
+
+	status = WdfRequestRetrieveInputBuffer(request, sizeof(*in), &in, NULL);
+	if (!NT_SUCCESS(status))
+		goto exit;
+	ctx->intr_enabled = in->enable != 0;
+	status = STATUS_SUCCESS;
+exit:
+	WdfRequestComplete(request, status);
+}
 
-    PNETUIO_CONTEXT_DATA  ctx;
-    ctx = netuio_get_context_data(device);
+static void
+netuio_ioctl_intr_query(NETUIO_CONTEXT_DATA *ctx, WDFREQUEST request)
+{
+	struct netuio_intr_query *in;
+	NETUIO_INTR_CTX *intr_ctx;
+	NTSTATUS status;
+
+	status = WdfRequestRetrieveInputBuffer(request, sizeof(*in), &in, NULL);
+	if (!NT_SUCCESS(status))
+		goto error;
+	if (in->vector >= ctx->intr_n) {
+		status = STATUS_INVALID_PARAMETER;
+		goto error;
+	}
+	intr_ctx = netuio_intr_ctx_get(ctx->intr[in->vector]);
+	WdfRequestForwardToIoQueue(request, intr_ctx->queue);
+	return;
+error:
+	WdfRequestComplete(request, status);
+}
 
-    if (IoControlCode != IOCTL_NETUIO_PCI_CONFIG_IO)
-    {
-        status = STATUS_INVALID_DEVICE_REQUEST;
-        goto end;
-    }
+static void
+netuio_ioctl_pci_config_io(NETUIO_CONTEXT_DATA *ctx, WDFREQUEST Request)
+{
+    PVOID input_buf = NULL, output_buf = NULL;
+    size_t input_buf_size, output_buf_size;
+    size_t bytes_returned = 0;
+    NTSTATUS status;
 
     // First retrieve the input buffer and see if it matches our device
     status = WdfRequestRetrieveInputBuffer(Request, sizeof(struct dpdk_pci_config_io), &input_buf, &input_buf_size);
@@ -298,7 +310,6 @@ netuio_evt_IO_device_control(WDFQUEUE Queue, WDFREQUEST Request,
     if (!NT_SUCCESS(status)) {
         goto end;
     }
-    ASSERT(output_buf_size == OutputBufferLength);
 
     if (dpdk_pci_io_input->op == PCI_IO_READ) {
         *(UINT64 *)output_buf = 0;
@@ -329,6 +340,37 @@ netuio_evt_IO_device_control(WDFQUEUE Queue, WDFREQUEST Request,
 
 end:
     WdfRequestCompleteWithInformation(Request, status, bytes_returned);
+}
 
-    return;
+/*
+Routine Description:
+    This event is invoked when the framework receives IRP_MJ_DEVICE_CONTROL request.
+
+Return Value:
+    None
+ */
+_Use_decl_annotations_
+VOID
+netuio_evt_IO_device_control(WDFQUEUE Queue, WDFREQUEST Request,
+		size_t OutputBufferLength, size_t InputBufferLength,
+		ULONG IoControlCode)
+{
+	WDFDEVICE device = WdfIoQueueGetDevice(Queue);
+	PNETUIO_CONTEXT_DATA ctx = netuio_get_context_data(device);
+
+	UNREFERENCED_PARAMETER(OutputBufferLength);
+	UNREFERENCED_PARAMETER(InputBufferLength);
+	switch (IoControlCode) {
+	case IOCTL_NETUIO_PCI_CONFIG_IO:
+		netuio_ioctl_pci_config_io(ctx, Request);
+		break;
+	case IOCTL_NETUIO_INTR_CONTROL:
+		netuio_ioctl_intr_control(ctx, Request);
+		break;
+	case IOCTL_NETUIO_INTR_QUERY:
+		netuio_ioctl_intr_query(ctx, Request);
+		break;
+	default:
+		WdfRequestComplete(Request, STATUS_INVALID_DEVICE_REQUEST);
+	}
 }
-- 
2.29.3

Re: [dpdk-dev] [kmods PATCH] windows/netuio: add interrupt support

[William Tu]

On Mon, Oct 11, 2021 at 6:11 PM Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:
>
> Add IOCTL for interrupt event delivery to user space.
>
> * IOCTL_NETUIO_INTR_CONTROL:
>   Enable/disable the delivery of interrupt events.
>   This is a software switch to ignore received interrupts.
>   Programming the HW to stop issuing interrupts is up to the user.
>
> * IOCTL_NETUIO_INTR_QUERY:
>   Wait for an interrupt event on the specified vector.
>   More than one outstanding request may be issued for the same vector.
>   This can be used to balance the interrupt servicing if the requests
>   are issued using distinct handles waited by different threads.
>   If N identical requests are issued using the same handle,
>   one request will be completed each time the interrupt is triggered.
>
> MSI-X support is advertised for the driver, but the implementation
> works with any type of interrupts assigned by the kernel.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---

Hi Dmitry,
Is there a corresponding userspace code so I can give it a try?
s.t like sending IOCTL_NETUIO_INTR_CONTROL to enable a device's interrupt,
and blocked at IOCTL_NETUIO_INTR_QUERY for incoming request in the intr_queue.
Thanks
William

Re: [kmods PATCH] windows/netuio: add interrupt support

[Dmitry Kozlyuk]

2021-10-12 04:11 (UTC+0300), Dmitry Kozlyuk:
> [...]

This patchset is not supposed to be merged for now.
It should have been marked as RFC, sorry for the misleading title.

Offline discussions revealed a conflict between
edge-triggered interrupts in DPDK contract
and level-triggered IO completions used by this patch
and the corresponding userspace part.
I'm experimenting with alternative approaches
and will send a new implementation
when both the kernel and the userspace parts are ready.
Thanks to William for preliminary testing
and to Mark for sharing his experience with some alternatives.