diff mbox

[v12,10/11] common: add a new mappable resource type: XENMEM_resource_grant_table

Message ID 20171017132432.24093-11-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant Oct. 17, 2017, 1:24 p.m. UTC
This patch allows grant table frames to be mapped using the
XENMEM_acquire_resource memory op.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

v12:
 - Dropped limit checks as requested by Jan.

v10:
 - Addressed comments from Jan.

v8:
 - The functionality was originally incorporated into the earlier patch
   "x86/mm: add HYPERVISOR_memory_op to acquire guest resources".
---
 xen/common/grant_table.c      | 49 ++++++++++++++++++++++++++++++++++++++-----
 xen/common/memory.c           | 45 ++++++++++++++++++++++++++++++++++++++-
 xen/include/public/memory.h   |  6 ++++++
 xen/include/xen/grant_table.h |  4 ++++
 4 files changed, 98 insertions(+), 6 deletions(-)

Comments

Jan Beulich Oct. 26, 2017, 3:46 p.m. UTC | #1
>>> On 17.10.17 at 15:24, <paul.durrant@citrix.com> wrote:
> v12:
>  - Dropped limit checks as requested by Jan.

Thanks, but ...

> +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
> +                            mfn_t *mfn)
> +{
> +    struct grant_table *gt = d->grant_table;
> +    int rc;
> +
> +    /* write lock required as version may change and/or table may grow */
> +    grant_write_lock(gt);
> +    rc = gnttab_get_frame(d, idx | XENMAPIDX_grant_table_status, mfn);

... this ORing won't work then. You need to pass the function a
separate bool, which would likely call for a prereq patch.

> @@ -965,12 +966,49 @@ static long xatp_permission_check(struct domain *d, unsigned int space)
>      return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
>  }
>  
> +static int acquire_grant_table(struct domain *d, unsigned int id,
> +                               unsigned long frame,
> +                               unsigned int nr_frames,
> +                               unsigned long mfn_list[])
> +{
> +    unsigned int i = nr_frames;
> +
> +    /* Iterate backwards in case table needs to grow */
> +    while ( i-- != 0 )
> +    {
> +        mfn_t mfn = INVALID_MFN;
> +        int rc;
> +
> +        switch ( id )
> +        {
> +        case XENMEM_resource_grant_table_id_grant:
> +            rc = gnttab_get_grant_frame(d, frame + i, &mfn);
> +            break;
> +
> +        case XENMEM_resource_grant_table_id_status:
> +            rc = gnttab_get_status_frame(d, frame + i, &mfn);
> +            break;
> +
> +        default:
> +            rc = -EINVAL;
> +            break;
> +        }
> +
> +        if ( rc )
> +            return rc;
> +
> +        mfn_list[i] = mfn_x(mfn);
> +    }
> +
> +    return 0;
> +}
> +
>  static int acquire_resource(
>      XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
>  {
>      struct domain *d, *currd = current->domain;
>      xen_mem_acquire_resource_t xmar;
> -    unsigned long mfn_list[2];
> +    unsigned long mfn_list[32];

Together with the GFN array further down this makes up for 512
bytes of stack space, if my calculation wasn't wrong. That's quite
a lot and certainly something that shouldn't be further increased.
I can accept it remaining this way for now (provided ARM has a
large enough stack as well), but please add a comment indicating
that if further growth is needed, converting to e.g. per-CPU
arrays is going to be necessary.

Jan
diff mbox

Patch

diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 6d20b17739..886579a7b0 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3756,13 +3756,12 @@  int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
 }
 #endif
 
-int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
-                     mfn_t *mfn)
+/* Caller must hold write lock as version may change and table may grow */
+static int gnttab_get_frame(struct domain *d, unsigned long idx,
+                            mfn_t *mfn)
 {
-    int rc = 0;
     struct grant_table *gt = d->grant_table;
-
-    grant_write_lock(gt);
+    int rc = 0;
 
     if ( gt->gt_version == 0 )
         gt->gt_version = 1;
@@ -3787,6 +3786,18 @@  int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
             rc = -EINVAL;
     }
 
+    return rc;
+}
+
+int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
+                     mfn_t *mfn)
+{
+    struct grant_table *gt = d->grant_table;
+    int rc;
+
+    grant_write_lock(gt);
+
+    rc = gnttab_get_frame(d, idx, mfn);
     if ( !rc )
         gnttab_set_frame_gfn(gt, idx, gfn);
 
@@ -3795,6 +3806,34 @@  int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
     return rc;
 }
 
+int gnttab_get_grant_frame(struct domain *d, unsigned long idx,
+                           mfn_t *mfn)
+{
+    struct grant_table *gt = d->grant_table;
+    int rc;
+
+    /* write lock required as version may change and/or table may grow */
+    grant_write_lock(gt);
+    rc = gnttab_get_frame(d, idx, mfn);
+    grant_write_unlock(gt);
+
+    return rc;
+}
+
+int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+                            mfn_t *mfn)
+{
+    struct grant_table *gt = d->grant_table;
+    int rc;
+
+    /* write lock required as version may change and/or table may grow */
+    grant_write_lock(gt);
+    rc = gnttab_get_frame(d, idx | XENMAPIDX_grant_table_status, mfn);
+    grant_write_unlock(gt);
+
+    return rc;
+}
+
 static void gnttab_usage_print(struct domain *rd)
 {
     int first = 1;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index b27a71c4f1..ae46d95885 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -23,6 +23,7 @@ 
 #include <xen/numa.h>
 #include <xen/mem_access.h>
 #include <xen/trace.h>
+#include <xen/grant_table.h>
 #include <asm/current.h>
 #include <asm/hardirq.h>
 #include <asm/p2m.h>
@@ -965,12 +966,49 @@  static long xatp_permission_check(struct domain *d, unsigned int space)
     return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
 }
 
+static int acquire_grant_table(struct domain *d, unsigned int id,
+                               unsigned long frame,
+                               unsigned int nr_frames,
+                               unsigned long mfn_list[])
+{
+    unsigned int i = nr_frames;
+
+    /* Iterate backwards in case table needs to grow */
+    while ( i-- != 0 )
+    {
+        mfn_t mfn = INVALID_MFN;
+        int rc;
+
+        switch ( id )
+        {
+        case XENMEM_resource_grant_table_id_grant:
+            rc = gnttab_get_grant_frame(d, frame + i, &mfn);
+            break;
+
+        case XENMEM_resource_grant_table_id_status:
+            rc = gnttab_get_status_frame(d, frame + i, &mfn);
+            break;
+
+        default:
+            rc = -EINVAL;
+            break;
+        }
+
+        if ( rc )
+            return rc;
+
+        mfn_list[i] = mfn_x(mfn);
+    }
+
+    return 0;
+}
+
 static int acquire_resource(
     XEN_GUEST_HANDLE_PARAM(xen_mem_acquire_resource_t) arg)
 {
     struct domain *d, *currd = current->domain;
     xen_mem_acquire_resource_t xmar;
-    unsigned long mfn_list[2];
+    unsigned long mfn_list[32];
     int rc;
 
     if ( copy_from_guest(&xmar, arg, 1) )
@@ -1016,6 +1054,11 @@  static int acquire_resource(
                                          xmar.nr_frames, mfn_list);
         break;
 
+    case XENMEM_resource_grant_table:
+        rc = acquire_grant_table(d, xmar.id, xmar.frame, xmar.nr_frames,
+                                 mfn_list);
+        break;
+
     default:
         rc = -EOPNOTSUPP;
         break;
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 9596ebf2c7..553304009a 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -611,14 +611,20 @@  struct xen_mem_acquire_resource {
     uint16_t type;
 
 #define XENMEM_resource_ioreq_server 0
+#define XENMEM_resource_grant_table 1
 
     /*
      * IN - a type-specific resource identifier, which must be zero
      *      unless stated otherwise.
      *
      * type == XENMEM_resource_ioreq_server -> id == ioreq server id
+     * type == XENMEM_resource_grant_table -> id defined below
      */
     uint32_t id;
+
+#define XENMEM_resource_grant_table_id_grant 0
+#define XENMEM_resource_grant_table_id_status 1
+
     /* IN/OUT - As an IN parameter number of frames of the resource
      *          to be mapped. However, if the specified value is 0 and
      *          frame_list is NULL then this field will be set to the
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index b3a95fda58..e9125e43e7 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -55,6 +55,10 @@  int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
 
 int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
                      mfn_t *mfn);
+int gnttab_get_grant_frame(struct domain *d, unsigned long idx,
+                           mfn_t *mfn);
+int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+                            mfn_t *mfn);
 
 unsigned int gnttab_dom0_frames(void);