[PATCH 2/5] tests: Add firmware-auto-efi-sev-snp

Andrea Bolognani via Devel posted 5 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 1 month, 1 week ago
This test case demonstrates how firmware autoselection doesn't
currently work correctly for domains using SEV-SNP: the
descriptor for a suitable firmware exists, and yet it doesn't
get picked up.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 ...-auto-efi-sev-snp.x86_64-latest+amdsev.err |  1 +
 ...-auto-efi-sev-snp.x86_64-latest+amdsev.xml | 38 +++++++++++++++++++
 .../firmware-auto-efi-sev-snp.xml             | 20 ++++++++++
 tests/qemuxmlconftest.c                       |  5 +++
 4 files changed, 64 insertions(+)
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.xml

diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err
new file mode 100644
index 0000000000..3edb2b3451
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.err
@@ -0,0 +1 @@
+operation failed: Unable to find 'efi' firmware that is compatible with the current configuration
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
new file mode 100644
index 0000000000..81ac7888ea
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
@@ -0,0 +1,38 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-10.0'>hvm</type>
+    <loader format='raw'/>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0' model='none'/>
+    <controller type='sata' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pcie-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <watchdog model='itco' action='reset'/>
+    <memballoon model='none'/>
+  </devices>
+  <launchSecurity type='sev-snp'>
+    <policy>0x00030000</policy>
+  </launchSecurity>
+</domain>
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.xml b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.xml
new file mode 100644
index 0000000000..ea980c06c2
--- /dev/null
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.xml
@@ -0,0 +1,20 @@
+<domain type='kvm'>
+  <name>guest</name>
+  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <vcpu placement='static'>1</vcpu>
+  <os firmware='efi'>
+    <type arch='x86_64' machine='pc-q35-10.0'>hvm</type>
+  </os>
+  <features>
+    <acpi/>
+  </features>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' model='none'/>
+    <memballoon model='none'/>
+  </devices>
+  <launchSecurity type="sev-snp">
+    <policy>0x30000</policy>
+  </launchSecurity>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 678d2efbc3..272d314195 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -1477,6 +1477,11 @@ mymain(void)
     DO_TEST_CAPS_ARCH_LATEST_ABI_UPDATE("firmware-auto-efi-format-loader-raw", "aarch64");
     DO_TEST_CAPS_LATEST("firmware-auto-efi-format-mismatch");
 
+    DO_TEST_CAPS_ARCH_LATEST_FULL("firmware-auto-efi-sev-snp", "x86_64",
+                                  ARG_FLAGS, FLAG_EXPECT_FAILURE,
+                                  ARG_CAPS_VARIANT, "+amdsev",
+                                  ARG_END);
+
     DO_TEST_CAPS_LATEST("clock-utc");
     DO_TEST_CAPS_LATEST("clock-localtime");
     DO_TEST_CAPS_LATEST("clock-localtime-basis-localtime");
-- 
2.50.1
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 3 weeks, 5 days ago
On 7/31/25 09:45, Andrea Bolognani via Devel wrote:
> This test case demonstrates how firmware autoselection doesn't
> currently work correctly for domains using SEV-SNP: the
> descriptor for a suitable firmware exists, and yet it doesn't
> get picked up.

On my test system, autoselection for SEV-SNP guests does work after making the 
firmware descriptor changes suggested by Gerd

https://src.fedoraproject.org/fork/kraxel/rpms/edk2/c/5146a0c3e9bf821d045e0cc3600ad715aca14588

It fails for SEV and SEV-ES guests. As a first step, I tried "importing" the 
descriptor changes to tests/qemufirmwaredata/, but as always I'm fighting with 
fixing up the tests :-/.

Regards,
Jim
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 3 weeks, 5 days ago
On Tue, Aug 12, 2025 at 05:26:19PM -0600, Jim Fehlig wrote:
> On 7/31/25 09:45, Andrea Bolognani via Devel wrote:
> > This test case demonstrates how firmware autoselection doesn't
> > currently work correctly for domains using SEV-SNP: the
> > descriptor for a suitable firmware exists, and yet it doesn't
> > get picked up.
>
> On my test system, autoselection for SEV-SNP guests does work after making
> the firmware descriptor changes suggested by Gerd
>
> https://src.fedoraproject.org/fork/kraxel/rpms/edk2/c/5146a0c3e9bf821d045e0cc3600ad715aca14588
>
> It fails for SEV and SEV-ES guests. As a first step, I tried "importing" the
> descriptor changes to tests/qemufirmwaredata/, but as always I'm fighting
> with fixing up the tests :-/.

