[PATCH v4 6/8] dm_op: convert HVMOP_set_mem_type

[Paul Durrant <paul.durrant@citrix.com>]

This patch removes the need for handling HVMOP restarts, so that
infrastructure is removed.

NOTE: This patch also modifies the type of the 'nr' argument of
      xc_hvm_set_mem_type() from uint64_t to uint32_t. In practice the
      value passed was always truncated to 32 bits.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>

v4:
- Added initializers as requested by Jan.

v3:
- Addressed more comments from Jan.

v2:
- Addressed several comments from Jan.
---
 tools/libxc/include/xenctrl.h       |   2 +-
 tools/libxc/xc_misc.c               |  29 +++-----
 xen/arch/x86/hvm/dm.c               |  90 ++++++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c              | 136 +-----------------------------------
 xen/include/public/hvm/dm_op.h      |  22 ++++++
 xen/include/public/hvm/hvm_op.h     |  20 ------
 xen/xsm/flask/policy/access_vectors |   2 +-
 7 files changed, 125 insertions(+), 176 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index a5c234f..13431bb 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1634,7 +1634,7 @@ int xc_hvm_modified_memory(
  * Allowed types are HVMMEM_ram_rw, HVMMEM_ram_ro, HVMMEM_mmio_dm
  */
 int xc_hvm_set_mem_type(
-    xc_interface *xch, domid_t dom, hvmmem_type_t memtype, uint64_t first_pfn, uint64_t nr);
+    xc_interface *xch, domid_t dom, hvmmem_type_t memtype, uint64_t first_pfn, uint32_t nr);
 
 /*
  * Injects a hardware/software CPU trap, to take effect the next time the HVM 
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 597df99..5b06d6b 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -590,30 +590,21 @@ int xc_hvm_modified_memory(
 }
 
 int xc_hvm_set_mem_type(
-    xc_interface *xch, domid_t dom, hvmmem_type_t mem_type, uint64_t first_pfn, uint64_t nr)
+    xc_interface *xch, domid_t dom, hvmmem_type_t mem_type, uint64_t first_pfn, uint32_t nr)
 {
-    DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_mem_type, arg);
-    int rc;
-
-    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
-    if ( arg == NULL )
-    {
-        PERROR("Could not allocate memory for xc_hvm_set_mem_type hypercall");
-        return -1;
-    }
+    struct xen_dm_op op;
+    struct xen_dm_op_set_mem_type *data;
 
-    arg->domid        = dom;
-    arg->hvmmem_type  = mem_type;
-    arg->first_pfn    = first_pfn;
-    arg->nr           = nr;
+    memset(&op, 0, sizeof(op));
 
-    rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-                  HVMOP_set_mem_type,
-                  HYPERCALL_BUFFER_AS_ARG(arg));
+    op.op = XEN_DMOP_set_mem_type;
+    data = &op.u.set_mem_type;
 
-    xc_hypercall_buffer_free(xch, arg);
+    data->mem_type = mem_type;
+    data->first_pfn = first_pfn;
+    data->nr = nr;
 
-    return rc;
+    return do_dm_op(xch, dom, 1, &op, sizeof(op));
 }
 
 int xc_hvm_inject_trap(
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index dd81116..b3c91f8 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -159,6 +159,82 @@ static int modified_memory(struct domain *d, xen_pfn_t *first_pfn,
     return rc;
 }
 
+static bool allow_p2m_type_change(p2m_type_t old, p2m_type_t new)
+{
+    return p2m_is_ram(old) ||
+           (p2m_is_hole(old) && new == p2m_mmio_dm) ||
+           (old == p2m_ioreq_server && new == p2m_ram_rw);
+}
+
+static int set_mem_type(struct domain *d, hvmmem_type_t mem_type,
+                        xen_pfn_t *first_pfn, unsigned int *nr)
+{
+    xen_pfn_t last_pfn = *first_pfn + *nr - 1;
+    unsigned int iter = 0;
+    int rc = 0;
+
+    /* Interface types to internal p2m types */
+    static const p2m_type_t memtype[] = {
+        [HVMMEM_ram_rw]  = p2m_ram_rw,
+        [HVMMEM_ram_ro]  = p2m_ram_ro,
+        [HVMMEM_mmio_dm] = p2m_mmio_dm,
+        [HVMMEM_unused] = p2m_invalid,
+        [HVMMEM_ioreq_server] = p2m_ioreq_server
+    };
+
+    if ( (*first_pfn > last_pfn) ||
+         (last_pfn > domain_get_maximum_gpfn(d)) )
+        return -EINVAL;
+
+    if ( mem_type >= ARRAY_SIZE(memtype) ||
+         unlikely(mem_type == HVMMEM_unused) )
+        return -EINVAL;
+
+    while ( iter < *nr )
+    {
+        unsigned long pfn = *first_pfn + iter;
+        p2m_type_t t;
+
+        get_gfn_unshare(d, pfn, &t);
+        if ( p2m_is_paging(t) )
+        {
+            put_gfn(d, pfn);
+            p2m_mem_paging_populate(d, pfn);
+            return -EAGAIN;
+        }
+
+        if ( p2m_is_shared(t) )
+            rc = -EAGAIN;
+        else if ( !allow_p2m_type_change(t, memtype[mem_type]) )
+            rc = -EINVAL;
+        else
+            rc = p2m_change_type_one(d, pfn, t, memtype[mem_type]);
+
+        put_gfn(d, pfn);
+
+        if ( rc )
+            break;
+
+        iter++;
+
+        /*
+         * Check for continuation every 256th iteration and if the
+         * iteration is not the last.
+         */
+        if ( (iter < *nr) && ((iter & 0xff) == 0) &&
+             hypercall_preempt_check() )
+        {
+            *first_pfn += iter;
+            *nr -= iter;
+
+            rc = -ERESTART;
+            break;
+        }
+    }
+
+    return rc;
+}
+
 static int dm_op(domid_t domid,
                  unsigned int nr_bufs,
                  xen_dm_op_buf_t bufs[])
