[PATCH] qemu: don't warn about missing SMM for CVM firmware
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
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
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 :|
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
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
participants (3)
-
Andrea Bolognani -
Daniel P. Berrangé -
Ján Tomko