Patch importing the changes attached.

Can you be more specific about the issue you're experiencing for
SEV(-ES) guests? Based on the patch, the behavior doesn't seem to
change at all there. Are you able to successfully start those guests
when you use unmodified libvirt and edk2?

Then again, the existing SEV tests look... Questionable. They all use
the i440fx machine type and default (BIOS) firmware, whereas
according to the documentation[1] you really want q35 and UEFI. So at
best our test coverage is lacking.

Stressing again the fact that I know very little about SEV and its
variants, my impression is that generally speaking stateless firmware
is preferred for the use case; however in Fedora the descriptors for
"regular" edk2 builds with no Secure Boot[2] advertise support for
the "amd-sev" and "amd-sev-es" firmware features, and since they sort
before the SEV-specific builds[3] libvirt will pick them up unless
you specifically ask for the firmware to be stateless.

Not sure if the best way to get out of this situation is to shuffle
the descriptors around, drop the SEV-specific features from other
descriptors, or tweak the libvirt algorithm so that it will prefer
stateless firmware for SEV unless told otherwise.

Very much interested in hearing everyone's thoughts on the topic.


[1] https://libvirt.org/kbase/launch_security_sev.html
[2] /usr/share/qemu/firmware/5*-edk2-ovmf-*-x64-nosb.json
[3] /usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev*.json
-- 
Andrea Bolognani / Red Hat / Virtualization
From 9fa2cf19ea8fa659d3b7fd5a1dafa510a04612f4 Mon Sep 17 00:00:00 2001
From: Andrea Bolognani <abologna@redhat.com>
Date: Wed, 13 Aug 2025 11:10:32 +0200
Subject: [PATCH] DONOTMERGE update firmware descriptors

---
 .../qemu/firmware/60-edk2-ovmf-x64-amdsev.json     |  1 -
 .../qemu/firmware/60-edk2-ovmf-x64-amdsev.json     |  3 +--
 .../qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json} | 14 ++++++--------
 tests/qemufirmwaretest.c                           |  2 ++
 ...ware-auto-efi-sev-snp.x86_64-latest+amdsev.args |  5 ++---
 ...mware-auto-efi-sev-snp.x86_64-latest+amdsev.xml |  2 +-
 ...unch-security-sev-snp.x86_64-latest+amdsev.args |  5 ++---
 ...aunch-security-sev-snp.x86_64-latest+amdsev.xml |  2 +-
 .../launch-security-sev-snp.x86_64-latest.args     |  5 ++---
 .../launch-security-sev-snp.x86_64-latest.xml      |  2 +-
 10 files changed, 18 insertions(+), 23 deletions(-)
 copy tests/qemufirmwaredata/{out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json => usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json} (57%)

diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
index d83d394ba7..2d3b821acb 100644
--- a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
+++ b/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
@@ -21,7 +21,6 @@
     "features": [
         "amd-sev",
         "amd-sev-es",
-        "amd-sev-snp",
         "verbose-dynamic"
     ]
 }