@@ -333,6 +409,20 @@ static int dm_op(domid_t domid,
         break;
     }
 
+    case XEN_DMOP_set_mem_type:
+    {
+        struct xen_dm_op_set_mem_type *data =
+            &op.u.set_mem_type;
+
+        rc = -EINVAL;
+        if ( data->pad )
+            break;
+
+        rc = set_mem_type(d, data->mem_type, &data->first_pfn,
+                          &data->nr);
+        break;
+    }
+
     default:
         rc = -EOPNOTSUPP;
         break;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 32dc6d7..6b6f3ee 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4846,132 +4846,11 @@ static int hvmop_get_mem_type(
     return rc;
 }
 
-/*
- * Note that this value is effectively part of the ABI, even if we don't need
- * to make it a formal part of it: A guest suspended for migration in the
- * middle of a continuation would fail to work if resumed on a hypervisor
- * using a different value.
- */
-#define HVMOP_op_mask 0xff
-
-static bool_t hvm_allow_p2m_type_change(p2m_type_t old, p2m_type_t new)
-{
-    if ( p2m_is_ram(old) ||
-         (p2m_is_hole(old) && new == p2m_mmio_dm) ||
-         (old == p2m_ioreq_server && new == p2m_ram_rw) )
-        return 1;
-
-    return 0;
-}
-
-static int hvmop_set_mem_type(
-    XEN_GUEST_HANDLE_PARAM(xen_hvm_set_mem_type_t) arg,
-    unsigned long *iter)
-{
-    unsigned long start_iter = *iter;
-    struct xen_hvm_set_mem_type a;
-    struct domain *d;
-    int rc;
-
-    /* Interface types to internal p2m types */
-    static const p2m_type_t memtype[] = {
-        [HVMMEM_ram_rw]  = p2m_ram_rw,
-        [HVMMEM_ram_ro]  = p2m_ram_ro,
-        [HVMMEM_mmio_dm] = p2m_mmio_dm,
-        [HVMMEM_unused] = p2m_invalid,
-        [HVMMEM_ioreq_server] = p2m_ioreq_server
-    };
-
-    if ( copy_from_guest(&a, arg, 1) )
-        return -EFAULT;
-
-    rc = rcu_lock_remote_domain_by_id(a.domid, &d);
-    if ( rc != 0 )
-        return rc;
-
-    rc = -EINVAL;
-    if ( !is_hvm_domain(d) )
-        goto out;
-
-    rc = xsm_hvm_control(XSM_DM_PRIV, d, HVMOP_set_mem_type);
-    if ( rc )
-        goto out;
-
-    rc = -EINVAL;
-    if ( a.nr < start_iter ||
-         ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
-         ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
-        goto out;
-
-    if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
-         unlikely(a.hvmmem_type == HVMMEM_unused) )
-        goto out;
-
-    while ( a.nr > start_iter )
-    {
-        unsigned long pfn = a.first_pfn + start_iter;
-        p2m_type_t t;
-
-        get_gfn_unshare(d, pfn, &t);
-        if ( p2m_is_paging(t) )
-        {
-            put_gfn(d, pfn);
-            p2m_mem_paging_populate(d, pfn);
-            rc = -EAGAIN;
-            goto out;
-        }
-        if ( p2m_is_shared(t) )
-        {
-            put_gfn(d, pfn);
-            rc = -EAGAIN;
-            goto out;
-        }
-        if ( !hvm_allow_p2m_type_change(t, memtype[a.hvmmem_type]) )
-        {
-            put_gfn(d, pfn);
-            goto out;
-        }
-
-        rc = p2m_change_type_one(d, pfn, t, memtype[a.hvmmem_type]);
-        put_gfn(d, pfn);
-
-        if ( rc )
-            goto out;
-
-        /* Check for continuation if it's not the last interation */
-        if ( a.nr > ++start_iter && !(start_iter & HVMOP_op_mask) &&
-             hypercall_preempt_check() )
-        {
-            rc = -ERESTART;
-            goto out;
-        }
-    }
-    rc = 0;
-
- out:
-    rcu_unlock_domain(d);
-    *iter = start_iter;
-
-    return rc;
-}
-
 long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    unsigned long start_iter, mask;
     long rc = 0;
 
