On Thu, Jul 4, 2024 at 2:01 AM Michael Roth <michael.roth@xxxxxxx> wrote:
> Currently if the 'legacy-vm-type' property of the sev-guest object is
> left unset, QEMU will attempt to use the newer KVM_SEV_INIT2 kernel
> interface in conjunction with the newer KVM_X86_SEV_VM and
> KVM_X86_SEV_ES_VM KVM VM types.
>
> This can lead to measurement changes if, for instance, an SEV guest was
> created on a host that originally had an older kernel that didn't
> support KVM_SEV_INIT2, but is booted on the same host later on after the
> host kernel was upgraded.
I think this is the right thing to do for SEV-ES. I agree that it's
bad to require a very new kernel (6.10 will be released only a month
before QEMU 9.1), on the other hand the KVM_SEV_ES_INIT API is broken
in several ways. As long as there is a way to go back to it, and it's
not changed by old machine types, not using it for SEV-ES is the
better choice for upstream.
On the other hand, I think it makes no difference for SEV? Should we
always use KVM_SEV_INIT, or alternatively fall back as it was before
this patch?
Paolo
> Cc: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> cc: kvm@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
> ---
> qapi/qom.json | 11 ++++++-----
> target/i386/sev.c | 30 ++++++++++++++++++++++++------
> 2 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 8bd299265e..a212c009aa 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -912,11 +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.
> +# 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. Set
> +# this to force compatibility with older QEMU or kernel
> +# versions that rely on legacy KVM_SEV_INIT behavior.
> +# Otherwise, QEMU will require KVM_SEV_INIT2 for SEV guests.
> # (default: false) (since 9.1)
> #
> # Since: 2.12
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 3ab8b3c28b..8f56c0cf0c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1347,14 +1347,22 @@ static int sev_kvm_type(X86ConfidentialGuest *cg)
> goto out;
> }
>
> + if (sev_guest->legacy_vm_type) {
> + sev_common->kvm_type = KVM_X86_DEFAULT_VM;
> + goto out;
> + }
> +
> kvm_type = (sev_guest->policy & SEV_POLICY_ES) ?
> KVM_X86_SEV_ES_VM : KVM_X86_SEV_VM;
> - if (kvm_is_vm_type_supported(kvm_type) && !sev_guest->legacy_vm_type) {
> - sev_common->kvm_type = kvm_type;
> - } else {
> - sev_common->kvm_type = KVM_X86_DEFAULT_VM;
> + if (!kvm_is_vm_type_supported(kvm_type)) {
> + error_report("SEV: host kernel does not support requested %s VM type. To allow use of "
> + "legacy KVM_X86_DEFAULT_VM VM type, the 'legacy-vm-type' argument must be "
> + "set to true for the sev-guest object.",
> + kvm_type == KVM_X86_SEV_VM ? "KVM_X86_SEV_VM" : "KVM_X86_SEV_ES_VM");
> + return -1;
> }
>
> + sev_common->kvm_type = kvm_type;
> out:
> return sev_common->kvm_type;
> }
> @@ -1445,14 +1453,24 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
> }
>
> trace_kvm_sev_init();
> - if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) == KVM_X86_DEFAULT_VM) {
> + switch (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common))) {
> + case 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 {
> + break;
> + case KVM_X86_SEV_VM:
> + case KVM_X86_SEV_ES_VM:
> + case KVM_X86_SNP_VM: {
> struct kvm_sev_init args = { 0 };
>
> ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
> + break;
> + }
> + default:
> + error_setg(errp, "%s: host kernel does not support the requested SEV configuration.",
> + __func__);
> + return -1;
> }
>
> if (ret) {
> --
> 2.25.1
>
