x86/viridian: Re-purpose the HVM parameter to be a feature mask
authorPaul Durrant <paul.durrant@citrix.com>
Tue, 23 Sep 2014 10:40:09 +0000 (11:40 +0100)
committerIan Campbell <ian.campbell@citrix.com>
Tue, 23 Sep 2014 14:17:08 +0000 (15:17 +0100)
The following commits introduced the time reference counter MSR and
TSC/APIC frequency MSRs into the viridian feature set respectively:

e36cd2cdc9674a7a4855d21fb7b3e6e17c4bb33b
84657efd9116f40924aa13c9d5a349e007da716f

The time reference counter MSR feature was then reverted by commit

1cd4fab14ce25859efa4a2af13475e6650a5506c

because a flaw in the implementation meant the counter was reset on
migration.

All of these changes were made without any addtional options being
added to the VM configuration, or any compatibility checks being made
in the domain save/restore code. Hence setting the single boolean
'viridian' option in the VM configuration yields a different set of
features depending on which version of Xen the VM is started on, and the
feature set can change across migration (so new MSRs can magically appear).

This patch grandfathers in the current viridian features set and calls them
the 'base' and 'freq' feature sets. HVM_PARAM_VIRIDIAN is re-purposed as
a feature mask. The hypervisor has only ever allowed it ot be set to 0
or 1, so the presence of the base and freq sets are indicated by setting
bit 0. The freq set can then be turned off by setting bit 1, thus
restoring the pre-Xen-4.4 base set. Newly implemented viridian features
can be optionally enabled in future by setting further bits.

The viridian option in xl.cfg(5) has also been changed to a list so
that the sets can be individually enabled or disabled. For compatibility,
if the option is specified as a boolean, then a true (1) value will enable
the base and freq sets and a false (0) value will not enable any
enlightenments.

This patch also alters the allowed write accesses to HVM_PARAM_VIRIDIAN.
Currently there is nothing to stop the guest writing this value (which,
while harmless to anything else, should not happen) and nothing to
stop a toolstack from setting the value back to zero whilst the guest is
running, causing CPUID leaves to disappear and MSR accesses to start
causing GPFs in the guest. Both of these possibilities are now disallowed:
Once the parameter is set to a non-zero value it may not be modified (only
re-written with the same value), and guests no longer have any write
access.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: David Scott <dave.scott@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
docs/man/xl.cfg.pod.5
tools/libxc/xc_domain_restore.c
tools/libxl/libxl.h
tools/libxl/libxl_dom.c
tools/libxl/libxl_types.idl
tools/libxl/xl_cmdimpl.c
xen/arch/x86/hvm/hvm.c
xen/arch/x86/hvm/viridian.c
xen/include/asm-x86/hvm/hvm.h
xen/include/public/hvm/params.h

index 517ae2fb92db9ce359a7eab9f61ac166077c793f..1bc7fbc7687e11858dc88762773369d529a4e315 100644 (file)
@@ -1134,18 +1134,62 @@ Windows L<http://wiki.xen.org/wiki/XenWindowsGplPv>.
 Setting B<xen_platform_pci=0> with the default device_model "qemu-xen"
 requires at least QEMU 1.6.
 
