Re: [Xen-devel] [PATCH] Paging and memory sharing for HVM guests

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

>>> Grzegorz Milos <gm281@xxxxxxxxx> 17.12.09 00:14 >>>
>--- a/xen/include/public/domctl.h      Tue Dec 15 10:47:36 2009 -0800
>+++ b/xen/include/public/domctl.h      Tue Dec 15 10:47:42 2009 -0800
>@@ -691,6 +691,31 @@
> 
> };
> 
>+/*
>+ * Memory event operations
>+ */
>+
>+#define XEN_DOMCTL_mem_event_op  56
>+
>+/* Add and remove memory handlers */
>+#define XEN_DOMCTL_MEM_EVENT_OP_ENABLE     0
>+#define XEN_DOMCTL_MEM_EVENT_OP_DISABLE    1
>+
>+struct xen_domctl_mem_event_op {
>+    uint32_t       op;           /* XEN_DOMCTL_MEM_EVENT_OP_* */
>+    uint32_t       mode;         /* XEN_DOMCTL_MEM_EVENT_ENABLE_* */
>+
>+    /* OP_ENABLE */
>+    unsigned long  shared_addr;  /* IN:  Virtual address of shared page */
>+    unsigned long  ring_addr;    /* IN:  Virtual address of ring page */
>+
>+    /* Other OPs */
>+    unsigned long  gfn;          /* IN:  gfn of page being operated on */
>+};
>+typedef struct xen_domctl_mem_event_op xen_domctl_mem_event_op_t;
>+DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_event_op_t);
>+
>+
> struct xen_domctl {
>     uint32_t cmd;
>     uint32_t interface_version; /* XEN_DOMCTL_INTERFACE_VERSION */

The use of (unsigned) long in domctl.h and sysctl.h is invalid, as it breaks
32-bit dom0 on 64-bit hv.

>--- a/xen/include/public/domctl.h      Tue Dec 15 13:05:35 2009 -0800
>+++ b/xen/include/public/domctl.h      Tue Dec 15 13:05:38 2009 -0800
>...
>@@ -103,6 +104,7 @@
>     uint32_t flags;              /* XEN_DOMINF_* */
>     uint64_aligned_t tot_pages;
>     uint64_aligned_t max_pages;
>+    uint64_aligned_t shr_pages;
>     uint64_aligned_t shared_info_frame; /* GMFN of shared_info struct */
>     uint64_aligned_t cpu_time;
>     uint32_t nr_online_vcpus;    /* Number of VCPUs currently online. */

This change is not valid without changing XEN_DOMCTL_INTERFACE_VERSION,
since tools outside the Xen tree (namely, libvirt, which is currently broken
because of this) rely on knowing all (valid) (version, structure-layout) pairs.

>@@ -727,6 +729,52 @@
> typedef struct xen_domctl_mem_event_op xen_domctl_mem_event_op_t;
> DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_event_op_t);
> 
>+/*
>+ * Memory sharing operations
>+ */
>+#define XEN_DOMCTL_mem_sharing_op  58
>+
>+#define XEN_DOMCTL_MEM_SHARING_OP_CONTROL        0
>+#define XEN_DOMCTL_MEM_SHARING_OP_NOMINATE_GFN   1
>+#define XEN_DOMCTL_MEM_SHARING_OP_NOMINATE_GREF  2
>+#define XEN_DOMCTL_MEM_SHARING_OP_SHARE          3
>+#define XEN_DOMCTL_MEM_SHARING_OP_RESUME         4
>+#define XEN_DOMCTL_MEM_SHARING_OP_DEBUG_GFN      5
>+#define XEN_DOMCTL_MEM_SHARING_OP_DEBUG_MFN      6
>+#define XEN_DOMCTL_MEM_SHARING_OP_DEBUG_GREF     7
>+
>+#define XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID  (-10)
>+#define XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID  (-9)
>+
>+struct xen_domctl_mem_sharing_op {
>+    uint8_t       op;            /* XEN_DOMCTL_MEM_EVENT_OP_* */
>+
>+    union {
>+        int enable;                       /* for OP_CONTROL            */
>+
>+        struct mem_sharing_op_nominate {  /* for OP_NOMINATE           */
>+            union {
>+                unsigned long gfn;        /* IN: gfn to nominate       */
>+                uint32_t      grant_ref;  /* IN: grant ref to nominate */
>+            };
>+            uint64_t       handle;        /* OUT: the handle           */
>+        } nominate;
>+        struct mem_sharing_op_share {
>+            uint64_t source_handle;       /* IN: handle to the source page */
>+            uint64_t client_handle;       /* IN: handle to the client page */
>+        } share; 
>+        struct mem_sharing_op_debug {
>+            union {
>+                unsigned long  gfn;        /* IN: gfn to debug          */
>+                unsigned long  mfn;        /* IN: mfn to debug          */
>+                grant_ref_t    gref;       /* IN: gref to debug         */
>+            };
>+        } debug;
>+    };
>+};
>+typedef struct xen_domctl_mem_sharing_op xen_domctl_mem_sharing_op_t;
>+DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_sharing_op_t);
>+
> 
> struct xen_domctl {
>     uint32_t cmd;

Like above, using unsigned long here is invalid. Using int doesn't seem
to be too good an idea either, for portability reasons. I think fixed-size
integer types should be used everywhere.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel