Re: [win-pv-devel] [PATCH 04/14 v2] Move console handler to static Pdo

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of owen.smith@xxxxxxxxxx
> Sent: 23 February 2018 14:22
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith <owen.smith@xxxxxxxxxx>
> Subject: [win-pv-devel] [PATCH 04/14 v2] Move console handler to static Pdo
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> Changes the Fdo into a simple bus driver that only enumerates
> the single static Pdo.Moves the interface and all console operations
> to the static Pdo.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  src/xencons/fdo.c | 151 +------------------------------------------------
>  src/xencons/pdo.c | 164
> ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 161 insertions(+), 154 deletions(-)
> 
> diff --git a/src/xencons/fdo.c b/src/xencons/fdo.c
> index 1c5323c..45a1fdd 100644
> --- a/src/xencons/fdo.c
> +++ b/src/xencons/fdo.c
> @@ -41,7 +41,6 @@
>  #include <suspend_interface.h>
>  #include <store_interface.h>
>  #include <console_interface.h>
> -#include <xencons_device.h>
>  #include <version.h>
> 
>  #include "driver.h"
> @@ -49,7 +48,6 @@
>  #include "fdo.h"
>  #include "pdo.h"
>  #include "mutex.h"
> -#include "console.h"
>  #include "thread.h"
>  #include "names.h"
>  #include "dbg_print.h"
> @@ -92,8 +90,6 @@ struct _XENCONS_FDO {
> 
>      FDO_RESOURCE                Resource[RESOURCE_COUNT];
> 
> -    PXENCONS_CONSOLE            Console;
> -
>      XENBUS_DEBUG_INTERFACE      DebugInterface;
>      XENBUS_SUSPEND_INTERFACE    SuspendInterface;
>      XENBUS_STORE_INTERFACE      StoreInterface;
> @@ -1096,12 +1092,6 @@ FdoD3ToD0(
> 
>      __FdoReleaseMutex(Fdo);
> 
> -    status = ConsoleD3ToD0(Fdo->Console);
> -    ASSERT(NT_SUCCESS(status));
> -
> -#pragma prefast(suppress:28123)
> -    (VOID) IoSetDeviceInterfaceState(&Fdo->Dx->Link, TRUE);
> -
>      Trace("<====\n");
> 
>      return STATUS_SUCCESS;
> @@ -1144,11 +1134,6 @@ FdoD0ToD3(
> 
>      Trace("====>\n");
> 
> -#pragma prefast(suppress:28123)
> -    (VOID) IoSetDeviceInterfaceState(&Fdo->Dx->Link, FALSE);
> -
> -    ConsoleD0ToD3(Fdo->Console);
> -
>      __FdoAcquireMutex(Fdo);
> 
>      for (ListEntry = Fdo->Dx->ListEntry.Flink;
> @@ -2484,85 +2469,6 @@ done:
>  }
> 
>  static DECLSPEC_NOINLINE NTSTATUS
> -FdoDispatchCreate(
> -    IN  PXENCONS_FDO    Fdo,
> -    IN  PIRP            Irp
> -    )
> -{
> -    PIO_STACK_LOCATION  StackLocation;
> -    NTSTATUS            status;
> -
> -    StackLocation = IoGetCurrentIrpStackLocation(Irp);
> -
> -    status = ConsoleOpen(Fdo->Console, StackLocation->FileObject);
> -
> -    Irp->IoStatus.Status = status;
> -    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> -
> -    return status;
> -}
> -
> -static DECLSPEC_NOINLINE NTSTATUS
> -FdoDispatchCleanup(
> -    IN  PXENCONS_FDO    Fdo,
> -    IN  PIRP            Irp
> -    )
> -{
> -    PIO_STACK_LOCATION  StackLocation;
> -    NTSTATUS            status;
> -
> -    StackLocation = IoGetCurrentIrpStackLocation(Irp);
> -
> -    status = ConsoleClose(Fdo->Console, StackLocation->FileObject);
> -
> -    Irp->IoStatus.Status = status;
> -    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> -
> -    return status;
> -}
> -
> -static DECLSPEC_NOINLINE NTSTATUS
> -FdoDispatchClose(
> -    IN  PXENCONS_FDO    Fdo,
> -    IN  PIRP            Irp
> -    )
> -{
> -    NTSTATUS            status;
> -
> -    UNREFERENCED_PARAMETER(Fdo);
> -
> -    status = STATUS_SUCCESS;
> -
> -    Irp->IoStatus.Status = status;
> -    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> -
> -    return status;
> -}
> -
> -static DECLSPEC_NOINLINE NTSTATUS
> -FdoDispatchReadWrite(
> -    IN  PXENCONS_FDO    Fdo,
> -    IN  PIRP            Irp
> -    )
> -{
> -    NTSTATUS            status;
> -
> -    status = ConsolePutQueue(Fdo->Console, Irp);
> -    if (status != STATUS_PENDING)
> -        goto fail1;
> -
> -    return STATUS_PENDING;
> -
> -fail1:
> -    Error("fail1 (%08x)\n", status);
> -
> -    Irp->IoStatus.Status = status;
> -    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> -
> -    return status;
> -}
> -
> -static DECLSPEC_NOINLINE NTSTATUS
>  FdoDispatchDefault(
>      IN  PXENCONS_FDO    Fdo,
>      IN  PIRP            Irp
> @@ -2602,23 +2508,6 @@ FdoDispatch(
>          status = FdoDispatchPower(Fdo, Irp);
>          break;
> 
> -    case IRP_MJ_CREATE:
> -        status = FdoDispatchCreate(Fdo, Irp);
> -        break;
> -
> -    case IRP_MJ_CLEANUP:
> -        status = FdoDispatchCleanup(Fdo, Irp);
> -        break;
> -
> -    case IRP_MJ_CLOSE:
> -        status = FdoDispatchClose(Fdo, Irp);
> -        break;
> -
> -    case IRP_MJ_READ:
> -    case IRP_MJ_WRITE:
> -        status = FdoDispatchReadWrite(Fdo, Irp);
> -        break;
> -
>      default:
>          status = FdoDispatchDefault(Fdo, Irp);
>          break;
> @@ -2734,9 +2623,6 @@ DEFINE_FDO_GET_INTERFACE(Suspend,
> PXENBUS_SUSPEND_INTERFACE)
>  DEFINE_FDO_GET_INTERFACE(Store, PXENBUS_STORE_INTERFACE)
>  DEFINE_FDO_GET_INTERFACE(Console, PXENBUS_CONSOLE_INTERFACE)
> 
> -#pragma warning(push)
> -#pragma warning(disable:6014) // Leaking memory '&Dx->Link'
> -
>  NTSTATUS
>  FdoCreate(
>      IN  PDEVICE_OBJECT      PhysicalDeviceObject
> @@ -2762,13 +2648,6 @@ FdoCreate(
>      Dx = (PXENCONS_DX)FunctionDeviceObject->DeviceExtension;
>      RtlZeroMemory(Dx, sizeof (XENCONS_DX));
> 
> -    status = IoRegisterDeviceInterface(PhysicalDeviceObject,
> -                                       &GUID_XENCONS_DEVICE,
> -                                       NULL,
> -                                       &Dx->Link);
> -    if (!NT_SUCCESS(status))
> -        goto fail2;
> -
>      Dx->Type = FUNCTION_DEVICE_OBJECT;
>      Dx->DeviceObject = FunctionDeviceObject;
>      Dx->DevicePnpState = Added;
> @@ -2845,12 +2724,6 @@ FdoCreate(
>      if (!NT_SUCCESS(status))
>          goto fail11;
> 
> -    status = ConsoleCreate(Fdo, &Fdo->Console);
> -    if (!NT_SUCCESS(status))
> -        goto fail12;
> -
> -    FunctionDeviceObject->Flags |= DO_BUFFERED_IO;
> -
>      Dx->Fdo = Fdo;
> 
>      InitializeMutex(&Fdo->Mutex);
> @@ -2863,13 +2736,13 @@ FdoCreate(
> 
>      status = PdoCreate(Fdo, NULL);
>      if (!NT_SUCCESS(status))
> -        goto fail13;
> +        goto fail12;
> 
>      FunctionDeviceObject->Flags &= ~DO_DEVICE_INITIALIZING;
>      return STATUS_SUCCESS;
> 
> -fail13:
> -    Error("fail13\n");
> +fail12:
> +    Error("fail12\n");
> 
>      Dx->Fdo = Fdo;
> 
> @@ -2877,12 +2750,6 @@ fail13:
>      RtlZeroMemory(&Dx->ListEntry, sizeof(LIST_ENTRY));
>      Fdo->References = 0;
> 
> -    ConsoleDestroy(Fdo->Console);
> -    Fdo->Console = NULL;
> -
> -fail12:
> -    Error("fail12\n");
> -
>      RtlZeroMemory(&Fdo->ConsoleInterface,
>                    sizeof(XENBUS_CONSOLE_INTERFACE));
> 
> @@ -2944,11 +2811,6 @@ fail4:
>  fail3:
>      Error("fail3\n");
> 
> -    RtlFreeUnicodeString(&Dx->Link);
> -
> -fail2:
> -    Error("fail2\n");
> -
>      IoDeleteDevice(FunctionDeviceObject);
> 
>  fail1:
> @@ -2979,9 +2841,6 @@ FdoDestroy(
> 
>      Dx->Fdo = NULL;
> 
> -    ConsoleDestroy(Fdo->Console);
> -    Fdo->Console = NULL;
> -
>      RtlZeroMemory(&Fdo->ConsoleInterface,
>                    sizeof (XENBUS_CONSOLE_INTERFACE));
> 
> @@ -3016,9 +2875,5 @@ FdoDestroy(
>      ASSERT(IsZeroMemory(Fdo, sizeof (XENCONS_FDO)));
>      __FdoFree(Fdo);
> 
> -    RtlFreeUnicodeString(&Dx->Link);
> -
>      IoDeleteDevice(FunctionDeviceObject);
>  }
> -
> -#pragma warning(pop)
> diff --git a/src/xencons/pdo.c b/src/xencons/pdo.c
> index 5e90844..077c7e2 100755
> --- a/src/xencons/pdo.c
> +++ b/src/xencons/pdo.c
> @@ -37,12 +37,14 @@
>  #include <stdlib.h>
> 
>  #include <suspend_interface.h>
> +#include <xencons_device.h>
>  #include <version.h>
> 
>  #include "driver.h"
>  #include "names.h"
>  #include "fdo.h"
>  #include "pdo.h"
> +#include "console.h"
>  #include "thread.h"
>  #include "dbg_print.h"
>  #include "assert.h"
> @@ -68,6 +70,8 @@ struct _XENCONS_PDO {
> 
>      XENBUS_SUSPEND_INTERFACE    SuspendInterface;
>      PXENBUS_SUSPEND_CALLBACK    SuspendCallbackLate;
> +
> +    PXENCONS_CONSOLE            Console;
>  };
> 
>  static FORCEINLINE PVOID
> @@ -437,10 +441,27 @@ PdoD3ToD0(
> 
>      KeLowerIrql(Irql);
> 
> +    status = ConsoleD3ToD0(Pdo->Console);
> +    if (!NT_SUCCESS(status))
> +        goto fail4;
> +
> +#pragma prefast(suppress:28123)
> +    (VOID) IoSetDeviceInterfaceState(&Pdo->Dx->Link, TRUE);
> +
>      Trace("(%s) <====\n", __PdoGetName(Pdo));
> 
>      return STATUS_SUCCESS;
> 
> +fail4:
> +    Error("fail4\n");
> +
> +    KeRaiseIrql(DISPATCH_LEVEL, &Irql);
> +
> +    XENBUS_SUSPEND(Deregister,
> +                   &Pdo->SuspendInterface,
> +                   Pdo->SuspendCallbackLate);
> +    Pdo->SuspendCallbackLate = NULL;
> +
>  fail3:
>      Error("fail3\n");
> 
> @@ -471,6 +492,11 @@ PdoD0ToD3(
> 
>      ASSERT3U(KeGetCurrentIrql(), == , PASSIVE_LEVEL);
> 
> +#pragma prefast(suppress:28123)
> +    (VOID) IoSetDeviceInterfaceState(&Pdo->Dx->Link, FALSE);
> +
> +    ConsoleD0ToD3(Pdo->Console);
> +
>      KeRaiseIrql(DISPATCH_LEVEL, &Irql);
> 
>      XENBUS_SUSPEND(Deregister,
> @@ -527,10 +553,21 @@ PdoStartDevice(
>  {
>      NTSTATUS            status;
> 
> -    status = PdoD3ToD0(Pdo);
> +    if (Pdo->Dx->Link.Length != 0)
> +        goto done;
> +
> +    status = IoRegisterDeviceInterface(__PdoGetDeviceObject(Pdo),
> +                                       &GUID_XENCONS_DEVICE,
> +                                       NULL,
> +                                       &Pdo->Dx->Link);
>      if (!NT_SUCCESS(status))
>          goto fail1;
> 
> +done:
> +    status = PdoD3ToD0(Pdo);
> +    if (!NT_SUCCESS(status))
> +        goto fail2;
> +
>      __PdoSetDevicePnpState(Pdo, Started);
> 
>      Irp->IoStatus.Status = STATUS_SUCCESS;
> @@ -538,6 +575,9 @@ PdoStartDevice(
> 
>      return STATUS_SUCCESS;
> 
> +fail2:
> +    Error("fail2\n");
> +
>  fail1:
>      Error("fail1 (%08x)\n", status);
> 
> @@ -1639,6 +1679,85 @@ PdoDispatchPower(
>  }
> 
>  static DECLSPEC_NOINLINE NTSTATUS
> +PdoDispatchCreate(
> +    IN  PXENCONS_PDO    Pdo,
> +    IN  PIRP            Irp
> +    )
> +{
> +    PIO_STACK_LOCATION  StackLocation;
> +    NTSTATUS            status;
> +
> +    StackLocation = IoGetCurrentIrpStackLocation(Irp);
> +
> +    status = ConsoleOpen(Pdo->Console, StackLocation->FileObject);
> +
> +    Irp->IoStatus.Status = status;
> +    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> +
> +    return status;
> +}
> +
> +static DECLSPEC_NOINLINE NTSTATUS
> +PdoDispatchCleanup(
> +    IN  PXENCONS_PDO    Pdo,
> +    IN  PIRP            Irp
> +    )
> +{
> +    PIO_STACK_LOCATION  StackLocation;
> +    NTSTATUS            status;
> +
> +    StackLocation = IoGetCurrentIrpStackLocation(Irp);
> +
> +    status = ConsoleClose(Pdo->Console, StackLocation->FileObject);
> +
> +    Irp->IoStatus.Status = status;
> +    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> +
> +    return status;
> +}
> +
> +static DECLSPEC_NOINLINE NTSTATUS
> +PdoDispatchClose(
> +    IN  PXENCONS_PDO    Pdo,
> +    IN  PIRP            Irp
> +    )
> +{
> +    NTSTATUS            status;
> +
> +    UNREFERENCED_PARAMETER(Pdo);
> +
> +    status = STATUS_SUCCESS;
> +
> +    Irp->IoStatus.Status = status;
> +    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> +
> +    return status;
> +}
> +
> +static DECLSPEC_NOINLINE NTSTATUS
> +PdoDispatchReadWrite(
> +    IN  PXENCONS_PDO    Pdo,
> +    IN  PIRP            Irp
> +    )
> +{
> +    NTSTATUS            status;
> +
> +    status = ConsolePutQueue(Pdo->Console, Irp);
> +    if (status != STATUS_PENDING)
> +        goto fail1;
> +
> +    return STATUS_PENDING;
> +
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    Irp->IoStatus.Status = status;
> +    IoCompleteRequest(Irp, IO_NO_INCREMENT);
> +
> +    return status;
> +}
> +
> +static DECLSPEC_NOINLINE NTSTATUS
>  PdoDispatchDefault(
>      IN  PXENCONS_PDO    Pdo,
>      IN  PIRP            Irp
> @@ -1674,6 +1793,23 @@ PdoDispatch(
>          status = PdoDispatchPower(Pdo, Irp);
>          break;
> 
> +    case IRP_MJ_CREATE:
> +        status = PdoDispatchCreate(Pdo, Irp);
> +        break;
> +
> +    case IRP_MJ_CLEANUP:
> +        status = PdoDispatchCleanup(Pdo, Irp);
> +        break;
> +
> +    case IRP_MJ_CLOSE:
> +        status = PdoDispatchClose(Pdo, Irp);
> +        break;
> +
> +    case IRP_MJ_READ:
> +    case IRP_MJ_WRITE:
> +        status = PdoDispatchReadWrite(Pdo, Irp);
> +        break;
> +
>      default:
>          status = PdoDispatchDefault(Pdo, Irp);
>          break;
> @@ -1755,31 +1891,42 @@ PdoCreate(
> 
>      Dx->Pdo = Pdo;
> 
> -    status = FdoAddPhysicalDeviceObject(Fdo, Pdo);
> +    status = ConsoleCreate(Fdo, &Pdo->Console);
>      if (!NT_SUCCESS(status))
>          goto fail5;
> 
> +    status = FdoAddPhysicalDeviceObject(Fdo, Pdo);
> +    if (!NT_SUCCESS(status))
> +        goto fail6;
> +
>      status = STATUS_UNSUCCESSFUL;
>      if (__PdoIsEjectRequested(Pdo))
> -        goto fail6;
> +        goto fail7;
> 
>      Info("%p (%s)\n",
>           PhysicalDeviceObject,
>           __PdoGetName(Pdo));
> 
> +    PhysicalDeviceObject->Flags |= DO_BUFFERED_IO;
>      PhysicalDeviceObject->Flags &= ~DO_DEVICE_INITIALIZING;
>      return STATUS_SUCCESS;
> 
> +fail7:
> +    Error("fail7\n");
> +
> +    FdoRemovePhysicalDeviceObject(Fdo, Pdo);
> +
>  fail6:
>      Error("fail6\n");
> 
> -    FdoRemovePhysicalDeviceObject(Fdo, Pdo);
> +    (VOID)__PdoClearEjectRequested(Pdo);
> +
> +    ConsoleDestroy(Pdo->Console);
> +    Pdo->Console = NULL;
> 
>  fail5:
>      Error("fail5\n");
> 
> -    (VOID)__PdoClearEjectRequested(Pdo);
> -
>      Dx->Pdo = NULL;
> 
>      RtlZeroMemory(&Pdo->SuspendInterface,
> @@ -1843,6 +1990,11 @@ PdoDestroy(
> 
>      Dx->Pdo = NULL;
> 
> +    ConsoleDestroy(Pdo->Console);
> +    Pdo->Console = NULL;
> +
> +    RtlFreeUnicodeString(&Pdo->Dx->Link);
> +
>      RtlZeroMemory(&Pdo->SuspendInterface,
>                    sizeof(XENBUS_SUSPEND_INTERFACE));
> 
> --
> 2.8.3
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel