On Thu, Mar 31, 2022 at 11:48 AM Marc Orr <marcorr@xxxxxxxxxx> wrote: > > On Thu, Mar 31, 2022 at 10:40 AM Marc Orr <marcorr@xxxxxxxxxx> wrote: > > > > 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. > > Though... this patch makes this pr_info redundant! Since we'll now > report this in userspace. Actually, I'd be OK to remove it. I'll make this 2 patches. This current patch and another to rate limit this pr_info() I think this patch is doing a lot already so would prefer to just add a second. Is that reasonable? > > > 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 > > Reading this again, I now see that "reason_set" sounds like "The > reason code is set". I bet that's how Sean read it during his review. > So yeah, this needs comments :-)! I'll add comments but I agree with Marc. These are part of the GHCB spec so for the very specific SEV-ES termination reason we should include all the data the spec allows. Sounds OK?