Ping.
On Thu, Feb 17, 2022 at 11:54:07AM +0000, Daniel P. Berrangé wrote:
> Currently the 'nvram_template' entry is mandatory when parsing the
> firmware descriptor based on flash. QEMU is extending the firmware
> descriptor spec to make the 'nvram_template' optional, depending
> on the value of a new 'mode' field:
>
> - "split"
> * "executable" contains read-only CODE
> * "nvram_template" contains read-write VARS
>
> - "combined"
> * "executable" contains read-write CODE and VARs
> * "nvram_template" not present, as the "executable"
> is effectively the template on its own
>
> - "stateless"
> * "executable" contains read-only CODE and VARs
> * "nvram_template" not present
>
> In the latter case, the guest OS can write vars but the firmware will
> make no attempt to persist them, so any changes will be lost at
> poweroff.
>
> For now we parse this new 'mode' but discard any firmware which is not
> 'mode=split' when matching for a domain. This is the minimum required
> to have libvirt not break when seeing the new firmware descriptors.
> Future changes will support the new modes.
>
> In the tests we have a mixture of files with and without the mode
> attribute.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/qemu/qemu_firmware.c | 79 ++++++++++++++++---
> .../share/qemu/firmware/50-ovmf-sb-keys.json | 33 ++++++++
> .../out/usr/share/qemu/firmware/61-ovmf.json | 31 ++++++++
> .../out/usr/share/qemu/firmware/70-aavmf.json | 28 +++++++
> .../qemu/firmware/45-ovmf-sev-stateless.json | 31 ++++++++
> .../qemu/firmware/55-ovmf-sb-combined.json | 33 ++++++++
> .../usr/share/qemu/firmware/60-ovmf-sb.json | 1 +
> tests/qemufirmwaretest.c | 31 ++++++--
> 8 files changed, 246 insertions(+), 21 deletions(-)
> create mode 100644 tests/qemufirmwaredata/out/usr/share/qemu/firmware/50-ovmf-sb-keys.json
> create mode 100644 tests/qemufirmwaredata/out/usr/share/qemu/firmware/61-ovmf.json
> create mode 100644 tests/qemufirmwaredata/out/usr/share/qemu/firmware/70-aavmf.json
> create mode 100644 tests/qemufirmwaredata/usr/share/qemu/firmware/45-ovmf-sev-stateless.json
> create mode 100644 tests/qemufirmwaredata/usr/share/qemu/firmware/55-ovmf-sb-combined.json
>
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index ff364996b8..e403ee98e4 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -59,6 +59,22 @@ VIR_ENUM_IMPL(qemuFirmwareOSInterface,
> );
>
>
> +typedef enum {
> + QEMU_FIRMWARE_FLASH_MODE_SPLIT,
> + QEMU_FIRMWARE_FLASH_MODE_COMBINED,
> + QEMU_FIRMWARE_FLASH_MODE_STATELESS,
> +
> + QEMU_FIRMWARE_FLASH_MODE_LAST,
> +} qemuFirmwareFlashMode;
> +
> +VIR_ENUM_DECL(qemuFirmwareFlashMode);
> +VIR_ENUM_IMPL(qemuFirmwareFlashMode,
> + QEMU_FIRMWARE_FLASH_MODE_LAST,
> + "split",
> + "combined",
> + "stateless",
> +);
> +
> typedef struct _qemuFirmwareFlashFile qemuFirmwareFlashFile;
> struct _qemuFirmwareFlashFile {
> char *filename;
> @@ -68,6 +84,7 @@ struct _qemuFirmwareFlashFile {
>
> typedef struct _qemuFirmwareMappingFlash qemuFirmwareMappingFlash;
> struct _qemuFirmwareMappingFlash {
> + qemuFirmwareFlashMode mode;
> qemuFirmwareFlashFile executable;
> qemuFirmwareFlashFile nvram_template;
> };
> @@ -359,9 +376,31 @@ qemuFirmwareMappingFlashParse(const char *path,
> virJSONValue *doc,
> qemuFirmwareMappingFlash *flash)
> {
> + virJSONValue *mode;
> virJSONValue *executable;
> virJSONValue *nvram_template;
>
> + if (!(mode = virJSONValueObjectGet(doc, "mode"))) {
> + /* Historical default */
> + flash->mode = QEMU_FIRMWARE_FLASH_MODE_SPLIT;
> + } else {
> + const char *modestr = virJSONValueGetString(mode);
> + int modeval;
> + if (!modestr) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Firmware flash mode value was malformed"));
> + return -1;
> + }
> + modeval = qemuFirmwareFlashModeTypeFromString(modestr);
> + if (modeval < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Firmware flash mode value '%s' unexpected"),
> + modestr);
> + return -1;
> + }
> + flash->mode = modeval;
> + }
> +
> if (!(executable = virJSONValueObjectGet(doc, "executable"))) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("missing 'executable' in '%s'"),
> @@ -372,15 +411,17 @@ qemuFirmwareMappingFlashParse(const char *path,
> if (qemuFirmwareFlashFileParse(path, executable, &flash->executable) < 0)
> return -1;
>
> - if (!(nvram_template = virJSONValueObjectGet(doc, "nvram-template"))) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("missing 'nvram-template' in '%s'"),
> - path);
> - return -1;
> - }
> + if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
> + if (!(nvram_template = virJSONValueObjectGet(doc, "nvram-template"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("missing 'nvram-template' in '%s'"),
> + path);
> + return -1;
> + }
>
> - if (qemuFirmwareFlashFileParse(path, nvram_template, &flash->nvram_template) < 0)
> - return -1;
> + if (qemuFirmwareFlashFileParse(path, nvram_template, &flash->nvram_template) < 0)
> + return -1;
> + }
>
> return 0;
> }
> @@ -693,10 +734,12 @@ qemuFirmwareMappingFlashFormat(virJSONValue *mapping,
> g_autoptr(virJSONValue) executable = NULL;
> g_autoptr(virJSONValue) nvram_template = NULL;
>
> - if (!(executable = qemuFirmwareFlashFileFormat(flash->executable)))
> + if (virJSONValueObjectAppendString(mapping,
> + "mode",
> + qemuFirmwareFlashModeTypeToString(flash->mode)) < 0)
> return -1;
>
> - if (!(nvram_template = qemuFirmwareFlashFileFormat(flash->nvram_template)))
> + if (!(executable = qemuFirmwareFlashFileFormat(flash->executable)))
> return -1;
>
> if (virJSONValueObjectAppend(mapping,
> @@ -704,11 +747,15 @@ qemuFirmwareMappingFlashFormat(virJSONValue *mapping,
> &executable) < 0)
> return -1;
>
> + if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
> + if (!(nvram_template = qemuFirmwareFlashFileFormat(flash->nvram_template)))
> + return -1;
>
> - if (virJSONValueObjectAppend(mapping,
> + if (virJSONValueObjectAppend(mapping,
> "nvram-template",
> - &nvram_template) < 0)
> - return -1;
> + &nvram_template) < 0)
> + return -1;
> + }
>
> return 0;
> }
> @@ -1070,6 +1117,12 @@ qemuFirmwareMatchDomain(const virDomainDef *def,
> return false;
> }
>
> + if (fw->mapping.device == QEMU_FIRMWARE_DEVICE_FLASH &&
> + fw->mapping.data.flash.mode != QEMU_FIRMWARE_FLASH_MODE_SPLIT) {
> + VIR_DEBUG("Discarding loader without split flash");
> + return false;
> + }
> +
> if (def->sec) {
> switch ((virDomainLaunchSecurity) def->sec->sectype) {
> case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/50-ovmf-sb-keys.json b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/50-ovmf-sb-keys.json
> new file mode 100644
> index 0000000000..c251682cd9
> --- /dev/null
> +++ b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/50-ovmf-sb-keys.json
> @@ -0,0 +1,33 @@
> +{
> + "interface-types": [
> + "uefi"
> + ],
> + "mapping": {
> + "device": "flash",
> + "mode": "split",
> + "executable": {
> + "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> + "format": "raw"
> + },
> + "nvram-template": {
> + "filename": "/usr/share/OVMF/OVMF_VARS.secboot.fd",
> + "format": "raw"
> + }
> + },
> + "targets": [
> + {
> + "architecture": "x86_64",
> + "machines": [
> + "pc-q35-*"
> + ]
> + }
> + ],
> + "features": [
> + "acpi-s3",
> + "amd-sev",
> + "enrolled-keys",
> + "requires-smm",
> + "secure-boot",
> + "verbose-dynamic"
> + ]
> +}
> diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/61-ovmf.json b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/61-ovmf.json
> new file mode 100644
> index 0000000000..2a9aa23efb
> --- /dev/null
> +++ b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/61-ovmf.json
> @@ -0,0 +1,31 @@
> +{
> + "interface-types": [
> + "uefi"
> + ],
> + "mapping": {
> + "device": "flash",
> + "mode": "split",
> + "executable": {
> + "filename": "/usr/share/OVMF/OVMF_CODE.fd",
> + "format": "raw"
> + },
> + "nvram-template": {
> + "filename": "/usr/share/OVMF/OVMF_VARS.fd",
> + "format": "raw"
> + }
> + },
> + "targets": [
> + {
> + "architecture": "x86_64",
> + "machines": [
> + "pc-i440fx-*",
> + "pc-q35-*"
> + ]
> + }
> + ],
> + "features": [
> + "acpi-s3",
> + "amd-sev",
> + "verbose-dynamic"
> + ]
> +}
> diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/70-aavmf.json b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/70-aavmf.json
> new file mode 100644
> index 0000000000..9bd5ac2868
> --- /dev/null
> +++ b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/70-aavmf.json
> @@ -0,0 +1,28 @@
> +{
> + "interface-types": [
> + "uefi"
> + ],
> + "mapping": {
> + "device": "flash",
> + "mode": "split",
> + "executable": {
> + "filename": "/usr/share/AAVMF/AAVMF_CODE.fd",
> + "format": "raw"
> + },
> + "nvram-template": {
> + "filename": "/usr/share/AAVMF/AAVMF_VARS.fd",
> + "format": "raw"
> + }
> + },
> + "targets": [
> + {
> + "architecture": "aarch64",
> + "machines": [
> + "virt-*"
> + ]
> + }
> + ],
> + "features": [
> +
> + ]
> +}
> diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/45-ovmf-sev-stateless.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/45-ovmf-sev-stateless.json
> new file mode 100644
> index 0000000000..5a619f3ab0
> --- /dev/null
> +++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/45-ovmf-sev-stateless.json
> @@ -0,0 +1,31 @@
> +{
> + "description": "OVMF for x86_64, with SEV, without SB, without SMM, with NO varstore",
> + "interface-types": [
> + "uefi"
> + ],
> + "mapping": {
> + "device": "flash",
> + "mode": "stateless",
> + "executable": {
> + "filename": "/usr/share/OVMF/OVMF.sev.fd",
> + "format": "raw"
> + }
> + },
> + "targets": [
> + {
> + "architecture": "x86_64",
> + "machines": [
> + "pc-q35-*"
> + ]
> + }
> + ],
> + "features": [
> + "acpi-s3",
> + "amd-sev",
> + "amd-sev-es",
> + "verbose-dynamic"
> + ],
> + "tags": [
> +
> + ]
> +}
> diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/55-ovmf-sb-combined.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/55-ovmf-sb-combined.json
> new file mode 100644
> index 0000000000..eb3332e4ab
> --- /dev/null
> +++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/55-ovmf-sb-combined.json
> @@ -0,0 +1,33 @@
> +{
> + "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled",
> + "interface-types": [
> + "uefi"
> + ],
> + "mapping": {
> + "device": "flash",
> + "mode": "combined",
> + "executable": {
> + "filename": "/usr/share/OVMF/OVMF.secboot.fd",
> + "format": "raw"
> + }
> + },
> + "targets": [
> + {
> + "architecture": "x86_64",
> + "machines": [
> + "pc-q35-*"
> + ]
> + }
> + ],
> + "features": [
> + "acpi-s3",
> + "amd-sev",
> + "enrolled-keys",
> + "requires-smm",
> + "secure-boot",
> + "verbose-dynamic"
> + ],
> + "tags": [
> +
> + ]
> +}
> diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/60-ovmf-sb.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-ovmf-sb.json
> index 5e8a94ae78..a5273a5e8b 100644
> --- a/tests/qemufirmwaredata/usr/share/qemu/firmware/60-ovmf-sb.json
> +++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-ovmf-sb.json
> @@ -5,6 +5,7 @@
> ],
> "mapping": {
> "device": "flash",
> + "mode": "split",
> "executable": {
> "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
> "format": "raw"
> diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
> index cad4b6d383..fc3416b2ae 100644
> --- a/tests/qemufirmwaretest.c
> +++ b/tests/qemufirmwaretest.c
> @@ -17,22 +17,31 @@ static int
> testParseFormatFW(const void *opaque)
> {
> const char *filename = opaque;
> - g_autofree char *path = NULL;
> + g_autofree char *inpath = NULL;
> + g_autofree char *outpath = NULL;
> g_autoptr(qemuFirmware) fw = NULL;
> - g_autofree char *buf = NULL;
> g_autoptr(virJSONValue) json = NULL;
> g_autofree char *expected = NULL;
> g_autofree char *actual = NULL;
> + g_autofree char *buf = NULL;
>
> - path = g_strdup_printf("%s/qemufirmwaredata/%s", abs_srcdir, filename);
> + inpath = g_strdup_printf("%s/qemufirmwaredata/%s", abs_srcdir, filename);
> + outpath = g_strdup_printf("%s/qemufirmwaredata/out/%s", abs_srcdir, filename);
>
> - if (!(fw = qemuFirmwareParse(path)))
> + if (!(fw = qemuFirmwareParse(inpath)))
> return -1;
>
> - if (virFileReadAll(path,
> - 1024 * 1024, /* 1MiB */
> - &buf) < 0)
> - return -1;
> + if (virFileExists(outpath)) {
> + if (virFileReadAll(outpath,
> + 1024 * 1024, /* 1MiB */
> + &buf) < 0)
> + return -1;
> + } else {
> + if (virFileReadAll(inpath,
> + 1024 * 1024, /* 1MiB */
> + &buf) < 0)
> + return -1;
> + }
>
> if (!(json = virJSONValueFromString(buf)))
> return -1;
> @@ -60,7 +69,9 @@ testFWPrecedence(const void *opaque G_GNUC_UNUSED)
> const char *expected[] = {
> PREFIX "/share/qemu/firmware/40-bios.json",
> SYSCONFDIR "/qemu/firmware/40-ovmf-sb-keys.json",
> + PREFIX "/share/qemu/firmware/45-ovmf-sev-stateless.json",
> PREFIX "/share/qemu/firmware/50-ovmf-sb-keys.json",
> + PREFIX "/share/qemu/firmware/55-ovmf-sb-combined.json",
> PREFIX "/share/qemu/firmware/61-ovmf.json",
> PREFIX "/share/qemu/firmware/70-aavmf.json",
> NULL
> @@ -218,7 +229,9 @@ mymain(void)
> } while (0)
>
> DO_PARSE_TEST("usr/share/qemu/firmware/40-bios.json");
> + DO_PARSE_TEST("usr/share/qemu/firmware/45-ovmf-sev-stateless.json");
> DO_PARSE_TEST("usr/share/qemu/firmware/50-ovmf-sb-keys.json");
> + DO_PARSE_TEST("usr/share/qemu/firmware/55-ovmf-sb-combined.json");
> DO_PARSE_TEST("usr/share/qemu/firmware/60-ovmf-sb.json");
> DO_PARSE_TEST("usr/share/qemu/firmware/61-ovmf.json");
> DO_PARSE_TEST("usr/share/qemu/firmware/70-aavmf.json");
> @@ -250,6 +263,8 @@ mymain(void)
> DO_SUPPORTED_TEST("pc-q35-3.1", VIR_ARCH_X86_64, true,
> "/usr/share/seabios/bios-256k.bin:NULL:"
> "/usr/share/OVMF/OVMF_CODE.secboot.fd:/usr/share/OVMF/OVMF_VARS.secboot.fd:"
> + "/usr/share/OVMF/OVMF.sev.fd:NULL:"
> + "/usr/share/OVMF/OVMF.secboot.fd:NULL:"
> "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd",
> VIR_DOMAIN_OS_DEF_FIRMWARE_BIOS,
> VIR_DOMAIN_OS_DEF_FIRMWARE_EFI);
> --
> 2.34.1
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
