1. Patchew
  2. Libvirt
  3. View series

[PATCH] qemu: don't warn about missing SMM for CVM firmware

Daniel P. Berrangé via Devel posted 1 patch 1 month, 1 week ago
Failed in applying to current master (apply log)
src/qemu/qemu_firmware.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
[PATCH] qemu: don't warn about missing SMM for CVM firmware
Posted by Daniel P. Berrangé via Devel 1 month, 1 week ago 
From: Daniel P. Berrangé <berrange@redhat.com>

Neither Intel TDX / AMD SEV(SNP) allow use of SMM, but the EDK2
firmware none the less supports secureboot. Libvirt currently
issues bogus warnings about Fedora firmware

  warning : qemuFirmwareSanityCheck:1575 : Firmware description
  '/usr/share/qemu/firmware/60-edk2-ovmf-x64-inteltdx.json' has
  invalid set of features: requires-smm = 0, secure-boot = 1,
  enrolled-keys = 1

This removes the warning if the firmware descriptor indicates use
of any confidential VM technology.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_firmware.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index f10137144e..dbec068738 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
     bool requiresSMM = false;
     bool supportsSecureBoot = false;
     bool hasEnrolledKeys = false;
+    bool cvm = false;
 
     for (i = 0; i < fw->nfeatures; i++) {
         switch (fw->features[i]) {
@@ -1552,13 +1553,15 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
         case QEMU_FIRMWARE_FEATURE_ENROLLED_KEYS:
             hasEnrolledKeys = true;
             break;
-        case QEMU_FIRMWARE_FEATURE_NONE:
-        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_AMD_SEV_SNP:
         case QEMU_FIRMWARE_FEATURE_INTEL_TDX:
+            cvm = true;
+            break;
+        case QEMU_FIRMWARE_FEATURE_NONE:
+        case QEMU_FIRMWARE_FEATURE_ACPI_S3:
+        case QEMU_FIRMWARE_FEATURE_ACPI_S4:
         case QEMU_FIRMWARE_FEATURE_VERBOSE_DYNAMIC:
         case QEMU_FIRMWARE_FEATURE_VERBOSE_STATIC:
         case QEMU_FIRMWARE_FEATURE_LAST:
@@ -1566,7 +1569,8 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
         }
     }
 
