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

From: Brijesh Singh <brijesh.singh@amd.com>
To: Sean Christopherson <seanjc@google.com>
Cc: brijesh.singh@amd.com, 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: Tue, 11 May 2021 13:34:48 -0500	[thread overview]
Message-ID: <07ce14c1-d9b2-2a88-9297-cae4e8dbea1f@amd.com> (raw)
In-Reply-To: <YJmjAaogWeFgCEmb@google.com>

On 5/10/21 4:17 PM, Sean Christopherson wrote:
> 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.

The GHCB is finalized and released so I don't think it will be covered
in v2. But I will go ahead and file the report so that it is considered
in the next updates. Having said that, I do see that for other commands
(e.g, HV door bell page) the spec spell out that if guest provides an
invalid GPA then inject a #GP. I guess we need to move that statement
and apply to all the commands. Until then I am fine with not injecting
#GP to not divert from the spec.

> "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.