[libvirt PATCH] qemu_firmware: select correct firmware for AMD SEV-ES
When using firmware auto-selection and user enables AMD SEV-ES we need to pick correct firmware that actually supports it. This can be detected by having `amd-sev-es` in the firmware JSON description. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_firmware.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 2aeac635da..8d10497717 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -129,6 +129,7 @@ typedef enum { QEMU_FIRMWARE_FEATURE_ACPI_S3, QEMU_FIRMWARE_FEATURE_ACPI_S4, QEMU_FIRMWARE_FEATURE_AMD_SEV, + QEMU_FIRMWARE_FEATURE_AMD_SEV_ES, QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS, QEMU_FIRMWARE_FEATURE_REQUIRES_SMM, QEMU_FIRMWARE_FEATURE_SECURE_BOOT, @@ -145,6 +146,7 @@ VIR_ENUM_IMPL(qemuFirmwareFeature, "acpi-s3", "acpi-s4", "amd-sev", + "amd-sev-es", "enrolled-keys", "requires-smm", "secure-boot", @@ -913,6 +915,9 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) } +#define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2) + + static bool qemuFirmwareMatchDomain(const virDomainDef *def, const qemuFirmware *fw, @@ -924,6 +929,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, bool supportsS4 = false; bool requiresSMM = false; bool supportsSEV = false; + bool supportsSEVES = false; bool supportsSecureBoot = false; bool hasEnrolledKeys = false; int reqSecureBoot; @@ -972,6 +978,11 @@ qemuFirmwareMatchDomain(const virDomainDef *def, case QEMU_FIRMWARE_FEATURE_AMD_SEV: supportsSEV = true; break; + + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + supportsSEVES = true; + break; + case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM: requiresSMM = true; break; @@ -1043,10 +1054,16 @@ qemuFirmwareMatchDomain(const virDomainDef *def, } if (def->sev && - def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && - !supportsSEV) { - VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); - return false; + def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + if (!supportsSEV) { + VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); + return false; + } + + if (def->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && !supportsSEVES) { + VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", path); + return false; + } } VIR_DEBUG("Firmware '%s' matches domain requirements", path); @@ -1148,6 +1165,7 @@ qemuFirmwareEnableFeatures(virQEMUDriver *driver, case QEMU_FIRMWARE_FEATURE_ACPI_S3: case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: @@ -1181,6 +1199,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw, case QEMU_FIRMWARE_FEATURE_ACPI_S3: case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC: @@ -1412,6 +1431,7 @@ qemuFirmwareGetSupported(const char *machine, case QEMU_FIRMWARE_FEATURE_ACPI_S3: case QEMU_FIRMWARE_FEATURE_ACPI_S4: case QEMU_FIRMWARE_FEATURE_AMD_SEV: + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS: case QEMU_FIRMWARE_FEATURE_SECURE_BOOT: case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC: -- 2.31.1
On 6/11/21 2:27 PM, Pavel Hrdina wrote:
When using firmware auto-selection and user enables AMD SEV-ES we need to pick correct firmware that actually supports it. This can be detected by having `amd-sev-es` in the firmware JSON description.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_firmware.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 2aeac635da..8d10497717 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -129,6 +129,7 @@ typedef enum { QEMU_FIRMWARE_FEATURE_ACPI_S3, QEMU_FIRMWARE_FEATURE_ACPI_S4, QEMU_FIRMWARE_FEATURE_AMD_SEV, + QEMU_FIRMWARE_FEATURE_AMD_SEV_ES, QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS, QEMU_FIRMWARE_FEATURE_REQUIRES_SMM, QEMU_FIRMWARE_FEATURE_SECURE_BOOT, @@ -145,6 +146,7 @@ VIR_ENUM_IMPL(qemuFirmwareFeature, "acpi-s3", "acpi-s4", "amd-sev", + "amd-sev-es", "enrolled-keys", "requires-smm", "secure-boot", @@ -913,6 +915,9 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) }
+#define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2) + + static bool qemuFirmwareMatchDomain(const virDomainDef *def, const qemuFirmware *fw, @@ -924,6 +929,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, bool supportsS4 = false; bool requiresSMM = false; bool supportsSEV = false; + bool supportsSEVES = false; bool supportsSecureBoot = false; bool hasEnrolledKeys = false; int reqSecureBoot; @@ -972,6 +978,11 @@ qemuFirmwareMatchDomain(const virDomainDef *def, case QEMU_FIRMWARE_FEATURE_AMD_SEV: supportsSEV = true; break; + + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + supportsSEVES = true; + break; + case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM: requiresSMM = true; break; @@ -1043,10 +1054,16 @@ qemuFirmwareMatchDomain(const virDomainDef *def, }
if (def->sev && - def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && - !supportsSEV) { - VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); - return false; + def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + if (!supportsSEV) { + VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); + return false; + } + + if (def->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && !supportsSEVES) {
Please break this ^^ long line.
+ VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", path); + return false; + } }
We have src/qemu/qemu_firmware.c which tests this code. Mind posting a follow up patch? For this one though: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Mon, Jun 14, 2021 at 11:15:07AM +0200, Michal Prívozník wrote:
On 6/11/21 2:27 PM, Pavel Hrdina wrote:
When using firmware auto-selection and user enables AMD SEV-ES we need to pick correct firmware that actually supports it. This can be detected by having `amd-sev-es` in the firmware JSON description.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_firmware.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c index 2aeac635da..8d10497717 100644 --- a/src/qemu/qemu_firmware.c +++ b/src/qemu/qemu_firmware.c @@ -129,6 +129,7 @@ typedef enum { QEMU_FIRMWARE_FEATURE_ACPI_S3, QEMU_FIRMWARE_FEATURE_ACPI_S4, QEMU_FIRMWARE_FEATURE_AMD_SEV, + QEMU_FIRMWARE_FEATURE_AMD_SEV_ES, QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS, QEMU_FIRMWARE_FEATURE_REQUIRES_SMM, QEMU_FIRMWARE_FEATURE_SECURE_BOOT, @@ -145,6 +146,7 @@ VIR_ENUM_IMPL(qemuFirmwareFeature, "acpi-s3", "acpi-s4", "amd-sev", + "amd-sev-es", "enrolled-keys", "requires-smm", "secure-boot", @@ -913,6 +915,9 @@ qemuFirmwareOSInterfaceTypeFromOsDefFirmware(int fw) }
+#define VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY (1 << 2) + + static bool qemuFirmwareMatchDomain(const virDomainDef *def, const qemuFirmware *fw, @@ -924,6 +929,7 @@ qemuFirmwareMatchDomain(const virDomainDef *def, bool supportsS4 = false; bool requiresSMM = false; bool supportsSEV = false; + bool supportsSEVES = false; bool supportsSecureBoot = false; bool hasEnrolledKeys = false; int reqSecureBoot; @@ -972,6 +978,11 @@ qemuFirmwareMatchDomain(const virDomainDef *def, case QEMU_FIRMWARE_FEATURE_AMD_SEV: supportsSEV = true; break; + + case QEMU_FIRMWARE_FEATURE_AMD_SEV_ES: + supportsSEVES = true; + break; + case QEMU_FIRMWARE_FEATURE_REQUIRES_SMM: requiresSMM = true; break; @@ -1043,10 +1054,16 @@ qemuFirmwareMatchDomain(const virDomainDef *def, }
if (def->sev && - def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV && - !supportsSEV) { - VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); - return false; + def->sev->sectype == VIR_DOMAIN_LAUNCH_SECURITY_SEV) { + if (!supportsSEV) { + VIR_DEBUG("Domain requires SEV, firmware '%s' doesn't support it", path); + return false; + } + + if (def->sev->policy & VIR_QEMU_FIRMWARE_AMD_SEV_ES_POLICY && !supportsSEVES) {
Please break this ^^ long line.
Will do.
+ VIR_DEBUG("Domain requires SEV-ES, firmware '%s' doesn't support it", path); + return false; + } }
We have src/qemu/qemu_firmware.c which tests this code. Mind posting a follow up patch? For this one though:
I guess you mean tests/qemufirmwaretest.c. I'll post a follow up.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks, Pavel
participants (2)
-
Michal Prívozník -
Pavel Hrdina