[PATCHv3 0/6] tools: rework VM Generation ID

[PATCHv3 0/6] tools: rework VM Generation ID

[David Vrabel]

This series reworks the VM Generation ID to a) conform to the
published spec from Microsoft; b) simplify the save/restore code; and
c) extend the libxl API to allow toolstacks to use this feature.

The VM Generation ID must be regenerated with a new random ID after
certain VM operations. Since xl lacks infrastructure for tracking the
life-cycle of snapshots and clones (etc), the safe option of always
using a new generation ID is used.

You can download the spec from:

  http://www.microsoft.com/en-us/download/details.aspx?id=30707

Changes in v3:

- Specify generation ID in libxl_domain_build_info instead of having
  to call a function to set it.
- Improve docs.

Changes in v2:

- Use libxl_uuid for the generation ID.
- Add "generation_id" option to xl domain configuration file and use
  this to set a random generation ID every time.

David

[PATCH 1/6] docs: update docs for the ~/platform/generation-id key

[David Vrabel]

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 docs/misc/xenstore-paths.markdown |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
index 70ab7f4..22fcfaa 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown,
 below) even if the guest has advertised support for the event channel
 protocol.
 
-#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL]
-
-The hexadecimal representation of the address of the domain's
-"generation id".
-
 #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL]
 
 If the default low MMIO hole (below 4GiB) is not big enough for all
@@ -193,9 +188,22 @@ Various platform properties.
 
 #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL]
 
-Two 64 bit values that represent the Windows Generation ID.
-Is used by the BIOS initializer to get this value.
-If not present or "0:0" (all zeroes) device will not be present to the machine.
+The upper and lower 64-bit words of the 128-bit VM Generation ID.
+
+This key is used by hvmloader to create the ACPI VM Generation ID
+device.  It initialises a 16 octet region of guest memory with this
+value.  The guest physical address of this region is saved in the
+HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter.
+
+If this key is not present, is empty, or is all-zeros ("0:0") then the
+ACPI device is not created.
+
+When restoring a guest, the toolstack may (in certain circumstances)
+need generate a new random generation ID and write it to guest memory
+at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR.
+
+See Microsoft's "Virtual Machine Generation ID" specification for the
+circumstances where the generation ID needs to be changed.
 
 ### Frontend device paths
 
-- 
1.7.10.4

[PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[David Vrabel]

HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
VM Generation ID.  This parameter will be written by hvmloader and read
by the toolstack when updating the gen. ID (e.g., after restoring from a
snapshot).

A HVM parameter is easier for the save/restore code to work with (than
a XenStore key).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 xen/include/public/hvm/params.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
index 517a184..4d7c692 100644
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -145,6 +145,9 @@
 /* SHUTDOWN_* action in case of a triple fault */
 #define HVM_PARAM_TRIPLE_FAULT_REASON 31
 
-#define HVM_NR_PARAMS          32
+/* Location of the VM Generation ID in guest physical address space. */
+#define HVM_PARAM_VM_GENERATION_ID_ADDR 32
+
+#define HVM_NR_PARAMS          33
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
-- 
1.7.10.4

[PATCH 3/6] hvmloader: add helper functions to get/set HVM params

[David Vrabel]

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 tools/firmware/hvmloader/Makefile    |    1 +
 tools/firmware/hvmloader/hvm_param.c |   36 ++++++++++++++++++++++++++++++++++
 tools/firmware/hvmloader/hvmloader.c |   14 ++-----------
 tools/firmware/hvmloader/util.h      |    9 +++++++++
 tools/firmware/hvmloader/xenbus.c    |   14 +++++--------
 5 files changed, 53 insertions(+), 21 deletions(-)
 create mode 100644 tools/firmware/hvmloader/hvm_param.c

diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
index 00ee952..46a79c5 100644
--- a/tools/firmware/hvmloader/Makefile
+++ b/tools/firmware/hvmloader/Makefile
@@ -31,6 +31,7 @@ CFLAGS += $(CFLAGS_xeninclude)
 OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
 OBJS += smp.o cacheattr.o xenbus.o
 OBJS += e820.o pci.o pir.o ctype.o
+OBJS += hvm_param.o
 ifeq ($(debug),y)
 OBJS += tests.o
 endif
diff --git a/tools/firmware/hvmloader/hvm_param.c b/tools/firmware/hvmloader/hvm_param.c
new file mode 100644
index 0000000..f7d8720
--- /dev/null
+++ b/tools/firmware/hvmloader/hvm_param.c
@@ -0,0 +1,36 @@
+/*
+ * hvm_param.c: get/set HVM params.
+ *
+ * Copyright (C) 2014 Citrix Systems R&D Ltd.
+ */
+#include "util.h"
+#include "config.h"
+#include "hypercall.h"
+
+#include <xen/hvm/params.h>
+
+int hvm_param_get(uint32_t index, uint64_t *value)
+{
+    struct xen_hvm_param p;
+    int ret;
+
+    p.domid = DOMID_SELF;
+    p.index = index;
+
+    ret = hypercall_hvm_op(HVMOP_get_param, &p);
+    if (ret == 0)
+        *value = p.value;
+
+    return ret;
+}
+
+int hvm_param_set(uint32_t index, uint64_t value)
+{
+    struct xen_hvm_param p;
+
+    p.domid = DOMID_SELF;
+    p.index = index;
+    p.value = value;
+
+    return hypercall_hvm_op(HVMOP_set_param, &p);
+}
diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
index 1cc8cf2..7b0da38 100644
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -176,14 +176,10 @@ static void cmos_write_memory_size(void)
 static void init_vm86_tss(void)
 {
     void *tss;
-    struct xen_hvm_param p;
 
     tss = mem_alloc(128, 128);
     memset(tss, 0, 128);
-    p.domid = DOMID_SELF;
-    p.index = HVM_PARAM_VM86_TSS;
-    p.value = virt_to_phys(tss);
-    hypercall_hvm_op(HVMOP_set_param, &p);
+    hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss));
     printf("vm86 TSS at %08lx\n", virt_to_phys(tss));
 }
 
@@ -314,12 +310,6 @@ int main(void)
 
     if ( acpi_enabled )
     {
-        struct xen_hvm_param p = {
-            .domid = DOMID_SELF,
-            .index = HVM_PARAM_ACPI_IOPORTS_LOCATION,
-            .value = 1,
-        };
-
         if ( bios->acpi_build_tables )
         {
             printf("Loading ACPI ...\n");
@@ -328,7 +318,7 @@ int main(void)
 
         acpi_enable_sci();
 
-        hypercall_hvm_op(HVMOP_set_param, &p);
+        hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1);
     }
 
     init_vm86_tss();
diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
index 9ccb905..e4b6be4 100644
--- a/tools/firmware/hvmloader/util.h
+++ b/tools/firmware/hvmloader/util.h
@@ -210,6 +210,15 @@ const char *xenstore_read(const char *path, const char *default_resp);
  */
 int xenstore_write(const char *path, const char *value);
 
+
+/* Get a HVM param.
+ */
+int hvm_param_get(uint32_t index, uint64_t *value);
+
+/* Set a HVM param.
+ */
+int hvm_param_set(uint32_t index, uint64_t value);
+
 /* Setup PCI bus */
 void pci_setup(void);
 
diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
index fe72e97..64c2176 100644
--- a/tools/firmware/hvmloader/xenbus.c
+++ b/tools/firmware/hvmloader/xenbus.c
@@ -41,21 +41,17 @@ static char payload[XENSTORE_PAYLOAD_MAX + 1];  /* Unmarshalling area */
  * Call once, before any other xenbus actions. */
 void xenbus_setup(void)
 {
-    xen_hvm_param_t param;
+    uint64_t val;
 
     /* Ask Xen where the xenbus shared page is. */
-    param.domid = DOMID_SELF;
-    param.index = HVM_PARAM_STORE_PFN;
-    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
+    if ( hvm_param_get(HVM_PARAM_STORE_PFN, &val) )
         BUG();
-    rings = (void *) (unsigned long) (param.value << PAGE_SHIFT);
+    rings = (void *) (unsigned long) (val << PAGE_SHIFT);
 
     /* Ask Xen where the xenbus event channel is. */
-    param.domid = DOMID_SELF;
-    param.index = HVM_PARAM_STORE_EVTCHN;
-    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
+    if ( hvm_param_get(HVM_PARAM_STORE_EVTCHN, &val) )
         BUG();
-    event = param.value;
+    event = val;
 
     printf("Xenbus rings @0x%lx, event channel %lu\n",
            (unsigned long) rings, (unsigned long) event);
-- 
1.7.10.4

[PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation

[David Vrabel]

The VM generation ID support in libxc/libxl was based on a draft
specification which subsequently changed considerably.  Remove much of
the code in anticipation of introducing something simpler that
conforms to the current specification from Microsoft.

Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead
of the hvmloader/generation-id-address XenStore key.  This simplifies
save/restore since it only needs to transfer the value of this param.

There are no changes to the migration stream format or the public
libxl API.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 tools/firmware/hvmloader/acpi/build.c |    9 +++----
 tools/libxc/xc_domain_restore.c       |   44 +++------------------------------
 tools/libxc/xc_domain_save.c          |    5 ++--
 tools/libxc/xenguest.h                |    8 ++----
 tools/libxl/libxl_create.c            |   12 +++------
 tools/libxl/libxl_dom.c               |   25 ++-----------------
 tools/libxl/libxl_internal.h          |    8 ++----
 tools/libxl/libxl_save_callout.c      |   10 +++-----
 tools/libxl/libxl_save_helper.c       |    9 +++----
 tools/libxl/libxl_save_msgs_gen.pl    |    3 +--
 10 files changed, 26 insertions(+), 107 deletions(-)

diff --git a/tools/firmware/hvmloader/acpi/build.c b/tools/firmware/hvmloader/acpi/build.c
index 5c42d89..1431296 100644
--- a/tools/firmware/hvmloader/acpi/build.c
+++ b/tools/firmware/hvmloader/acpi/build.c
@@ -24,6 +24,7 @@
 #include "../config.h"
 #include "../util.h"
 #include <xen/hvm/hvm_xs_strings.h>
+#include <xen/hvm/params.h>
 
 #define ACPI_MAX_SECONDARY_TABLES 16
 
@@ -362,7 +363,6 @@ static int construct_secondary_tables(unsigned long *table_ptrs,
 static int new_vm_gid(struct acpi_info *acpi_info)
 {
     uint64_t vm_gid[2], *buf;
-    char addr[12];
     const char * s;
     char *end;
 
@@ -383,12 +383,9 @@ static int new_vm_gid(struct acpi_info *acpi_info)
         return 0;
     memcpy(buf, vm_gid, sizeof(vm_gid));
 
-    /* set into ACPI table and XenStore the address */
+    /* set into ACPI table and HVM param the address */
     acpi_info->vm_gid_addr = virt_to_phys(buf);
-    if ( snprintf(addr, sizeof(addr), "0x%lx", virt_to_phys(buf))
-         >= sizeof(addr) )
-        return 0;
-    xenstore_write("hvmloader/generation-id-address", addr);
+    hvm_param_set(HVM_PARAM_VM_GENERATION_ID_ADDR, acpi_info->vm_gid_addr);
 
     return 1;
 }
diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
index bcb0ae0..cf1c489 100644
--- a/tools/libxc/xc_domain_restore.c
+++ b/tools/libxc/xc_domain_restore.c
@@ -1408,8 +1408,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       domid_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, domid_t console_domid,
                       unsigned int hvm, unsigned int pae, int superpages,
-                      int no_incr_generationid, int checkpointed_stream,
-                      unsigned long *vm_generationid_addr,
+                      int checkpointed_stream,
                       struct restore_callbacks *callbacks)
 {
     DECLARE_DOMCTL;
@@ -1634,44 +1633,9 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                 xc_set_hvm_param(xch, dom, HVM_PARAM_VM86_TSS, pagebuf.vm86_tss);
             if ( pagebuf.console_pfn )
                 console_pfn = pagebuf.console_pfn;
-            if ( pagebuf.vm_generationid_addr ) {
-                if ( !no_incr_generationid ) {
-                    unsigned int offset;
-                    unsigned char *buf;
-                    unsigned long long generationid;
-
-                    /*
-                     * Map the VM generation id buffer and inject the new value.
-                     */
-
-                    pfn = pagebuf.vm_generationid_addr >> PAGE_SHIFT;
-                    offset = pagebuf.vm_generationid_addr & (PAGE_SIZE - 1);
-                
-                    if ( (pfn >= dinfo->p2m_size) ||
-                         (pfn_type[pfn] != XEN_DOMCTL_PFINFO_NOTAB) )
-                    {
-                        ERROR("generation id buffer frame is bad");
-                        goto out;
-                    }
-
-                    mfn = ctx->p2m[pfn];
-                    buf = xc_map_foreign_range(xch, dom, PAGE_SIZE,
-                                               PROT_READ | PROT_WRITE, mfn);
-                    if ( buf == NULL )
-                    {
-                        ERROR("xc_map_foreign_range for generation id"
-                              " buffer failed");
-                        goto out;
-                    }
-
-                    generationid = *(unsigned long long *)(buf + offset);
-                    *(unsigned long long *)(buf + offset) = generationid + 1;
-
-                    munmap(buf, PAGE_SIZE);
-                }
-
-                *vm_generationid_addr = pagebuf.vm_generationid_addr;
-            }
+            if ( pagebuf.vm_generationid_addr )
+                xc_set_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR,
+                                 pagebuf.vm_generationid_addr);
 
             break;  /* our work here is done */
         }
diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
index 71f9b59..42094e8 100644
--- a/tools/libxc/xc_domain_save.c
+++ b/tools/libxc/xc_domain_save.c
@@ -802,8 +802,7 @@ static int save_tsc_info(xc_interface *xch, uint32_t dom, int io_fd)
 
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                    uint32_t max_factor, uint32_t flags,
-                   struct save_callbacks* callbacks, int hvm,
-                   unsigned long vm_generationid_addr)
+                   struct save_callbacks* callbacks, int hvm)
 {
     xc_dominfo_t info;
     DECLARE_DOMCTL;
@@ -1634,7 +1633,7 @@ int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iter
         } chunk = { 0, };
 
         chunk.id = XC_SAVE_ID_HVM_GENERATION_ID_ADDR;
-        chunk.data = vm_generationid_addr;
+        xc_get_hvm_param(xch, dom, HVM_PARAM_VM_GENERATION_ID_ADDR, &chunk.data);
 
         if ( (chunk.data != 0) &&
              wrexact(io_fd, &chunk, sizeof(chunk)) )
diff --git a/tools/libxc/xenguest.h b/tools/libxc/xenguest.h
index 1f216cd..40bbac8 100644
--- a/tools/libxc/xenguest.h
+++ b/tools/libxc/xenguest.h
@@ -86,8 +86,7 @@ struct save_callbacks {
  */
 int xc_domain_save(xc_interface *xch, int io_fd, uint32_t dom, uint32_t max_iters,
                    uint32_t max_factor, uint32_t flags /* XCFLAGS_xxx */,
-                   struct save_callbacks* callbacks, int hvm,
-                   unsigned long vm_generationid_addr);
+                   struct save_callbacks* callbacks, int hvm);
 
 
 /* callbacks provided by xc_domain_restore */
@@ -113,9 +112,7 @@ struct restore_callbacks {
  * @parm hvm non-zero if this is a HVM restore
  * @parm pae non-zero if this HVM domain has PAE support enabled
  * @parm superpages non-zero to allocate guest memory with superpages
- * @parm no_incr_generationid non-zero if generation id is NOT to be incremented
  * @parm checkpointed_stream non-zero if the far end of the stream is using checkpointing
- * @parm vm_generationid_addr returned with the address of the generation id buffer
  * @parm callbacks non-NULL to receive a callback to restore toolstack
  *       specific data
  * @return 0 on success, -1 on failure
@@ -125,8 +122,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
                       domid_t store_domid, unsigned int console_evtchn,
                       unsigned long *console_mfn, domid_t console_domid,
                       unsigned int hvm, unsigned int pae, int superpages,
-                      int no_incr_generationid, int checkpointed_stream,
-                      unsigned long *vm_generationid_addr,
+                      int checkpointed_stream,
                       struct restore_callbacks *callbacks);
 /**
  * xc_domain_restore writes a file to disk that contains the device
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index d015cf4..cae6f53 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -584,12 +584,7 @@ retry_transaction:
                         ARRAY_SIZE(rwperm));
     }
 
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
-        libxl__xs_mkdir(gc, t,
-            libxl__sprintf(gc, "%s/hvmloader/generation-id-address", dom_path),
-                        rwperm, ARRAY_SIZE(rwperm));
-
-                    vm_list = libxl_list_vm(ctx, &nb_vm);
+    vm_list = libxl_list_vm(ctx, &nb_vm);
     if (!vm_list) {
         LOG(ERROR, "cannot get number of running guests");
         rc = ERROR_FAIL;
@@ -903,7 +898,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
         goto out;
     }
     libxl__xc_domain_restore(egc, dcs,
-                             hvm, pae, superpages, 1);
+                             hvm, pae, superpages);
     return;
 
  out:
@@ -911,7 +906,7 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 }
 
 void libxl__srm_callout_callback_restore_results(unsigned long store_mfn,
-          unsigned long console_mfn, unsigned long genidad, void *user)
+          unsigned long console_mfn, void *user)
 {
     libxl__save_helper_state *shs = user;
     libxl__domain_create_state *dcs = CONTAINER_OF(shs, *dcs, shs);
@@ -920,7 +915,6 @@ void libxl__srm_callout_callback_restore_results(unsigned long store_mfn,
 
     state->store_mfn =            store_mfn;
     state->console_mfn =          console_mfn;
-    state->vm_generationid_addr = genidad;
     shs->need_results =           0;
 }
 
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 661999c..4804fdf 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -279,7 +279,6 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
 
     state->store_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->store_domid);
     state->console_port = xc_evtchn_alloc_unbound(ctx->xch, domid, state->console_domid);
-    state->vm_generationid_addr = 0;
 
     if (info->type == LIBXL_DOMAIN_TYPE_HVM)
         hvm_set_conf_params(ctx->xch, domid, info);
@@ -297,7 +296,7 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     libxl_ctx *ctx = libxl__gc_owner(gc);
     char *dom_path, *vm_path;
     xs_transaction_t t;
-    char **ents, **hvm_ents;
+    char **ents;
     int i, rc;
 
     rc = libxl_domain_sched_params_set(CTX, domid, &info->sched_params);
@@ -334,13 +333,6 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
                             ? "online" : "offline";
     }
 
-    hvm_ents = NULL;
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
-        hvm_ents = libxl__calloc(gc, 3, sizeof(char *));
-        hvm_ents[0] = "hvmloader/generation-id-address";
-        hvm_ents[1] = GCSPRINTF("0x%lx", state->vm_generationid_addr);
-    }
-
     dom_path = libxl__xs_get_dompath(gc, domid);
     if (!dom_path) {
         return ERROR_FAIL;
@@ -351,9 +343,6 @@ retry_transaction:
     t = xs_transaction_start(ctx->xsh);
 
     libxl__xs_writev(gc, t, dom_path, ents);
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
-        libxl__xs_writev(gc, t, dom_path, hvm_ents);
-
     libxl__xs_writev(gc, t, dom_path, local_ents);
     libxl__xs_writev(gc, t, vm_path, vms_ents);
 
@@ -1502,7 +1491,6 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
     STATE_AO_GC(dss->ao);
     int port;
     int rc = ERROR_FAIL;
-    unsigned long vm_generationid_addr;
 
     /* Convenience aliases */
     const uint32_t domid = dss->domid;
@@ -1521,19 +1509,10 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
 
     switch (type) {
     case LIBXL_DOMAIN_TYPE_HVM: {
-        char *path;
-        char *addr;
-
-        path = GCSPRINTF("%s/hvmloader/generation-id-address",
-                              libxl__xs_get_dompath(gc, domid));
-        addr = libxl__xs_read(gc, XBT_NULL, path);
-
-        vm_generationid_addr = (addr) ? strtoul(addr, NULL, 0) : 0;
         dss->hvm = 1;
         break;
     }
     case LIBXL_DOMAIN_TYPE_PV:
-        vm_generationid_addr = 0;
         dss->hvm = 0;
         break;
     default:
@@ -1580,7 +1559,7 @@ void libxl__domain_suspend(libxl__egc *egc, libxl__domain_suspend_state *dss)
     callbacks->switch_qemu_logdirty = libxl__domain_suspend_common_switch_qemu_logdirty;
     dss->shs.callbacks.save.toolstack_save = libxl__toolstack_save;
 
-    libxl__xc_domain_save(egc, dss, vm_generationid_addr);
+    libxl__xc_domain_save(egc, dss);
     return;
 
  out:
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 082749e..457424f 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -960,8 +960,6 @@ typedef struct {
     uint32_t console_domid;
     unsigned long console_mfn;
 
-    unsigned long vm_generationid_addr;
-
     char *saved_state;
 
     libxl__file_reference pv_kernel;
@@ -2757,8 +2755,7 @@ _hidden void libxl__domain_suspend(libxl__egc *egc,
 
 
 /* calls libxl__xc_domain_suspend_done when done */
-_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*,
-                                   unsigned long vm_generationid_addr);
+_hidden void libxl__xc_domain_save(libxl__egc*, libxl__domain_suspend_state*);
 /* If rc==0 then retval is the return value from xc_domain_save
  * and errnoval is the errno value it provided.
  * If rc!=0, retval and errnoval are undefined. */
@@ -2782,8 +2779,7 @@ _hidden int libxl__toolstack_save(uint32_t domid, uint8_t **buf,
 /* calls libxl__xc_domain_restore_done when done */
 _hidden void libxl__xc_domain_restore(libxl__egc *egc,
                                       libxl__domain_create_state *dcs,
-                                      int hvm, int pae, int superpages,
-                                      int no_incr_generationid);
+                                      int hvm, int pae, int superpages);
 /* If rc==0 then retval is the return value from xc_domain_save
  * and errnoval is the errno value it provided.
  * If rc!=0, retval and errnoval are undefined. */
diff --git a/tools/libxl/libxl_save_callout.c b/tools/libxl/libxl_save_callout.c
index c35da4d..1c9f806 100644
--- a/tools/libxl/libxl_save_callout.c
+++ b/tools/libxl/libxl_save_callout.c
@@ -41,8 +41,7 @@ static void helper_done(libxl__egc *egc, libxl__save_helper_state *shs);
 /*----- entrypoints -----*/
 
 void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
-                              int hvm, int pae, int superpages,
-                              int no_incr_generationid)
+                              int hvm, int pae, int superpages)
 {
     STATE_AO_GC(dcs->ao);
 
@@ -59,7 +58,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
         state->store_port,
         state->store_domid, state->console_port,
         state->console_domid,
-        hvm, pae, superpages, no_incr_generationid,
+        hvm, pae, superpages,
         cbflags, dcs->checkpointed_stream,
     };
 
@@ -75,8 +74,7 @@ void libxl__xc_domain_restore(libxl__egc *egc, libxl__domain_create_state *dcs,
                argnums, ARRAY_SIZE(argnums));
 }
 
-void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss,
-                           unsigned long vm_generationid_addr)
+void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss)
 {
     STATE_AO_GC(dss->ao);
     int r, rc, toolstack_data_fd = -1;
@@ -112,7 +110,7 @@ void libxl__xc_domain_save(libxl__egc *egc, libxl__domain_suspend_state *dss,
     }
 
     const unsigned long argnums[] = {
-        dss->domid, 0, 0, dss->xcflags, dss->hvm, vm_generationid_addr,
+        dss->domid, 0, 0, dss->xcflags, dss->hvm,
         toolstack_data_fd, toolstack_data_len,
         cbflags,
     };
diff --git a/tools/libxl/libxl_save_helper.c b/tools/libxl/libxl_save_helper.c
index b259bd0..c760dd3 100644
--- a/tools/libxl/libxl_save_helper.c
+++ b/tools/libxl/libxl_save_helper.c
@@ -211,7 +211,6 @@ int main(int argc, char **argv)
         uint32_t max_factor =      strtoul(NEXTARG,0,10);
         uint32_t flags =           strtoul(NEXTARG,0,10);
         int hvm =                  atoi(NEXTARG);
-        unsigned long genidad =    strtoul(NEXTARG,0,10);
         toolstack_save_fd  =       atoi(NEXTARG);
         toolstack_save_len =       strtoul(NEXTARG,0,10);
         unsigned cbflags =         strtoul(NEXTARG,0,10);
@@ -224,7 +223,7 @@ int main(int argc, char **argv)
 
         startup("save");
         r = xc_domain_save(xch, io_fd, dom, max_iters, max_factor, flags,
-                           &helper_save_callbacks, hvm, genidad);
+                           &helper_save_callbacks, hvm);
         complete(r);
 
     } else if (!strcmp(mode,"--restore-domain")) {
@@ -238,7 +237,6 @@ int main(int argc, char **argv)
         unsigned int hvm =         strtoul(NEXTARG,0,10);
         unsigned int pae =         strtoul(NEXTARG,0,10);
         int superpages =           strtoul(NEXTARG,0,10);
-        int no_incr_genidad =      strtoul(NEXTARG,0,10);
         unsigned cbflags =         strtoul(NEXTARG,0,10);
         int checkpointed =         strtoul(NEXTARG,0,10);
         assert(!*++argv);
@@ -247,15 +245,14 @@ int main(int argc, char **argv)
 
         unsigned long store_mfn = 0;
         unsigned long console_mfn = 0;
-        unsigned long genidad = 0;
 
         startup("restore");
         r = xc_domain_restore(xch, io_fd, dom, store_evtchn, &store_mfn,
                               store_domid, console_evtchn, &console_mfn,
                               console_domid, hvm, pae, superpages,
-                              no_incr_genidad, checkpointed, &genidad,
+                              checkpointed,
                               &helper_restore_callbacks);
-        helper_stub_restore_results(store_mfn,console_mfn,genidad,0);
+        helper_stub_restore_results(store_mfn,console_mfn,0);
         complete(r);
 
     } else {
diff --git a/tools/libxl/libxl_save_msgs_gen.pl b/tools/libxl/libxl_save_msgs_gen.pl
index 745e2ac..88f4921 100755
--- a/tools/libxl/libxl_save_msgs_gen.pl
+++ b/tools/libxl/libxl_save_msgs_gen.pl
@@ -32,8 +32,7 @@ our @msgs = (
     [  7, 'rcxW',   "toolstack_restore",     [qw(uint32_t domid
                                                 BLOCK tsdata)] ],
     [  8, 'r',      "restore_results",       ['unsigned long', 'store_mfn',
-                                              'unsigned long', 'console_mfn',
-                                              'unsigned long', 'genidad'] ],
+                                              'unsigned long', 'console_mfn'] ],
     [  9, 'srW',    "complete",              [qw(int retval
                                                  int errnoval)] ],
 );
-- 
1.7.10.4

[PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

Toolstacks may specify a VM generation ID using the
u.hvm.generation_id field into libxl_domain_build_info structure, when
creating a domain.

The toolstack is responsible for providing the correct generation ID
according to the Microsoft specification (e.g., generating new random
ones as appropriate when restoring).

Although the specification requires that a ACPI Notify event is raised
if the generation ID is changed, the generation ID is never changed
when the domain is in a state to receive such an event (it's either
newly created or suspended).

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 tools/libxl/Makefile         |    1 +
 tools/libxl/libxl.h          |    5 +++
 tools/libxl/libxl_dom.c      |   10 +++++
 tools/libxl/libxl_internal.h |    4 ++
 tools/libxl/libxl_types.idl  |    1 +
 tools/libxl/libxl_vm_genid.c |   89 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 110 insertions(+)
 create mode 100644 tools/libxl/libxl_vm_genid.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 4cfa275..5d0e2ec 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -77,6 +77,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
 			libxl_json.o libxl_aoutils.o libxl_numa.o \
 			libxl_save_callout.o _libxl_save_msgs_callout.o \
 			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+LIBXL_OBJS += libxl_vm_genid.o
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
 
 LIBXL_TESTS += timedereg
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 80947c3..9dd57fd 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -95,6 +95,11 @@
 #define LIBXL_HAVE_BUILDINFO_EVENT_CHANNELS 1
 
 /*
+ * libxl_domain_build_info has the u.hvm.generation_id field.
+ */
+#define LIBXL_HAVE_BUILDINFO_HVM_GENERATION_ID 1
+
+/*
  * LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE indicates that a
  * 'direct_io_safe' field (of boolean type) is present in
  * libxl_device_disk.
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 4804fdf..80bc9ff 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -314,6 +314,16 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
     if (info->cpuid != NULL)
         libxl_cpuid_set(ctx, domid, info->cpuid);
 
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM 
+            && !libxl_uuid_is_nil(&info->u.hvm.generation_id)) {
+        rc = libxl__vm_generation_id_set(CTX, domid,
+                                         &info->u.hvm.generation_id);
+        if (rc) {
+            LOG(ERROR, "Failed to set VM Generation ID");
+            return rc;
+        }
+    }
+
     ents = libxl__calloc(gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
     ents[0] = "memory/static-max";
     ents[1] = GCSPRINTF("%"PRId64, info->max_memkb);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 457424f..d938b90 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3052,6 +3052,10 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
     libxl_bitmap_copy(CTX, &cndt->nodemap, nodemap);
 }
 
+_hidden int libxl__vm_generation_id_set(libxl_ctx *ctx, uint32_t domid,
+                                        const libxl_uuid *uuid);
+
+
 /* Som handy macros for defbool type. */
 #define LIBXL__DEFBOOL_DEFAULT     (0)
 #define LIBXL__DEFBOOL_FALSE       (-1)
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 52f1aa9..7800509 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -373,6 +373,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("xen_platform_pci", libxl_defbool),
                                        ("usbdevice_list",   libxl_string_list),
                                        ("vendor_device",    libxl_vendor_device),
+                                       ("generation_id",    libxl_uuid),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
diff --git a/tools/libxl/libxl_vm_genid.c b/tools/libxl/libxl_vm_genid.c
new file mode 100644
index 0000000..378dcf2
--- /dev/null
+++ b/tools/libxl/libxl_vm_genid.c
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2014 Citrix Systems R&D Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+#include <xenctrl.h>
+#include <xen/hvm/params.h>
+
+/*
+ * Set a HVM domain's VM Generation ID.
+ *
+ * For further details, refer to the "Virtual Machine Generation ID"
+ * document from Microsoft.
+ * 
+ *   http://www.microsoft.com/en-us/download/details.aspx?id=30707
+ */
+int libxl__vm_generation_id_set(libxl_ctx *ctx, uint32_t domid,
+                                const libxl_uuid *uuid)
+{
+    GC_INIT(ctx);
+    char *dom_path;
+    char *key;
+    uint64_t genid[2];
+    uint64_t paddr = 0;
+    int rc;
+
+    memcpy(genid, libxl_uuid_bytearray((libxl_uuid *)uuid), 16);
+
+    /*
+     * Set the "platform/generation-id" XenStore key to pass the ID to
+     * hvmloader.
+     */
+    dom_path = libxl__xs_get_dompath(gc, domid);
+    if (!dom_path) {
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    key = libxl__sprintf(gc, "%s/platform/generation-id", dom_path);
+    rc = libxl__xs_write(gc, XBT_NULL, key, "%"PRIu64 ":%" PRIu64,
+                         genid[0], genid[1]);
+    if (rc < 0)
+        goto out;
+
+    /*
+     * Update the ID in guest memory (if available).
+     */
+    xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_VM_GENERATION_ID_ADDR, &paddr);
+    if (paddr) {
+        void *vaddr;
+
+        vaddr = xc_map_foreign_range(ctx->xch, domid, XC_PAGE_SIZE,
+                                     PROT_READ | PROT_WRITE,
+                                     paddr >> XC_PAGE_SHIFT);
+        if (vaddr == NULL) {
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        memcpy(vaddr + (paddr & ~XC_PAGE_MASK), genid, 2 * sizeof(*genid));
+        munmap(vaddr, XC_PAGE_SIZE);
+
+        /*
+         * The spec requries an ACPI Notify event is injected into the
+         * guest when the generation ID is changed.
+         *
+         * This is only called for domains that are suspended or newly
+         * created and they won't be in a state to receive such an
+         * event.
+         */
+    }
+
+    rc = 0;
+
+  out:
+    GC_FREE;
+    return rc;
+}
-- 
1.7.10.4

[PATCH 6/6] xl: generate a new random VM generation ID if requested

[David Vrabel]

If the "generation_id" option is set in the domain configuration,
generate and set a new random VM generation ID every time a domain is
created or restored.

xl lacks the infrastructure to fully track the lifecycle of VM images
as they are snapshotted and cloned (etc) so always using a new ID is
the safe option and ensures that a new one will be used where it matters.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 docs/man/xl.cfg.pod.5    |   14 ++++++++++++++
 tools/libxl/xl_cmdimpl.c |    4 ++++
 2 files changed, 18 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a94d037..b74064d 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -936,6 +936,20 @@ number of vendor defined SMBIOS structures (type 128 - 255). Since SMBIOS
 structures do not present their overall size, each entry in the file must be
 preceded by a 32b integer indicating the size of the next structure.
 
+=item B<generation_id=BOOLEAN>
+
+Provide a VM generation ID to the guest.
+
+The VM generation ID as a 128-bit random number that a guest may use
+to determine if the guest has been restored from an earlier snapshot,
+or cloned.
+
+This is required for Microsoft Windows Server 2012 (and later) domain
+controllers.
+
+See also "Virtual Machine Generation ID" by Microsoft
+(http://www.microsoft.com/en-us/download/details.aspx?id=30707).
+
 =back 
 
 =head3 Guest Virtual Time Controls
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 5195914..a611095 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1058,6 +1058,10 @@ static void parse_config_data(const char *config_source,
                                &b_info->u.hvm.smbios_firmware, 0);
         xlu_cfg_replace_string(config, "acpi_firmware",
                                &b_info->u.hvm.acpi_firmware, 0);
+
+        /* Generate and set a new random VM Generation ID? */
+        if (!xlu_cfg_get_long(config, "generation_id", &l, 0))
+            libxl_uuid_generate(&b_info->u.hvm.generation_id);
         break;
     case LIBXL_DOMAIN_TYPE_PV:
     {
-- 
1.7.10.4

Re: [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[Andrew Cooper]

On 03/06/14 14:15, David Vrabel wrote:
> HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
> VM Generation ID.  This parameter will be written by hvmloader and read
> by the toolstack when updating the gen. ID (e.g., after restoring from a
> snapshot).
>
> A HVM parameter is easier for the save/restore code to work with (than
> a XenStore key).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

This needs rebasing as params 32 and 33 was taken by the ioreq-server
series.

> ---
>  xen/include/public/hvm/params.h |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 517a184..4d7c692 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -145,6 +145,9 @@
>  /* SHUTDOWN_* action in case of a triple fault */
>  #define HVM_PARAM_TRIPLE_FAULT_REASON 31
>  
> -#define HVM_NR_PARAMS          32
> +/* Location of the VM Generation ID in guest physical address space. */
> +#define HVM_PARAM_VM_GENERATION_ID_ADDR 32
> +
> +#define HVM_NR_PARAMS          33
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Andrew Cooper]

On 03/06/14 14:15, David Vrabel wrote:
> Toolstacks may specify a VM generation ID using the
> u.hvm.generation_id field into libxl_domain_build_info structure, when
> creating a domain.
>
> The toolstack is responsible for providing the correct generation ID
> according to the Microsoft specification (e.g., generating new random
> ones as appropriate when restoring).
>
> Although the specification requires that a ACPI Notify event is raised
> if the generation ID is changed, the generation ID is never changed
> when the domain is in a state to receive such an event (it's either
> newly created or suspended).
>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  tools/libxl/Makefile         |    1 +
>  tools/libxl/libxl.h          |    5 +++
>  tools/libxl/libxl_dom.c      |   10 +++++
>  tools/libxl/libxl_internal.h |    4 ++
>  tools/libxl/libxl_types.idl  |    1 +
>  tools/libxl/libxl_vm_genid.c |   89 ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 110 insertions(+)
>  create mode 100644 tools/libxl/libxl_vm_genid.c
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 4cfa275..5d0e2ec 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -77,6 +77,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
>  			libxl_json.o libxl_aoutils.o libxl_numa.o \
>  			libxl_save_callout.o _libxl_save_msgs_callout.o \
>  			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
> +LIBXL_OBJS += libxl_vm_genid.o
>  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
>  
>  LIBXL_TESTS += timedereg
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 80947c3..9dd57fd 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -95,6 +95,11 @@
>  #define LIBXL_HAVE_BUILDINFO_EVENT_CHANNELS 1
>  
>  /*
> + * libxl_domain_build_info has the u.hvm.generation_id field.
> + */
> +#define LIBXL_HAVE_BUILDINFO_HVM_GENERATION_ID 1
> +
> +/*
>   * LIBXL_HAVE_DEVICE_DISK_DIRECT_IO_SAFE indicates that a
>   * 'direct_io_safe' field (of boolean type) is present in
>   * libxl_device_disk.
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 4804fdf..80bc9ff 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -314,6 +314,16 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
>      if (info->cpuid != NULL)
>          libxl_cpuid_set(ctx, domid, info->cpuid);
>  
> +    if (info->type == LIBXL_DOMAIN_TYPE_HVM 
> +            && !libxl_uuid_is_nil(&info->u.hvm.generation_id)) {
> +        rc = libxl__vm_generation_id_set(CTX, domid,
> +                                         &info->u.hvm.generation_id);
> +        if (rc) {
> +            LOG(ERROR, "Failed to set VM Generation ID");
> +            return rc;
> +        }
> +    }
> +
>      ents = libxl__calloc(gc, 12 + (info->max_vcpus * 2) + 2, sizeof(char *));
>      ents[0] = "memory/static-max";
>      ents[1] = GCSPRINTF("%"PRId64, info->max_memkb);
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 457424f..d938b90 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -3052,6 +3052,10 @@ void libxl__numa_candidate_put_nodemap(libxl__gc *gc,
>      libxl_bitmap_copy(CTX, &cndt->nodemap, nodemap);
>  }
>  
> +_hidden int libxl__vm_generation_id_set(libxl_ctx *ctx, uint32_t domid,
> +                                        const libxl_uuid *uuid);
> +
> +
>  /* Som handy macros for defbool type. */
>  #define LIBXL__DEFBOOL_DEFAULT     (0)
>  #define LIBXL__DEFBOOL_FALSE       (-1)
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 52f1aa9..7800509 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -373,6 +373,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         ("xen_platform_pci", libxl_defbool),
>                                         ("usbdevice_list",   libxl_string_list),
>                                         ("vendor_device",    libxl_vendor_device),
> +                                       ("generation_id",    libxl_uuid),
>                                         ])),
>                   ("pv", Struct(None, [("kernel", string),
>                                        ("slack_memkb", MemKB),
> diff --git a/tools/libxl/libxl_vm_genid.c b/tools/libxl/libxl_vm_genid.c
> new file mode 100644
> index 0000000..378dcf2
> --- /dev/null
> +++ b/tools/libxl/libxl_vm_genid.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (C) 2014 Citrix Systems R&D Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +
> +#include "libxl_internal.h"
> +
> +#include <xenctrl.h>
> +#include <xen/hvm/params.h>
> +
> +/*
> + * Set a HVM domain's VM Generation ID.
> + *
> + * For further details, refer to the "Virtual Machine Generation ID"
> + * document from Microsoft.
> + * 
> + *   http://www.microsoft.com/en-us/download/details.aspx?id=30707
> + */
> +int libxl__vm_generation_id_set(libxl_ctx *ctx, uint32_t domid,
> +                                const libxl_uuid *uuid)
> +{
> +    GC_INIT(ctx);
> +    char *dom_path;
> +    char *key;
> +    uint64_t genid[2];
> +    uint64_t paddr = 0;
> +    int rc;
> +
> +    memcpy(genid, libxl_uuid_bytearray((libxl_uuid *)uuid), 16);
> +
> +    /*
> +     * Set the "platform/generation-id" XenStore key to pass the ID to
> +     * hvmloader.
> +     */
> +    dom_path = libxl__xs_get_dompath(gc, domid);
> +    if (!dom_path) {
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +    key = libxl__sprintf(gc, "%s/platform/generation-id", dom_path);
> +    rc = libxl__xs_write(gc, XBT_NULL, key, "%"PRIu64 ":%" PRIu64,
> +                         genid[0], genid[1]);
> +    if (rc < 0)
> +        goto out;
> +
> +    /*
> +     * Update the ID in guest memory (if available).
> +     */
> +    xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_VM_GENERATION_ID_ADDR, &paddr);

This will break 32bit toolstack builds, because xc_get_hvm_param() takes
a pointer to an unsigned long pointer rather than a uint64_t pointer.

It appears that it will also silently truncate the 64bit hvm param to
32bits for a 32bit toolstack...

~Andrew

> +    if (paddr) {
> +        void *vaddr;
> +
> +        vaddr = xc_map_foreign_range(ctx->xch, domid, XC_PAGE_SIZE,
> +                                     PROT_READ | PROT_WRITE,
> +                                     paddr >> XC_PAGE_SHIFT);
> +        if (vaddr == NULL) {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        memcpy(vaddr + (paddr & ~XC_PAGE_MASK), genid, 2 * sizeof(*genid));
> +        munmap(vaddr, XC_PAGE_SIZE);
> +
> +        /*
> +         * The spec requries an ACPI Notify event is injected into the
> +         * guest when the generation ID is changed.
> +         *
> +         * This is only called for domains that are suspended or newly
> +         * created and they won't be in a state to receive such an
> +         * event.
> +         */
> +    }
> +
> +    rc = 0;
> +
> +  out:
> +    GC_FREE;
> +    return rc;
> +}

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 03/06/14 14:28, Andrew Cooper wrote:
> On 03/06/14 14:15, David Vrabel wrote:
>
>> +    /*
>> +     * Update the ID in guest memory (if available).
>> +     */
>> +    xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_VM_GENERATION_ID_ADDR, &paddr);
> 
> This will break 32bit toolstack builds, because xc_get_hvm_param() takes
> a pointer to an unsigned long pointer rather than a uint64_t pointer.

Well that's rather foolish of it.  I've fixed up the libxc functions to
take the correct uint32_t index and uint64_t value parameters.

David

ps. please trim your replies.

Re: [PATCH 3/6] hvmloader: add helper functions to get/set HVM params

[Konrad Rzeszutek Wilk]

On Tue, Jun 03, 2014 at 02:15:37PM +0100, David Vrabel wrote:
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  tools/firmware/hvmloader/Makefile    |    1 +
>  tools/firmware/hvmloader/hvm_param.c |   36 ++++++++++++++++++++++++++++++++++
>  tools/firmware/hvmloader/hvmloader.c |   14 ++-----------
>  tools/firmware/hvmloader/util.h      |    9 +++++++++
>  tools/firmware/hvmloader/xenbus.c    |   14 +++++--------
>  5 files changed, 53 insertions(+), 21 deletions(-)
>  create mode 100644 tools/firmware/hvmloader/hvm_param.c
> 
> diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> index 00ee952..46a79c5 100644
> --- a/tools/firmware/hvmloader/Makefile
> +++ b/tools/firmware/hvmloader/Makefile
> @@ -31,6 +31,7 @@ CFLAGS += $(CFLAGS_xeninclude)
>  OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
>  OBJS += smp.o cacheattr.o xenbus.o
>  OBJS += e820.o pci.o pir.o ctype.o
> +OBJS += hvm_param.o
>  ifeq ($(debug),y)
>  OBJS += tests.o
>  endif
> diff --git a/tools/firmware/hvmloader/hvm_param.c b/tools/firmware/hvmloader/hvm_param.c
> new file mode 100644
> index 0000000..f7d8720
> --- /dev/null
> +++ b/tools/firmware/hvmloader/hvm_param.c
> @@ -0,0 +1,36 @@
> +/*
> + * hvm_param.c: get/set HVM params.
> + *
> + * Copyright (C) 2014 Citrix Systems R&D Ltd.
> + */
> +#include "util.h"
> +#include "config.h"
> +#include "hypercall.h"
> +
> +#include <xen/hvm/params.h>
> +
> +int hvm_param_get(uint32_t index, uint64_t *value)
> +{
> +    struct xen_hvm_param p;
> +    int ret;
> +
> +    p.domid = DOMID_SELF;
> +    p.index = index;
> +
> +    ret = hypercall_hvm_op(HVMOP_get_param, &p);
> +    if (ret == 0)
> +        *value = p.value;
> +
> +    return ret;
> +}
> +
> +int hvm_param_set(uint32_t index, uint64_t value)
> +{
> +    struct xen_hvm_param p;
> +
> +    p.domid = DOMID_SELF;
> +    p.index = index;
> +    p.value = value;
> +
> +    return hypercall_hvm_op(HVMOP_set_param, &p);
> +}
> diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
> index 1cc8cf2..7b0da38 100644
> --- a/tools/firmware/hvmloader/hvmloader.c
> +++ b/tools/firmware/hvmloader/hvmloader.c
> @@ -176,14 +176,10 @@ static void cmos_write_memory_size(void)
>  static void init_vm86_tss(void)
>  {
>      void *tss;
> -    struct xen_hvm_param p;
>  
>      tss = mem_alloc(128, 128);
>      memset(tss, 0, 128);
> -    p.domid = DOMID_SELF;
> -    p.index = HVM_PARAM_VM86_TSS;
> -    p.value = virt_to_phys(tss);
> -    hypercall_hvm_op(HVMOP_set_param, &p);
> +    hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss));
>      printf("vm86 TSS at %08lx\n", virt_to_phys(tss));
>  }
>  
> @@ -314,12 +310,6 @@ int main(void)
>  
>      if ( acpi_enabled )
>      {
> -        struct xen_hvm_param p = {
> -            .domid = DOMID_SELF,
> -            .index = HVM_PARAM_ACPI_IOPORTS_LOCATION,
> -            .value = 1,
> -        };
> -
>          if ( bios->acpi_build_tables )
>          {
>              printf("Loading ACPI ...\n");
> @@ -328,7 +318,7 @@ int main(void)
>  
>          acpi_enable_sci();
>  
> -        hypercall_hvm_op(HVMOP_set_param, &p);
> +        hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1);
>      }
>  
>      init_vm86_tss();
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 9ccb905..e4b6be4 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -210,6 +210,15 @@ const char *xenstore_read(const char *path, const char *default_resp);
>   */
>  int xenstore_write(const char *path, const char *value);
>  
> +
> +/* Get a HVM param.
> + */
> +int hvm_param_get(uint32_t index, uint64_t *value);
> +
> +/* Set a HVM param.
> + */
> +int hvm_param_set(uint32_t index, uint64_t value);
> +
>  /* Setup PCI bus */
>  void pci_setup(void);
>  
> diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> index fe72e97..64c2176 100644
> --- a/tools/firmware/hvmloader/xenbus.c
> +++ b/tools/firmware/hvmloader/xenbus.c
> @@ -41,21 +41,17 @@ static char payload[XENSTORE_PAYLOAD_MAX + 1];  /* Unmarshalling area */
>   * Call once, before any other xenbus actions. */
>  void xenbus_setup(void)
>  {
> -    xen_hvm_param_t param;
> +    uint64_t val;
>  
>      /* Ask Xen where the xenbus shared page is. */
> -    param.domid = DOMID_SELF;
> -    param.index = HVM_PARAM_STORE_PFN;
> -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> +    if ( hvm_param_get(HVM_PARAM_STORE_PFN, &val) )
>          BUG();
> -    rings = (void *) (unsigned long) (param.value << PAGE_SHIFT);
> +    rings = (void *) (unsigned long) (val << PAGE_SHIFT);
>  
>      /* Ask Xen where the xenbus event channel is. */
> -    param.domid = DOMID_SELF;
> -    param.index = HVM_PARAM_STORE_EVTCHN;
> -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> +    if ( hvm_param_get(HVM_PARAM_STORE_EVTCHN, &val) )
>          BUG();
> -    event = param.value;
> +    event = val;
>  
>      printf("Xenbus rings @0x%lx, event channel %lu\n",
>             (unsigned long) rings, (unsigned long) event);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1/6] docs: update docs for the ~/platform/generation-id key

[Ian Campbell]

On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

AIUI (some of) the things said here are only true after this series has
been applied, which is fine but means I won't pick it up now independent
of the rest. If you wanted to merge it into the patch which actually
makes the changes or push it to the end then that would be fine, or you
are welcome to just leave it here, no big deal.

Ian.

> ---
>  docs/misc/xenstore-paths.markdown |   24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/misc/xenstore-paths.markdown b/docs/misc/xenstore-paths.markdown
> index 70ab7f4..22fcfaa 100644
> --- a/docs/misc/xenstore-paths.markdown
> +++ b/docs/misc/xenstore-paths.markdown
> @@ -166,11 +166,6 @@ use the xenstore-based protocol instead (see ~/control/shutdown,
>  below) even if the guest has advertised support for the event channel
>  protocol.
>  
> -#### ~/hvmloader/generation-id-address = ADDRESS [r,HVM,INTERNAL]
> -
> -The hexadecimal representation of the address of the domain's
> -"generation id".
> -
>  #### ~/hvmloader/allow-memory-relocate = ("1"|"0") [HVM,INTERNAL]
>  
>  If the default low MMIO hole (below 4GiB) is not big enough for all
> @@ -193,9 +188,22 @@ Various platform properties.
>  
>  #### ~/platform/generation-id = INTEGER ":" INTEGER [HVM,INTERNAL]
>  
> -Two 64 bit values that represent the Windows Generation ID.
> -Is used by the BIOS initializer to get this value.
> -If not present or "0:0" (all zeroes) device will not be present to the machine.
> +The upper and lower 64-bit words of the 128-bit VM Generation ID.
> +
> +This key is used by hvmloader to create the ACPI VM Generation ID
> +device.  It initialises a 16 octet region of guest memory with this
> +value.  The guest physical address of this region is saved in the
> +HVM_PARAM_VM_GENERATION_ID_ADDR HVM parameter.
> +
> +If this key is not present, is empty, or is all-zeros ("0:0") then the
> +ACPI device is not created.
> +
> +When restoring a guest, the toolstack may (in certain circumstances)
> +need generate a new random generation ID and write it to guest memory
> +at the guest physical address in HVM_PARAM_VM_GENERATION_ID_ADDR.
> +
> +See Microsoft's "Virtual Machine Generation ID" specification for the
> +circumstances where the generation ID needs to be changed.
>  
>  ### Frontend device paths
>  

Re: [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[Ian Campbell]

On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
> VM Generation ID.  This parameter will be written by hvmloader and read
> by the toolstack when updating the gen. ID (e.g., after restoring from a
> snapshot).
> 
> A HVM parameter is easier for the save/restore code to work with (than
> a XenStore key).
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix,com>

It's a bit ambiguous for hvm params which the hypervisor doesn't
actually interpret but strictly speaking this needs to be CCd to the h/v
guys. Added Jan/Keir.Tim.

> ---
>  xen/include/public/hvm/params.h |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h
> index 517a184..4d7c692 100644
> --- a/xen/include/public/hvm/params.h
> +++ b/xen/include/public/hvm/params.h
> @@ -145,6 +145,9 @@
>  /* SHUTDOWN_* action in case of a triple fault */
>  #define HVM_PARAM_TRIPLE_FAULT_REASON 31
>  
> -#define HVM_NR_PARAMS          32
> +/* Location of the VM Generation ID in guest physical address space. */
> +#define HVM_PARAM_VM_GENERATION_ID_ADDR 32
> +
> +#define HVM_NR_PARAMS          33
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

Re: [PATCH 3/6] hvmloader: add helper functions to get/set HVM params

[Ian Campbell]

On Tue, 2014-06-03 at 15:43 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 03, 2014 at 02:15:37PM +0100, David Vrabel wrote:
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

In the past Keir used to have a say in the maintenance of this even
though it lives under tools (it is in some loose sense considered part
of the hypervisor). CCing the hypervisor guys for opinions on a) this
patch and b) the idea of adding a hvmloader entry to the MAINTAINERS
file and/or who should be listed there.

Ian.


> > ---
> >  tools/firmware/hvmloader/Makefile    |    1 +
> >  tools/firmware/hvmloader/hvm_param.c |   36 ++++++++++++++++++++++++++++++++++
> >  tools/firmware/hvmloader/hvmloader.c |   14 ++-----------
> >  tools/firmware/hvmloader/util.h      |    9 +++++++++
> >  tools/firmware/hvmloader/xenbus.c    |   14 +++++--------
> >  5 files changed, 53 insertions(+), 21 deletions(-)
> >  create mode 100644 tools/firmware/hvmloader/hvm_param.c
> > 
> > diff --git a/tools/firmware/hvmloader/Makefile b/tools/firmware/hvmloader/Makefile
> > index 00ee952..46a79c5 100644
> > --- a/tools/firmware/hvmloader/Makefile
> > +++ b/tools/firmware/hvmloader/Makefile
> > @@ -31,6 +31,7 @@ CFLAGS += $(CFLAGS_xeninclude)
> >  OBJS  = hvmloader.o mp_tables.o util.o smbios.o 
> >  OBJS += smp.o cacheattr.o xenbus.o
> >  OBJS += e820.o pci.o pir.o ctype.o
> > +OBJS += hvm_param.o
> >  ifeq ($(debug),y)
> >  OBJS += tests.o
> >  endif
> > diff --git a/tools/firmware/hvmloader/hvm_param.c b/tools/firmware/hvmloader/hvm_param.c
> > new file mode 100644
> > index 0000000..f7d8720
> > --- /dev/null
> > +++ b/tools/firmware/hvmloader/hvm_param.c
> > @@ -0,0 +1,36 @@
> > +/*
> > + * hvm_param.c: get/set HVM params.
> > + *
> > + * Copyright (C) 2014 Citrix Systems R&D Ltd.
> > + */
> > +#include "util.h"
> > +#include "config.h"
> > +#include "hypercall.h"
> > +
> > +#include <xen/hvm/params.h>
> > +
> > +int hvm_param_get(uint32_t index, uint64_t *value)
> > +{
> > +    struct xen_hvm_param p;
> > +    int ret;
> > +
> > +    p.domid = DOMID_SELF;
> > +    p.index = index;
> > +
> > +    ret = hypercall_hvm_op(HVMOP_get_param, &p);
> > +    if (ret == 0)
> > +        *value = p.value;
> > +
> > +    return ret;
> > +}
> > +
> > +int hvm_param_set(uint32_t index, uint64_t value)
> > +{
> > +    struct xen_hvm_param p;
> > +
> > +    p.domid = DOMID_SELF;
> > +    p.index = index;
> > +    p.value = value;
> > +
> > +    return hypercall_hvm_op(HVMOP_set_param, &p);
> > +}
> > diff --git a/tools/firmware/hvmloader/hvmloader.c b/tools/firmware/hvmloader/hvmloader.c
> > index 1cc8cf2..7b0da38 100644
> > --- a/tools/firmware/hvmloader/hvmloader.c
> > +++ b/tools/firmware/hvmloader/hvmloader.c
> > @@ -176,14 +176,10 @@ static void cmos_write_memory_size(void)
> >  static void init_vm86_tss(void)
> >  {
> >      void *tss;
> > -    struct xen_hvm_param p;
> >  
> >      tss = mem_alloc(128, 128);
> >      memset(tss, 0, 128);
> > -    p.domid = DOMID_SELF;
> > -    p.index = HVM_PARAM_VM86_TSS;
> > -    p.value = virt_to_phys(tss);
> > -    hypercall_hvm_op(HVMOP_set_param, &p);
> > +    hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss));
> >      printf("vm86 TSS at %08lx\n", virt_to_phys(tss));
> >  }
> >  
> > @@ -314,12 +310,6 @@ int main(void)
> >  
> >      if ( acpi_enabled )
> >      {
> > -        struct xen_hvm_param p = {
> > -            .domid = DOMID_SELF,
> > -            .index = HVM_PARAM_ACPI_IOPORTS_LOCATION,
> > -            .value = 1,
> > -        };
> > -
> >          if ( bios->acpi_build_tables )
> >          {
> >              printf("Loading ACPI ...\n");
> > @@ -328,7 +318,7 @@ int main(void)
> >  
> >          acpi_enable_sci();
> >  
> > -        hypercall_hvm_op(HVMOP_set_param, &p);
> > +        hvm_param_set(HVM_PARAM_ACPI_IOPORTS_LOCATION, 1);
> >      }
> >  
> >      init_vm86_tss();
> > diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> > index 9ccb905..e4b6be4 100644
> > --- a/tools/firmware/hvmloader/util.h
> > +++ b/tools/firmware/hvmloader/util.h
> > @@ -210,6 +210,15 @@ const char *xenstore_read(const char *path, const char *default_resp);
> >   */
> >  int xenstore_write(const char *path, const char *value);
> >  
> > +
> > +/* Get a HVM param.
> > + */
> > +int hvm_param_get(uint32_t index, uint64_t *value);
> > +
> > +/* Set a HVM param.
> > + */
> > +int hvm_param_set(uint32_t index, uint64_t value);
> > +
> >  /* Setup PCI bus */
> >  void pci_setup(void);
> >  
> > diff --git a/tools/firmware/hvmloader/xenbus.c b/tools/firmware/hvmloader/xenbus.c
> > index fe72e97..64c2176 100644
> > --- a/tools/firmware/hvmloader/xenbus.c
> > +++ b/tools/firmware/hvmloader/xenbus.c
> > @@ -41,21 +41,17 @@ static char payload[XENSTORE_PAYLOAD_MAX + 1];  /* Unmarshalling area */
> >   * Call once, before any other xenbus actions. */
> >  void xenbus_setup(void)
> >  {
> > -    xen_hvm_param_t param;
> > +    uint64_t val;
> >  
> >      /* Ask Xen where the xenbus shared page is. */
> > -    param.domid = DOMID_SELF;
> > -    param.index = HVM_PARAM_STORE_PFN;
> > -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> > +    if ( hvm_param_get(HVM_PARAM_STORE_PFN, &val) )
> >          BUG();
> > -    rings = (void *) (unsigned long) (param.value << PAGE_SHIFT);
> > +    rings = (void *) (unsigned long) (val << PAGE_SHIFT);
> >  
> >      /* Ask Xen where the xenbus event channel is. */
> > -    param.domid = DOMID_SELF;
> > -    param.index = HVM_PARAM_STORE_EVTCHN;
> > -    if ( hypercall_hvm_op(HVMOP_get_param, &param) )
> > +    if ( hvm_param_get(HVM_PARAM_STORE_EVTCHN, &val) )
> >          BUG();
> > -    event = param.value;
> > +    event = val;
> >  
> >      printf("Xenbus rings @0x%lx, event channel %lu\n",
> >             (unsigned long) rings, (unsigned long) event);
> > -- 
> > 1.7.10.4
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

Re: [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[Jan Beulich]

>>> On 10.06.14 at 12:27, <Ian.Campbell@citrix.com> wrote:
> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>> HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
>> VM Generation ID.  This parameter will be written by hvmloader and read
>> by the toolstack when updating the gen. ID (e.g., after restoring from a
>> snapshot).
>> 
>> A HVM parameter is easier for the save/restore code to work with (than
>> a XenStore key).
>> 
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix,com>
> 
> It's a bit ambiguous for hvm params which the hypervisor doesn't
> actually interpret but strictly speaking this needs to be CCd to the h/v
> guys. Added Jan/Keir.Tim.

Adding a parameter used by the tools only in general would seem fine,
yet in the case at hand it looks to be a quite questionable one: I
didn't look too closely at the series so far, and hence it's unclear to
me how any part of the tools can safely know the location of any
particular data item inside the guest kernel. Plus I don't see what
would guarantee that physical address to not change (after all
Windows does swap certain parts of kernel memory if necessary).

Jan

Re: [PATCH 4/6] libxc, libxl, hvmloader: strip out outdated VM generation ID implementation

[Ian Campbell]

On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> The VM generation ID support in libxc/libxl was based on a draft
> specification which subsequently changed considerably.  Remove much of
> the code in anticipation of introducing something simpler that
> conforms to the current specification from Microsoft.
> 
> Switch to using a HVM param (HVM_PARAM_VM_GENERATION_ID_ADDR) instead
> of the hvmloader/generation-id-address XenStore key.  This simplifies
> save/restore since it only needs to transfer the value of this param.
> 
> There are no changes to the migration stream format or the public
> libxl API.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[Andrew Cooper]

On 10/06/14 11:40, Jan Beulich wrote:
>>>> On 10.06.14 at 12:27, <Ian.Campbell@citrix.com> wrote:
>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>>> HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
>>> VM Generation ID.  This parameter will be written by hvmloader and read
>>> by the toolstack when updating the gen. ID (e.g., after restoring from a
>>> snapshot).
>>>
>>> A HVM parameter is easier for the save/restore code to work with (than
>>> a XenStore key).
>>>
>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>> Acked-by: Ian Campbell <ian.campbell@citrix,com>
>>
>> It's a bit ambiguous for hvm params which the hypervisor doesn't
>> actually interpret but strictly speaking this needs to be CCd to the h/v
>> guys. Added Jan/Keir.Tim.
> Adding a parameter used by the tools only in general would seem fine,
> yet in the case at hand it looks to be a quite questionable one: I
> didn't look too closely at the series so far, and hence it's unclear to
> me how any part of the tools can safely know the location of any
> particular data item inside the guest kernel. Plus I don't see what
> would guarantee that physical address to not change (after all
> Windows does swap certain parts of kernel memory if necessary).
>
> Jan

hvmloader allocates a physical area, marked as reserved in the E820,
then tells window "your generation ID is as this physical address".

Its location is never going to change after boot, but tools subsequently
need to know where it is in the guest to update its value.

~Andrew

Re: [PATCH 2/6] hvm: add HVM_PARAM_VM_GENERATION_ID_ADDR

[Jan Beulich]

>>> On 10.06.14 at 12:44, <andrew.cooper3@citrix.com> wrote:
> On 10/06/14 11:40, Jan Beulich wrote:
>>>>> On 10.06.14 at 12:27, <Ian.Campbell@citrix.com> wrote:
>>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>>>> HVM_PARAM_VM_GENERATION_ID_ADDR is the guest physical address of the
>>>> VM Generation ID.  This parameter will be written by hvmloader and read
>>>> by the toolstack when updating the gen. ID (e.g., after restoring from a
>>>> snapshot).
>>>>
>>>> A HVM parameter is easier for the save/restore code to work with (than
>>>> a XenStore key).
>>>>
>>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>>> Acked-by: Ian Campbell <ian.campbell@citrix,com>
>>>
>>> It's a bit ambiguous for hvm params which the hypervisor doesn't
>>> actually interpret but strictly speaking this needs to be CCd to the h/v
>>> guys. Added Jan/Keir.Tim.
>> Adding a parameter used by the tools only in general would seem fine,
>> yet in the case at hand it looks to be a quite questionable one: I
>> didn't look too closely at the series so far, and hence it's unclear to
>> me how any part of the tools can safely know the location of any
>> particular data item inside the guest kernel. Plus I don't see what
>> would guarantee that physical address to not change (after all
>> Windows does swap certain parts of kernel memory if necessary).
> 
> hvmloader allocates a physical area, marked as reserved in the E820,
> then tells window "your generation ID is as this physical address".
> 
> Its location is never going to change after boot, but tools subsequently
> need to know where it is in the guest to update its value.

Ah, okay, that's fine then. I.e. for the interface addition:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> Toolstacks may specify a VM generation ID using the
> u.hvm.generation_id field into libxl_domain_build_info structure, when
> creating a domain.
> 
> The toolstack is responsible for providing the correct generation ID
> according to the Microsoft specification (e.g., generating new random
> ones as appropriate when restoring).

Any reason not to make the libxl interface a (def)bool and have libxl
generate a suitable ID internally? Does anything ever care about the
specific UUID?

Should we consider calling the API field name something more specific,
like "ms_vgid"? I'm thinking of the case where some other OS vendor
reinvents the wheel. (I don't care about the internals, just the API).

> Although the specification requires that a ACPI Notify event is raised
> if the generation ID is changed, the generation ID is never changed
> when the domain is in a state to receive such an event (it's either
> newly created or suspended).
> 
> diff --git a/tools/libxl/libxl_vm_genid.c b/tools/libxl/libxl_vm_genid.c
> new file mode 100644
> index 0000000..378dcf2
> --- /dev/null
> +++ b/tools/libxl/libxl_vm_genid.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (C) 2014 Citrix Systems R&D Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +
> +#include "libxl_internal.h"
> +
> +#include <xenctrl.h>
> +#include <xen/hvm/params.h>
> +
> +/*
> + * Set a HVM domain's VM Generation ID.
> + *
> + * For further details, refer to the "Virtual Machine Generation ID"
> + * document from Microsoft.
> + * 
> + *   http://www.microsoft.com/en-us/download/details.aspx?id=30707
> + */
> +int libxl__vm_generation_id_set(libxl_ctx *ctx, uint32_t domid,
> +                                const libxl_uuid *uuid)

Internal (libxl__) functions should take a libxl__gc *gc rather than a
libxl_ctx, which means you can then drop all the GC_INIT stuff here and
rely on the callers gc..

> +{
> +    GC_INIT(ctx);
> +    char *dom_path;
> +    char *key;

Both of these can be const I think.

> +    uint64_t genid[2];
> +    uint64_t paddr = 0;
> +    int rc;
> +
> +    memcpy(genid, libxl_uuid_bytearray((libxl_uuid *)uuid), 16);

Please can you make libxl_uuid_bytearray const correct instead of
casting away the const here.

Do you not need to worry about endianess when memcpy'ing out of a uuid?

> +
> +    /*
> +     * Set the "platform/generation-id" XenStore key to pass the ID to
> +     * hvmloader.
> +     */
> +    dom_path = libxl__xs_get_dompath(gc, domid);
> +    if (!dom_path) {
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +    key = libxl__sprintf(gc, "%s/platform/generation-id", dom_path);

Please use GCSPRINTF(). That might even shorten things enough that you
can inline it into the xs_write call.

> +    rc = libxl__xs_write(gc, XBT_NULL, key, "%"PRIu64 ":%" PRIu64,
> +                         genid[0], genid[1]);
> +    if (rc < 0)
> +        goto out;
> +
> +    /*
> +     * Update the ID in guest memory (if available).
> +     */
> +    xc_get_hvm_param(ctx->xch, domid, HVM_PARAM_VM_GENERATION_ID_ADDR, &paddr);
> +    if (paddr) {
> +        void *vaddr;
> +
> +        vaddr = xc_map_foreign_range(ctx->xch, domid, XC_PAGE_SIZE,
> +                                     PROT_READ | PROT_WRITE,
> +                                     paddr >> XC_PAGE_SHIFT);
> +        if (vaddr == NULL) {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        memcpy(vaddr + (paddr & ~XC_PAGE_MASK), genid, 2 * sizeof(*genid));
> +        munmap(vaddr, XC_PAGE_SIZE);
> +
> +        /*
> +         * The spec requries an ACPI Notify event is injected into the

"requires"

> +         * guest when the generation ID is changed.
> +         *
> +         * This is only called for domains that are suspended or newly
> +         * created and they won't be in a state to receive such an
> +         * event.
> +         */
> +    }
> +
> +    rc = 0;
> +
> +  out:
> +    GC_FREE;
> +    return rc;
> +}

Re: [PATCH 6/6] xl: generate a new random VM generation ID if requested

[Ian Campbell]

On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> If the "generation_id" option is set in the domain configuration,
> generate and set a new random VM generation ID every time a domain is
> created or restored.
> 
> xl lacks the infrastructure to fully track the lifecycle of VM images
> as they are snapshotted and cloned (etc) so always using a new ID is
> the safe option and ensures that a new one will be used where it matters.
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

(modulo my comment/query on the previous patch wrt to making this is a
boolean).

Ian.

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 10/06/14 12:01, Ian Campbell wrote:
> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>> Toolstacks may specify a VM generation ID using the
>> u.hvm.generation_id field into libxl_domain_build_info structure, when
>> creating a domain.
>>
>> The toolstack is responsible for providing the correct generation ID
>> according to the Microsoft specification (e.g., generating new random
>> ones as appropriate when restoring).
> 
> Any reason not to make the libxl interface a (def)bool and have libxl
> generate a suitable ID internally? Does anything ever care about the
> specific UUID?

Other toolstacks (like xapi) have a better idea of the life-cycle of a
VM than xl does and thus need only use a new ID for snapshots -- they
can retain the same ID if the domain is just rebooted/migrated.

David

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Tue, 2014-06-10 at 13:35 +0100, David Vrabel wrote:
> On 10/06/14 12:01, Ian Campbell wrote:
> > On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> >> Toolstacks may specify a VM generation ID using the
> >> u.hvm.generation_id field into libxl_domain_build_info structure, when
> >> creating a domain.
> >>
> >> The toolstack is responsible for providing the correct generation ID
> >> according to the Microsoft specification (e.g., generating new random
> >> ones as appropriate when restoring).
> > 
> > Any reason not to make the libxl interface a (def)bool and have libxl
> > generate a suitable ID internally? Does anything ever care about the
> > specific UUID?
> 
> Other toolstacks (like xapi) have a better idea of the life-cycle of a
> VM than xl does and thus need only use a new ID for snapshots -- they
> can retain the same ID if the domain is just rebooted/migrated.

They should set the field to false then and libxl won't regenerate it.

Ian.

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 10/06/14 14:37, Ian Campbell wrote:
> On Tue, 2014-06-10 at 13:35 +0100, David Vrabel wrote:
>> On 10/06/14 12:01, Ian Campbell wrote:
>>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>>>> Toolstacks may specify a VM generation ID using the
>>>> u.hvm.generation_id field into libxl_domain_build_info structure, when
>>>> creating a domain.
>>>>
>>>> The toolstack is responsible for providing the correct generation ID
>>>> according to the Microsoft specification (e.g., generating new random
>>>> ones as appropriate when restoring).
>>>
>>> Any reason not to make the libxl interface a (def)bool and have libxl
>>> generate a suitable ID internally? Does anything ever care about the
>>> specific UUID?
>>
>> Other toolstacks (like xapi) have a better idea of the life-cycle of a
>> VM than xl does and thus need only use a new ID for snapshots -- they
>> can retain the same ID if the domain is just rebooted/migrated.
> 
> They should set the field to false then and libxl won't regenerate it.

They still need an interface to specify the ID.

I don't see any value in adding a use_random_genid field just to avoid a
single libxl_uuid_generate() call in the toolstack.

David

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Tue, 2014-06-10 at 14:41 +0100, David Vrabel wrote:
> On 10/06/14 14:37, Ian Campbell wrote:
> > On Tue, 2014-06-10 at 13:35 +0100, David Vrabel wrote:
> >> On 10/06/14 12:01, Ian Campbell wrote:
> >>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> >>>> Toolstacks may specify a VM generation ID using the
> >>>> u.hvm.generation_id field into libxl_domain_build_info structure, when
> >>>> creating a domain.
> >>>>
> >>>> The toolstack is responsible for providing the correct generation ID
> >>>> according to the Microsoft specification (e.g., generating new random
> >>>> ones as appropriate when restoring).
> >>>
> >>> Any reason not to make the libxl interface a (def)bool and have libxl
> >>> generate a suitable ID internally? Does anything ever care about the
> >>> specific UUID?
> >>
> >> Other toolstacks (like xapi) have a better idea of the life-cycle of a
> >> VM than xl does and thus need only use a new ID for snapshots -- they
> >> can retain the same ID if the domain is just rebooted/migrated.
> > 
> > They should set the field to false then and libxl won't regenerate it.
> 
> They still need an interface to specify the ID.

Oh, because of the reboot case. That makes sense.

> I don't see any value in adding a use_random_genid field just to avoid a
> single libxl_uuid_generate() call in the toolstack.

The point is to get as much common stuff into the library as we can, but
as you've now made clear this isn't common.

Ian.

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 10/06/14 12:01, Ian Campbell wrote:
> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> Should we consider calling the API field name something more specific,
> like "ms_vgid"? I'm thinking of the case where some other OS vendor
> reinvents the wheel. (I don't care about the internals, just the API).

generation_id matches the platform/generation-id xenstore key so I would
keep the libxl field name as-is.

> Do you not need to worry about endianess when memcpy'ing out of a uuid?

No. The conversion of uuid to the two 64-bit integers is arbitrary, it
need only be consistent.  The integers in guest memory are in native
endianness.

David

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Tue, 2014-06-10 at 18:59 +0100, David Vrabel wrote:
> On 10/06/14 12:01, Ian Campbell wrote:
> > On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> > Should we consider calling the API field name something more specific,
> > like "ms_vgid"? I'm thinking of the case where some other OS vendor
> > reinvents the wheel. (I don't care about the internals, just the API).
> 
> generation_id matches the platform/generation-id xenstore key so I would
> keep the libxl field name as-is.

One is an internal implementation detail (we can update libx? and
hvmloader in parallel if we have to) while the other is a stable API.

> > Do you not need to worry about endianess when memcpy'ing out of a uuid?
> 
> No. The conversion of uuid to the two 64-bit integers is arbitrary, it
> need only be consistent.  The integers in guest memory are in native
> endianness.

OK. I was wondering if we might want to preserve the byte order so as
not to trample any UUID format which is used (i.e. the stuff described
in
http://en.wikipedia.org/wiki/Universally_unique_identifier#Variants_and_versions ). Or even just be sure that what libxl logs and what is observed in the guest matched (e.g. for debug purposes).

Ian.

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 11/06/14 09:22, Ian Campbell wrote:
> On Tue, 2014-06-10 at 18:59 +0100, David Vrabel wrote:
>> On 10/06/14 12:01, Ian Campbell wrote:
>>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>>> Should we consider calling the API field name something more specific,
>>> like "ms_vgid"? I'm thinking of the case where some other OS vendor
>>> reinvents the wheel. (I don't care about the internals, just the API).
>>
>> generation_id matches the platform/generation-id xenstore key so I would
>> keep the libxl field name as-is.
> 
> One is an internal implementation detail (we can update libx? and
> hvmloader in parallel if we have to) while the other is a stable API.

The specification is for a generic, non-Microsoft specific ACPI device
so I don't see the need to include ms_ in the field name.

>>> Do you not need to worry about endianess when memcpy'ing out of a uuid?
>>
>> No. The conversion of uuid to the two 64-bit integers is arbitrary, it
>> need only be consistent.  The integers in guest memory are in native
>> endianness.
> 
> OK. I was wondering if we might want to preserve the byte order so as
> not to trample any UUID format which is used.

The specification doesn't require that the ID is a UUID.

David

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Wed, 2014-06-11 at 11:53 +0100, David Vrabel wrote:
> On 11/06/14 09:22, Ian Campbell wrote:
> > On Tue, 2014-06-10 at 18:59 +0100, David Vrabel wrote:
> >> On 10/06/14 12:01, Ian Campbell wrote:
> >>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> >>> Should we consider calling the API field name something more specific,
> >>> like "ms_vgid"? I'm thinking of the case where some other OS vendor
> >>> reinvents the wheel. (I don't care about the internals, just the API).
> >>
> >> generation_id matches the platform/generation-id xenstore key so I would
> >> keep the libxl field name as-is.
> > 
> > One is an internal implementation detail (we can update libx? and
> > hvmloader in parallel if we have to) while the other is a stable API.
> 
> The specification is for a generic, non-Microsoft specific ACPI device
> so I don't see the need to include ms_ in the field name.

The name should reflect the spec then. acpi_vm_gen_counter perhaps
(matching the ACPI _CID)

> >>> Do you not need to worry about endianess when memcpy'ing out of a uuid?
> >>
> >> No. The conversion of uuid to the two 64-bit integers is arbitrary, it
> >> need only be consistent.  The integers in guest memory are in native
> >> endianness.
> > 
> > OK. I was wondering if we might want to preserve the byte order so as
> > not to trample any UUID format which is used.
> 
> The specification doesn't require that the ID is a UUID.

True. It does say "cryptographically random integer value". Are you sure
that the result of libxl_uuid_generate meets that standard (rather than
say one of the non-crypto schemes for generating uniqueness)?

Ian.

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[David Vrabel]

On 11/06/14 12:01, Ian Campbell wrote:
> On Wed, 2014-06-11 at 11:53 +0100, David Vrabel wrote:
>> On 11/06/14 09:22, Ian Campbell wrote:
>>> On Tue, 2014-06-10 at 18:59 +0100, David Vrabel wrote:
>>>> On 10/06/14 12:01, Ian Campbell wrote:
>>>>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
>>>>> Should we consider calling the API field name something more specific,
>>>>> like "ms_vgid"? I'm thinking of the case where some other OS vendor
>>>>> reinvents the wheel. (I don't care about the internals, just the API).
>>>>
>>>> generation_id matches the platform/generation-id xenstore key so I would
>>>> keep the libxl field name as-is.
>>>
>>> One is an internal implementation detail (we can update libx? and
>>> hvmloader in parallel if we have to) while the other is a stable API.
>>
>> The specification is for a generic, non-Microsoft specific ACPI device
>> so I don't see the need to include ms_ in the field name.
> 
> The name should reflect the spec then. acpi_vm_gen_counter perhaps
> (matching the ACPI _CID)

It does match the spec;  "Generation ID" is the name of the feature.
Using acpi_vm_gen_counter would just be confusing because the device
does not provide a counter.

>>>>> Do you not need to worry about endianess when memcpy'ing out of a uuid?
>>>>
>>>> No. The conversion of uuid to the two 64-bit integers is arbitrary, it
>>>> need only be consistent.  The integers in guest memory are in native
>>>> endianness.
>>>
>>> OK. I was wondering if we might want to preserve the byte order so as
>>> not to trample any UUID format which is used.
>>
>> The specification doesn't require that the ID is a UUID.
> 
> True. It does say "cryptographically random integer value". Are you sure
> that the result of libxl_uuid_generate meets that standard (rather than
> say one of the non-crypto schemes for generating uniqueness)?

Yes (although it does only provide 122 bits of randomness but I think
the trade-off of a convenient, UUID-base interface is worth it).

David

Re: [PATCH 5/6] libxl: allow a generation ID to be specified at domain creation

[Ian Campbell]

On Wed, 2014-06-11 at 12:47 +0100, David Vrabel wrote:
> On 11/06/14 12:01, Ian Campbell wrote:
> > On Wed, 2014-06-11 at 11:53 +0100, David Vrabel wrote:
> >> On 11/06/14 09:22, Ian Campbell wrote:
> >>> On Tue, 2014-06-10 at 18:59 +0100, David Vrabel wrote:
> >>>> On 10/06/14 12:01, Ian Campbell wrote:
> >>>>> On Tue, 2014-06-03 at 14:15 +0100, David Vrabel wrote:
> >>>>> Should we consider calling the API field name something more specific,
> >>>>> like "ms_vgid"? I'm thinking of the case where some other OS vendor
> >>>>> reinvents the wheel. (I don't care about the internals, just the API).
> >>>>
> >>>> generation_id matches the platform/generation-id xenstore key so I would
> >>>> keep the libxl field name as-is.
> >>>
> >>> One is an internal implementation detail (we can update libx? and
> >>> hvmloader in parallel if we have to) while the other is a stable API.
> >>
> >> The specification is for a generic, non-Microsoft specific ACPI device
> >> so I don't see the need to include ms_ in the field name.

BTW, the fact is that microsoft are defining the spec, regardless of
whether it is generic in nature, so I'm not really sure I buy this
argument.

e.g. if the Linux guys decided they wanted something similar but
incompatible with this spec and therefore a different ACPI device how
would we name that field to avoid confusion?

> > The name should reflect the spec then. acpi_vm_gen_counter perhaps
> > (matching the ACPI _CID)
> 
> It does match the spec;  "Generation ID" is the name of the feature.
> Using acpi_vm_gen_counter would just be confusing because the device
> does not provide a counter.

That's the name it has in the spec.

> >>>>> Do you not need to worry about endianess when memcpy'ing out of a uuid?
> >>>>
> >>>> No. The conversion of uuid to the two 64-bit integers is arbitrary, it
> >>>> need only be consistent.  The integers in guest memory are in native
> >>>> endianness.
> >>>
> >>> OK. I was wondering if we might want to preserve the byte order so as
> >>> not to trample any UUID format which is used.
> >>
> >> The specification doesn't require that the ID is a UUID.
> > 
> > True. It does say "cryptographically random integer value". Are you sure
> > that the result of libxl_uuid_generate meets that standard (rather than
> > say one of the non-crypto schemes for generating uniqueness)?
> 
> Yes (although it does only provide 122 bits of randomness but I think
> the trade-off of a convenient, UUID-base interface is worth it).

Even on BSD?

Ian.