-    switch ( op & HVMOP_op_mask )
-    {
-    default:
-        mask = ~0UL;
-        break;
-    case HVMOP_set_mem_type:
-        mask = HVMOP_op_mask;
-        break;
-    }
-
-    start_iter = op & ~mask;
-    switch ( op &= mask )
+    switch ( op )
     {
     case HVMOP_set_evtchn_upcall_vector:
         rc = hvmop_set_evtchn_upcall_vector(
@@ -5002,12 +4881,6 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
             guest_handle_cast(arg, xen_hvm_get_mem_type_t));
         break;
 
-    case HVMOP_set_mem_type:
-        rc = hvmop_set_mem_type(
-            guest_handle_cast(arg, xen_hvm_set_mem_type_t),
-            &start_iter);
-        break;
-
     case HVMOP_pagetable_dying:
     {
         struct xen_hvm_pagetable_dying a;
@@ -5116,13 +4989,6 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
     }
     }
 
-    if ( rc == -ERESTART )
-    {
-        ASSERT(!(start_iter & mask));
-        rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh",
-                                           op | start_iter, arg);
-    }
-
     return rc;
 }
 
diff --git a/xen/include/public/hvm/dm_op.h b/xen/include/public/hvm/dm_op.h
index 8514ca4..8e9bef1 100644
--- a/xen/include/public/hvm/dm_op.h
+++ b/xen/include/public/hvm/dm_op.h
@@ -252,6 +252,27 @@ struct xen_dm_op_modified_memory {
     uint64_aligned_t first_pfn;
 };
 
+/*
+ * XEN_DMOP_set_mem_type: Notify that a region of memory is to be treated
+ *                        in a specific way. (See definition of
+ *                        hvmmem_type_t).
+ *
+ * NOTE: In the event of a continuation (return code -ERESTART), the
+ *       @first_pfn is set to the value of the pfn of the remaining
+ *       region and @nr reduced to the size of the remaining region.
+ */
+#define XEN_DMOP_set_mem_type 12
+
+struct xen_dm_op_set_mem_type {
+    /* IN - number of contiguous pages */
+    uint32_t nr;
+    /* IN - new hvmmem_type_t of region */
+    uint16_t mem_type;
+    uint16_t pad;
+    /* IN - first pfn in region */
+    uint64_aligned_t first_pfn;
+};
+
 struct xen_dm_op {
     uint32_t op;
     uint32_t pad;
@@ -267,6 +288,7 @@ struct xen_dm_op {
         struct xen_dm_op_set_isa_irq_level set_isa_irq_level;
         struct xen_dm_op_set_pci_link_route set_pci_link_route;
         struct xen_dm_op_modified_memory modified_memory;
+        struct xen_dm_op_set_mem_type set_mem_type;
     } u;
 };
 
diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 76e1b78..d7e2f12 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -96,26 +96,6 @@ typedef enum {
     HVMMEM_ioreq_server
 } hvmmem_type_t;
 
-/* Following tools-only interfaces may change in future. */
-#if defined(__XEN__) || defined(__XEN_TOOLS__)
-
-#define HVMOP_set_mem_type    8
-/* Notify that a region of memory is to be treated in a specific way. */
-struct xen_hvm_set_mem_type {
-    /* Domain to be updated. */
-    domid_t domid;
-    /* Memory type */
-    uint16_t hvmmem_type;
-    /* Number of pages. */
-    uint32_t nr;
-    /* First pfn. */
-    uint64_aligned_t first_pfn;
-};
-typedef struct xen_hvm_set_mem_type xen_hvm_set_mem_type_t;
-DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_mem_type_t);
-
-#endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
-
 /* Hint from PV drivers for pagetable destruction. */
 #define HVMOP_pagetable_dying        9
 struct xen_hvm_pagetable_dying {
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 361e7aa..381f659 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -262,7 +262,7 @@ class hvm
     bind_irq
 # XEN_DOMCTL_pin_mem_cacheattr
     cacheattr
-# HVMOP_get_mem_type, HVMOP_set_mem_type,
+# HVMOP_get_mem_type,
 # HVMOP_set_mem_access, HVMOP_get_mem_access, HVMOP_pagetable_dying,
 # HVMOP_inject_trap
     hvmctl
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel