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
