[PATCH 0/3] libxl: A few firmware improvments
Upstream ovmf will be removing support for Xen in the traditional ovmf package, moving it to a separate OvmfXen package https://bugzilla.tianocore.org/show_bug.cgi?id=2122 This motivated me to verify the impact on libvirt+ovmf+xen. Fortunately it is minimal since the new OvmfXen package is basically a reduced version of the traditional package. I.e. it should contain everything the traditional package contains and thus is binary compatible and should not affect existing guests using the traditional package. Indeed my initial testing has shown this to be true. While testing I cooked up a few improvements to the libxl driver firmware handling, including adding support for automatic firmware selection. Thanks for review/comments! Jim Fehlig (3): libxl: Introduce domain def validate callback libxl: Forbid domain definition with secure boot enabled libxl: Support firmware autoselection src/libxl/libxl_conf.c | 17 ++++++++++++-- src/libxl/libxl_domain.c | 49 ++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 12 deletions(-) -- 2.31.1
Introduce libxlDomainDefValidate and move the existing validation check from libxlDomainDefPostParse. Additional validation will be introduced in subsequent patches. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 625e04a9b0..9630f12568 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -379,17 +379,9 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDef *dev, static int libxlDomainDefPostParse(virDomainDef *def, unsigned int parseFlags G_GNUC_UNUSED, - void *opaque, + void *opaque G_GNUC_UNUSED, void *parseOpaque G_GNUC_UNUSED) { - libxlDriverPrivate *driver = opaque; - g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); - - if (!virCapabilitiesDomainSupported(cfg->caps, def->os.type, - def->os.arch, - def->virtType)) - return -1; - /* Xen PV domains always have a PV console, so add one to the domain config * via post-parse callback if not explicitly specified in the XML. */ if (def->os.type != VIR_DOMAIN_OSTYPE_HVM && def->nconsoles == 0) { @@ -441,11 +433,28 @@ libxlDomainDefPostParse(virDomainDef *def, return 0; } +static int +libxlDomainDefValidate(const virDomainDef *def, + void *opaque, + void *parseOpaque G_GNUC_UNUSED) +{ + libxlDriverPrivate *driver = opaque; + g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); + + if (!virCapabilitiesDomainSupported(cfg->caps, def->os.type, + def->os.arch, + def->virtType)) + return -1; + + return 0; +} + virDomainDefParserConfig libxlDomainDefParserConfig = { .macPrefix = { 0x00, 0x16, 0x3e }, .netPrefix = LIBXL_GENERATED_PREFIX_XEN, .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, + .domainValidateCallback = libxlDomainDefValidate, .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; -- 2.31.1
Xen+ovmf does not support secure boot. Fail domain def validation if secure boot is enabled. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_domain.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 9630f12568..14d000511a 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -446,6 +446,16 @@ libxlDomainDefValidate(const virDomainDef *def, def->virtType)) return -1; + /* Xen+ovmf does not support secure boot */ + if (virDomainDefHasOldStyleUEFI(def)) { + if (def->os.loader && + def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is not supported on Xen")); + return -1; + } + } + return 0; } -- 2.31.1
Xen only supports one firmware, making autoselection easy to implement. In fact, <os firmware='efi'> is probably preferable in the Xen driver, where libxl supports a firmware setting with accepted values such as bios, ovmf, uefi (currently same semantics as ovmf), seabios, etc. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/libxl/libxl_conf.c | 17 +++++++++++++++-- src/libxl/libxl_domain.c | 22 ++++++++++++++++------ 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index fc4268db01..aef93fae53 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -627,15 +627,28 @@ libxlMakeDomBuildInfo(virDomainDef *def, /* * Currently libxl only allows specifying the type of BIOS. - * If the type is PFLASH, we assume OVMF and set libxl_bios_type + * If automatic firmware selection is enabled or the loader + * type is PFLASH, we assume OVMF and set libxl_bios_type * to LIBXL_BIOS_TYPE_OVMF. The path to the OVMF firmware is * configured when building Xen using '--with-system-ovmf='. If * not specified, LIBXL_FIRMWARE_DIR/ovmf.bin is used. In the * future, Xen will support a user-specified firmware path. See * https://lists.xenproject.org/archives/html/xen-devel/2016-03/msg01628.html */ - if (virDomainDefHasOldStyleUEFI(def)) + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) { + if (def->os.loader == NULL) + def->os.loader = g_new0(virDomainLoaderDef, 1); + if (def->os.loader->path == NULL) + def->os.loader->path = g_strdup(cfg->firmwares[0]->name); + if (def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_NONE) + def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; + if (def->os.loader->readonly == VIR_TRISTATE_BOOL_ABSENT) + def->os.loader->readonly = VIR_TRISTATE_BOOL_YES; b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF; + def->os.firmware = VIR_DOMAIN_OS_DEF_FIRMWARE_NONE; + } else if (virDomainDefHasOldStyleUEFI(def)) { + b_info->u.hvm.bios = LIBXL_BIOS_TYPE_OVMF; + } if (def->emulator) { if (!virFileExists(def->emulator)) { diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 14d000511a..59d26d5e2b 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -440,6 +440,7 @@ libxlDomainDefValidate(const virDomainDef *def, { libxlDriverPrivate *driver = opaque; g_autoptr(libxlDriverConfig) cfg = libxlDriverConfigGet(driver); + bool reqSecureBoot = false; if (!virCapabilitiesDomainSupported(cfg->caps, def->os.type, def->os.arch, @@ -447,13 +448,20 @@ libxlDomainDefValidate(const virDomainDef *def, return -1; /* Xen+ovmf does not support secure boot */ + if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI) { + if (def->os.firmwareFeatures && + def->os.firmwareFeatures[VIR_DOMAIN_OS_DEF_FIRMWARE_FEATURE_SECURE_BOOT]) + reqSecureBoot = true; + } if (virDomainDefHasOldStyleUEFI(def)) { if (def->os.loader && - def->os.loader->secure == VIR_TRISTATE_BOOL_YES) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Secure boot is not supported on Xen")); - return -1; - } + def->os.loader->secure == VIR_TRISTATE_BOOL_YES) + reqSecureBoot = true; + } + if (reqSecureBoot) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Secure boot is not supported on Xen")); + return -1; } return 0; @@ -465,7 +473,9 @@ virDomainDefParserConfig libxlDomainDefParserConfig = { .devicesPostParseCallback = libxlDomainDeviceDefPostParse, .domainPostParseCallback = libxlDomainDefPostParse, .domainValidateCallback = libxlDomainDefValidate, - .features = VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, + + .features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT | + VIR_DOMAIN_DEF_FEATURE_NET_MODEL_STRING, }; -- 2.31.1
On 6/4/21 7:27 PM, Jim Fehlig wrote:
Upstream ovmf will be removing support for Xen in the traditional ovmf package, moving it to a separate OvmfXen package
https://bugzilla.tianocore.org/show_bug.cgi?id=2122
This motivated me to verify the impact on libvirt+ovmf+xen. Fortunately it is minimal since the new OvmfXen package is basically a reduced version of the traditional package. I.e. it should contain everything the traditional package contains and thus is binary compatible and should not affect existing guests using the traditional package. Indeed my initial testing has shown this to be true.
While testing I cooked up a few improvements to the libxl driver firmware handling, including adding support for automatic firmware selection.
Thanks for review/comments!
Jim Fehlig (3): libxl: Introduce domain def validate callback libxl: Forbid domain definition with secure boot enabled libxl: Support firmware autoselection
src/libxl/libxl_conf.c | 17 ++++++++++++-- src/libxl/libxl_domain.c | 49 ++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 12 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Jim Fehlig -
Michal Prívozník