Re: [libvirt] [PATCH 1/3] firmware: include arch and features in firmware file list

Currently qemu.conf contains a nvram parameter which lists firmware code files and the corresponding nvram file path. We need to know which architecture and features are associated with each firmware file for future enhancement. This extends the syntax in a backwards compatible manner to record this info. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/qemu/qemu.conf | 14 ++++-- src/qemu/test_libvirtd_qemu.aug.in | 6 +-- src/util/virfirmware.c | 94 ++++++++++++++++++++++++++++++++++---- src/util/virfirmware.h | 7 +++ 5 files changed, 105 insertions(+), 17 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68d0ea9..a2f0b83 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1628,6 +1628,7 @@ virFirewallStartTransaction; # util/virfirmware.h +virFirmwareFind; virFirmwareFreeList; virFirmwareParse; virFirmwareParseList; diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index e4c2aae..56a0e55 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -595,16 +595,22 @@ # using this master file as image. Each UEFI firmware can, # however, have different variables store. Therefore the nvram is # a list of strings when a single item is in form of: -# ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}. +# +# ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}:${ARCH}[:${FEATURE}:...]. +# +# Current valid features include: +# +# 'secboot' - the firmware has secure boot enabled +# # Later, when libvirt creates per domain variable store, this list is # searched for the master image. The UEFI firmware can be called # differently for different guest architectures. For instance, it's OVMF # for x86_64 and i686, but it's AAVMF for aarch64. The libvirt default # follows this scheme. #nvram = [ -# "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd", -# "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd", -# "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" +# "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64", +# "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64:secboot", +# "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:aarch64" #] # The backend to use for handling stdout/stderr output from diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index cd162ae..b78a0b3 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -83,8 +83,8 @@ module Test_libvirtd_qemu = { "migration_port_max" = "49215" } { "log_timestamp" = "0" } { "nvram" - { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd" } - { "2" = "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd" } - { "3" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd" } + { "1" = "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64" } + { "2" = "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.fd:x86_64:secboot" } + { "3" = "/usr/share/AAVMF/AAVMF_CODE.fd:/usr/share/AAVMF/AAVMF_VARS.fd:aarch64" } } { "stdio_handler" = "logd" } diff --git a/src/util/virfirmware.c b/src/util/virfirmware.c index 6b20c06..117e8ef 100644 --- a/src/util/virfirmware.c +++ b/src/util/virfirmware.c @@ -61,30 +61,83 @@ int virFirmwareParse(const char *str, virFirmwarePtr firmware) { int ret = -1; - char **token; + char **token, **tmp; + size_t ntoken = 0; + int arch; - if (!(token = virStringSplit(str, ":", 0))) + if (!(tmp = token = virStringSplit(str, ":", 0)))

goto cleanup; - if (token[0]) { - virSkipSpaces((const char **) &token[0]); - if (token[1]) - virSkipSpaces((const char **) &token[1]); + while (tmp && *tmp) { + virSkipSpaces((const char **) &tmp); + if (STREQ(*tmp, "")) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Invalid nvram format for '%s', token " + "must not be the empty string"), + str); + goto cleanup; + } + ntoken++; + tmp++; } - /* Exactly two tokens are expected */ - if (!token[0] || !token[1] || token[2] || - STREQ(token[0], "") || STREQ(token[1], "")) { + if (ntoken < 2 || ntoken > 4) { virReportError(VIR_ERR_CONF_SYNTAX, - _("Invalid nvram format: '%s'"), + _("Invalid nvram format for '%s', expected " + "CODE-PATH:NVRAM-PATH:[ARCH:[FEATURE,...]]"),

str); goto cleanup; } + if (ntoken > 2) { + if ((arch = virArchFromString(token[2])) < 0) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Unknown arch in nvram config '%s'"), + str); + goto cleanup; + } + firmware->arch = arch; + } else { + if (strstr(token[0], "OVMF")) { + firmware->arch = VIR_ARCH_X86_64;

+ } else if (strstr(token[1], "AVMF")) { + firmware->arch = VIR_ARCH_AARCH64; + } else { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Cannot guest arch for nvram config '%s', " + "please specify it explicitly"),

+ str); + goto cleanup; + } + } + if (VIR_STRDUP(firmware->name, token[0]) < 0 || VIR_STRDUP(firmware->nvram, token[1]) < 0) goto cleanup; + /* Remaining tokens are feature flags */ + if (ntoken > 3) { + tmp = token + 3; + while (*tmp) { + if (STREQ(*tmp, "secboot")) { + firmware->secboot = true; + } else { + virReportError(VIR_ERR_CONF_SYNTAX, + _("Unknown feature flag in nvram config '%s'"), + str);

+ goto cleanup; + } + tmp++; + } + } else { + if (strstr(firmware->name, "secboot")) + firmware->secboot = true;

+ } + + VIR_DEBUG("Parsed firmware code='%s' nvram='%s' arch='%s' secboot='%d'", + firmware->name, firmware->nvram, + virArchToString(firmware->arch), firmware->secboot); + ret = 0; cleanup: virStringFreeList(token); @@ -135,3 +188,24 @@ virFirmwareParseList(const char *list, virStringFreeList(token); return ret; } + + +virFirmwarePtr virFirmwareFind(virFirmwarePtr *firmwares,

+ size_t nfirmwares, + virArch arch, + bool secboot) +{ + size_t i; + + for (i = 0; i < nfirmwares; i++) { + if (firmwares[i]->arch == arch && + firmwares[i]->secboot == secboot) { + return firmwares[i]; + } + } + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Cannot find a firmware for arch %s with secboot=%d"), + virArchToString(arch), secboot); + return NULL; +} diff --git a/src/util/virfirmware.h b/src/util/virfirmware.h index 682a865..f3ac36a 100644 --- a/src/util/virfirmware.h +++ b/src/util/virfirmware.h @@ -24,11 +24,14 @@ # define __VIR_FIRMWARE_H__ # include "internal.h" +# include "virarch.h" typedef struct _virFirmware virFirmware; typedef virFirmware *virFirmwarePtr; struct _virFirmware { + virArch arch; + bool secboot; char *name; char *nvram; }; @@ -47,5 +50,9 @@ virFirmwareParseList(const char *list, size_t *nfirmwares) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +virFirmwarePtr virFirmwareFind(virFirmwarePtr *firmwares, + size_t nfirmwares, + virArch arch, + bool secboot); #endif /* __VIR_FIRMWARE_H__ */