Re: [PATCH] i386/sev: Don't allow automatic fallback to legacy KVM_SEV*_INIT

[Michael Roth <michael.roth@amd.com>]

On Thu, Jul 04, 2024 at 11:53:33AM +0200, Paolo Bonzini wrote:
> On Thu, Jul 4, 2024 at 11:39 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > The debug_swap parameter simply could not be enabled in the old API
> > > without breaking measurements. The new API *is the fix* to allow using
> > > it (though QEMU doesn't have the option plumbed in yet). There is no
> > > extensibility.
> > >
> > > Enabling debug_swap by default is also a thorny problem; it cannot be
> > > enabled by default because not all CPUs support it, and also we'd have
> > > the same problem that we cannot enable debug_swap on new machine types
> > > without requiring a new kernel. Tying the default to the -cpu model
> > > would work but it is confusing.
> >
> > Presumably we can tie it to '-cpu host' without much problem, and
> > then just leave it as an opt-in feature flag for named CPU models.
> 
> '-cpu host' for SEV-SNP is also problematic, since CPUID is part of
> the measurement. It's okay for starting guests in a quick and dirty
> manner, but it cannot be used if measurement is in use.
> 
> It's weird to have "-cpu" provide the default for "-object", since
> -object is created much earlier than CPUs. But then "-cpu" itself is
> weird because it's a kind of "factory" for future objects. Maybe we
> should redo the same exercise I did to streamline machine
> initialization, this time focusing on -cpu/-machine/-accel, to
> understand the various phases and where sev-{,snp-}guest
> initialization fits.
> 
> > > I think it's reasonable if the fix is displayed right into the error
> > > message. It's only needed for SEV-ES though, SEV can use the old and
> > > new APIs interchangeably.
> >
> > FYI currently it is proposed to unconditionally force set legacy-vm-type=true
> > in libvirt, so QEMU guests would *never* use the new ioctl, to fix what we
> > consider to be a QEMU / kernel guest ABI regression.
> 
> Ok, so it's probably best to apply both this and your patch for now.
> Later debug-swap can be enabled and will automatically disable
> legacy-vm-type if the user left it to the default.

I think this seems like the ideal default behavior, where QEMU will
continue to stick with legacy interface unless the user specifically
enables a new option like debug-swap that relies on KVM_SEV_INIT2.
So I reworked this patch to make legacy-vm-type an OnOffAuto field,
where 'auto' implements the above semantics, and 'on'/'off' continue
to behave as they do here in v1.

At the moment, since QEMU doesn't actually expose anything that requires
KVM_SEV_INIT2 for SEV-ES, setting legacy-vm-type to 'auto' or 'on' end
up being equivalent, but by defaulting to 'auto' things will continue to
"just work" on the libvirt side even when new features are enabled. And
by adding a bit of that infrastructure now it's less likely that some
option gets exposed in a way that doesn't abide by the above semantics.

So as part of v2 I switched the default for 9.1+ to 'auto'. But if
v1+Daniel's patch is still preferred that should be fine too.

-Mike

> 
> If you want to test this combo and send a pull request (either to me
> or to Richard), that would help because I'm mostly away for a few
> days.
> 
> Paolo
>