-=item B<viridian=BOOLEAN>
-
-Turns on or off the exposure of MicroSoft Hyper-V (AKA viridian)
-compatible enlightenments to the guest.  These can improve performance
-of Microsoft Windows guests from Windows Vista and Windows 2008
-onwards and setting this option for such guests is strongly
-recommended. This option should be harmless for other versions of
-Windows (although it will not give any benefit) and the majority of
-other non-Windows OSes. However it is known to be incompatible with
-some other Operating Systems and in some circumstance can prevent
-Xen's own paravirtualisation interfaces for HVM guests from being
-used.
+=item B<viridian=[ "GROUP", "GROUP", ...]>
+
+The groups of Microsoft Hyper-V (AKA viridian) compatible enlightenments
+exposed to the guest. The following groups of enlightenments may be
+specified:
+
+=over 4
+
+=item B<base>
+
+This group incorporates the Hypercall MSRs, Virtual processor index MSR,
+and APIC access MSRs. These enlightenments can improve performance of
+Windows Vista and Windows Server 2008 onwards and setting this option
+for such guests is strongly recommended.
+This group is also a pre-requisite for all others. If it is disabled
+then it is an error to attempt to enable any other group.
+
+=item B<freq>
+
+This group incorporates the TSC and APIC frequency MSRs. These
+enlightenments can improve performance of Windows 7 and Windows
+Server 2008 R2 onwards.
+
+=item B<defaults>
+
+This is a special value that enables the default set of groups, which
+is currently the B<base> and B<freq> groups.
+
+=item B<all>
+
+This is a special value that enables all available groups.
+
+=back
+
+Groups can be disabled by prefixing the name with '!'. So, for example,
+to enable all groups except B<freq>, specify:
+
+=over 4
+
+B<viridian=[ "all", "!freq" ]>
+
+=back
+
+For details of the enlightenments see the latest version of Microsoft's
+Hypervisor Top-Level Functional Specification.
+
+The enlightenments should be harmless for other versions of Windows
+(although they will not give any benefit) and the majority of other
+non-Windows OSes.
+However it is known that they are incompatible with some other Operating
+Systems and in some circumstance can prevent Xen's own paravirtualisation
+interfaces for HVM guests from being used.
+
+The viridian option can be specified as a boolean. A value of true (1)
+is equivalent to the list [ "defaults" ], and a value of false (0) is
+equivalent to an empty list.
 
 =back
 
index fb4ddfc6b2a0c4920ac238fb14f96c73d0dc718e..d8bd9b38570da74a1755aced68ed1003eed7a5f5 100644 (file)
@@ -922,7 +922,7 @@ static int pagebuf_get_one(xc_interface *xch, struct restore_ctx *ctx,
         if ( RDEXACT(fd, &buf->viridian, sizeof(uint32_t)) ||
              RDEXACT(fd, &buf->viridian, sizeof(uint64_t)) )
         {
-            PERROR("error read the viridian flag");
+            PERROR("error reading the viridian enlightenments");
             return -1;
         }
         return pagebuf_get_one(xch, ctx, buf, fd, dom);
@@ -1747,7 +1747,7 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
     }
 
     if (pagebuf.viridian != 0)
-        xc_hvm_param_set(xch, dom, HVM_PARAM_VIRIDIAN, 1);
+        xc_hvm_param_set(xch, dom, HVM_PARAM_VIRIDIAN, pagebuf.viridian);
 
     /*
      * If we are migrating in from a host that does not support
index b7f4f39a4e4c9e41b8bff46ff31aa5d786c9ba2a..9ae0fccc6b871f15ef40289b45a9ea972d1f6560 100644 (file)
  */
 #define LIBXL_HAVE_SCHED_RTDS 1
 