diff --git a/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
index 9a561bc7eb..ca88ef9176 100644
--- a/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
+++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
@@ -1,5 +1,5 @@
 {
-    "description": "OVMF with SEV-ES support",
+    "description": "OVMF with SEV + SEV-ES support",
     "interface-types": [
         "uefi"
     ],
@@ -22,7 +22,6 @@
     "features": [
         "amd-sev",
         "amd-sev-es",
-        "amd-sev-snp",
         "verbose-dynamic"
     ],
     "tags": [
diff --git a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json
similarity index 57%
copy from tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
copy to tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json
index d83d394ba7..99e51c3d00 100644
--- a/tests/qemufirmwaredata/out/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json
+++ b/tests/qemufirmwaredata/usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json
@@ -1,14 +1,11 @@
 {
+    "description": "OVMF with SEV-SNP support",
     "interface-types": [
         "uefi"
     ],
     "mapping": {
-        "device": "flash",
-        "mode": "stateless",
-        "executable": {
-            "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
-            "format": "raw"
-        }
+        "device": "memory",
+        "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
     },
     "targets": [
         {
@@ -19,9 +16,10 @@
         }
     ],
     "features": [
-        "amd-sev",
-        "amd-sev-es",
         "amd-sev-snp",
         "verbose-dynamic"
+    ],
+    "tags": [
+
     ]
 }
diff --git a/tests/qemufirmwaretest.c b/tests/qemufirmwaretest.c
index a4fb5c9b9c..c18ee85c0a 100644
--- a/tests/qemufirmwaretest.c
+++ b/tests/qemufirmwaretest.c
@@ -100,6 +100,7 @@ testFWPrecedence(const void *opaque G_GNUC_UNUSED)
         PREFIX "/share/qemu/firmware/53-edk2-aarch64-verbose-raw.json",
         SYSCONFDIR "/qemu/firmware/59-combined.json",
         PREFIX "/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json",
+        PREFIX "/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json",
         PREFIX "/share/qemu/firmware/60-edk2-ovmf-x64-inteltdx.json",
         PREFIX "/share/qemu/firmware/90-combined.json",
         PREFIX "/share/qemu/firmware/91-bios.json",
@@ -279,6 +280,7 @@ mymain(void)
     DO_PARSE_TEST("usr/share/qemu/firmware/52-edk2-aarch64-verbose-qcow2.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/53-edk2-aarch64-verbose-raw.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsev.json");
+    DO_PARSE_TEST("usr/share/qemu/firmware/60-edk2-ovmf-x64-amdsevsnp.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/60-edk2-ovmf-x64-inteltdx.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/90-combined.json");
     DO_PARSE_TEST("usr/share/qemu/firmware/91-bios.json");
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args
index 99350f600c..624039d1a2 100644
--- a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args
@@ -10,11 +10,10 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \
 -name guest=guest,debug-threads=on \
 -S \
 -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \
--blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.amdsev.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
--machine pc-q35-10.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,pflash0=libvirt-pflash0-format,acpi=on \
+-machine pc-q35-10.0,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,acpi=on \
 -accel kvm \
 -cpu qemu64 \
+-bios /usr/share/edk2/ovmf/OVMF.amdsev.fd \
 -m size=1048576k \
 -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}' \
 -overcommit mem-lock=off \
diff --git a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
index 6ea58f3361..10a1a3a22d 100644
--- a/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
+++ b/tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml
@@ -10,7 +10,7 @@
       <feature enabled='no' name='enrolled-keys'/>
       <feature enabled='no' name='secure-boot'/>
     </firmware>
-    <loader readonly='yes' type='pflash' stateless='yes' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
+    <loader type='rom' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
     <boot dev='hd'/>
   </os>
   <features>
diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.args b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.args
index b3bc7fcf04..c191b62070 100644
--- a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.args
+++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.args
@@ -10,11 +10,10 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -name guest=QEMUGuest1,debug-threads=on \
 -S \
 -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
--blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.amdsev.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
--machine pc-q35-8.2,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,pflash0=libvirt-pflash0-format,acpi=on \
+-machine pc-q35-8.2,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,acpi=on \
 -accel kvm \
 -cpu qemu64 \
+-bios /usr/share/edk2/ovmf/OVMF.amdsev.fd \
 -m size=219136k \
 -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
 -overcommit mem-lock=off \
diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.xml b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.xml
index d9bf146993..f356fb798a 100644
--- a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.xml
+++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest+amdsev.xml
@@ -10,7 +10,7 @@
       <feature enabled='no' name='enrolled-keys'/>
       <feature enabled='no' name='secure-boot'/>
     </firmware>
-    <loader readonly='yes' type='pflash' stateless='yes' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
+    <loader type='rom' stateless='yes' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
     <boot dev='hd'/>
   </os>
   <features>
diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args
index b3bc7fcf04..c191b62070 100644
--- a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.args
@@ -10,11 +10,10 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -name guest=QEMUGuest1,debug-threads=on \
 -S \
 -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
--blockdev '{"driver":"file","filename":"/usr/share/edk2/ovmf/OVMF.amdsev.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}' \
--machine pc-q35-8.2,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,pflash0=libvirt-pflash0-format,acpi=on \
+-machine pc-q35-8.2,usb=off,dump-guest-core=off,memory-backend=pc.ram,confidential-guest-support=lsec0,acpi=on \
 -accel kvm \
 -cpu qemu64 \
+-bios /usr/share/edk2/ovmf/OVMF.amdsev.fd \
 -m size=219136k \
 -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
 -overcommit mem-lock=off \
diff --git a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml
index d9bf146993..f356fb798a 100644
--- a/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/launch-security-sev-snp.x86_64-latest.xml
@@ -10,7 +10,7 @@
       <feature enabled='no' name='enrolled-keys'/>
       <feature enabled='no' name='secure-boot'/>
     </firmware>
-    <loader readonly='yes' type='pflash' stateless='yes' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
+    <loader type='rom' stateless='yes' format='raw'>/usr/share/edk2/ovmf/OVMF.amdsev.fd</loader>
     <boot dev='hd'/>
   </os>
   <features>
-- 
2.50.1

Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 2 weeks, 5 days ago
On 8/13/25 09:01, Andrea Bolognani wrote:
> On Tue, Aug 12, 2025 at 05:26:19PM -0600, Jim Fehlig wrote:
>> On 7/31/25 09:45, Andrea Bolognani via Devel wrote:
>>> This test case demonstrates how firmware autoselection doesn't
>>> currently work correctly for domains using SEV-SNP: the
>>> descriptor for a suitable firmware exists, and yet it doesn't
>>> get picked up.
>>
>> On my test system, autoselection for SEV-SNP guests does work after making
>> the firmware descriptor changes suggested by Gerd
>>
>> https://src.fedoraproject.org/fork/kraxel/rpms/edk2/c/5146a0c3e9bf821d045e0cc3600ad715aca14588
>>
>> It fails for SEV and SEV-ES guests. As a first step, I tried "importing" the
>> descriptor changes to tests/qemufirmwaredata/, but as always I'm fighting
>> with fixing up the tests :-/.
> 
> Patch importing the changes attached.
> 
> Can you be more specific about the issue you're experiencing for
> SEV(-ES) guests? Based on the patch, the behavior doesn't seem to
> change at all there. Are you able to successfully start those guests
> when you use unmodified libvirt and edk2?
> 
> Then again, the existing SEV tests look... Questionable. They all use
> the i440fx machine type and default (BIOS) firmware, whereas
> according to the documentation[1] you really want q35 and UEFI. So at
> best our test coverage is lacking.

FYI, I've been working on a series that updates the existing SEV tests, includes 
your descriptor import, and slightly tweaked patches from this series

https://gitlab.com/jfehlig/libvirt/-/tree/coco-firmware-autoselect-improvements?ref_type=heads

> Stressing again the fact that I know very little about SEV and its
> variants, my impression is that generally speaking stateless firmware
> is preferred for the use case; however in Fedora the descriptors for
> "regular" edk2 builds with no Secure Boot[2] advertise support for
> the "amd-sev" and "amd-sev-es" firmware features, and since they sort
> before the SEV-specific builds[3] libvirt will pick them up unless
> you specifically ask for the firmware to be stateless.
> 
> Not sure if the best way to get out of this situation is to shuffle
> the descriptors around, drop the SEV-specific features from other
> descriptors, or tweak the libvirt algorithm so that it will prefer
> stateless firmware for SEV unless told otherwise.

My WIP series drops the SEV features from the incompatible descriptors.

I will be off the remainder of the week, but can tidy the series and post a V1 
next week if there's interest.

Regards,
Jim
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 2 weeks, 5 days ago
On Tue, Aug 19, 2025 at 04:09:28PM -0600, Jim Fehlig via Devel wrote:
> On 8/13/25 09:01, Andrea Bolognani wrote:
> > Stressing again the fact that I know very little about SEV and its
> > variants, my impression is that generally speaking stateless firmware
> > is preferred for the use case; however in Fedora the descriptors for
> > "regular" edk2 builds with no Secure Boot[2] advertise support for
> > the "amd-sev" and "amd-sev-es" firmware features, and since they sort
> > before the SEV-specific builds[3] libvirt will pick them up unless
> > you specifically ask for the firmware to be stateless.
> >
> > Not sure if the best way to get out of this situation is to shuffle
> > the descriptors around, drop the SEV-specific features from other
> > descriptors, or tweak the libvirt algorithm so that it will prefer
> > stateless firmware for SEV unless told otherwise.
>
> My WIP series drops the SEV features from the incompatible descriptors.

That feels premature. I'm okay with going in that direction, but it's
not a change that we should make to the libvirt test suite before
reaching an agreement and having the change applied to the edk2
package. The libvirt test suite is intended to match the real life
behavior as closely as possible.

> I will be off the remainder of the week, but can tidy the series and post a
> V1 next week if there's interest.

AFAICT you've made no code change other than squashing in the fixup
that I had provided shortly after posting v1. Did I miss something?

Your patch updating the SEV(-ES) tests to use q35 and UEFI looks
reasonable from a quick look. I'll take a closer one and report back.

Overall it doesn't IMO make sense for you to post a series off that
branch. I can pick up your test suite changes, squash in my fix and
post v2 next week.

But before we can consider pushing any of this, we need to solve the
SEV(-ES) issue you've mentioned elsewhere in the thread and reach an
overall agreement on what the descriptors for firmware targeting all
SEV variants should look like going forward.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 1 week, 6 days ago
On 8/20/25 09:24, Andrea Bolognani wrote:
> On Tue, Aug 19, 2025 at 04:09:28PM -0600, Jim Fehlig via Devel wrote:
>> On 8/13/25 09:01, Andrea Bolognani wrote:
>>> Stressing again the fact that I know very little about SEV and its
>>> variants, my impression is that generally speaking stateless firmware
>>> is preferred for the use case; however in Fedora the descriptors for
>>> "regular" edk2 builds with no Secure Boot[2] advertise support for
>>> the "amd-sev" and "amd-sev-es" firmware features, and since they sort
>>> before the SEV-specific builds[3] libvirt will pick them up unless
>>> you specifically ask for the firmware to be stateless.
>>>
>>> Not sure if the best way to get out of this situation is to shuffle
>>> the descriptors around, drop the SEV-specific features from other
>>> descriptors, or tweak the libvirt algorithm so that it will prefer
>>> stateless firmware for SEV unless told otherwise.
>>
>> My WIP series drops the SEV features from the incompatible descriptors.
> 
> That feels premature. I'm okay with going in that direction, but it's
> not a change that we should make to the libvirt test suite before
> reaching an agreement and having the change applied to the edk2
> package. The libvirt test suite is intended to match the real life
> behavior as closely as possible.
> 
>> I will be off the remainder of the week, but can tidy the series and post a
>> V1 next week if there's interest.
> 
> AFAICT you've made no code change other than squashing in the fixup
> that I had provided shortly after posting v1. Did I miss something?

No, you didn't miss anything. Poor choice of wording on my part. Instead of 
"I've been working on a series..." it would have been clearer to say "I've been 
assembling a series...". Afterall, 4 of the 6 patches are yours :-).

> Your patch updating the SEV(-ES) tests to use q35 and UEFI looks
> reasonable from a quick look. I'll take a closer one and report back.
> 
> Overall it doesn't IMO make sense for you to post a series off that
> branch. I can pick up your test suite changes, squash in my fix and
> post v2 next week.

Agree it's better for you to continue with your series :-). I was just trying to 
help move this along while I had some free cycles.

Regards,
Jim
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 2 weeks ago
On Wed, Aug 20, 2025 at 08:24:51AM -0700, Andrea Bolognani wrote:
> On Tue, Aug 19, 2025 at 04:09:28PM -0600, Jim Fehlig via Devel wrote:
> > I will be off the remainder of the week, but can tidy the series and post a
> > V1 next week if there's interest.
>
> AFAICT you've made no code change other than squashing in the fixup
> that I had provided shortly after posting v1. Did I miss something?
>
> Your patch updating the SEV(-ES) tests to use q35 and UEFI looks
> reasonable from a quick look. I'll take a closer one and report back.
>
> Overall it doesn't IMO make sense for you to post a series off that
> branch. I can pick up your test suite changes, squash in my fix and
> post v2 next week.

Done.

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/IBI4ZG6YDMW35WIEZVPHIQCVFZ5GMQZZ/

> > My WIP series drops the SEV features from the incompatible descriptors.
>
> That feels premature. I'm okay with going in that direction, but it's
> not a change that we should make to the libvirt test suite before
> reaching an agreement and having the change applied to the edk2
> package. The libvirt test suite is intended to match the real life
> behavior as closely as possible.

I just realized that one of the commits in my v2 pretty much matches
this. Oops. Anyway I've marked it at DONOTMERGE because of the
rationale above: we should never get ahead of the edk2 package, so
that commit is just intended as a way to foster discussion.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 3 weeks, 3 days ago
On 8/13/25 09:01, Andrea Bolognani wrote:
> On Tue, Aug 12, 2025 at 05:26:19PM -0600, Jim Fehlig wrote:
>> On 7/31/25 09:45, Andrea Bolognani via Devel wrote:
>>> This test case demonstrates how firmware autoselection doesn't
>>> currently work correctly for domains using SEV-SNP: the
>>> descriptor for a suitable firmware exists, and yet it doesn't
>>> get picked up.
>>
>> On my test system, autoselection for SEV-SNP guests does work after making
>> the firmware descriptor changes suggested by Gerd
>>
>> https://src.fedoraproject.org/fork/kraxel/rpms/edk2/c/5146a0c3e9bf821d045e0cc3600ad715aca14588
>>
>> It fails for SEV and SEV-ES guests. As a first step, I tried "importing" the
>> descriptor changes to tests/qemufirmwaredata/, but as always I'm fighting
>> with fixing up the tests :-/.
> 
> Patch importing the changes attached.

Thanks! My trial and error approach to fixing the test suite would have 
eventually got there :-).

