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

[Brijesh Singh]

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.