Re: [PATCH] i386: revert defaults to 'legacy-vm-type=true' for SEV(-ES) guests

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 24, 2024 at 01:38:56PM +0100, Daniel P. Berrangé wrote:
> On Mon, Jun 24, 2024 at 08:27:01AM -0400, Michael S. Tsirkin wrote:
> > On Fri, Jun 14, 2024 at 11:39:24AM +0100, Daniel P. Berrangé wrote:
> > > The KVM_SEV_INIT2 ioctl was only introduced in Linux 6.10, which will
> > > only have been released for a bit over a month when QEMU 9.1 is
> > > released.
> > > 
> > > The SEV(-ES) support in QEMU has been present since 2.12 dating back
> > > to 2018. With this in mind, the overwhealming majority of users of
> > > SEV(-ES) are unlikely to be running Linux >= 6.10, any time in the
> > > forseeable future.
> > > 
> > > IOW, defaulting new QEMU to 'legacy-vm-type=false' means latest QEMU
> > > machine types will be broken out of the box for most SEV(-ES) users.
> > > Even if the kernel is new enough, it also affects the guest measurement,
> > > which means that their existing tools for validating measurements will
> > > also be broken by the new default.
> > > 
> > > This is not a sensible default choice at this point in time. Revert to
> > > the historical behaviour which is compatible with what most users are
> > > currently running.
> > > 
> > > This can be re-evaluated a few years down the line, though it is more
> > > likely that all attention will be on SEV-SNP by this time. Distro
> > > vendors may still choose to change this default downstream to align
> > > with their new major releases where they can guarantee the kernel
> > > will always provide the required functionality.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > This makes sense superficially, so
> > 
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > and I'll let kvm maintainers merge this.
> > 
> > However I wonder, wouldn't it be better to refactor this:
> > 
> >     if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) == KVM_X86_DEFAULT_VM) {
> >         cmd = sev_es_enabled() ? KVM_SEV_ES_INIT : KVM_SEV_INIT;
> >         
> >         ret = sev_ioctl(sev_common->sev_fd, cmd, NULL, &fw_error);
> >     } else {
> >         struct kvm_sev_init args = { 0 };
> >                 
> >         ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> >     }   
> > 
> > to something like:
> > 
> > if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) != KVM_X86_DEFAULT_VM) {
> >         struct kvm_sev_init args = { 0 };
> >                 
> >         ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> > 	if (ret && errno == ENOTTY) {
> > 		cmd = sev_es_enabled() ? KVM_SEV_ES_INIT : KVM_SEV_INIT;
> > 
> > 		ret = sev_ioctl(sev_common->sev_fd, cmd, NULL, &fw_error);
> > 	}
> > }
> > 
> > 
> > Yes I realize this means measurement will then depend on the host
> > but it seems nicer than failing guest start, no?
> 
> IMHO having an invariant measurement for a given guest configuration
> is a critical guarantee. We should not be allowing guest attestation
> to break as a side-effect of upgrading a software component, while
> keeping the guest config unchanged.

Well attenstation can change for a variety of reasons involving software
upgrades: host or guest. It is up to user to either trust both old and
new attestion, or pick one. Seems better than forcing policy host side.

> IOW, I'd view measurement as being "guest ABI", and versioned machine
> types are there to provide invariant guest ABI.

In practice we can't always do this exactly: e.g. vhost has
a rich feature mask and what we do is clear features not
supported by a specific host kernel.

Similarly for vhost-user where the ABI depends on an
external component.

So things can change if you move across host kernels.




> Personally, if we want simplicitly then just not using KVM_SEV_INIT2
> at all would be the easiest option. SEV/SEV-ES are legacy technology
> at this point, so we could be justified in leaving it unchanged and
> only focusing on SEV-SNP. Unless someone can say what the critical
> *must have* benefit of using KVM_SEV_INIT2 is ?


No objection.

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