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

["Daniel P. Berrangé" <berrange@redhat.com>]

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.

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

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 ?

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