> Can you be more specific about the issue you're experiencing for
> SEV(-ES) guests?

I'm seeing the same issue we were trying to solve for SNP guests with this series

ERROR    operation failed: Unable to find 'efi' firmware that is compatible with 
the current configuration

> Based on the patch, the behavior doesn't seem to
> change at all there. Are you able to successfully start those guests
> when you use unmodified libvirt and edk2?

No, the same issue exists with SEV and SEV-ES guests. Sorry if that was not 
clear. Prior to separating SNP into its own descriptor, all three features 
shared the "stateless, pflash" descriptor, and all three suffered the problem 
you're fixing with this series.

SNP works after providing a descriptor which properly identifies it as requiring 
a 'memory' device. AFAICT, the existing autoselection code already handles 
non-pflash devices.

> 
> Then again, the existing SEV tests look... Questionable. They all use
> the i440fx machine type and default (BIOS) firmware, whereas
> according to the documentation[1] you really want q35 and UEFI. So at
> best our test coverage is lacking.

Agreed.

> 
> Stressing again the fact that I know very little about SEV and its
> variants, my impression is that generally speaking stateless firmware
> is preferred for the use case; however in Fedora the descriptors for
> "regular" edk2 builds with no Secure Boot[2] advertise support for
> the "amd-sev" and "amd-sev-es" firmware features, and since they sort
> before the SEV-specific builds[3] libvirt will pick them up unless
> you specifically ask for the firmware to be stateless.
> 
> Not sure if the best way to get out of this situation is to shuffle
> the descriptors around, drop the SEV-specific features from other
> descriptors, or tweak the libvirt algorithm so that it will prefer
> stateless firmware for SEV unless told otherwise.
> 
> Very much interested in hearing everyone's thoughts on the topic.

