Re: [win-pv-devel] [PATCH 1/3] Add foreign page mapping functions to the GNTTAB interface

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx [mailto:win-pv-devel-
> bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf Of Rafal Wojdyla
> Sent: 11 September 2015 13:30
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: [win-pv-devel] [PATCH 1/3] Add foreign page mapping functions to
> the GNTTAB interface
> 
> GNTTAB interface now includes functions to map and unmap memory pages
> granted by a foreign domain. The page(s) are mapped under an address
> allocated from the PCI BAR space.
> 
> Signed-off-by: Rafal Wojdyla <omeg@xxxxxxxxxxxxxxxxxxxxxx>

There's one or two minor stylistic issues but I'll fix those up myself.

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

> ---
>  include/gnttab_interface.h |  54 ++++++++++++++-
>  include/xen.h              |  19 ++++++
>  src/xen/grant_table.c      | 116 +++++++++++++++++++++++++++++++
>  src/xenbus/gnttab.c        | 165
> +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 352 insertions(+), 2 deletions(-)
> 
> diff --git a/include/gnttab_interface.h b/include/gnttab_interface.h
> index d29440a..b0f4adf 100644
> --- a/include/gnttab_interface.h
> +++ b/include/gnttab_interface.h
> @@ -163,6 +163,39 @@ typedef VOID
>      IN  PXENBUS_GNTTAB_CACHE    Cache
>      );
> 
> +/*! \typedef XENBUS_GNTTAB_MAP_FOREIGN_PAGES
> +    \brief Map foreign memory pages into the system address space
> +
> +    \param Interface The interface header
> +    \param Domain The domid of the foreign domain that granted the pages
> +    \param NumberPages Number of pages to map
> +    \param References Array of grant reference numbers shared by the
> foreign domain
> +    \param ReadOnly If TRUE, pages are mapped with read-only access
> +    \param Address The physical address that the foreign pages are mapped
> under
> +*/
> +
> +typedef NTSTATUS
> +(*XENBUS_GNTTAB_MAP_FOREIGN_PAGES)(
> +    IN  PINTERFACE              Interface,
> +    IN  USHORT                  Domain,
> +    IN  ULONG                   NumberPages,
> +    IN  PULONG                  References,
> +    IN  BOOLEAN                 ReadOnly,
> +    OUT PHYSICAL_ADDRESS        *Address
> +    );
> +
> +/*! \typedef XENBUS_GNTTAB_UNMAP_FOREIGN_PAGES
> +    \brief Unmap foreign memory pages from the system address space
> +
> +    \param Interface The interface header
> +    \param Address The physical address that the foreign pages are mapped
> under
> +*/
> +typedef NTSTATUS
> +(*XENBUS_GNTTAB_UNMAP_FOREIGN_PAGES)(
> +    IN  PINTERFACE              Interface,
> +    IN  PHYSICAL_ADDRESS        Address
> +    );
> +
>  // {763679C5-E5C2-4A6D-8B88-6BB02EC42D8E}
>  DEFINE_GUID(GUID_XENBUS_GNTTAB_INTERFACE,
>  0x763679c5, 0xe5c2, 0x4a6d, 0x8b, 0x88, 0x6b, 0xb0, 0x2e, 0xc4, 0x2d, 0x8e);
> @@ -182,7 +215,24 @@ struct _XENBUS_GNTTAB_INTERFACE_V1 {
>      XENBUS_GNTTAB_DESTROY_CACHE         GnttabDestroyCache;
>  };
> 
> -typedef struct _XENBUS_GNTTAB_INTERFACE_V1
> XENBUS_GNTTAB_INTERFACE, *PXENBUS_GNTTAB_INTERFACE;
> +/*! \struct _XENBUS_GNTTAB_INTERFACE_V2
> +    \brief GNTTAB interface version 2
> +    \ingroup interfaces
> +*/
> +struct _XENBUS_GNTTAB_INTERFACE_V2 {
> +    INTERFACE                           Interface;
> +    XENBUS_GNTTAB_ACQUIRE               GnttabAcquire;
> +    XENBUS_GNTTAB_RELEASE               GnttabRelease;
> +    XENBUS_GNTTAB_CREATE_CACHE          GnttabCreateCache;
> +    XENBUS_GNTTAB_PERMIT_FOREIGN_ACCESS
> GnttabPermitForeignAccess;
> +    XENBUS_GNTTAB_REVOKE_FOREIGN_ACCESS
> GnttabRevokeForeignAccess;
> +    XENBUS_GNTTAB_GET_REFERENCE         GnttabGetReference;
> +    XENBUS_GNTTAB_DESTROY_CACHE         GnttabDestroyCache;
> +    XENBUS_GNTTAB_MAP_FOREIGN_PAGES     GnttabMapForeignPages;
> +    XENBUS_GNTTAB_UNMAP_FOREIGN_PAGES
> GnttabUnmapForeignPages;
> +};
> +
> +typedef struct _XENBUS_GNTTAB_INTERFACE_V2
> XENBUS_GNTTAB_INTERFACE, *PXENBUS_GNTTAB_INTERFACE;
> 
>  /*! \def XENBUS_GNTTAB
>      \brief Macro at assist in method invocation
> @@ -193,7 +243,7 @@ typedef struct _XENBUS_GNTTAB_INTERFACE_V1
> XENBUS_GNTTAB_INTERFACE, *PXENBUS_GNT
>  #endif  // _WINDLL
> 
>  #define XENBUS_GNTTAB_INTERFACE_VERSION_MIN 1
> -#define XENBUS_GNTTAB_INTERFACE_VERSION_MAX 1
> +#define XENBUS_GNTTAB_INTERFACE_VERSION_MAX 2
> 
>  #endif  // _XENBUS_GNTTAB_INTERFACE_H
> 
> diff --git a/include/xen.h b/include/xen.h
> index 6007582..23c7ac0 100644
> --- a/include/xen.h
> +++ b/include/xen.h
> @@ -258,6 +258,25 @@ GrantTableCopy(
>      IN  ULONG               Count
>      );
> 
> +__checkReturn
> +XEN_API
> +NTSTATUS
> +GrantTableMapForeignPage(
> +    IN  USHORT                  Domain,
> +    IN  ULONG                   GrantRef,
> +    IN  PHYSICAL_ADDRESS        Address,
> +    IN  BOOLEAN                 ReadOnly,
> +    OUT ULONG                   *Handle
> +    );
> +
> +__checkReturn
> +XEN_API
> +NTSTATUS
> +GrantTableUnmapForeignPage(
> +    IN  ULONG                   Handle,
> +    IN  PHYSICAL_ADDRESS        Address
> +    );
> +
>  // SCHED
> 
>  __checkReturn
> diff --git a/src/xen/grant_table.c b/src/xen/grant_table.c
> index 6facb3f..6b00f65 100644
> --- a/src/xen/grant_table.c
> +++ b/src/xen/grant_table.c
> @@ -38,6 +38,35 @@
>  #include "dbg_print.h"
>  #include "assert.h"
> 
> +#pragma warning(disable:4127)   // conditional expression is constant
> +
> +// Most of the GNTST_* values don't have meaningful NTSTATUS
> counterparts,
> +// this macro translates those that do.
> +#define GNTST_TO_STATUS(_gntst, _status)                    \
> +        do {                                                \
> +            switch (_gntst) {                               \
> +            case GNTST_okay:                                \
> +                _status = STATUS_SUCCESS;                   \
> +                break;                                      \
> +                                                            \
> +            case GNTST_bad_handle:                          \
> +                _status = STATUS_INVALID_HANDLE;            \
> +                break;                                      \
> +                                                            \
> +            case GNTST_permission_denied:                   \
> +                _status = STATUS_ACCESS_DENIED;             \
> +                break;                                      \
> +                                                            \
> +            case GNTST_eagain:                              \
> +                _status = STATUS_RETRY;                     \
> +                break;                                      \
> +                                                            \
> +            default:                                        \
> +                _status = STATUS_UNSUCCESSFUL;              \
> +                break;                                      \
> +            }                                               \
> +        } while (FALSE)
> +
>  static LONG_PTR
>  GrantTableOp(
>      IN  ULONG   Command,
> @@ -131,3 +160,90 @@ fail1:
> 
>      return status;
>  }
> +
> +__checkReturn
> +XEN_API
> +NTSTATUS
> +GrantTableMapForeignPage(
> +    IN  USHORT                  Domain,
> +    IN  ULONG                   GrantRef,
> +    IN  PHYSICAL_ADDRESS        Address,
> +    IN  BOOLEAN                 ReadOnly,
> +    OUT ULONG                   *Handle
> +    )
> +{
> +    struct gnttab_map_grant_ref op;
> +    LONG_PTR                    rc;
> +    NTSTATUS                    status;
> +
> +    RtlZeroMemory(&op, sizeof(op));
> +    op.dom = Domain;
> +    op.ref = GrantRef;
> +    op.flags = GNTMAP_host_map;
> +    if (ReadOnly)
> +        op.flags |= GNTMAP_readonly;
> +    op.host_addr = Address.QuadPart;
> +
> +    rc = GrantTableOp(GNTTABOP_map_grant_ref, &op, 1);
> +
> +    if (rc < 0) {
> +        ERRNO_TO_STATUS(-rc, status);
> +        goto fail1;
> +    }
> +
> +    if (op.status != GNTST_okay) {
> +        GNTST_TO_STATUS(op.status, status);
> +        Error("hypercall status: %d\n", op.status);
> +        goto fail2;
> +    }
> +
> +    *Handle = op.handle;
> +
> +    return STATUS_SUCCESS;
> +
> +fail2:
> +    Error("fail2\n");
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
> +}
> +
> +__checkReturn
> +XEN_API
> +NTSTATUS
> +GrantTableUnmapForeignPage(
> +    IN  ULONG                     Handle,
> +    IN  PHYSICAL_ADDRESS          Address
> +    )
> +{
> +    struct gnttab_unmap_grant_ref op;
> +    LONG_PTR                      rc;
> +    NTSTATUS                      status;
> +
> +    RtlZeroMemory(&op, sizeof(op));
> +    op.handle = Handle;
> +    op.host_addr = Address.QuadPart;
> +
> +    rc = GrantTableOp(GNTTABOP_unmap_grant_ref, &op, 1);
> +
> +    if (rc < 0) {
> +        ERRNO_TO_STATUS(-rc, status);
> +        goto fail1;
> +    }
> +
> +    if (op.status != GNTST_okay) {
> +        GNTST_TO_STATUS(op.status, status);
> +        Error("hypercall status: %d\n", op.status);
> +        goto fail2;
> +    }
> +
> +    return STATUS_SUCCESS;
> +
> +fail2:
> +    Error("fail2\n");
> +fail1:
> +    Error("fail1 (%08x)\n", status);
> +
> +    return status;
> +}
> diff --git a/src/xenbus/gnttab.c b/src/xenbus/gnttab.c
> index 165e38f..32b43c7 100644
> --- a/src/xenbus/gnttab.c
> +++ b/src/xenbus/gnttab.c
> @@ -40,6 +40,7 @@
>  #include "dbg_print.h"
>  #include "assert.h"
>  #include "util.h"
> +#include "hash_table.h"
> 
>  #define XENBUS_GNTTAB_MAXIMUM_FRAME_COUNT  32
>  #define XENBUS_GNTTAB_ENTRY_PER_FRAME      (PAGE_SIZE / sizeof
> (grant_entry_v1_t))
> @@ -68,6 +69,11 @@ struct _XENBUS_GNTTAB_ENTRY {
>      grant_entry_v1_t    Entry;
>  };
> 
> +typedef struct _XENBUS_GNTTAB_MAP_ENTRY {
> +    ULONG               NumberPages;
> +    ULONG               MapHandles[1];
> +} XENBUS_GNTTAB_MAP_ENTRY, *PXENBUS_GNTTAB_MAP_ENTRY;
> +
>  struct _XENBUS_GNTTAB_CONTEXT {
>      PXENBUS_FDO                 Fdo;
>      KSPIN_LOCK                  Lock;
> @@ -82,6 +88,7 @@ struct _XENBUS_GNTTAB_CONTEXT {
>      PXENBUS_SUSPEND_CALLBACK    SuspendCallbackEarly;
>      XENBUS_DEBUG_INTERFACE      DebugInterface;
>      PXENBUS_DEBUG_CALLBACK      DebugCallback;
> +    PXENBUS_HASH_TABLE          MapTable;
>      LIST_ENTRY                  List;
>  };
> 
> @@ -534,6 +541,124 @@ GnttabGetReference(
>      return (ULONG)Entry->Reference;
>  }
> 
> +static NTSTATUS
> +GnttabMapForeignPages(
> +    IN  PINTERFACE              Interface,
> +    IN  USHORT                  Domain,
> +    IN  ULONG                   NumberPages,
> +    IN  PULONG                  References,
> +    IN  BOOLEAN                 ReadOnly,
> +    OUT PHYSICAL_ADDRESS        *Address
> +    )
> +{
> +    NTSTATUS                    status;
> +    PXENBUS_GNTTAB_CONTEXT      Context = Interface->Context;
> +    ULONG                       PageIndex;
> +    PHYSICAL_ADDRESS            PageAddress;
> +    PXENBUS_GNTTAB_MAP_ENTRY    MapEntry;
> +
> +    status = FdoAllocateIoSpace(Context->Fdo, NumberPages * PAGE_SIZE,
> Address);
> +    if (!NT_SUCCESS(status))
> +        goto fail1;
> +
> +    status = STATUS_INSUFFICIENT_RESOURCES;
> +    MapEntry =
> __GnttabAllocate(FIELD_OFFSET(XENBUS_GNTTAB_MAP_ENTRY,
> MapHandles) + NumberPages*sizeof(ULONG));
> +    if (!MapEntry)
> +        goto fail2;
> +
> +    PageAddress.QuadPart = Address->QuadPart;
> +    MapEntry->NumberPages = NumberPages;
> +
> +    for (PageIndex = 0; PageIndex < NumberPages; PageIndex++) {
> +        status = GrantTableMapForeignPage(Domain,
> +                                          References[PageIndex],
> +                                          PageAddress,
> +                                          ReadOnly,
> +                                          &MapEntry->MapHandles[PageIndex]);
> +        if (!NT_SUCCESS(status))
> +            goto fail3;
> +
> +        PageAddress.QuadPart += PAGE_SIZE;
> +    }
> +
> +    status = HashTableAdd(Context->MapTable,
> +                          (ULONG_PTR)Address->QuadPart,
> +                          (ULONG_PTR)MapEntry);
> +
> +    if (!NT_SUCCESS(status))
> +        goto fail4;
> +
> +    return STATUS_SUCCESS;
> +
> +fail4:
> +    Error("fail4\n");
> +
> +fail3:
> +    Error("fail3\n");
> +
> +    while (PageIndex > 0) {
> +        --PageIndex;
> +        PageAddress.QuadPart -= PAGE_SIZE;
> +        ASSERT(NT_SUCCESS(GrantTableUnmapForeignPage(MapEntry-
> >MapHandles[PageIndex], PageAddress)));
> +    }
> +
> +    __GnttabFree(MapEntry);
> +
> +fail2:
> +    Error("fail2\n");
> +    FdoFreeIoSpace(Context->Fdo, *Address, NumberPages * PAGE_SIZE);
> +
> +fail1:
> +    Error("fail1: (%08x)\n", status);
> +    return status;
> +}
> +
> +static NTSTATUS
> +GnttabUnmapForeignPages(
> +    IN  PINTERFACE              Interface,
> +    IN  PHYSICAL_ADDRESS        Address
> +    )
> +{
> +    NTSTATUS                    Status;
> +    PXENBUS_GNTTAB_CONTEXT      Context = Interface->Context;
> +    ULONG                       PageIndex;
> +    PHYSICAL_ADDRESS            PageAddress;
> +    PXENBUS_GNTTAB_MAP_ENTRY    MapEntry;
> +
> +    Status = HashTableLookup(Context->MapTable,
> (ULONG_PTR)Address.QuadPart, (PULONG_PTR)&MapEntry);
> +    if (!NT_SUCCESS(Status))
> +        goto fail1;
> +
> +    Status = HashTableRemove(Context->MapTable,
> (ULONG_PTR)Address.QuadPart);
> +    if (!NT_SUCCESS(Status))
> +        goto fail2;
> +
> +    PageAddress.QuadPart = Address.QuadPart;
> +
> +    for (PageIndex = 0; PageIndex < MapEntry->NumberPages;
> PageIndex++) {
> +        Status = GrantTableUnmapForeignPage(MapEntry-
> >MapHandles[PageIndex], PageAddress);
> +        if (!NT_SUCCESS(Status))
> +            goto fail3;
> +
> +        PageAddress.QuadPart += PAGE_SIZE;
> +    }
> +
> +    FdoFreeIoSpace(Context->Fdo, Address, MapEntry->NumberPages *
> PAGE_SIZE);
> +    __GnttabFree(MapEntry);
> +    return STATUS_SUCCESS;
> +
> +fail3:
> +    Error("fail3\n");
> +    KeBugCheckEx(MEMORY_MANAGEMENT,
> (ULONG_PTR)XENBUS_GNTTAB_TAG, (ULONG_PTR)PageAddress.QuadPart,
> (ULONG_PTR)MapEntry->MapHandles[PageIndex],
> (ULONG_PTR)Address.QuadPart);
> +
> +fail2:
> +    Error("fail2\n");
> +
> +fail1:
> +    Error("fail1: (%08x)\n", Status);
> +    return Status;
> +}
> +
>  static VOID
>  GnttabSuspendCallbackEarly(
>      IN  PVOID               Argument
> @@ -789,6 +914,19 @@ static struct _XENBUS_GNTTAB_INTERFACE_V1
> GnttabInterfaceVersion1 = {
>      GnttabDestroyCache
>  };
> 
> +static struct _XENBUS_GNTTAB_INTERFACE_V2   GnttabInterfaceVersion2 =
> {
> +    { sizeof(struct _XENBUS_GNTTAB_INTERFACE_V2), 2, NULL, NULL, NULL
> },
> +    GnttabAcquire,
> +    GnttabRelease,
> +    GnttabCreateCache,
> +    GnttabPermitForeignAccess,
> +    GnttabRevokeForeignAccess,
> +    GnttabGetReference,
> +    GnttabDestroyCache,
> +    GnttabMapForeignPages,
> +    GnttabUnmapForeignPages
> +};
> +
>  NTSTATUS
>  GnttabInitialize(
>      IN  PXENBUS_FDO             Fdo,
> @@ -836,12 +974,19 @@ GnttabInitialize(
>      InitializeListHead(&(*Context)->List);
>      KeInitializeSpinLock(&(*Context)->Lock);
> 
> +    status = HashTableCreate(&(*Context)->MapTable);
> +    if (!NT_SUCCESS(status))
> +        goto fail2;
> +
>      (*Context)->Fdo = Fdo;
> 
>      Trace("<====\n");
> 
>      return STATUS_SUCCESS;
> 
> +fail2:
> +    Error("fail2\n");
> +
>  fail1:
>      Error("fail1 (%08x)\n", status);
> 
> @@ -878,6 +1023,23 @@ GnttabGetInterface(
>          status = STATUS_SUCCESS;
>          break;
>      }
> +    case 2: {
> +        struct _XENBUS_GNTTAB_INTERFACE_V2  *GnttabInterface;
> +
> +        GnttabInterface = (struct _XENBUS_GNTTAB_INTERFACE_V2
> *)Interface;
> +
> +        status = STATUS_BUFFER_OVERFLOW;
> +        if (Size < sizeof(struct _XENBUS_GNTTAB_INTERFACE_V2))
> +            break;
> +
> +        *GnttabInterface = GnttabInterfaceVersion2;
> +
> +        ASSERT3U(Interface->Version, ==, Version);
> +        Interface->Context = Context;
> +
> +        status = STATUS_SUCCESS;
> +        break;
> +    }
>      default:
>          status = STATUS_NOT_SUPPORTED;
>          break;
> @@ -895,6 +1057,9 @@ GnttabTeardown(
> 
>      Context->Fdo = NULL;
> 
> +    HashTableDestroy(Context->MapTable);
> +    Context->MapTable = NULL;
> +
>      RtlZeroMemory(&Context->Lock, sizeof (KSPIN_LOCK));
>      RtlZeroMemory(&Context->List, sizeof (LIST_ENTRY));
> 
> --
> 1.8.1.msysgit.1
> 
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel

_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel