On Tue, Aug 26, 2025 at 10:39:30AM -0600, Jim Fehlig wrote: > On 8/26/25 09:30, Andrea Bolognani wrote: > > On Mon, Aug 25, 2025 at 05:12:57PM -0600, Jim Fehlig wrote: > > > On 8/25/25 10:19, Andrea Bolognani via Devel wrote: > > > > One of the new test cases 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. > > > > > > But the descriptor is incorrect. Autoselection using current git master > > > works fine with a proper descriptor for SNP. > > > > It's true, the current descriptor for SEV-SNP is incorrect as it > > causes libvirt to use pflash instead of rom. But the fact that > > libvirt will ignore the current descriptor unless > > > > <loader stateless='yes'/> > > > > is present in the domain configuration, as demonstrated by the test > > case that I'm adding in this patch, is a problem of its own, and > > indeed the one that you reported in the first place ;) > > Yep, no arguing that point. > > > So yes, we need to fix both issues, the one in libvirt and the one in > > the descriptors. Solving the latter first would merely sweep the > > former under the carpet, not make it go away. > > I think the same could be said by fixing libvirt first.
Not really, because after you've fixed libvirt (patch 05/10) the test case lands in an "intermediate" state where autoselection succeeds, but you still get the wrong mode being used (pflash instead of ROM). Only after you fix the descriptors too (patch 08/10) you end up with the desired state. Doing things in this order gives you the full progression of the fix clearly visible in the git log. > > To be clear, I'm tentatively in favor of moving towards a world in > > which stateless firmware is used consistently across the board for > > SEV guests, but we need to ensure that we don't cause disruption for > > existing users in the process. > > Agreed. Changing the actual edk2 descriptors per patch 9 may cause > disruptions for users wanting a persistent variable store in pflash for > their SEV(-ES) guests. Maybe we can cater to that use case by adding a low-priority descriptor that is identical to the regular edk2 one but advertises amd-sev and amd-sev-es features, and by asking users that want a persistent variable store to ask for <loader stateless='no'/> instead? Assuming people looking to stateful SEV are a small minority, this sounds like it might be feasible. And then we could use the ROM loader for everyone else. We wouldn't even need separate descriptors for SEV-SNP and SEV(-ES), just for stateless SEV and stateful SEV. -- Andrea Bolognani / Red Hat / Virtualization