AFAIK, only stateless firmware should be used with SEV and SEV-ES guests. 
Probably best to drop those features from the other descriptors.

BTW, I often tend to ignore SEV and SEV-ES. IMO they are not used and can 
hopefully be removed from upstream projects in the future. This opinion was 
recently strengthened when I discovered that new BIOS updates to our Genoa and 
Turin systems will not allow the SEV-ES and SEV-SNP features to work together.

Regards,
Jim
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 2 weeks, 5 days ago
On Thu, Aug 14, 2025 at 03:07:10PM -0600, Jim Fehlig wrote:
> On 8/13/25 09:01, Andrea Bolognani wrote:
> > Can you be more specific about the issue you're experiencing for
> > SEV(-ES) guests?
>
> I'm seeing the same issue we were trying to solve for SNP guests with this series
>
> ERROR    operation failed: Unable to find 'efi' firmware that is compatible
> with the current configuration

Please share the debug output showing what happens during the
firmware selection process. That will tell us why the amdsev.json
descriptor is not considered suitable. I'm really surprised by this
because things seem to work correctly in the context of the test
suite, but clearly there's something going on.

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 2 weeks ago
On 8/20/25 09:12, Andrea Bolognani wrote:
> On Thu, Aug 14, 2025 at 03:07:10PM -0600, Jim Fehlig wrote:
>> On 8/13/25 09:01, Andrea Bolognani wrote:
>>> Can you be more specific about the issue you're experiencing for
>>> SEV(-ES) guests?
>>
>> I'm seeing the same issue we were trying to solve for SNP guests with this series
>>
>> ERROR    operation failed: Unable to find 'efi' firmware that is compatible
>> with the current configuration
> 
> Please share the debug output showing what happens during the
> firmware selection process. That will tell us why the amdsev.json
> descriptor is not considered suitable. I'm really surprised by this
> because things seem to work correctly in the context of the test
> suite, but clearly there's something going on.

