Re: [libvirt PATCH] qemu_firmware: select correct firmware for AMD SEV-ES

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(a)redhat.com&gt; > --- > 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(a)redhat.com&gt;