-    if ((supportsSecureBoot != requiresSMM) ||
+    if ((!cvm &&
+         (supportsSecureBoot != requiresSMM)) ||
         (hasEnrolledKeys && !supportsSecureBoot)) {
         VIR_WARN("Firmware description '%s' has invalid set of features: "
                  "%s = %d, %s = %d, %s = %d",
-- 
2.50.1
Re: [PATCH] qemu: don't warn about missing SMM for CVM firmware
Posted by Ján Tomko via Devel 1 month ago 
On a Thursday in 2025, Daniel P. Berrangé via Devel wrote:
>From: Daniel P. Berrangé <berrange@redhat.com>
>
>Neither Intel TDX / AMD SEV(SNP) allow use of SMM, but the EDK2
>firmware none the less supports secureboot. Libvirt currently
>issues bogus warnings about Fedora firmware
>
>  warning : qemuFirmwareSanityCheck:1575 : Firmware description
>  '/usr/share/qemu/firmware/60-edk2-ovmf-x64-inteltdx.json' has
>  invalid set of features: requires-smm = 0, secure-boot = 1,
>  enrolled-keys = 1
>
>This removes the warning if the firmware descriptor indicates use
>of any confidential VM technology.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/qemu/qemu_firmware.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
>index f10137144e..dbec068738 100644
>--- a/src/qemu/qemu_firmware.c
>+++ b/src/qemu/qemu_firmware.c
>@@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
>     bool requiresSMM = false;
>     bool supportsSecureBoot = false;
>     bool hasEnrolledKeys = false;
>+    bool cvm = false;
>

Naming this, for example, confidential VM would be more descriptive.

>     for (i = 0; i < fw->nfeatures; i++) {
>         switch (fw->features[i]) {

Regardless:
Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] qemu: don't warn about missing SMM for CVM firmware
Posted by Andrea Bolognani via Devel 1 month ago 
On Thu, Jul 31, 2025 at 07:33:21PM +0100, Daniel P. Berrangé via Devel wrote:
> +++ b/src/qemu/qemu_firmware.c
> @@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
>      bool requiresSMM = false;
>      bool supportsSecureBoot = false;
>      bool hasEnrolledKeys = false;
> +    bool cvm = false;

Maybe isConfidential instead, to follow the existing convention and
be a little more descriptive?

> @@ -1566,7 +1569,8 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
>          }
>      }
>
> -    if ((supportsSecureBoot != requiresSMM) ||
> +    if ((!cvm &&
> +         (supportsSecureBoot != requiresSMM)) ||
>          (hasEnrolledKeys && !supportsSecureBoot)) {
>          VIR_WARN("Firmware description '%s' has invalid set of features: "
>                   "%s = %d, %s = %d, %s = %d",

This could use a short comment explaining why firmware intended for
CVM doesn't need SSM for Secure Boot.

Regardless of whether you want to act on any of the above
suggestions, the change makes sense so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH] qemu: don't warn about missing SMM for CVM firmware
Posted by Daniel P. Berrangé via Devel 1 month ago 
On Tue, Aug 05, 2025 at 08:54:02AM -0500, Andrea Bolognani wrote:
> On Thu, Jul 31, 2025 at 07:33:21PM +0100, Daniel P. Berrangé via Devel wrote:
> > +++ b/src/qemu/qemu_firmware.c
> > @@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> >      bool requiresSMM = false;
> >      bool supportsSecureBoot = false;
> >      bool hasEnrolledKeys = false;
> > +    bool cvm = false;
> 
> Maybe isConfidential instead, to follow the existing convention and
> be a little more descriptive?
> 
> > @@ -1566,7 +1569,8 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> >          }
> >      }
> >
> > -    if ((supportsSecureBoot != requiresSMM) ||
> > +    if ((!cvm &&
> > +         (supportsSecureBoot != requiresSMM)) ||
> >          (hasEnrolledKeys && !supportsSecureBoot)) {
> >          VIR_WARN("Firmware description '%s' has invalid set of features: "
> >                   "%s = %d, %s = %d, %s = %d",
> 
> This could use a short comment explaining why firmware intended for
> CVM doesn't need SSM for Secure Boot.
> 
> Regardless of whether you want to act on any of the above
> suggestions, the change makes sense so

I made both those changes and pushed.


With 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 :|
Re: [PATCH] qemu: don't warn about missing SMM for CVM firmware
Posted by Andrea Bolognani via Devel 1 month ago 
On Tue, Aug 05, 2025 at 04:28:18PM +0100, Daniel P. Berrangé wrote:
> On Tue, Aug 05, 2025 at 08:54:02AM -0500, Andrea Bolognani wrote:
> > On Thu, Jul 31, 2025 at 07:33:21PM +0100, Daniel P. Berrangé via Devel wrote:
> > > +++ b/src/qemu/qemu_firmware.c
> > > @@ -1540,6 +1540,7 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> > >      bool requiresSMM = false;
> > >      bool supportsSecureBoot = false;
> > >      bool hasEnrolledKeys = false;
> > > +    bool cvm = false;
> >
> > Maybe isConfidential instead, to follow the existing convention and
> > be a little more descriptive?
> >
> > > @@ -1566,7 +1569,8 @@ qemuFirmwareSanityCheck(const qemuFirmware *fw,
> > >          }
> > >      }
> > >
> > > -    if ((supportsSecureBoot != requiresSMM) ||
> > > +    if ((!cvm &&
> > > +         (supportsSecureBoot != requiresSMM)) ||
> > >          (hasEnrolledKeys && !supportsSecureBoot)) {
> > >          VIR_WARN("Firmware description '%s' has invalid set of features: "
> > >                   "%s = %d, %s = %d, %s = %d",
> >
> > This could use a short comment explaining why firmware intended for
> > CVM doesn't need SSM for Secure Boot.
> >
> > Regardless of whether you want to act on any of the above
> > suggestions, the change makes sense so
>
> I made both those changes and pushed.

Looks great, thank you!

-- 
Andrea Bolognani / Red Hat / Virtualization

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.