On Thu, Mar 31, 2022 at 10:11 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> +Paolo and Vitaly
>
> In the future, I highly recommend using scripts/get_maintainers.pl.
>
> On Wed, Mar 30, 2022, Peter Gonda wrote:
> > SEV-ES guests can request termination using the GHCB's MSR protocol. See
> > AMD's GHCB spec section '4.1.13 Termination Request'. Currently when a
> > guest does this the userspace VMM sees an KVM_EXIT_UNKNOWN (-EVINAL)
> > return code from KVM_RUN. By adding a KVM_EXIT_SHUTDOWN_ENTRY to kvm_run
> > struct the userspace VMM can clearly see the guest has requested a SEV-ES
> > termination including the termination reason code set and reason code.
> >
> > Signed-off-by: Peter Gonda <pgonda@xxxxxxxxxx>
> >
> > ---
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 75fa6dd268f0..5f9d37dd3f6f 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2735,8 +2735,13 @@ static int sev_handle_vmgexit_msr_protocol(struct vcpu_svm *svm)
> > pr_info("SEV-ES guest requested termination: %#llx:%#llx\n",
> > reason_set, reason_code);
>
> This pr_info() should be removed. A malicious usersepace could spam the kernel
> by constantly running a vCPU that requests termination.
Ah, good catch. But actually, I've found this specific pr_info _very_
useful in debugging. Sean, would you be OK to convert it to a rate
limited print?
> > - ret = -EINVAL;
> > - break;
> > + vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
> > + vcpu->run->shutdown.reason = KVM_SHUTDOWN_SEV_TERM;
> > + vcpu->run->shutdown.ndata = 2;
> > + vcpu->run->shutdown.data[0] = reason_set;
> > + vcpu->run->shutdown.data[1] = reason_code;
>
> Does KVM really need to split the reason_set and reason_code? Without reading
> the spec, it's not even clear what "set" means. Assuming it's something like
> "the reason code is valid", then I don't see any reason (lol) to split the two.
> If we do split them, then arguably the reason_code should be filled if and only
> if reason_set is true, and that's just extra work.
I think KVM should split the reason_set and reason_code. This code is
based on the GHCB spec after all. And reason_set and reason_code are
both a part of the GHCB spec. But I agree, folks shouldn't have to go
to the spec to understand what reason_set and reason_code are. Rather
than not splitting them up, can we add comments in the source to
explain what they mean?
Also, my understanding from reading the spec is that reason_code
should always be filled, even when reason_set is 0. See below. But
basically, you can have reason_set: 0 and reason_code: non-zero.
Quoting the spec:
The reason code set is meant to provide hypervisors with their own
termination SEV-ES Guest-Hypervisor Communication Block
Standardization reason codes. This document defines and owns reason
code set 0x0 and the following reason codes (GHCBData[23:16]):
0x00 – General termination request
0x01 – SEV-ES / GHCB Protocol range is not supported.
0x02 – SEV-SNP features not supported
