diff mbox

[v4,5/6] libxl: add options to enable/disable emulated devices

Message ID 1453395092-88090-6-git-send-email-roger.pau@citrix.com
State New, archived
Headers show

Commit Message

Roger Pau Monné Jan. 21, 2016, 4:51 p.m. UTC
Allow enabling or disabling emulated devices from the libxl domain
configuration file. For HVM guests with a device model all the emulated
devices are enabled. For HVM guests without a device model no devices are
enabled by default, although they can be enabled using the options provided.
The arbiter of whether a combination is posible or not is always Xen, libxl
doesn't do any kind of check.

This set of options is also propagated inside of the libxl migration record
as part of the contents of the libxl_domain_build_info struct, so that when
the other end (restore) creates the domain the same set of devices are
enabled. This is important for future compatibility, in case we decide to
enable some emulated devices by default for HVMlite guests, old HVMlite
guests migrated to newer versions should continue to see the same set of
emulated devices.

It has been discussed that it would be better to avoid having this
information inside of the libxl stream, and to instead rely on which devices
get their context loaded inside of Xen on resume. This of course requires
more work and it also has certain issues, like the fact that some devices
don't restore a context at all (like VGA). The consensus is that the
solution presented in this patch is not going to prevent further
developments in this direction, and can always be used as a check to make
sure the Xen context and the libxl context are in sync.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v3:
 - Fix spelling mistake in xl.cfg man page.
 - Error out if a classic HVM guest tries to use some of the HVMlite
   options.
 - Modify libxl__update_domain_configuration so the fields relevant to the
   set of enabled devices are stored.
 - Expand commit message to clarify the approach taken.
---
 docs/man/xl.cfg.pod.5        | 43 +++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/libxl.h          | 11 +++++++++++
 tools/libxl/libxl_create.c   | 26 +++++++++++++++++++++++++-
 tools/libxl/libxl_internal.c | 25 +++++++++++++++++++++++++
 tools/libxl/libxl_types.idl  |  6 ++++++
 tools/libxl/libxl_x86.c      | 33 ++++++++++++++++++++++++++++-----
 tools/libxl/xl_cmdimpl.c     |  7 +++++++
 7 files changed, 145 insertions(+), 6 deletions(-)

Comments

Roger Pau Monné Jan. 22, 2016, 5:04 p.m. UTC | #1
El 21/01/16 a les 17.51, Roger Pau Monne ha escrit:
> Allow enabling or disabling emulated devices from the libxl domain
> configuration file. For HVM guests with a device model all the emulated
> devices are enabled. For HVM guests without a device model no devices are
> enabled by default, although they can be enabled using the options provided.
> The arbiter of whether a combination is posible or not is always Xen, libxl
> doesn't do any kind of check.
> 
> This set of options is also propagated inside of the libxl migration record
> as part of the contents of the libxl_domain_build_info struct, so that when
> the other end (restore) creates the domain the same set of devices are
> enabled. This is important for future compatibility, in case we decide to
> enable some emulated devices by default for HVMlite guests, old HVMlite
> guests migrated to newer versions should continue to see the same set of
> emulated devices.
> 
> It has been discussed that it would be better to avoid having this
> information inside of the libxl stream, and to instead rely on which devices
> get their context loaded inside of Xen on resume. This of course requires
> more work and it also has certain issues, like the fact that some devices
> don't restore a context at all (like VGA). The consensus is that the
> solution presented in this patch is not going to prevent further
> developments in this direction, and can always be used as a check to make
> sure the Xen context and the libxl context are in sync.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

To tools committers: please refrain from pushing this until we have a
clear view on which devices we might want to enable in the future.
Adding a bunch of libxl options that are never going to be allowed
doesn't make any sense.

Roger.
Ian Campbell Jan. 25, 2016, 9:33 a.m. UTC | #2
On Fri, 2016-01-22 at 18:04 +0100, Roger Pau Monné wrote:
> El 21/01/16 a les 17.51, Roger Pau Monne ha escrit:
> > Allow enabling or disabling emulated devices from the libxl domain
> > configuration file. For HVM guests with a device model all the emulated
> > devices are enabled. For HVM guests without a device model no devices
> > are
> > enabled by default, although they can be enabled using the options
> > provided.
> > The arbiter of whether a combination is posible or not is always Xen,
> > libxl
> > doesn't do any kind of check.
> > 
> > This set of options is also propagated inside of the libxl migration
> > record
> > as part of the contents of the libxl_domain_build_info struct, so that
> > when
> > the other end (restore) creates the domain the same set of devices are
> > enabled. This is important for future compatibility, in case we decide
> > to
> > enable some emulated devices by default for HVMlite guests, old HVMlite
> > guests migrated to newer versions should continue to see the same set
> > of
> > emulated devices.
> > 
> > It has been discussed that it would be better to avoid having this
> > information inside of the libxl stream, and to instead rely on which
> > devices
> > get their context loaded inside of Xen on resume. This of course
> > requires
> > more work and it also has certain issues, like the fact that some
> > devices
> > don't restore a context at all (like VGA). The consensus is that the
> > solution presented in this patch is not going to prevent further
> > developments in this direction, and can always be used as a check to
> > make
> > sure the Xen context and the libxl context are in sync.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> 
> To tools committers:

Ack.

>  please refrain from pushing this until we have a
> clear view on which devices we might want to enable in the future.
> Adding a bunch of libxl options that are never going to be allowed
> doesn't make any sense.
> 
> Roger.
>
diff mbox

Patch

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8899f75..dc47888 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1762,6 +1762,49 @@  See F<docs/misc/pci-device-reservations.txt> for more information.
 
 =back
 
+=head3 HVM without a device model options
+
+These options can be used to change the set of emulated devices provided
+to guests without a device model. Note that Xen might not support all
+possible combinations. By default HVM guests without a device model
+don't have any of them enabled.
+
+It is important to notice that these options (except the hpet one) are
+not available to HVM guests with a device model, and trying to set them
+will cause xl to exit with an error.
+
+=over 4
+
+=item B<lapic=BOOLEAN>
+
+Enables or disables the Local APIC.
+
+=item B<ioapic=BOOLEAN>
+
+Enables or disables the IO APIC.
+
+=item B<rtc=BOOLEAN>
+
+Enables or disables the RTC.
+
+=item B<power_management=BOOLEAN>
+
+Enables or disables the ACPI power management timer and control interfaces.
+
+=item B<pic=BOOLEAN>
+
+Enables or disables the PIC.
+
+=item B<pit=BOOLEAN>
+
+Enables or disables the PIT.
+
+=item B<hpet=BOOLEAN>
+
+Enables or disables the HPET.
+
+=back
+
 =head2 Device-Model Options
 
 The following options control the selection of the device-model.  This
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index ab76d85..228f192 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -886,6 +886,17 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
  */
 #define LIBXL_HAVE_VGA_INTERFACE_TYPE_UNKNOWN 1
 
+/*
+ * LIBXL_HAVE_EMULATED_DEVS_OPTIONS
+ *
+ * In the case that LIBXL_HAVE_EMULATED_DEVS_OPTIONS is set libxl
+ * allows enabling or disabling emulated devices for HVM guests
+ * without a device model. The following fields are added to the
+ * hvm structure inside of libxl_domain_build_info: lapic, ioapic,
+ * rtc, power_management, pic, pit.
+ */
+#define LIBXL_HAVE_EMULATED_DEVS_OPTIONS 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index c7700a7..7d0f570 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -295,13 +295,37 @@  int libxl__domain_build_info_setdefault(libxl__gc *gc,
         libxl_defbool_setdefault(&b_info->u.hvm.acpi_s4,            true);
         libxl_defbool_setdefault(&b_info->u.hvm.nx,                 true);
         libxl_defbool_setdefault(&b_info->u.hvm.viridian,           false);
-        libxl_defbool_setdefault(&b_info->u.hvm.hpet,               true);
         libxl_defbool_setdefault(&b_info->u.hvm.vpt_align,          true);
         libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
         libxl_defbool_setdefault(&b_info->u.hvm.altp2m,             false);
         libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);
         libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
 
