[PATCH v2 0/4] libxl: add e820_host option to the libvirt config
Context: - old thread on xen-devel: http://xen.markmail.org/thread/awcswnywzei4s65e - new thread here: https://www.redhat.com/archives/libvir-list/2020-April/msg00447.html Marek Marczykowski-Górecki (4): conf: add xen specific feature: e820_host libxl: make use of e820_host feature xenconfig: parse e820_host option tests: check e820_host option handling docs/formatdomain.html.in | 20 ++++- docs/schemas/domaincommon.rng | 16 +++- src/conf/domain_conf.c | 102 ++++++++++++++++++- src/conf/domain_conf.h | 9 ++- src/libxl/libxl_conf.c | 14 ++- src/libxl/xen_common.c | 13 ++- src/qemu/qemu_validate.c | 1 +- tests/xlconfigdata/test-paravirt-e820_host.cfg | 13 ++- tests/xlconfigdata/test-paravirt-e820_host.xml | 37 +++++++- tests/xlconfigtest.c | 1 +- 10 files changed, 226 insertions(+) create mode 100644 tests/xlconfigdata/test-paravirt-e820_host.cfg create mode 100644 tests/xlconfigdata/test-paravirt-e820_host.xml base-commit: 7118bdee1550b6022e7362402ca8204add4cf80b -- git-series 0.9.1
e820_host is a Xen-specific option, only available for PV domains, that provides the domain a virtual e820 memory map based on the host one. It is required when using PCI passthrough and is generally considered safe for any PV kernel. e820_host is silently ignored if set in HVM domain configuration. See xl.cfg(5) man page in the Xen documentation for more details. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- Changes in v2: - add documentation - use virXMLPropString - add enum entries at the end --- docs/formatdomain.html.in | 20 +++++++- docs/schemas/domaincommon.rng | 16 +++++- src/conf/domain_conf.c | 102 +++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 9 +++- src/qemu/qemu_validate.c | 1 +- 5 files changed, 148 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6f43976..11d970c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2061,6 +2061,9 @@ <hidden state='on'/> <hint-dedicated state='on'/> </kvm> + <xen> + <e820_host state='on'/> + </xen> <pvspinlock state='on'/> <gic version='2'/> <ioapic driver='qemu'/> @@ -2242,6 +2245,23 @@ </tr> </table> </dd> + <dt><code>xen</code></dt> + <dd>Various features to change the behavior of the Xen hypervisor. + <table class="top_table"> + <tr> + <th>Feature</th> + <th>Description</th> + <th>Value</th> + <th>Since</th> + </tr> + <tr> + <td>e820_host</td> + <td>Expose the host e820 to the guest (PV only)</td> + <td>on, off</td> + <td><span class="since">6.3.0</span></td> + </tr> + </table> + </dd> <dt><code>pmu</code></dt> <dd>Depending on the <code>state</code> attribute (values <code>on</code>, <code>off</code>, default <code>on</code>) enable or disable the diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 65d6580..2b5f844 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5349,6 +5349,9 @@ <ref name="kvm"/> </optional> <optional> + <ref name="xen"/> + </optional> + <optional> <element name="privnet"> <empty/> </element> @@ -6337,6 +6340,19 @@ </element> </define> + <!-- Optional Xen features --> + <define name="xen"> + <element name="xen"> + <interleave> + <optional> + <element name="e820_host"> + <ref name="featurestate"/> + </element> + </optional> + </interleave> + </element> + </define> + <!-- Optional capabilities features --> <define name="capabilities"> <element name="capabilities"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8e81463..473adea 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -173,6 +173,7 @@ VIR_ENUM_IMPL(virDomainFeature, "nested-hv", "msrs", "ccf-assist", + "xen", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -206,6 +207,11 @@ VIR_ENUM_IMPL(virDomainKVM, "hint-dedicated", ); +VIR_ENUM_IMPL(virDomainXen, + VIR_DOMAIN_XEN_LAST, + "e820_host" +); + VIR_ENUM_IMPL(virDomainMsrsUnknown, VIR_DOMAIN_MSRS_UNKNOWN_LAST, "ignore", @@ -20862,6 +20868,7 @@ virDomainDefParseXML(xmlDocPtr xml, case VIR_DOMAIN_FEATURE_HYPERV: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_MSRS: + case VIR_DOMAIN_FEATURE_XEN: def->features[val] = VIR_TRISTATE_SWITCH_ON; break; @@ -21172,6 +21179,51 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(nodes); } + if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + int feature; + int value; + if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0) + goto error; + + for (i = 0; i < n; i++) { + feature = virDomainXenTypeFromString((const char *)nodes[i]->name); + if (feature < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Xen feature: %s"), + nodes[i]->name); + goto error; + } + + switch ((virDomainXen) feature) { + case VIR_DOMAIN_XEN_E820_HOST: + if (!(tmp = virXMLPropString(nodes[i], "state"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Xen feature '%s'"), + nodes[i]->name); + goto error; + } + + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Xen feature '%s'"), + nodes[i]->name); + goto error; + } + + VIR_FREE(tmp); + def->xen_features[feature] = value; + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_XEN_LAST: + break; + } + } + VIR_FREE(nodes); + } + if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) { int rv = virDomainParseScaledValue("string(./features/smm/tseg)", "string(./features/smm/tseg/@unit)", @@ -23173,6 +23225,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, case VIR_DOMAIN_FEATURE_PRIVNET: case VIR_DOMAIN_FEATURE_HYPERV: case VIR_DOMAIN_FEATURE_KVM: + case VIR_DOMAIN_FEATURE_XEN: case VIR_DOMAIN_FEATURE_PVSPINLOCK: case VIR_DOMAIN_FEATURE_PMU: case VIR_DOMAIN_FEATURE_VMPORT: @@ -23344,6 +23397,30 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } } + /* xen */ + if (src->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + for (i = 0; i < VIR_DOMAIN_XEN_LAST; i++) { + switch ((virDomainXen) i) { + case VIR_DOMAIN_XEN_E820_HOST: + if (src->xen_features[i] != dst->xen_features[i]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("State of Xen feature '%s' differs: " + "source: '%s', destination: '%s'"), + virDomainXenTypeToString(i), + virTristateSwitchTypeToString(src->xen_features[i]), + virTristateSwitchTypeToString(dst->xen_features[i])); + return false; + } + + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_XEN_LAST: + break; + } + } + } + /* kvm */ if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) { @@ -28959,6 +29036,31 @@ virDomainDefFormatFeatures(virBufferPtr buf, virBufferAddLit(&childBuf, "</kvm>\n"); break; + case VIR_DOMAIN_FEATURE_XEN: + if (def->features[i] != VIR_TRISTATE_SWITCH_ON) + break; + + virBufferAddLit(&childBuf, "<xen>\n"); + virBufferAdjustIndent(&childBuf, 2); + for (j = 0; j < VIR_DOMAIN_XEN_LAST; j++) { + switch ((virDomainXen) j) { + case VIR_DOMAIN_XEN_E820_HOST: + if (def->xen_features[j]) + virBufferAsprintf(&childBuf, "<%s state='%s'/>\n", + virDomainXenTypeToString(j), + virTristateSwitchTypeToString( + def->xen_features[j])); + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_XEN_LAST: + break; + } + } + virBufferAdjustIndent(&childBuf, -2); + virBufferAddLit(&childBuf, "</xen>\n"); + break; + case VIR_DOMAIN_FEATURE_CAPABILITIES: for (j = 0; j < VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST; j++) { if (def->caps_features[j] != VIR_TRISTATE_SWITCH_ABSENT) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index aad3f82..0374494 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1814,6 +1814,7 @@ typedef enum { VIR_DOMAIN_FEATURE_NESTED_HV, VIR_DOMAIN_FEATURE_MSRS, VIR_DOMAIN_FEATURE_CCF_ASSIST, + VIR_DOMAIN_FEATURE_XEN, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; @@ -1860,6 +1861,12 @@ typedef enum { } virDomainMsrsUnknown; typedef enum { + VIR_DOMAIN_XEN_E820_HOST = 0, + + VIR_DOMAIN_XEN_LAST +} virDomainXen; + +typedef enum { VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT = 0, VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW, VIR_DOMAIN_CAPABILITIES_POLICY_DENY, @@ -2484,6 +2491,7 @@ struct _virDomainDef { int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; int kvm_features[VIR_DOMAIN_KVM_LAST]; int msrs_features[VIR_DOMAIN_MSRS_LAST]; + int xen_features[VIR_DOMAIN_XEN_LAST]; unsigned int hyperv_spinlocks; int hyperv_stimer_direct; virGICVersion gic_version; @@ -3529,6 +3537,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode); VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy); VIR_ENUM_DECL(virDomainHyperv); VIR_ENUM_DECL(virDomainKVM); +VIR_ENUM_DECL(virDomainXen); VIR_ENUM_DECL(virDomainMsrsUnknown); VIR_ENUM_DECL(virDomainRNGModel); VIR_ENUM_DECL(virDomainRNGBackend); diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 971e4a9..cb0ff8d 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -302,6 +302,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; + case VIR_DOMAIN_FEATURE_XEN: case VIR_DOMAIN_FEATURE_ACPI: case VIR_DOMAIN_FEATURE_PAE: case VIR_DOMAIN_FEATURE_HAP: -- git-series 0.9.1
On 4/13/20 8:37 PM, Marek Marczykowski-Górecki wrote:
e820_host is a Xen-specific option, only available for PV domains, that provides the domain a virtual e820 memory map based on the host one. It is required when using PCI passthrough and is generally considered safe for any PV kernel. e820_host is silently ignored if set in HVM domain configuration. See xl.cfg(5) man page in the Xen documentation for more details.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
This looks good to me now so Reviewed-by: Jim Fehlig <jfehlig@suse.com> But before pushing I'd like to settle on mapping the new 'passthrough' xl.cfg option to libvirt. See https://www.redhat.com/archives/libvir-list/2020-April/msg00696.html On xen 4.13 it seems 'passthrough' is enough for PV domains. Enabling it, and not e820_host, allows me to successfully hotplug a PCI device after PV domain creation. I haven't checked but I assume e820_host was needed to hotplug PCI devices with xen < 4.13? Regards, Jim
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b3f67f8..05d671b 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -692,6 +692,20 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, b_info->u.pv.kernel = g_strdup(def->os.kernel); } b_info->u.pv.ramdisk = g_strdup(def->os.initrd); + + if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + switch ((virTristateSwitch) def->xen_features[VIR_DOMAIN_XEN_E820_HOST]) { + case VIR_TRISTATE_SWITCH_ON: + libxl_defbool_set(&b_info->u.pv.e820_host, true); + break; + case VIR_TRISTATE_SWITCH_OFF: + libxl_defbool_set(&b_info->u.pv.e820_host, false); + break; + case VIR_TRISTATE_SWITCH_ABSENT: + case VIR_TRISTATE_SWITCH_LAST: + break; + } + } } /* only the 'xen' balloon device model is supported */ -- git-series 0.9.1
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/xen_common.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 9a385eb..bbb9739 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -592,6 +592,13 @@ xenParseCPUFeatures(virConfPtr conf, def->clock.timers[def->clock.ntimers - 1] = timer; } + } else { + if (xenConfigGetBool(conf, "e820_host", &val, 0) < 0) { + return -1; + } else if (val) { + def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON; + def->xen_features[VIR_DOMAIN_XEN_E820_HOST] = VIR_TRISTATE_SWITCH_ON; + } } return 0; @@ -2138,6 +2145,12 @@ xenFormatCPUFeatures(virConfPtr conf, virDomainDefPtr def) (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == VIR_TRISTATE_SWITCH_ON) ? 1 : 0) < 0) return -1; + } else { + if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) { + if (def->xen_features[VIR_DOMAIN_XEN_E820_HOST] == VIR_TRISTATE_SWITCH_ON) + if (xenConfigSetInt(conf, "e820_host", 1) < 0) + return -1; + } } for (i = 0; i < def->clock.ntimers; i++) { -- git-series 0.9.1
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jim Fehlig <jfehlig@suse.com> --- tests/xlconfigdata/test-paravirt-e820_host.cfg | 13 +++++++- tests/xlconfigdata/test-paravirt-e820_host.xml | 37 +++++++++++++++++++- tests/xlconfigtest.c | 1 +- 3 files changed, 51 insertions(+) create mode 100644 tests/xlconfigdata/test-paravirt-e820_host.cfg create mode 100644 tests/xlconfigdata/test-paravirt-e820_host.xml diff --git a/tests/xlconfigdata/test-paravirt-e820_host.cfg b/tests/xlconfigdata/test-paravirt-e820_host.cfg new file mode 100644 index 0000000..b9e5a48 --- /dev/null +++ b/tests/xlconfigdata/test-paravirt-e820_host.cfg @@ -0,0 +1,13 @@ +name = "XenGuest1" +uuid = "45b60f51-88a9-47a8-a3b3-5e66d71b2283" +maxmem = 512 +memory = 512 +vcpus = 4 +e820_host = 1 +localtime = 0 +on_poweroff = "preserve" +on_reboot = "restart" +on_crash = "preserve" +vif = [ "mac=5a:36:0e:be:00:09" ] +bootloader = "/usr/bin/pygrub" +disk = [ "format=qcow2,vdev=xvda,access=rw,backendtype=qdisk,target=/var/lib/xen/images/debian/disk.qcow2" ] diff --git a/tests/xlconfigdata/test-paravirt-e820_host.xml b/tests/xlconfigdata/test-paravirt-e820_host.xml new file mode 100644 index 0000000..955a780 --- /dev/null +++ b/tests/xlconfigdata/test-paravirt-e820_host.xml @@ -0,0 +1,37 @@ +<domain type='xen'> + <name>XenGuest1</name> + <uuid>45b60f51-88a9-47a8-a3b3-5e66d71b2283</uuid> + <memory unit='KiB'>524288</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>4</vcpu> + <bootloader>/usr/bin/pygrub</bootloader> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <features> + <xen> + <e820_host state='on'/> + </xen> + </features> + <clock offset='utc' adjustment='reset'/> + <on_poweroff>preserve</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>preserve</on_crash> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/xen/images/debian/disk.qcow2'/> + <target dev='xvda' bus='xen'/> + </disk> + <controller type='xenbus' index='0'/> + <interface type='ethernet'> + <mac address='5a:36:0e:be:00:09'/> + </interface> + <console type='pty'> + <target type='xen' port='0'/> + </console> + <input type='mouse' bus='xen'/> + <input type='keyboard' bus='xen'/> + <memballoon model='xen'/> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index 8faef16..8ea2503 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -300,6 +300,7 @@ mymain(void) DO_TEST("vif-multi-ip"); DO_TEST("usb"); DO_TEST("usbctrl"); + DO_TEST("paravirt-e820_host"); testXLFreeDriver(driver); -- git-series 0.9.1
participants (2)
-
Jim Fehlig -
Marek Marczykowski-Górecki