Debug output attached. I've also attached the amdsev.json equivalent I'm using 
for testing. And for completeness, here's the virt-install command

virt-install --virt-type kvm --hvm --arch x86_64 --name sev-es-temp --vcpus 
2,maxvcpus=4 --memory 2048,maxmemory=4096 --memtune hard_limit=4563402 --boot 
uefi --disk path=/vm_images/jim/images/sev-temp/disk0.qcow2,size=60,format=qcow2 
--network bridge=br0,model=virtio --location 
http://blabla/install/sles15sp7/x86_64 --autoconsole text --extra-args 
"console=ttyS0,115200n8" --extra-args "textmode=1" --graphics vnc --serial pty 
--launchSecurity sev,policy=0x03 --machine q35 --events on_reboot=destroy

Regards,
Jim
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Andrea Bolognani via Devel 1 week, 5 days ago
On Mon, Aug 25, 2025 at 11:05:02AM -0600, Jim Fehlig wrote:
> On 8/20/25 09:12, Andrea Bolognani wrote:
> > On Thu, Aug 14, 2025 at 03:07:10PM -0600, Jim Fehlig wrote:
> > > On 8/13/25 09:01, Andrea Bolognani wrote:
> > > > Can you be more specific about the issue you're experiencing for
> > > > SEV(-ES) guests?
> > >
> > > I'm seeing the same issue we were trying to solve for SNP guests with this series
> > >
> > > ERROR    operation failed: Unable to find 'efi' firmware that is compatible
> > > with the current configuration
> >
> > Please share the debug output showing what happens during the
> > firmware selection process. That will tell us why the amdsev.json
> > descriptor is not considered suitable. I'm really surprised by this
> > because things seem to work correctly in the context of the test
> > suite, but clearly there's something going on.
>
> Debug output attached. I've also attached the amdsev.json equivalent I'm
> using for testing. And for completeness, here's the virt-install command
>
> virt-install --virt-type kvm --hvm --arch x86_64 --name sev-es-temp --vcpus
> 2,maxvcpus=4 --memory 2048,maxmemory=4096 --memtune hard_limit=4563402
> --boot uefi --disk
> path=/vm_images/jim/images/sev-temp/disk0.qcow2,size=60,format=qcow2
> --network bridge=br0,model=virtio --location
> http://blabla/install/sles15sp7/x86_64 --autoconsole text --extra-args
> "console=ttyS0,115200n8" --extra-args "textmode=1" --graphics vnc --serial
> pty --launchSecurity sev,policy=0x03 --machine q35 --events
> on_reboot=destroy

