See [1] for the discussion motivating these changes.

[1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/U33UEJEXZGZEWOLGVAA5UWPLYY4WHEHT/

Andrea Bolognani (5):
  tests: Tweak descriptor for combined firmware
  tests: Add firmware-auto-efi-sev-snp
  qemu: Fix matching for stateless/combined firmware
  qemu: Fix matching for read/write firmware
  news: Update for firmware selection fixes

 NEWS.rst                                      |  6 +++
 src/qemu/qemu_firmware.c                      | 39 +++++++++++++------
 .../usr/share/qemu/firmware/90-combined.json  |  4 +-
 tests/qemufirmwaretest.c                      |  2 +-
 ...ware-auto-efi-rw-pflash.x86_64-latest.args | 36 +++++++++++++++++
 ...mware-auto-efi-rw-pflash.x86_64-latest.err |  1 -
 ...mware-auto-efi-rw-pflash.x86_64-latest.xml |  6 ++-
 .../firmware-auto-efi-rw.x86_64-latest.args   | 36 +++++++++++++++++
 .../firmware-auto-efi-rw.x86_64-latest.err    |  1 -
 .../firmware-auto-efi-rw.x86_64-latest.xml    |  6 ++-
 ...auto-efi-sev-snp.x86_64-latest+amdsev.args | 37 ++++++++++++++++++
 ...auto-efi-sev-snp.x86_64-latest+amdsev.xml} |  9 ++++-
 .../firmware-auto-efi-sev-snp.xml             | 20 ++++++++++
 tests/qemuxmlconftest.c                       |  8 +++-
 14 files changed, 190 insertions(+), 21 deletions(-)
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.args
 delete mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw-pflash.x86_64-latest.err
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.args
 delete mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-rw.x86_64-latest.err
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.x86_64-latest+amdsev.args
 copy tests/qemuxmlconfdata/{firmware-auto-efi-rw-pflash.x86_64-latest.xml => firmware-auto-efi-sev-snp.x86_64-latest+amdsev.xml} (76%)
 create mode 100644 tests/qemuxmlconfdata/firmware-auto-efi-sev-snp.xml

-- 
2.50.1

On 7/31/25 09:45, Andrea Bolognani via Devel wrote:
> See [1] for the discussion motivating these changes.
> 
> [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/U33UEJEXZGZEWOLGVAA5UWPLYY4WHEHT/
> 
> Andrea Bolognani (5):
>    tests: Tweak descriptor for combined firmware
>    tests: Add firmware-auto-efi-sev-snp
>    qemu: Fix matching for stateless/combined firmware
>    qemu: Fix matching for read/write firmware
>    news: Update for firmware selection fixes

Apologies for not having time to look at this in more detail today, but I 
quickly tried these patches and now see

operation failed: unable to find any master var store for loader: 
/usr/share/qemu/ovmf-x86_64-sev.bin

The descriptor file for this firmware contains

{
     "description": "UEFI firmware for x86_64, with AMD SEV (Stateless)",
     "interface-types": [
         "uefi"
     ],
     "mapping": {
         "device": "flash",
	"mode": "stateless",
         "executable": {
             "filename": "/usr/share/qemu/ovmf-x86_64-sev.bin",
             "format": "raw"
         }
     },
     "targets": [
         {
             "architecture": "x86_64",
             "machines": [
                 "pc-q35-*"
             ]
         }
     ],
     "features": [
	"amd-sev",
	"amd-sev-es",
	"amd-sev-snp",
         "verbose-dynamic"
     ],
     "tags": [

     ]
}

Regards,
Jim

On Thu, Jul 31, 2025 at 11:33:08AM -0600, Jim Fehlig wrote:
> Apologies for not having time to look at this in more detail today, but I
> quickly tried these patches and now see
>
> operation failed: unable to find any master var store for loader:
> /usr/share/qemu/ovmf-x86_64-sev.bin

This happens at domain start time, right?

Looking at the test case I added, the <nvram> element somehow is
getting filled in even though it obviously shouldn't be.

Additionally, I noticed that the stateless=yes attribute is not added
either, so that's another detail that will to be sorted out.

I'll look into it.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Fri, Aug 01, 2025 at 02:06:44AM -0700, Andrea Bolognani wrote:
> On Thu, Jul 31, 2025 at 11:33:08AM -0600, Jim Fehlig wrote:
> > Apologies for not having time to look at this in more detail today, but I
> > quickly tried these patches and now see
> >
> > operation failed: unable to find any master var store for loader:
> > /usr/share/qemu/ovmf-x86_64-sev.bin
>
> This happens at domain start time, right?
>
> Looking at the test case I added, the <nvram> element somehow is
> getting filled in even though it obviously shouldn't be.
>
> Additionally, I noticed that the stateless=yes attribute is not added
> either, so that's another detail that will to be sorted out.
>
> I'll look into it.

Can you please try squashing in the diff below (and regenerating test
data of course)?


diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 0fb954993a..502988d8ff 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -1444,6 +1444,9 @@ qemuFirmwareEnableFeaturesModern(virDomainDef *def,
         else
             loader->readonly = VIR_TRISTATE_BOOL_YES;

+        if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_STATELESS)
+            loader->stateless = VIR_TRISTATE_BOOL_YES;
+
         VIR_FREE(loader->path);
         loader->path = g_strdup(flash->executable.filename);

-- 
Andrea Bolognani / Red Hat / Virtualization

On 8/1/25 03:14, Andrea Bolognani wrote:
> On Fri, Aug 01, 2025 at 02:06:44AM -0700, Andrea Bolognani wrote:
>> On Thu, Jul 31, 2025 at 11:33:08AM -0600, Jim Fehlig wrote:
>>> Apologies for not having time to look at this in more detail today, but I
>>> quickly tried these patches and now see
>>>
>>> operation failed: unable to find any master var store for loader:
>>> /usr/share/qemu/ovmf-x86_64-sev.bin
>>
>> This happens at domain start time, right?
>>
>> Looking at the test case I added, the <nvram> element somehow is
>> getting filled in even though it obviously shouldn't be.
>>
>> Additionally, I noticed that the stateless=yes attribute is not added
>> either, so that's another detail that will to be sorted out.
>>
>> I'll look into it.
> 
> Can you please try squashing in the diff below (and regenerating test
> data of course)?
> 
> 
> diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
> index 0fb954993a..502988d8ff 100644
> --- a/src/qemu/qemu_firmware.c
> +++ b/src/qemu/qemu_firmware.c
> @@ -1444,6 +1444,9 @@ qemuFirmwareEnableFeaturesModern(virDomainDef *def,
>           else
>               loader->readonly = VIR_TRISTATE_BOOL_YES;
> 
> +        if (flash->mode == QEMU_FIRMWARE_FLASH_MODE_STATELESS)
> +            loader->stateless = VIR_TRISTATE_BOOL_YES;
> +
>           VIR_FREE(loader->path);
>           loader->path = g_strdup(flash->executable.filename);
> 

With this addition, the correct firmware is detected, but it's not properly 
provided to qemu

internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'): 
2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM 
readonly memory support

The pertinent command line pieces being

-blockdev 
'{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'

But for SNP, it needs to be provided as bios, e.g.

-bios /usr/share/qemu/ovmf-x86_64-sev.bin

Are we correctly identifying this firmware in the descriptor file? It's 
advertised as a "flash" device, although I'm not sure if any of the other 
"FirmwareDevice" options [1] are appropriate. Perhaps the "FirmwareOSInterface" 
should be 'bios'?

Regards,
Jim

[1] 
https://gitlab.com/qemu/qemu/-/blob/master/docs/interop/firmware.json?ref_type=heads

On Fri, Aug 01, 2025 at 11:39:45AM -0600, Jim Fehlig via Devel wrote:
> With this addition, the correct firmware is detected, but it's not properly
> provided to qemu
>
> internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'):
> 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM
> readonly memory support
>
> The pertinent command line pieces being
>
> -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'
>
> But for SNP, it needs to be provided as bios, e.g.
>
> -bios /usr/share/qemu/ovmf-x86_64-sev.bin
>
> Are we correctly identifying this firmware in the descriptor file? It's
> advertised as a "flash" device, although I'm not sure if any of the other
> "FirmwareDevice" options [1] are appropriate. Perhaps the
> "FirmwareOSInterface" should be 'bios'?

Adding Michal and Daniel to the conversation so that they can provide
some insights. I have zero experience with SEV and no easy access to
the relevant hardware.

Assuming that

  * the need to use -bios for SEV-SNP is intended;

  * pflash still needs to be used for SEV (-ES?);

  * the edk2 build OVMF.amdsev.fd can be used for all variants;

then I think that we need to have the edk2 package ship two separate
descriptors pointing to the same file, one containing

  {
    "mapping": {
        "device": "flash",
        "mode": "stateless",
        "executable": {
            "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
            "format": "raw"
        }
    },
    "features": [
        "amd-sev",
        "amd-sev-es"
    ]
  }

for SEV(-ES) and one containing

  {
    "mapping": {
        "device": "memory",
        "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
    },
    "features": [
        "amd-sev-snp"
    ]
  }

for SEV-SNP.

With those in place, libvirt will be able to know how it should
instruct QEMU to load the firmware. Hacking those changes into the
test suite seems to confirm this. You can make similar changes to
your local installation and test for yourself too.

-- 
Andrea Bolognani / Red Hat / Virtualization

  Hi,

> Assuming that
> 
>   * the need to use -bios for SEV-SNP is intended;

Yes.  SEV-SNP (and TDX too) are by design incompatible with pflash
emulation.  Both do not allow the host change guest memory layout
after launch, and pflash needs to do that to switch between reading
mode and programming mode.

>   * pflash still needs to be used for SEV (-ES?);

You can use pflash with SEV + SEV-ES.  It makes sense to do that if
you want use a persistent variable store in pflash.  Otherwise it
doesn't make much of a difference whenever you use -bios or read-only
pflash for the firmware.

> then I think that we need to have the edk2 package ship two separate
> descriptors pointing to the same file, one containing
> 
>   {
>     "mapping": {
>         "device": "flash",
>         "mode": "stateless",
>         "executable": {
>             "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
>             "format": "raw"
>         }
>     },
>     "features": [
>         "amd-sev",
>         "amd-sev-es"
>     ]
>   }
> 
> for SEV(-ES) and one containing
> 
>   {
>     "mapping": {
>         "device": "memory",
>         "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
>     },
>     "features": [
>         "amd-sev-snp"
>     ]
>   }
> 
> for SEV-SNP.

That should work.  Using device=memory for all three amd-sev* variants
should work too I think.

take care,
  Gerd

On Tue, Aug 05, 2025 at 12:56:56PM +0200, Gerd Hoffmann wrote:
> > Assuming that
> >
> >   * the need to use -bios for SEV-SNP is intended;
>
> Yes.  SEV-SNP (and TDX too) are by design incompatible with pflash
> emulation.  Both do not allow the host change guest memory layout
> after launch, and pflash needs to do that to switch between reading
> mode and programming mode.

Thanks for providing the additional insight.

> >   * pflash still needs to be used for SEV (-ES?);
>
> You can use pflash with SEV + SEV-ES.  It makes sense to do that if
> you want use a persistent variable store in pflash.  Otherwise it
> doesn't make much of a difference whenever you use -bios or read-only
> pflash for the firmware.

The current descriptor uses mode=stateless so there is not going to
be a persistent variable store.

> > then I think that we need to have the edk2 package ship two separate
> > descriptors pointing to the same file, one containing
> >
> >   {
> >     "mapping": {
> >         "device": "flash",
> >         "mode": "stateless",
> >         "executable": {
> >             "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
> >             "format": "raw"
> >         }
> >     },
> >     "features": [
> >         "amd-sev",
> >         "amd-sev-es"
> >     ]
> >   }
> >
> > for SEV(-ES) and one containing
> >
> >   {
> >     "mapping": {
> >         "device": "memory",
> >         "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
> >     },
> >     "features": [
> >         "amd-sev-snp"
> >     ]
> >   }
> >
> > for SEV-SNP.
>
> That should work.  Using device=memory for all three amd-sev* variants
> should work too I think.

Daniel suggested that elsewhere in the thread and of course it's an
appealing proposition, as it would keep complexity down and unify
handling across CVM use cases.

However I wonder if changing things would break migration for
existing SEV(-ES) guests. I think it would be fine since the current
pflash-based configuration would be transmitted as part of the
migration XML, so they will simply keep using that.

If I'm right about the above, then I agree that we should just switch
the existing SEV descriptor to device=memory.

-- 
Andrea Bolognani / Red Hat / Virtualization

[adding Daniel to CC]

On Tue, Aug 05, 2025 at 05:17:14AM -0700, Andrea Bolognani wrote:
> On Tue, Aug 05, 2025 at 12:56:56PM +0200, Gerd Hoffmann wrote:
> > > Assuming that
> > >
> > >   * the need to use -bios for SEV-SNP is intended;
> >
> > Yes.  SEV-SNP (and TDX too) are by design incompatible with pflash
> > emulation.  Both do not allow the host change guest memory layout
> > after launch, and pflash needs to do that to switch between reading
> > mode and programming mode.
>
> Thanks for providing the additional insight.
>
> > >   * pflash still needs to be used for SEV (-ES?);
> >
> > You can use pflash with SEV + SEV-ES.  It makes sense to do that if
> > you want use a persistent variable store in pflash.  Otherwise it
> > doesn't make much of a difference whenever you use -bios or read-only
> > pflash for the firmware.
>
> The current descriptor uses mode=stateless so there is not going to
> be a persistent variable store.
>
> > > then I think that we need to have the edk2 package ship two separate
> > > descriptors pointing to the same file, one containing
> > >
> > >   {
> > >     "mapping": {
> > >         "device": "flash",
> > >         "mode": "stateless",
> > >         "executable": {
> > >             "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
> > >             "format": "raw"
> > >         }
> > >     },
> > >     "features": [
> > >         "amd-sev",
> > >         "amd-sev-es"
> > >     ]
> > >   }
> > >
> > > for SEV(-ES) and one containing
> > >
> > >   {
> > >     "mapping": {
> > >         "device": "memory",
> > >         "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
> > >     },
> > >     "features": [
> > >         "amd-sev-snp"
> > >     ]
> > >   }
> > >
> > > for SEV-SNP.
> >
> > That should work.  Using device=memory for all three amd-sev* variants
> > should work too I think.
>
> Daniel suggested that elsewhere in the thread and of course it's an
> appealing proposition, as it would keep complexity down and unify
> handling across CVM use cases.
>
> However I wonder if changing things would break migration for
> existing SEV(-ES) guests. I think it would be fine since the current
> pflash-based configuration would be transmitted as part of the
> migration XML, so they will simply keep using that.
>
> If I'm right about the above, then I agree that we should just switch
> the existing SEV descriptor to device=memory.

-- 
Andrea Bolognani / Red Hat / Virtualization

> [adding Daniel to CC]

> > Daniel suggested that elsewhere in the thread and of course it's an
> > appealing proposition, as it would keep complexity down and unify
> > handling across CVM use cases.
> >
> > However I wonder if changing things would break migration for
> > existing SEV(-ES) guests.

Yea.  I've went the conservative way and created a test build keeping
things as-is for SEV(-ES).  Just in case, I don't know for sure if there
are any failure modes if we switch guests from pflash to bios.

https://copr.fedorainfracloud.org/coprs/kraxel/edk2.testbuilds/build/9376581/

take care,
  Gerd

On 8/5/25 07:02, Gerd Hoffmann wrote:
>> [adding Daniel to CC]
> 
>>> Daniel suggested that elsewhere in the thread and of course it's an
>>> appealing proposition, as it would keep complexity down and unify
>>> handling across CVM use cases.
>>>
>>> However I wonder if changing things would break migration for
>>> existing SEV(-ES) guests.
> 
> Yea.  I've went the conservative way and created a test build keeping
> things as-is for SEV(-ES).  Just in case, I don't know for sure if there
> are any failure modes if we switch guests from pflash to bios.
> 
> https://copr.fedorainfracloud.org/coprs/kraxel/edk2.testbuilds/build/9376581/

Thanks Gerd. I tend to agree with the conservative approach, which means the 
improvements in this series are still needed for firmware auto-selection  to 
work with SEV and SEV-ES guests.

Unfortunately, I've hit a regression somewhere in our stack and can no longer 
start existing SEV and SEV-ES guests. SNP is fine. I'll need to resolve that 
before continuing to test this series.

Regards,
Jim

On 8/4/25 05:31, Andrea Bolognani wrote:
> On Fri, Aug 01, 2025 at 11:39:45AM -0600, Jim Fehlig via Devel wrote:
>> With this addition, the correct firmware is detected, but it's not properly
>> provided to qemu
>>
>> internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'):
>> 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM
>> readonly memory support
>>
>> The pertinent command line pieces being
>>
>> -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'
>>
>> But for SNP, it needs to be provided as bios, e.g.
>>
>> -bios /usr/share/qemu/ovmf-x86_64-sev.bin
>>
>> Are we correctly identifying this firmware in the descriptor file? It's
>> advertised as a "flash" device, although I'm not sure if any of the other
>> "FirmwareDevice" options [1] are appropriate. Perhaps the
>> "FirmwareOSInterface" should be 'bios'?
> 
> Adding Michal and Daniel to the conversation so that they can provide
> some insights. I have zero experience with SEV and no easy access to
> the relevant hardware.

I don't follow qemu development close enough to know if pflash is now supported 
with SNP guests. AFAIK, only '-bios' was supported when the initial SNP 
enablement was merged.

> Assuming that
> 
>    * the need to use -bios for SEV-SNP is intended;
> 
>    * pflash still needs to be used for SEV (-ES?);
> 
>    * the edk2 build OVMF.amdsev.fd can be used for all variants;
> 
> then I think that we need to have the edk2 package ship two separate
> descriptors pointing to the same file, one containing
> 
>    {
>      "mapping": {
>          "device": "flash",
>          "mode": "stateless",
>          "executable": {
>              "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd",
>              "format": "raw"
>          }
>      },
>      "features": [
>          "amd-sev",
>          "amd-sev-es"
>      ]
>    }
> 
> for SEV(-ES) and one containing
> 
>    {
>      "mapping": {
>          "device": "memory",
>          "filename": "/usr/share/edk2/ovmf/OVMF.amdsev.fd"
>      },
>      "features": [
>          "amd-sev-snp"
>      ]
>    }
> 
> for SEV-SNP.
> 
> With those in place, libvirt will be able to know how it should
> instruct QEMU to load the firmware. Hacking those changes into the
> test suite seems to confirm this. You can make similar changes to
> your local installation and test for yourself too.

I created a new descriptor file for SNP following your example and was able to 
create/install an SNP-aware guest with

virt-install --boot uefi --launchSecurity sev-snp,policy=0x00030000 ...

Regards,
Jim

On Mon, Aug 04, 2025 at 02:15:01PM -0600, Jim Fehlig wrote:
> On 8/4/25 05:31, Andrea Bolognani wrote:
> > On Fri, Aug 01, 2025 at 11:39:45AM -0600, Jim Fehlig via Devel wrote:
> > > With this addition, the correct firmware is detected, but it's not properly
> > > provided to qemu
> > > 
> > > internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'):
> > > 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM
> > > readonly memory support
> > > 
> > > The pertinent command line pieces being
> > > 
> > > -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'
> > > 
> > > But for SNP, it needs to be provided as bios, e.g.
> > > 
> > > -bios /usr/share/qemu/ovmf-x86_64-sev.bin
> > > 
> > > Are we correctly identifying this firmware in the descriptor file? It's
> > > advertised as a "flash" device, although I'm not sure if any of the other
> > > "FirmwareDevice" options [1] are appropriate. Perhaps the
> > > "FirmwareOSInterface" should be 'bios'?
> > 
> > Adding Michal and Daniel to the conversation so that they can provide
> > some insights. I have zero experience with SEV and no easy access to
> > the relevant hardware.
> 
> I don't follow qemu development close enough to know if pflash is now
> supported with SNP guests. AFAIK, only '-bios' was supported when the
> initial SNP enablement was merged.

TDX/SNP are strictly -bios only and will remain that way.


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 08:08:14AM +0100, Daniel P. Berrangé wrote:
> On Mon, Aug 04, 2025 at 02:15:01PM -0600, Jim Fehlig wrote:
> > On 8/4/25 05:31, Andrea Bolognani wrote:
> > > On Fri, Aug 01, 2025 at 11:39:45AM -0600, Jim Fehlig via Devel wrote:
> > > > With this addition, the correct firmware is detected, but it's not properly
> > > > provided to qemu
> > > >
> > > > internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'):
> > > > 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM
> > > > readonly memory support
> > > >
> > > > The pertinent command line pieces being
> > > >
> > > > -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'
> > > >
> > > > But for SNP, it needs to be provided as bios, e.g.
> > > >
> > > > -bios /usr/share/qemu/ovmf-x86_64-sev.bin
> > > >
> > > > Are we correctly identifying this firmware in the descriptor file? It's
> > > > advertised as a "flash" device, although I'm not sure if any of the other
> > > > "FirmwareDevice" options [1] are appropriate. Perhaps the
> > > > "FirmwareOSInterface" should be 'bios'?
> > >
> > > Adding Michal and Daniel to the conversation so that they can provide
> > > some insights. I have zero experience with SEV and no easy access to
> > > the relevant hardware.
> >
> > I don't follow qemu development close enough to know if pflash is now
> > supported with SNP guests. AFAIK, only '-bios' was supported when the
> > initial SNP enablement was merged.
>
> TDX/SNP are strictly -bios only and will remain that way.

Got it.

The TDX descriptor is using device=memory already so it should work
correctly today.

Do you have any objections to the idea of separate descriptors for
SEV(-ES) (device=flash) and SEV-SNP (device=memory) pointing to the
same file? If not, I'll get the edk2 maintainer involved and make it
happen.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Tue, Aug 05, 2025 at 01:18:12AM -0700, Andrea Bolognani wrote:
> On Tue, Aug 05, 2025 at 08:08:14AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Aug 04, 2025 at 02:15:01PM -0600, Jim Fehlig wrote:
> > > On 8/4/25 05:31, Andrea Bolognani wrote:
> > > > On Fri, Aug 01, 2025 at 11:39:45AM -0600, Jim Fehlig via Devel wrote:
> > > > > With this addition, the correct firmware is detected, but it's not properly
> > > > > provided to qemu
> > > > >
> > > > > internal error: QEMU unexpectedly closed the monitor (vm='sles15sp7-snp'):
> > > > > 2025-08-01T17:11:20.589614Z qemu-system-x86_64: pflash with kvm requires KVM
> > > > > readonly memory support
> > > > >
> > > > > The pertinent command line pieces being
> > > > >
> > > > > -blockdev '{"driver":"file","filename":"/usr/share/qemu/ovmf-x86_64-sev.bin","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"}'
> > > > >
> > > > > But for SNP, it needs to be provided as bios, e.g.
> > > > >
> > > > > -bios /usr/share/qemu/ovmf-x86_64-sev.bin
> > > > >
> > > > > Are we correctly identifying this firmware in the descriptor file? It's
> > > > > advertised as a "flash" device, although I'm not sure if any of the other
> > > > > "FirmwareDevice" options [1] are appropriate. Perhaps the
> > > > > "FirmwareOSInterface" should be 'bios'?
> > > >
> > > > Adding Michal and Daniel to the conversation so that they can provide
> > > > some insights. I have zero experience with SEV and no easy access to
> > > > the relevant hardware.
> > >
> > > I don't follow qemu development close enough to know if pflash is now
> > > supported with SNP guests. AFAIK, only '-bios' was supported when the
> > > initial SNP enablement was merged.
> >
> > TDX/SNP are strictly -bios only and will remain that way.
> 
> Got it.
> 
> The TDX descriptor is using device=memory already so it should work
> correctly today.
> 
> Do you have any objections to the idea of separate descriptors for
> SEV(-ES) (device=flash) and SEV-SNP (device=memory) pointing to the
> same file? If not, I'll get the edk2 maintainer involved and make it
> happen.

Possibly we could just switch the existing descriptor, as with newer
QEMU IIUC SEV/ES can use either device 


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 :|