+        if (b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE)
+        {
+            libxl_defbool_setdefault(&b_info->u.hvm.hpet,               false);
+            libxl_defbool_setdefault(&b_info->u.hvm.lapic,              false);
+            libxl_defbool_setdefault(&b_info->u.hvm.ioapic,             false);
+            libxl_defbool_setdefault(&b_info->u.hvm.rtc,                false);
+            libxl_defbool_setdefault(&b_info->u.hvm.power_management,   false);
+            libxl_defbool_setdefault(&b_info->u.hvm.pic,                false);
+            libxl_defbool_setdefault(&b_info->u.hvm.pit,                false);
+        } else {
+            libxl_defbool_setdefault(&b_info->u.hvm.hpet,               true);
+
+            if (!libxl_defbool_is_default(b_info->u.hvm.lapic) ||
+                !libxl_defbool_is_default(b_info->u.hvm.ioapic) ||
+                !libxl_defbool_is_default(b_info->u.hvm.rtc) ||
+                !libxl_defbool_is_default(b_info->u.hvm.power_management) ||
+                !libxl_defbool_is_default(b_info->u.hvm.pic) ||
+                !libxl_defbool_is_default(b_info->u.hvm.pit)) {
+                LOG(ERROR, "the lapic, ioapic, rtc, power_management, pic and "
+                           "pit options are only available to HVM guests "
+                           "without a device model");
+                return ERROR_INVAL;
+            }
+        }
+
         libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
         if (!libxl_defbool_val(b_info->u.hvm.spice.enable) &&
             (b_info->u.hvm.spice.usbredirection > 0) ){
diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 328046b..791eae5 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -551,6 +551,31 @@  void libxl__update_domain_configuration(libxl__gc *gc,
 
     /* video ram */
     dst->b_info.video_memkb = src->b_info.video_memkb;
+
+    if (src->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
+#define check_and_save_hvm_field(src, dst, field)                           \
+    if (!libxl_defbool_is_default((src)->u.hvm.field))                      \
+        libxl_defbool_setdefault(&(dst)->u.hvm.field,                       \
+                                 libxl_defbool_val((src)->u.hvm.field))
+
+        /*
+         * Save the status of emulated devices.
+         *
+         * For HVMlite guests all those fields are forced to a specific
+         * value, so they are always saved. For HVM guests only the HPET
+         * one is used and thus set.
+         */
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, hpet);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, lapic);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, ioapic);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, rtc);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, power_management);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, pic);
+        check_and_save_hvm_field(&src->b_info, &dst->b_info, pit);
+#undef check_and_save_hvm_field
+
+        dst->b_info.u.hvm.vga.kind = src->b_info.u.hvm.vga.kind;
+    }
 }
 
 char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index e9e0da0..fdec23c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -519,6 +519,12 @@  libxl_domain_build_info = Struct("domain_build_info",[
                                        ("serial_list",      libxl_string_list),
                                        ("rdm", libxl_rdm_reserve),
                                        ("rdm_mem_boundary_memkb", MemKB),
+                                       ("lapic",            libxl_defbool),
+                                       ("ioapic",           libxl_defbool),
+                                       ("rtc",              libxl_defbool),
+                                       ("power_management", libxl_defbool),
+                                       ("pic",              libxl_defbool),
+                                       ("pit",              libxl_defbool),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 46cfafb..92f25fd 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -7,15 +7,38 @@  int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       libxl_domain_config *d_config,
                                       xc_domain_configuration_t *xc_config)
 {
+    struct libxl_domain_build_info *info = &d_config->b_info;
 
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
-        d_config->b_info.device_model_version !=
-        LIBXL_DEVICE_MODEL_VERSION_NONE) {
-        /* HVM domains with a device model. */
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) {
+        /* PV guests. */
+        xc_config->emulation_flags = XEN_X86_EMU_PIT;
+    } else if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        /* HVM guests with a device model. */
         xc_config->emulation_flags = XEN_X86_EMU_ALL;
     } else {
-        /* PV or HVM domains without a device model. */
+        /* HVM guests without a device model. */
         xc_config->emulation_flags = 0;
+
+        if (libxl_defbool_val(info->u.hvm.lapic))
+            xc_config->emulation_flags |= XEN_X86_EMU_LAPIC;
+        if (libxl_defbool_val(info->u.hvm.ioapic))
+            xc_config->emulation_flags |= XEN_X86_EMU_IOAPIC;
+        if (libxl_defbool_val(info->u.hvm.rtc))
+            xc_config->emulation_flags |= XEN_X86_EMU_RTC;
+        if (libxl_defbool_val(info->u.hvm.power_management))
+            xc_config->emulation_flags |= XEN_X86_EMU_PM;
+        if (libxl_defbool_val(info->u.hvm.pic))
+            xc_config->emulation_flags |= XEN_X86_EMU_PIC;
+        if (libxl_defbool_val(info->u.hvm.pit))
+            xc_config->emulation_flags |= XEN_X86_EMU_PIT;
+        if (libxl_defbool_val(info->u.hvm.hpet))
+            xc_config->emulation_flags |= XEN_X86_EMU_HPET;
+
+        if (info->u.hvm.vga.kind != LIBXL_VGA_INTERFACE_TYPE_NONE)
+            xc_config->emulation_flags |= XEN_X86_EMU_VGA;
+
+        if (d_config->num_pcidevs != 0)
+            xc_config->emulation_flags |= XEN_X86_EMU_IOMMU;
     }
 
     return 0;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 25507c7..3654097 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1514,6 +1514,13 @@  static void parse_config_data(const char *config_source,
         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, "hpet", &b_info->u.hvm.hpet, 0);
+        xlu_cfg_get_defbool(config, "lapic", &b_info->u.hvm.lapic, 0);
+        xlu_cfg_get_defbool(config, "ioapic", &b_info->u.hvm.ioapic, 0);
+        xlu_cfg_get_defbool(config, "rtc", &b_info->u.hvm.rtc, 0);
+        xlu_cfg_get_defbool(config, "power_management",
+                            &b_info->u.hvm.power_management, 0);
+        xlu_cfg_get_defbool(config, "pic", &b_info->u.hvm.pic, 0);
+        xlu_cfg_get_defbool(config, "pit", &b_info->u.hvm.pit, 0);
         xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
 
         switch (xlu_cfg_get_list(config, "viridian",