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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux