[ofw] [PATCH][RFC] winverbs/cq: implement CQ:Notify()

[Fab Tillier]

>Initial implementation of CQ:Notify() to support asynchronous notification
>of CQ events.
>
>Signed-off-by: Sean Hefty <sean.hefty at intel.com>
>---
>I'm not sure if the WdfObject*Lock() routines for an IO queue work at dispatch
>by default, but that looks easy enough to correct if they don't.
>
>Index: core/winverbs/kernel/wv_cq.c
>===================================================================
>--- core/winverbs/kernel/wv_cq.c        (revision 1090)
>+++ core/winverbs/kernel/wv_cq.c        (working copy)
>@@ -27,6 +27,7 @@
>  * SOFTWARE.
>  */
>
>+#include "wv_driver.h"
> #include "wv_cq.h"
> #include "wv_ioctl.h"
>
>@@ -68,13 +69,32 @@
>
> static void WvCqEventHandler(ib_event_rec_t *pEvent)
> {
>-       pEvent;
>+       WV_COMPLETION_QUEUE *cq = pEvent->context;
>+       WDFREQUEST                      request;
>+       NTSTATUS                        status;
>+
>+       WdfObjectAcquireLock(cq->Queue);
>+       status = WdfIoQueueRetrieveNextRequest(cq->Queue, &request);
>+       WdfObjectReleaseLock(cq->Queue);
>+
>+       if (NT_SUCCESS(status)) {
>+               WdfRequestComplete(request, STATUS_UNEXPECTED_IO_ERROR);
>+       }

What happens if a user has multiple Notify requests outstanding?  You only complete one?

It seems that all should get completed.

Also, why not provide richer status to distinguish overflow from catastrophic error?  Maybe something like STATUS_FAILURE for catastrophic failure, and STATUS_DATA_OVERRUN for CQ overflow?  Both are error status values.  Maybe it doesn't matter, though.

> }
>
> static void WvCqHandler(void *Context)
> {
>-        WV_COMPLETION_QUEUE *pCq = Context;
>-        pCq;
>+       WV_COMPLETION_QUEUE *cq = Context;
>+       WDFREQUEST                      request;
>+       NTSTATUS                        status;
>+
>+       WdfObjectAcquireLock(cq->Queue);
>+       status = WdfIoQueueRetrieveNextRequest(cq->Queue, &request);
>+       WdfObjectReleaseLock(cq->Queue);
>+
>+       if (NT_SUCCESS(status)) {
>+               WdfRequestComplete(request, STATUS_SUCCESS);
>+       }

Same here, all pending requests should get completed.

> }
>
> static NTSTATUS WvCqAlloc(WV_DEVICE *pDevice, UINT32 *pSize,
>@@ -82,8 +102,10 @@
> {
>        ib_api_status_t         ib_status;
>        WV_COMPLETION_QUEUE     *cq;
>+       WDF_IO_QUEUE_CONFIG     config;
>+       NTSTATUS                        status;
>
>-       cq = ExAllocatePoolWithTag(PagedPool, sizeof(WV_COMPLETION_QUEUE), 'qcvw');
>+       cq = ExAllocatePoolWithTag(NonPagedPool, sizeof(WV_COMPLETION_QUEUE), 'qcvw');
>        if (cq == NULL) {
>                return STATUS_NO_MEMORY;
>        }
>@@ -91,10 +113,18 @@
>        cq->Ref = 1;
>        KeInitializeEvent(&cq->Event, NotificationEvent, FALSE);
>
>+       WDF_IO_QUEUE_CONFIG_INIT(&config, WdfIoQueueDispatchManual);
>+       status = WdfIoQueueCreate(ControlDevice, &config,
>+                                                         WDF_NO_OBJECT_ATTRIBUTES, &cq->Queue);
>+       if (!NT_SUCCESS(status)) {
>+               goto err;
>+       }
>+
>        ib_status = pDevice->pVerbs->create_cq(pDevice->hVerbsDevice, cq,
>                                                                                   WvCqEventHandler, WvCqHandler,
>                                                                                   pSize, &cq->hVerbsCq, pVerbsData);
>        if (ib_status != IB_SUCCESS) {
>+               status = STATUS_UNSUCCESSFUL;
>                goto err;
>        }
>
>@@ -104,7 +134,7 @@
>
> err:
>        ExFreePool(cq);
>-       return STATUS_UNSUCCESSFUL;
>+       return status;
> }
>
> void WvCqCreate(WV_PROVIDER *pProvider, WDFREQUEST Request)
>@@ -249,3 +279,38 @@
> complete:
>        WdfRequestCompleteWithInformation(Request, status, len);
> }
>+
>+void WvCqNotify(WV_PROVIDER *pProvider, WDFREQUEST Request)
>+{
>+       WV_IO_ID                                *id;
>+       WV_COMPLETION_QUEUE             *cq;
>+       NTSTATUS                                status;
>+       ib_api_status_t                 ib_status;
>+
>+       status = WdfRequestRetrieveInputBuffer(Request, sizeof(WV_IO_ID), &id, NULL);
>+       if (!NT_SUCCESS(status)) {
>+               goto out;
>+       }
>+
>+       cq = WvCqAcquire(pProvider, id->Id);
>+       if (cq == NULL) {
>+               status = STATUS_NOT_FOUND;
>+               goto out;
>+       }
>+
>+       WdfObjectAcquireLock(cq->Queue);
>+       ib_status = cq->pVerbs->enable_cq_notify(cq->hVerbsCq,
>+                                                                                        id->Data ==
>WV_IO_CQ_NOTIFY_SOLICITED);

Are you sure this works - rearming a UM CQ in the kernel?  It should, but looking at the MTHCA code it appears that a doorbell for the CQ is only allocated in the kernel for kernel CQs (mthca_init_cq).

How do you support a client that wants error notifications, but not rearm?

>+       if (ib_status == IB_SUCCESS) {
>+               status = WdfRequestForwardToIoQueue(Request, cq->Queue);
>+       } else {
>+               status = STATUS_UNSUCCESSFUL;
>+       }
>+       WdfObjectReleaseLock(cq->Queue);
>+       WvCqRelease(cq);
>+
>+out:
>+       if (!NT_SUCCESS(status)) {
>+               WdfRequestComplete(Request, status);
>+       }
>+}