Re: [Xen-devel] [PATCH v2 07/11] hvmctl: convert HVMOP_set_mem_type

[Daniel De Graaf]

On 06/24/2016 06:32 AM, Jan Beulich wrote:

This allows elimination of the (ab)use of the high operation number
bits for encoding continuations.

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 


Acked-by: Daniel De Graaf 

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

[Xen-devel] [PATCH v2 07/11] hvmctl: convert HVMOP_set_mem_type

[Jan Beulich]

This allows elimination of the (ab)use of the high operation number
bits for encoding continuations.

Also limiting "nr" at the libxc level to 32 bits (the high 32 bits of
the previous 64-bit parameter got ignore so far).

Signed-off-by: Jan Beulich 
Reviewed-by: Wei Liu 
Reviewed-by: Andrew Cooper 

--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1627,7 +1627,8 @@ 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_gfn, uint32_t nr);
 
 /*
  * Injects a hardware/software CPU trap, to take effect the next time the HVM 
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -568,30 +568,15 @@ 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_gfn, uint32_t nr)
 {
-DECLARE_HYPERCALL_BUFFER(struct xen_hvm_set_mem_type, arg);
-int rc;
+DECLARE_HVMCTL(set_mem_type, dom,
+   .hvmmem_type = mem_type,
+   .first_gfn   = first_gfn,
+   .nr  = nr);
 
-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;
-}
-
-arg->domid= dom;
-arg->hvmmem_type  = mem_type;
-arg->first_pfn= first_pfn;
-arg->nr   = nr;
-
-rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op,
-  HVMOP_set_mem_type,
-  HYPERCALL_BUFFER_AS_ARG(arg));
-
-xc_hypercall_buffer_free(xch, arg);
-
-return rc;
+return do_hvmctl(xch, );
 }
 
 int xc_hvm_inject_trap(
--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -136,6 +136,70 @@ static int modified_memory(struct domain
 return 0;
 }
 
+static int set_mem_type(struct domain *d,
+const struct xen_hvm_set_mem_type *op, uint64_t *iter)
+{
+/* 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
+};
+
+if ( !is_hvm_domain(d) )
+return -EINVAL;
+
+if ( op->rsvd || op->nr < *iter ||
+ ((op->first_gfn + op->nr - 1) < op->first_gfn) ||
+ ((op->first_gfn + op->nr - 1) > domain_get_maximum_gpfn(d)) )
+return -EINVAL;
+
+if ( op->hvmmem_type >= ARRAY_SIZE(memtype) ||
+ unlikely(op->hvmmem_type == HVMMEM_unused) )
+return -EINVAL;
+
+while ( op->nr > *iter )
+{
+unsigned long gfn = op->first_gfn + *iter;
+p2m_type_t t;
+int rc;
+
+get_gfn_unshare(d, gfn, );
+
+if ( p2m_is_paging(t) )
+{
+put_gfn(d, gfn);
+p2m_mem_paging_populate(d, gfn);
+return -EAGAIN;
+}
+
+if ( p2m_is_shared(t) )
+rc = -EAGAIN;
+else if ( !p2m_is_ram(t) &&
+  (!p2m_is_hole(t) || op->hvmmem_type != HVMMEM_mmio_dm) &&
+  (t != p2m_mmio_write_dm || op->hvmmem_type != HVMMEM_ram_rw) 
)
+rc = -EINVAL;
+else
+rc = p2m_change_type_one(d, gfn, t, memtype[op->hvmmem_type]);
+
+put_gfn(d, gfn);
+
+if ( rc )
+return rc;
+
+/*
+ * Check for continuation every once in a while, and if it's not the
+ * last interation.
+ */
+if ( op->nr > ++*iter && !(*iter & 0xff) &&
+ hypercall_preempt_check() )
+return -ERESTART;
+}
+
+return 0;
+}
+
 long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xen_hvmctl_t) u_hvmctl)
 {
 xen_hvmctl_t op;
@@ -190,6 +254,10 @@ long do_hvmctl(XEN_GUEST_HANDLE_PARAM(xe
 rc = modified_memory(d, _memory, );
 break;
 
+case XEN_HVMCTL_set_mem_type:
+rc = set_mem_type(d, _mem_type, );
+break;
+
 default:
 rc = -EOPNOTSUPP;
 break;
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5215,31 +5215,11 @@ static int do_altp2m_op(
 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
-
 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 )