+/*
+ * libxl_domain_build_info has u.hvm.viridian_enable and _disable bitmaps
+ * of the specified width.
+ */
+#define LIBXL_HAVE_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE 1
+#define LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH 64
+
 /*
  * libxl ABI compatibility
  *
index ce0c4ac771d2ab48125a509ff83469c2bfb9e646..2c500ca0f9a40ba1ef60e8b8fd3c445d1d74629e 100644 (file)
@@ -209,14 +209,81 @@ static unsigned long timer_mode(const libxl_domain_build_info *info)
     return ((unsigned long)mode);
 }
 
+#if defined(__i386__) || defined(__x86_64__)
+static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid,
+                                     libxl_domain_build_info *const info)
+{
+    libxl_bitmap enlightenments;
+    libxl_viridian_enlightenment v;
+    uint64_t mask = 0;
+
+    libxl_bitmap_init(&enlightenments);
+    libxl_bitmap_alloc(CTX, &enlightenments,
+                       LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH);
+
+    if (libxl_defbool_val(info->u.hvm.viridian)) {
+        /* Enable defaults */
+        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE);
+        libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ);
+    }
+
+    libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) {
+        if (libxl_bitmap_test(&info->u.hvm.viridian_disable, v)) {
+            LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "%s group both enabled and disabled",
+                       libxl_viridian_enlightenment_to_string(v));
+            goto err;
+        }
+        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
+            libxl_bitmap_set(&enlightenments, v);
+    }
+
+    libxl_for_each_set_bit(v, info->u.hvm.viridian_disable)
+        if (libxl_viridian_enlightenment_to_string(v)) /* check validity */
+            libxl_bitmap_reset(&enlightenments, v);
+
+    /* The base set is a pre-requisite for all others */
+    if (!libxl_bitmap_is_empty(&enlightenments) &&
+        !libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) {
+        LIBXL__LOG(CTX, LIBXL__LOG_ERROR, "base group not enabled");
+        goto err;
+    }
+
+    libxl_for_each_set_bit(v, enlightenments)
+        LOG(DETAIL, "%s group enabled", libxl_viridian_enlightenment_to_string(v));
+
+    if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE)) {
+        mask |= HVMPV_base_freq;
+
+        if (!libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ))
+            mask |= HVMPV_no_freq;
+    }
+
+    if (mask != 0 &&
+        xc_hvm_param_set(CTX->xch,
+                         domid,
+                         HVM_PARAM_VIRIDIAN,
+                         mask) != 0) {
+        LIBXL__LOG_ERRNO(CTX, LIBXL__LOG_ERROR,
+                         "Couldn't set viridian feature mask (0x%"PRIx64")",
+                         mask);
+        goto err;
+    }
+
+    libxl_bitmap_dispose(&enlightenments);
+    return 0;
+
+err:
+    libxl_bitmap_dispose(&enlightenments);
+    return ERROR_FAIL;
+}
+#endif
+
 static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
                                 libxl_domain_build_info *const info)
 {
     xc_hvm_param_set(handle, domid, HVM_PARAM_PAE_ENABLED,
                     libxl_defbool_val(info->u.hvm.pae));
 #if defined(__i386__) || defined(__x86_64__)
-    xc_hvm_param_set(handle, domid, HVM_PARAM_VIRIDIAN,
-                    libxl_defbool_val(info->u.hvm.viridian));
     xc_hvm_param_set(handle, domid, HVM_PARAM_HPET_ENABLED,
                     libxl_defbool_val(info->u.hvm.hpet));
 #endif
@@ -350,8 +417,14 @@ 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);
 
-    if (info->type == LIBXL_DOMAIN_TYPE_HVM)
+    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
         hvm_set_conf_params(ctx->xch, domid, info);
+#if defined(__i386__) || defined(__x86_64__)
+        rc = hvm_set_viridian_features(gc, domid, info);
+        if (rc)
+            return rc;
+#endif
+    }
 
     rc = libxl__arch_domain_create(gc, d_config, domid);
 
index 15234d616c8c1eed82073cfc765e9f1aa2e7c399..407784e4943b43514f127defbe53dd7501c02815 100644 (file)
@@ -179,6 +179,12 @@ libxl_vendor_device = Enumeration("vendor_device", [
     (0, "NONE"),
     (1, "XENSERVER"),
     ])
+
+libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [
+    (0, "base"),
+    (1, "freq"),
+    ])
+
 #
 # Complex libxl types
 #
@@ -377,6 +383,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
                                        ("acpi_s4",          libxl_defbool),
                                        ("nx",               libxl_defbool),
                                        ("viridian",         libxl_defbool),
+                                       ("viridian_enable",  libxl_bitmap),
+                                       ("viridian_disable", libxl_bitmap),
                                        ("timeoffset",       string),
                                        ("hpet",             libxl_defbool),
                                        ("vpt_align",        libxl_defbool),
