1. Patchew
  2. Libvirt
  3. View series

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

Pavel Hrdina posted 1 patch 2 years, 10 months ago
Failed in applying to current master (apply log)
src/qemu/qemu_firmware.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
[libvirt PATCH] qemu_firmware: select correct firmware for AMD SEV-ES
Posted by Pavel Hrdina 2 years, 10 months ago 
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
Re: [libvirt PATCH] qemu_firmware: select correct firmware for AMD SEV-ES
Posted by Michal Prívozník 2 years, 10 months ago 
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
Re: [libvirt PATCH] qemu_firmware: select correct firmware for AMD SEV-ES
Posted by Pavel Hrdina 2 years, 10 months ago 
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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.