Re: [PATCH Part2 RFC v2 36/37] KVM: SVM: Provide support for SNP_GUEST_REQUEST NAE event

From: Sean Christopherson <seanjc@google.com>
To: Brijesh Singh <brijesh.singh@amd.com>
Cc: Peter Gonda <pgonda@google.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	kvm list <kvm@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	jroedel@suse.de, "Lendacky, Thomas" <thomas.lendacky@amd.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@intel.com>,
	David Rientjes <rientjes@google.com>,
	peterz@infradead.org, "H. Peter Anvin" <hpa@zytor.com>,
	tony.luck@intel.com
Subject: Re: [PATCH Part2 RFC v2 36/37] KVM: SVM: Provide support for SNP_GUEST_REQUEST NAE event
Date: Mon, 10 May 2021 21:17:53 +0000	[thread overview]
Message-ID: <YJmjAaogWeFgCEmb@google.com> (raw)
In-Reply-To: <f357d874-a369-93b6-ffa1-75c643596c81@amd.com>

On Mon, May 10, 2021, Brijesh Singh wrote:
> 
> On 5/10/21 1:57 PM, Peter Gonda wrote:
> >> +e_fail:
> >> +       ghcb_set_sw_exit_info_2(ghcb, rc);
> >> +       return;
> >> +
> >> +e_term:
> >> +       ghcb_set_sw_exit_info_1(ghcb, 1);
> >> +       ghcb_set_sw_exit_info_2(ghcb,
> >> +                               X86_TRAP_GP |
> >> +                               SVM_EVTINJ_TYPE_EXEPT |
> >> +                               SVM_EVTINJ_VALID);
> >> +}
> > I am probably missing something in the spec but I don't see any
> > references to #GP in the '4.1.7 SNP Guest Request' section. Why is
> > this different from e_fail?
> 
> The spec does not say to inject the #GP, I chose this because guest is
> not adhering to the spec and there was a not a good error code in the
> GHCB spec to communicate this condition. Per the spec, both the request
> and response page must be a valid GPA. If we detect that guest is not
> following the spec then its a guest BUG. IIRC, other places in the KVM
> does something similar when guest is trying invalid operation.

The GHCB spec should be updated to define an error code, even if it's a blanket
statement for all VMGEXITs that fail to adhere to the spec.  Arbitrarily choosing
an error code and/or exception number makes the information useless to the guest
because the guest can't take specific action for those failures.  E.g. if there
is a return code specifically for GHCB spec violation, then the guest can panic,
WARN, etc... knowing that it done messed up.

"Injecting" an exception is particularly bad, because if the guest kernel takes
that request literally and emulates a #GP, then we can end up in a situation
where someone files a bug report because VMGEXIT is hitting a #GP and confusion
ensues.