On Thu, Mar 31, 2022 at 12:54 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Thu, Mar 31, 2022, Peter Gonda wrote:
> > 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?
>
> I strongly prefer removing the pr_info() entirely. As Marc pointed out, the
> info is redundant when KVM properly reports the issue. And worse, the info is
> useless unless there's exactly one VM running. Even then, it doesn't capture
> which vCPU failed. This is exactly why Jim, myself, and others, have been pushing
> to avoid using dmesg to report guest errors. They're helpful for initial
> development, but dead weight for production, and if they're helpful for development
> then odds are good that having proper reporting in production would also be valuable.
Sounds good to me. Is a second patch OK with you? I think we get a lot
of cryptic cpu run exit reasons so fixing this up when we remove
pr_infos would be good. This would be a good example without this
pr_info or this change you'd have no idea whats going on.
>
> > > > 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?
>
> Ugh, so "set" means "set of reason codes"? That's heinous naming. I don't have a
> strong objection to splitting, but at the same time, why not punt it to userspace?
> Userspace is obviously going to have to understand what the hell "set" means
> anyways...
I am fine just copying the entire MSR to userspace.