Here are the relevant bits from the log:

  ...
  qemuInteropFetchConfigs:149 : firmware description path
'/usr/share/qemu/firmware/50-ovmf-x86_64-sev-snp.json' len=464
  qemuInteropFetchConfigs:149 : firmware description path
'/usr/share/qemu/firmware/50-ovmf-x86_64-sev.json' len=570
  qemuInteropFetchConfigs:149 : firmware description path
'/usr/share/qemu/firmware/50-seabios-256k.json' len=664
  ...
  qemuFirmwareMatchDomain:1361 : Domain requires SEV, firmware
'/usr/share/qemu/firmware/50-ovmf-x86_64-sev-snp.json' doesn't support
it
  qemuFirmwareMatchDomain:1311 : Discarding loader without split flash
  qemuFirmwareMatchDomain:1182 : No matching interface in
'/usr/share/qemu/firmware/50-seabios-256k.json'
  ...

So 50-ovmf-x86_64-sev.json is discarded because it advertises a
stateless firmware, while libvirt assumes that you want a stateful
one. Patch 05/10 from the v2 series should address this very problem
by making a stateless firmware eligible for this scenario. Can you
please try applying that series and checking whether that makes the
issue go away?

-- 
Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 2/5] tests: Add firmware-auto-efi-sev-snp
Posted by Jim Fehlig via Devel 1 week, 4 days ago
On 8/27/25 09:33, Andrea Bolognani wrote:
> On Mon, Aug 25, 2025 at 11:05:02AM -0600, Jim Fehlig wrote:
>> On 8/20/25 09:12, Andrea Bolognani wrote:
>>> On Thu, Aug 14, 2025 at 03:07:10PM -0600, Jim Fehlig wrote:
>>>> On 8/13/25 09:01, Andrea Bolognani wrote:
>>>>> Can you be more specific about the issue you're experiencing for
>>>>> SEV(-ES) guests?
>>>>
>>>> I'm seeing the same issue we were trying to solve for SNP guests with this series
>>>>
>>>> ERROR    operation failed: Unable to find 'efi' firmware that is compatible
>>>> with the current configuration
>>>
>>> Please share the debug output showing what happens during the
>>> firmware selection process. That will tell us why the amdsev.json
>>> descriptor is not considered suitable. I'm really surprised by this
>>> because things seem to work correctly in the context of the test
>>> suite, but clearly there's something going on.
>>
>> Debug output attached. I've also attached the amdsev.json equivalent I'm
>> using for testing. And for completeness, here's the virt-install command
>>
>> virt-install --virt-type kvm --hvm --arch x86_64 --name sev-es-temp --vcpus
>> 2,maxvcpus=4 --memory 2048,maxmemory=4096 --memtune hard_limit=4563402
>> --boot uefi --disk
>> path=/vm_images/jim/images/sev-temp/disk0.qcow2,size=60,format=qcow2
>> --network bridge=br0,model=virtio --location
>> http://blabla/install/sles15sp7/x86_64 --autoconsole text --extra-args
>> "console=ttyS0,115200n8" --extra-args "textmode=1" --graphics vnc --serial
>> pty --launchSecurity sev,policy=0x03 --machine q35 --events
>> on_reboot=destroy
> 
> Here are the relevant bits from the log:
> 
>    ...
>    qemuInteropFetchConfigs:149 : firmware description path
> '/usr/share/qemu/firmware/50-ovmf-x86_64-sev-snp.json' len=464
>    qemuInteropFetchConfigs:149 : firmware description path
> '/usr/share/qemu/firmware/50-ovmf-x86_64-sev.json' len=570
>    qemuInteropFetchConfigs:149 : firmware description path
> '/usr/share/qemu/firmware/50-seabios-256k.json' len=664
>    ...
>    qemuFirmwareMatchDomain:1361 : Domain requires SEV, firmware
> '/usr/share/qemu/firmware/50-ovmf-x86_64-sev-snp.json' doesn't support
> it
>    qemuFirmwareMatchDomain:1311 : Discarding loader without split flash
>    qemuFirmwareMatchDomain:1182 : No matching interface in
> '/usr/share/qemu/firmware/50-seabios-256k.json'
>    ...
> 
> So 50-ovmf-x86_64-sev.json is discarded because it advertises a
> stateless firmware, while libvirt assumes that you want a stateful
> one. Patch 05/10 from the v2 series should address this very problem
> by making a stateless firmware eligible for this scenario.

Nod. It's spread across threads and responses therein, but I've mentioned the 
patch works great for me :-).
  > Can you
> please try applying that series and checking whether that makes the
> issue go away?

I applied the full series on recent master and can verify autoselection works 
for SEV, SEV-ES and SEV-SNP guests.

I wasn't able to verify TDX since the hardware is occupied ATM, but it should 
work fine. The TDX descriptor advertises the type as 'memory', and in my testing 
libvirt already correctly handled autoselection for that firmware device type.

Regards,
Jim