From 3cde498c98be902fc8fe87c895dfeaaa95352b38 Mon Sep 17 00:00:00 2001

Message-Id: <3cde498c98be902fc8fe87c895dfeaaa95352b38@dist-git>

From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>

Date: Thu, 3 Feb 2022 13:43:18 +0000

Subject: [PATCH] qemu: support firmware descriptor flash 'mode' for optional

 NVRAM

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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

  - "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.

In the tests we have a mixture of files with and without the

mode attribute.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

(cherry picked from commit 32b9d8b0ae00669555f01f91ee11612a636c4b69)

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2057769

---

 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 529ab8d68e..7911d45aa0 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;

 }

@@ -1053,6 +1100,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.35.1