[libvirt PATCH] qemu: support parsing firmware descriptor flash 'mode' for optional NVRAM

Daniel P. Berrangé berrange at redhat.com
Thu Feb 17 11:54:07 UTC 2022


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 at redhat.com>
---
 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




More information about the libvir-list mailing list