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@xxxxxxxxxx> This makes sense superficially, so Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx> 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? > --- > hw/i386/pc.c | 1 - > qapi/qom.json | 12 ++++++------ > target/i386/sev.c | 7 +++++++ > 3 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 0469af00a7..b65843c559 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -82,7 +82,6 @@ > GlobalProperty pc_compat_9_0[] = { > { TYPE_X86_CPU, "x-l1-cache-per-thread", "false" }, > { TYPE_X86_CPU, "guest-phys-bits", "0" }, > - { "sev-guest", "legacy-vm-type", "true" }, > { TYPE_X86_CPU, "legacy-multi-node", "on" }, > }; > const size_t pc_compat_9_0_len = G_N_ELEMENTS(pc_compat_9_0); > diff --git a/qapi/qom.json b/qapi/qom.json > index 8bd299265e..714ebeec8b 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -912,12 +912,12 @@ > # @handle: SEV firmware handle (default: 0) > # > # @legacy-vm-type: Use legacy KVM_SEV_INIT KVM interface for creating the VM. > -# The newer KVM_SEV_INIT2 interface syncs additional vCPU > -# state when initializing the VMSA structures, which will > -# result in a different guest measurement. Set this to > -# maintain compatibility with older QEMU or kernel versions > -# that rely on legacy KVM_SEV_INIT behavior. > -# (default: false) (since 9.1) > +# The newer KVM_SEV_INIT2 interface, from Linux >= 6.10, syncs > +# additional vCPU state when initializing the VMSA structures, > +# which will result in a different guest measurement. Toggle > +# this to control compatibility with older QEMU or kernel > +# versions that rely on legacy KVM_SEV_INIT behavior. > +# (default: true) (since 9.1) > # > # Since: 2.12 > ## > diff --git a/target/i386/sev.c b/target/i386/sev.c > index 004c667ac1..16029282b7 100644 > --- a/target/i386/sev.c > +++ b/target/i386/sev.c > @@ -2086,6 +2086,13 @@ sev_guest_instance_init(Object *obj) > object_property_add_uint32_ptr(obj, "policy", &sev_guest->policy, > OBJ_PROP_FLAG_READWRITE); > object_apply_compat_props(obj); > + > + /* > + * KVM_SEV_INIT2 was only introduced in Linux 6.10. Avoid > + * breaking existing users of SEV, since the overwhealming > + * majority won't have a new enough kernel for a long time > + */ > + sev_guest->legacy_vm_type = true; > } > > /* guest info specific sev/sev-es */ > -- > 2.45.1