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

Michael Roth
2024-06-25T01:19:56Z

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.

Part of the reason for the change is that SEV-ES measurements are
already affected by some short-comings of the legacy KVM_SEV_ES_INIT
API. Namely, if the kvm_amd.debug-swap module param is used to enable
that SEV-ES feature, then that feature will get enabled on the KVM side
and change the initial guest measurement (due to VMSA_FEATURES field
of the vCPU's VMSA changing), and userspace has no way to control that
on a per-VM basis, so measurement for any particular invocation will
be somewhat random depending on the system configuration and kernel
level.

I think that's why users of newer QEMU machine types are highly
encouraged to switch to the new KVM_SEV_INIT2 interface. I do see this
causing issues for older QEMU machine types that previously relied on
the legacy interface, since we do want to avoid measurement changing
for an existing guest that was previously working on an older kernel,
which is why this flag defaults to true for pre-9.1 machine types. But
on newer kernels there is still potential for issues relating to
debug-swap (and other VMSA features that get added to KVM in the future)
and how they may cause measurement changes underneath the covers if we
don't allow userspace the ability to control what is/isn't disabled.

Because of that I think it's less headache for userspace to have to
opt-in to legacy interface when using newer machine models. It should be
a concious decision to keep using this deprecated interface with known
limitations that could affect measurement in unexpected ways.

I was actually planning to go the other direction on this because
currently for 9.1+, QEMU will try to use KVM_SEV_INIT2 if
KVM_CAP_VM_TYPES advertises its availability, but otherwise fall back to
the above KVM_SEV_ES_INIT interface and potential inherit the issues
noted above. So I was planning on getting rid of the fallback, and
basically only allowing legacy KVM_SEV_ES_INIT for 9.1+ if the user
manually sets sev_guest->legacy_vm_type via cmdline.

-Mike

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