index a8a36d8b9fb557d178b534dd2bc3d70d6e0d987b..d205f96a22b44abec13614a6b4af26ee3c42beaa 100644 (file)
@@ -808,8 +808,8 @@ static void parse_config_data(const char *config_source,
     long l;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms;
-    XLU_ConfigList *ioports, *irqs, *iomem;
-    int num_ioports, num_irqs, num_iomem, num_cpus;
+    XLU_ConfigList *ioports, *irqs, *iomem, *viridian;
+    int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
     int pci_permissive = 0;
@@ -1034,10 +1034,59 @@ static void parse_config_data(const char *config_source,
         xlu_cfg_get_defbool(config, "acpi_s3", &b_info->u.hvm.acpi_s3, 0);
         xlu_cfg_get_defbool(config, "acpi_s4", &b_info->u.hvm.acpi_s4, 0);
         xlu_cfg_get_defbool(config, "nx", &b_info->u.hvm.nx, 0);
-        xlu_cfg_get_defbool(config, "viridian", &b_info->u.hvm.viridian, 0);
         xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
         xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
 
+        switch (xlu_cfg_get_list(config, "viridian",
+                                 &viridian, &num_viridian, 1))
+        {
+        case 0: /* Success */
+            if (num_viridian) {
+                libxl_bitmap_alloc(ctx, &b_info->u.hvm.viridian_enable,
+                                   LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH);
+                libxl_bitmap_alloc(ctx, &b_info->u.hvm.viridian_disable,
+                                   LIBXL_BUILDINFO_HVM_VIRIDIAN_ENABLE_DISABLE_WIDTH);
+            }
+            for (i = 0; i < num_viridian; i++) {
+                libxl_viridian_enlightenment v;
+
+                buf = xlu_cfg_get_listitem(viridian, i);
+                if (strcmp(buf, "all") == 0)
+                    libxl_bitmap_set_any(&b_info->u.hvm.viridian_enable);
+                else if (strcmp(buf, "defaults") == 0)
+                    libxl_defbool_set(&b_info->u.hvm.viridian, true);
+                else {
+                    libxl_bitmap *s = &b_info->u.hvm.viridian_enable;
+                    libxl_bitmap *r = &b_info->u.hvm.viridian_disable;
+
+                    if (*buf == '!') {
+                        s = &b_info->u.hvm.viridian_disable;
+                        r = &b_info->u.hvm.viridian_enable;
+                        buf++;
+                    }
+
+                    e = libxl_viridian_enlightenment_from_string(buf, &v);
+                    if (e) {
+                        fprintf(stderr,
+                                "xl: unknown viridian enlightenment '%s'\n",
+                                buf);
+                        exit(-ERROR_FAIL);
+                    }
+
+                    libxl_bitmap_set(s, v);
+                    libxl_bitmap_reset(r, v);
+                }
+            }
+            break;
+        case ESRCH: break; /* Option not present */
+        case EINVAL:
+            xlu_cfg_get_defbool(config, "viridian", &b_info->u.hvm.viridian, 1);
+            break;
+        default:
+            fprintf(stderr,"xl: Unable to parse viridian enlightenments.\n");
+            exit(-ERROR_FAIL);
+        }
+
         if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
             const char *s = libxl_timer_mode_to_string(l);
             fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is deprecated. "
index bb45593aa99c9d7b63366a9afe0f10dfda19c147..5c7e0a46d04f2bbbfc0564049efc6cc5ab507fbc 100644 (file)
@@ -5555,8 +5555,24 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
                     rc = -EINVAL;
                 break;
             case HVM_PARAM_VIRIDIAN:
-                if ( a.value > 1 )
+                /* This should only ever be set once by the tools and read by the guest. */
+                rc = -EPERM;
+                if ( curr_d == d )
+                    break;
+
+                if ( a.value != d->arch.hvm_domain.params[a.index] )
+                {
+                    rc = -EEXIST;
+                    if ( d->arch.hvm_domain.params[a.index] != 0 )
+                        break;
+
                     rc = -EINVAL;
+                    if ( (a.value & ~HVMPV_feature_mask) ||
+                         !(a.value & HVMPV_base_freq) )
+                        break;
+                }
+
+                rc = 0;
                 break;
             case HVM_PARAM_IDENT_PT:
                 /* Not reflexive, as we must domain_pause(). */
index b646a8a36175a606f6a7bcda790fff65d3c7fe85..28c44799b7ef8cc8d9a279a64a15dba812e9063e 100644 (file)
@@ -90,8 +90,9 @@ int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
         /* Which hypervisor MSRs are available to the guest */
         *eax = (CPUID3A_MSR_APIC_ACCESS |
                 CPUID3A_MSR_HYPERCALL   |
-                CPUID3A_MSR_VP_INDEX    |
-                CPUID3A_MSR_FREQ);
+                CPUID3A_MSR_VP_INDEX);
+        if ( !(viridian_feature_mask(d) & HVMPV_no_freq) )
+            *eax |= CPUID3A_MSR_FREQ;
         break;
     case 4:
         /* Recommended hypercall usage. */
@@ -312,11 +313,17 @@ int rdmsr_viridian_regs(uint32_t idx, uint64_t *val)
         break;
 
     case VIRIDIAN_MSR_TSC_FREQUENCY:
+        if ( viridian_feature_mask(d) & HVMPV_no_freq )
+            return 0;
+
         perfc_incr(mshv_rdmsr_tsc_frequency);
         *val = (uint64_t)d->arch.tsc_khz * 1000ull;
         break;
 
     case VIRIDIAN_MSR_APIC_FREQUENCY:
+        if ( viridian_feature_mask(d) & HVMPV_no_freq )
+            return 0;
+
         perfc_incr(mshv_rdmsr_apic_frequency);
         *val = 1000000000ull / APIC_BUS_CYCLE_NS;
         break;
@@ -489,3 +496,13 @@ static int viridian_load_vcpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 
 HVM_REGISTER_SAVE_RESTORE(VIRIDIAN_VCPU, viridian_save_vcpu_ctxt,
                           viridian_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
index 121d053477a33eba4fcffa28f0a011cb209b79de..3e66276bbef1f1473a56bbd0d66447c3a7893c1a 100644 (file)
@@ -346,8 +346,11 @@ static inline unsigned long hvm_get_shadow_gs_base(struct vcpu *v)
     return hvm_funcs.get_shadow_gs_base(v);
 }
 
-#define is_viridian_domain(_d)                                             \
- (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
+#define viridian_feature_mask(d) \
+    ((d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN])
+
+#define is_viridian_domain(d) \
+    (is_hvm_domain(d) && (viridian_feature_mask(d) & HVMPV_base_freq))
 
 void hvm_hypervisor_cpuid_leaf(uint32_t sub_idx,
                                uint32_t *eax, uint32_t *ebx,
index 614ff5f2b4bf0147cff078ebc638a21ba1c29eda..68d26fdc0d0cbb5c8ab983b41badd0eddb5e3193 100644 (file)
 
 #if defined(__i386__) || defined(__x86_64__)
 
-/* Expose Viridian interfaces to this HVM guest? */
+/*
+ * Viridian enlightenments
+ *
+ * (See http://download.microsoft.com/download/A/B/4/AB43A34E-BDD0-4FA6-BDEF-79EEF16E880B/Hypervisor%20Top%20Level%20Functional%20Specification%20v4.0.docx)
+ *
+ * To expose viridian enlightenments to the guest set this parameter
+ * to the desired feature mask. The base feature set must be present
+ * in any valid feature mask.
+ */
 #define HVM_PARAM_VIRIDIAN     9
 
+/* Base+Freq viridian feature sets:
+ *
+ * - Hypercall MSRs (HV_X64_MSR_GUEST_OS_ID and HV_X64_MSR_HYPERCALL)
+ * - APIC access MSRs (HV_X64_MSR_EOI, HV_X64_MSR_ICR and HV_X64_MSR_TPR)
+ * - Virtual Processor index MSR (HV_X64_MSR_VP_INDEX)
+ * - Timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
+ *   HV_X64_MSR_APIC_FREQUENCY)
+ */
+#define _HVMPV_base_freq 0
+#define HVMPV_base_freq  (1 << _HVMPV_base_freq)
+
+/* Feature set modifications */
+
+/* Disable timer frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and
+ * HV_X64_MSR_APIC_FREQUENCY).
+ * This modification restores the viridian feature set to the
+ * original 'base' set exposed in releases prior to Xen 4.4.
+ */
+#define _HVMPV_no_freq 1
+#define HVMPV_no_freq  (1 << _HVMPV_no_freq)
+
+#define HVMPV_feature_mask (HVMPV_base_freq|HVMPV_no_freq)
+
 #endif
 
 /*