Re: [win-pv-devel] [PATCH 08/14 v2] Hide console access behind an interface

[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 08/14 v2] Hide console access behind an
> interface
> 
> From: Owen Smith <owen.smith@xxxxxxxxxx>
> 
> * Allows a different implementation of the console protocol for
>   non-default consoles
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

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

> ---
>  src/xencons/console.c     | 174
> +++++++++++++++++++++++++++++++++++++---------
>  src/xencons/console.h     |  40 +----------
>  src/xencons/console_abi.h |  99 ++++++++++++++++++++++++++
>  src/xencons/pdo.c         |  72 +++++++++----------
>  4 files changed, 279 insertions(+), 106 deletions(-)
>  create mode 100755 src/xencons/console_abi.h
> 
> diff --git a/src/xencons/console.c b/src/xencons/console.c
> index b9abe83..50fe145 100755
> --- a/src/xencons/console.c
> +++ b/src/xencons/console.c
> @@ -53,11 +53,12 @@ typedef struct _CONSOLE_HANDLE {
>      PXENCONS_STREAM Stream;
>  } CONSOLE_HANDLE, *PCONSOLE_HANDLE;
> 
> -struct _XENCONS_CONSOLE {
> +typedef struct _XENCONS_CONSOLE {
> +    LONG            RefCount;
>      PXENCONS_FDO    Fdo;
>      LIST_ENTRY      List;
>      KSPIN_LOCK      Lock;
> -};
> +} XENCONS_CONSOLE, *PXENCONS_CONSOLE;
> 
>  static FORCEINLINE PVOID
>  __ConsoleAllocate(
> @@ -136,7 +137,7 @@ fail1:
>      return NULL;
>  }
> 
> -NTSTATUS
> +static NTSTATUS
>  ConsoleOpen(
>      IN  PXENCONS_CONSOLE    Console,
>      IN  PFILE_OBJECT        FileObject
> @@ -178,7 +179,7 @@ fail1:
>      return status;
>  }
> 
> -NTSTATUS
> +static NTSTATUS
>  ConsoleClose(
>      IN  PXENCONS_CONSOLE    Console,
>      IN  PFILE_OBJECT        FileObject
> @@ -317,7 +318,7 @@ fail1:
>      return status;
>  }
> 
> -NTSTATUS
> +static NTSTATUS
>  ConsolePutQueue(
>      IN  PXENCONS_CONSOLE    Console,
>      IN  PIRP                Irp
> @@ -347,7 +348,7 @@ ConsolePutQueue(
>      return status;
>  }
> 
> -NTSTATUS
> +static NTSTATUS
>  ConsoleD3ToD0(
>      IN  PXENCONS_CONSOLE    Console
>      )
> @@ -361,7 +362,7 @@ ConsoleD3ToD0(
>      return STATUS_SUCCESS;
>  }
> 
> -VOID
> +static VOID
>  ConsoleD0ToD3(
>      IN  PXENCONS_CONSOLE    Console
>      )
> @@ -400,52 +401,157 @@ ConsoleD0ToD3(
>      Trace("<====\n");
>  }
> 
> -NTSTATUS
> -ConsoleCreate(
> -    IN  PXENCONS_FDO        Fdo,
> -    OUT PXENCONS_CONSOLE    *Console
> +static VOID
> +ConsoleDestroy(
> +    IN  PXENCONS_CONSOLE    Console
>      )
> -{
> -    NTSTATUS                status;
> -
> +{
>      Trace("====>\n");
> 
> -    *Console = __ConsoleAllocate(sizeof(XENCONS_CONSOLE));
> +    ASSERT(IsListEmpty(&Console->List));
> +    RtlZeroMemory(&Console->List, sizeof(LIST_ENTRY));
> 
> -    status = STATUS_NO_MEMORY;
> -    if (*Console == NULL)
> -        goto fail1;
> +    RtlZeroMemory(&Console->Lock, sizeof(KSPIN_LOCK));
> +
> +    Console->Fdo = NULL;
> 
> -    (*Console)->Fdo = Fdo;
> -    InitializeListHead(&(*Console)->List);
> -    KeInitializeSpinLock(&(*Console)->Lock);
> +    ASSERT(IsZeroMemory(Console, sizeof(XENCONS_CONSOLE)));
> +    __ConsoleFree(Console);
> 
>      Trace("<====\n");
> +}
> +
> +static NTSTATUS
> +ConsoleAbiAcquire(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    InterlockedIncrement(&Console->RefCount);
> 
>      return STATUS_SUCCESS;
> +}
> 
> -fail1:
> -    Error("fail1 (%08x)\n", status);
> +static VOID
> +ConsoleAbiRelease(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +    LONG                            Count;
> 
> -    return status;
> +    Count = InterlockedDecrement(&Console->RefCount);
> +
> +    if (Count == 0)
> +        ConsoleDestroy(Console);
>  }
> 
> -VOID
> -ConsoleDestroy(
> -    IN  PXENCONS_CONSOLE    Console
> +static BOOLEAN
> +ConsoleAbiIsDefault(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
>      )
> -{
> +{
> +    UNREFERENCED_PARAMETER(Context);
> +    return TRUE;
> +}
> +
> +static NTSTATUS
> +ConsoleAbiD3ToD0(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    return ConsoleD3ToD0(Console);
> +}
> +
> +static VOID
> +ConsoleAbiD0ToD3(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    ConsoleD0ToD3(Console);
> +}
> +
> +static NTSTATUS
> +ConsoleAbiOpen(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PFILE_OBJECT                FileObject
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    return ConsoleOpen(Console, FileObject);
> +}
> +
> +static NTSTATUS
> +ConsoleAbiClose(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PFILE_OBJECT                FileObject
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    return ConsoleClose(Console, FileObject);
> +}
> +
> +static NTSTATUS
> +ConsoleAbiPutQueue(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PIRP                        Irp
> +    )
> +{
> +    PXENCONS_CONSOLE                Console = (PXENCONS_CONSOLE)Context;
> +
> +    return ConsolePutQueue(Console, Irp);
> +}
> +
> +static XENCONS_CONSOLE_ABI ConsoleAbiV1 = {
> +    NULL,
> +    ConsoleAbiAcquire,
> +    ConsoleAbiRelease,
> +    ConsoleAbiIsDefault,
> +    ConsoleAbiD3ToD0,
> +    ConsoleAbiD0ToD3,
> +    ConsoleAbiOpen,
> +    ConsoleAbiClose,
> +    ConsoleAbiPutQueue
> +};
> +
> +NTSTATUS
> +ConsoleCreate(
> +    IN  PXENCONS_FDO            Fdo,
> +    IN  PXENCONS_CONSOLE_ABI    ConsoleAbi
> +    )
> +{
> +    PXENCONS_CONSOLE            Console;
> +    NTSTATUS                    status;
> +
>      Trace("====>\n");
> 
> -    ASSERT(IsListEmpty(&Console->List));
> -    RtlZeroMemory(&Console->List, sizeof(LIST_ENTRY));
> +    Console = __ConsoleAllocate(sizeof(XENCONS_CONSOLE));
> 
> -    RtlZeroMemory(&Console->Lock, sizeof(KSPIN_LOCK));
> +    status = STATUS_NO_MEMORY;
> +    if (Console == NULL)
> +        goto fail1;
> 
> -    Console->Fdo = NULL;
> +    Console->RefCount = 1;
> +    Console->Fdo = Fdo;
> +    InitializeListHead(&Console->List);
> +    KeInitializeSpinLock(&Console->Lock);
> 
> -    ASSERT(IsZeroMemory(Console, sizeof(XENCONS_CONSOLE)));
> -    __ConsoleFree(Console);
> +    *ConsoleAbi = ConsoleAbiV1;
> +    ConsoleAbi->Context = (PXENCONS_CONSOLE_CONTEXT)Console;
> 
>      Trace("<====\n");
> +
> +    return STATUS_SUCCESS;
> +
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
>  }
> diff --git a/src/xencons/console.h b/src/xencons/console.h
> index 1094f82..81a7c13 100755
> --- a/src/xencons/console.h
> +++ b/src/xencons/console.h
> @@ -35,46 +35,12 @@
>  #include <ntddk.h>
> 
>  #include "driver.h"
> -
> -typedef struct _XENCONS_CONSOLE XENCONS_CONSOLE,
> *PXENCONS_CONSOLE;
> +#include "console_abi.h"
> 
>  extern NTSTATUS
>  ConsoleCreate(
> -    IN  PXENCONS_FDO        Fdo,
> -    OUT PXENCONS_CONSOLE    *Console
> -    );
> -
> -extern VOID
> -ConsoleDestroy(
> -    IN  PXENCONS_CONSOLE    Console
> -    );
> -
> -extern NTSTATUS
> -ConsoleD3ToD0(
> -    IN  PXENCONS_CONSOLE    Console
> -    );
> -
> -extern VOID
> -ConsoleD0ToD3(
> -    IN  PXENCONS_CONSOLE    Console
> -    );
> -
> -extern NTSTATUS
> -ConsoleOpen(
> -    IN  PXENCONS_CONSOLE    Console,
> -    IN  PFILE_OBJECT        FileObject
> -    );
> -
> -extern NTSTATUS
> -ConsoleClose(
> -    IN  PXENCONS_CONSOLE    Console,
> -    IN  PFILE_OBJECT        FileObject
> -    );
> -
> -extern NTSTATUS
> -ConsolePutQueue(
> -    IN  PXENCONS_CONSOLE    Console,
> -    IN  PIRP                Irp
> +    IN  PXENCONS_FDO            Fdo,
> +    IN  PXENCONS_CONSOLE_ABI    ConsoleAbi
>      );
> 
>  #endif  // _XENCONS_CONSOLE_H
> diff --git a/src/xencons/console_abi.h b/src/xencons/console_abi.h
> new file mode 100755
> index 0000000..ab8d800
> --- /dev/null
> +++ b/src/xencons/console_abi.h
> @@ -0,0 +1,99 @@
> +/* Copyright (c) Citrix Systems Inc.
> +* All rights reserved.
> +*
> +* Redistribution and use in source and binary forms,
> +* with or without modification, are permitted provided
> +* that the following conditions are met:
> +*
> +* *   Redistributions of source code must retain the above
> +*     copyright notice, this list of conditions and the
> +*     following disclaimer.
> +* *   Redistributions in binary form must reproduce the above
> +*     copyright notice, this list of conditions and the
> +*     following disclaimer in the documentation and/or other
> +*     materials provided with the distribution.
> +*
> +* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> +* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
> +* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
> +* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
> +* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
> +* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> +* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> +* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> +* WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
> +* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +* SUCH DAMAGE.
> +*/
> +
> +#ifndef _XENCONS_CONSOLE_ABI_H
> +#define _XENCONS_CONSOLE_ABI_H
> +
> +#include <ntddk.h>
> +
> +#include "driver.h"
> +
> +typedef PVOID *PXENCONS_CONSOLE_CONTEXT;
> +
> +typedef NTSTATUS
> +(*XENCONS_CONSOLE_ABI_ACQUIRE)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    );
> +
> +typedef VOID
> +(*XENCONS_CONSOLE_ABI_RELEASE)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    );
> +
> +typedef BOOLEAN
> +(*XENCONS_CONSOLE_ABI_IS_DEFAULT)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    );
> +
> +typedef NTSTATUS
> +(*XENCONS_CONSOLE_ABI_D3TOD0)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    );
> +
> +typedef VOID
> +(*XENCONS_CONSOLE_ABI_D0TOD3)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context
> +    );
> +
> +typedef NTSTATUS
> +(*XENCONS_CONSOLE_ABI_OPEN)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PFILE_OBJECT                FileObject
> +    );
> +
> +typedef NTSTATUS
> +(*XENCONS_CONSOLE_ABI_CLOSE)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PFILE_OBJECT                FileObject
> +    );
> +
> +typedef NTSTATUS
> +(*XENCONS_CONSOLE_ABI_PUT_QUEUE)(
> +    IN  PXENCONS_CONSOLE_CONTEXT    Context,
> +    IN  PIRP                        Irp
> +    );
> +
> +typedef struct _XENCONS_CONSOLE_ABI {
> +    PXENCONS_CONSOLE_CONTEXT                Context;
> +    XENCONS_CONSOLE_ABI_ACQUIRE             ConsoleAbiAcquire;
> +    XENCONS_CONSOLE_ABI_RELEASE             ConsoleAbiRelease;
> +    XENCONS_CONSOLE_ABI_IS_DEFAULT          ConsoleAbiIsDefault;
> +    XENCONS_CONSOLE_ABI_D3TOD0              ConsoleAbiD3ToD0;
> +    XENCONS_CONSOLE_ABI_D0TOD3              ConsoleAbiD0ToD3;
> +    XENCONS_CONSOLE_ABI_OPEN                ConsoleAbiOpen;
> +    XENCONS_CONSOLE_ABI_CLOSE               ConsoleAbiClose;
> +    XENCONS_CONSOLE_ABI_PUT_QUEUE           ConsoleAbiPutQueue;
> +} XENCONS_CONSOLE_ABI, *PXENCONS_CONSOLE_ABI;
> +
> +#define XENCONS_CONSOLE_ABI(_Method, _Abi, ...)   \
> +    (_Abi)->ConsoleAbi ## _Method((_Abi)->Context, __VA_ARGS__)
> +
> +#endif  // _XENCONS_CONSOLE_ABI_H
> diff --git a/src/xencons/pdo.c b/src/xencons/pdo.c
> index 2acd86a..74be759 100755
> --- a/src/xencons/pdo.c
> +++ b/src/xencons/pdo.c
> @@ -44,6 +44,7 @@
>  #include "names.h"
>  #include "fdo.h"
>  #include "pdo.h"
> +#include "console_abi.h"
>  #include "console.h"
>  #include "thread.h"
>  #include "dbg_print.h"
> @@ -71,8 +72,7 @@ struct _XENCONS_PDO {
>      XENBUS_SUSPEND_INTERFACE    SuspendInterface;
>      PXENBUS_SUSPEND_CALLBACK    SuspendCallbackLate;
> 
> -    BOOLEAN                     IsDefault;
> -    PXENCONS_CONSOLE            Console;
> +    XENCONS_CONSOLE_ABI         ConsoleAbi;
>  };
> 
>  static FORCEINLINE PVOID
> @@ -308,7 +308,7 @@ __PdoIsDefault(
>      IN  PXENCONS_PDO    Pdo
>      )
>  {
> -    return Pdo->IsDefault;
> +    return XENCONS_CONSOLE_ABI(IsDefault, &Pdo->ConsoleAbi);
>  }
> 
>  BOOLEAN
> @@ -466,10 +466,7 @@ PdoD3ToD0(
> 
>      KeLowerIrql(Irql);
> 
> -    if (__PdoIsDefault(Pdo))
> -        status = ConsoleD3ToD0(Pdo->Console);
> -    else
> -        status = STATUS_SUCCESS;
> +    status = XENCONS_CONSOLE_ABI(D3ToD0, &Pdo->ConsoleAbi);
>      if (!NT_SUCCESS(status))
>          goto fail4;
> 
> @@ -523,8 +520,7 @@ PdoD0ToD3(
>  #pragma prefast(suppress:28123)
>      (VOID) IoSetDeviceInterfaceState(&Pdo->Dx->Link, FALSE);
> 
> -    if (__PdoIsDefault(Pdo))
> -        ConsoleD0ToD3(Pdo->Console);
> +    XENCONS_CONSOLE_ABI(D0ToD3, &Pdo->ConsoleAbi);
> 
>      KeRaiseIrql(DISPATCH_LEVEL, &Irql);
> 
> @@ -1718,10 +1714,9 @@ PdoDispatchCreate(
> 
>      StackLocation = IoGetCurrentIrpStackLocation(Irp);
> 
> -    if (__PdoIsDefault(Pdo))
> -        status = ConsoleOpen(Pdo->Console, StackLocation->FileObject);
> -    else
> -        status = STATUS_SUCCESS;
> +    status = XENCONS_CONSOLE_ABI(Open,
> +                                 &Pdo->ConsoleAbi,
> +                                 StackLocation->FileObject);
> 
>      Irp->IoStatus.Status = status;
>      IoCompleteRequest(Irp, IO_NO_INCREMENT);
> @@ -1740,10 +1735,9 @@ PdoDispatchCleanup(
> 
>      StackLocation = IoGetCurrentIrpStackLocation(Irp);
> 
> -    if (__PdoIsDefault(Pdo))
> -        status = ConsoleClose(Pdo->Console, StackLocation->FileObject);
> -    else
> -        status = STATUS_SUCCESS;
> +    status = XENCONS_CONSOLE_ABI(Close,
> +                                 &Pdo->ConsoleAbi,
> +                                 StackLocation->FileObject);
> 
>      Irp->IoStatus.Status = status;
>      IoCompleteRequest(Irp, IO_NO_INCREMENT);
> @@ -1777,12 +1771,12 @@ PdoDispatchReadWriteControl(
>  {
>      NTSTATUS            status;
> 
> -    if (__PdoIsDefault(Pdo))
> -        status = ConsolePutQueue(Pdo->Console, Irp);
> -    else
> -        status = STATUS_DEVICE_NOT_READY;
> +    status = XENCONS_CONSOLE_ABI(PutQueue,
> +                                 &Pdo->ConsoleAbi,
> +                                 Irp);
>      if (status == STATUS_SUCCESS)
>          goto done;
> +
>      if (status != STATUS_PENDING)
>          goto fail1;
> 
> @@ -1865,9 +1859,21 @@ PdoResume(
>      IN  PXENCONS_PDO    Pdo
>      )
>  {
> +    NTSTATUS            status;
> +
>      Trace("(%s) ====>\n", __PdoGetName(Pdo));
> +
> +    status = XENCONS_CONSOLE_ABI(Acquire, &Pdo->ConsoleAbi);
> +    if (!NT_SUCCESS(status))
> +        goto fail1;
> +
>      Trace("(%s) <====\n", __PdoGetName(Pdo));
>      return STATUS_SUCCESS;
> +
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
>  }
> 
>  VOID
> @@ -1876,6 +1882,9 @@ PdoSuspend(
>      )
>  {
>      Trace("(%s) ====>\n", __PdoGetName(Pdo));
> +
> +    XENCONS_CONSOLE_ABI(Release, &Pdo->ConsoleAbi);
> +
>      Trace("(%s) <====\n", __PdoGetName(Pdo));
>  }
> 
> @@ -1933,11 +1942,10 @@ PdoCreate(
> 
>      Dx->Pdo = Pdo;
> 
> -    Pdo->IsDefault = (Device == NULL);
> -    if (Pdo->IsDefault)
> -        status = ConsoleCreate(Fdo, &Pdo->Console);
> +    if (Device == NULL)
> +        status = ConsoleCreate(Fdo, &Pdo->ConsoleAbi);
>      else
> -        status = STATUS_SUCCESS;
> +        status = STATUS_SUCCESS;// TODO: create frontend
>      if (!NT_SUCCESS(status))
>          goto fail5;
> 
> @@ -1967,11 +1975,8 @@ fail6:
> 
>      (VOID)__PdoClearEjectRequested(Pdo);
> 
> -    if (__PdoIsDefault(Pdo))
> -        ConsoleDestroy(Pdo->Console);
> -    Pdo->Console = NULL;
> -
> -    Pdo->IsDefault = FALSE;
> +    XENCONS_CONSOLE_ABI(Release, &Pdo->ConsoleAbi);
> +    RtlZeroMemory(&Pdo->ConsoleAbi, sizeof(XENCONS_CONSOLE_ABI));
> 
>  fail5:
>      Error("fail5\n");
> @@ -2039,11 +2044,8 @@ PdoDestroy(
> 
>      Dx->Pdo = NULL;
> 
> -    if (__PdoIsDefault(Pdo))
> -        ConsoleDestroy(Pdo->Console);
> -    Pdo->Console = NULL;
> -
> -    Pdo->IsDefault = FALSE;
> +    XENCONS_CONSOLE_ABI(Release, &Pdo->ConsoleAbi);
> +    RtlZeroMemory(&Pdo->ConsoleAbi, sizeof(XENCONS_CONSOLE_ABI));
> 
>      RtlFreeUnicodeString(&Pdo->Dx->Link);
> 
> --
> 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