Hi Sean,
On 12/16/2024 6:41 PM, Sean Christopherson wrote:
> On Fri, Dec 06, 2024, Melody Wang wrote:
>> Convert VMGEXIT SW_EXITINFO1 codes from plain numbers to proper defines.
>>
>> No functionality changed.
>>
>> Signed-off-by: Melody Wang <huibo.wang@xxxxxxx>
>> Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>> Reviewed-by: Pavan Kumar Paluri <papaluri@xxxxxxx>
>> ---
>> arch/x86/include/asm/sev-common.h | 8 ++++++++
>> arch/x86/kvm/svm/sev.c | 12 ++++++------
>> arch/x86/kvm/svm/svm.c | 2 +-
>> 3 files changed, 15 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
>> index 98726c2b04f8..01d4744e880a 100644
>> --- a/arch/x86/include/asm/sev-common.h
>> +++ b/arch/x86/include/asm/sev-common.h
>> @@ -209,6 +209,14 @@ struct snp_psc_desc {
>>
>> #define GHCB_RESP_CODE(v) ((v) & GHCB_MSR_INFO_MASK)
>>
>> +/*
>> + * Error codes of the GHCB SW_EXITINFO1 related to GHCB input that can be
>> + * communicated back to the guest
>> + */
>> +#define GHCB_HV_RESP_SUCCESS 0
>
> Somewhat of a nit, but I don't think "SUCCESS" is appropriate due to the bizarre
> return codes for Page State Change (PSC) requests. For unknown reasons (really,
> why!?!?), PSC requests apparently always get back '0', but then put a bunch of
> errors into SW_EXITINFO2, including cases that are clearly not "success".
>
> FWIW, "no action" isn't much better, because it too directly conflicts with
> the documentation for PSC:
>
> The page state change request was interrupted, retry the request.
> ^^^^^^^^^^^^^^^^^
> I'm all for having svm_vmgexit_success(), but I think the macro needs to be
> NO_ACTION (even though it too is flawed), because I strongly suspect that patch 2
> deliberately avoided SUCCESS in snp_handle_guest_req() and snp_complete_psc()
> specifically because you knew SUCCESS would be misleading.
>
>> +#define GHCB_HV_RESP_ISSUE_EXCEPTION 1
>> +#define GHCB_HV_RESP_MALFORMED_INPUT 2
>
> Where is '2' actually documented? I looked all over the GHCB and the only ones
> I can find are '0' and '1'.
>
> 0x0000
> o No action requested by the hypervisor.
> 0x0001
> o The hypervisor has requested an exception be issued
>
GHCB spec (Pub# 56421, Rev:2.03), section 4.1 Invoking VMGEXIT documents
0x0002 as well.
0x0002
o The hypervisor encountered malformed input for the VMGEXIT.
Thanks,
Pavan
> And again, somewhat of a nit, but PSC ruins all the fun once more, because it
> quite clearly has multiple "malformed input" responses. So if PSC can get rejected
> with "bad input", why on earth would it not use GHCB_HV_RESP_MALFORMED_INPUT?
>
> o SW_EXITINFO2[31:0] == 0x00000001
> The page_state_change_header structure is not valid
>
> o SW_EXITINFO2[31:0] == 0x00000002
> The page_state_change_entry structure, identified by
> page_state_change_header.cur_entry, is not valid.
