| Message ID | 20241206221257.7167-2-huibo.wang@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: SVM: Make VMGEXIT GHCB exit codes more readable | expand |
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@amd.com> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com> > Reviewed-by: Pavan Kumar Paluri <papaluri@amd.com> > --- > 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 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.
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 +#define GHCB_HV_RESP_ISSUE_EXCEPTION 1 +#define GHCB_HV_RESP_MALFORMED_INPUT 2 + /* * Error codes related to GHCB input that can be communicated back to the guest * by setting the lower 32-bits of the GHCB SW_EXITINFO1 field to 2. diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 72674b8825c4..e7db7a5703b7 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -3433,7 +3433,7 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm) dump_ghcb(svm); } - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 2); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_MALFORMED_INPUT); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, reason); /* Resume the guest to "return" the error code. */ @@ -3577,7 +3577,7 @@ static int setup_vmgexit_scratch(struct vcpu_svm *svm, bool sync, u64 len) return 0; e_scratch: - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 2); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_MALFORMED_INPUT); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_SCRATCH_AREA); return 1; @@ -4124,7 +4124,7 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r return snp_handle_guest_req(svm, req_gpa, resp_gpa); request_invalid: - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 2); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_MALFORMED_INPUT); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT); return 1; /* resume guest */ } @@ -4317,7 +4317,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu) if (ret) return ret; - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 0); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_SUCCESS); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 0); exit_code = kvm_ghcb_get_sw_exit_code(control); @@ -4367,7 +4367,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu) default: pr_err("svm: vmgexit: unsupported AP jump table request - exit_info_1=%#llx\n", control->exit_info_1); - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 2); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_MALFORMED_INPUT); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT); } @@ -4397,7 +4397,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu) case SVM_VMGEXIT_AP_CREATION: ret = sev_snp_ap_creation(svm); if (ret) { - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 2); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_MALFORMED_INPUT); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT); } diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index dd15cc635655..58bce5f1ab0c 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -2977,7 +2977,7 @@ static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err) if (!err || !sev_es_guest(vcpu->kvm) || WARN_ON_ONCE(!svm->sev_es.ghcb)) return kvm_complete_insn_gp(vcpu, err); - ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, 1); + ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_ISSUE_EXCEPTION); ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, X86_TRAP_GP | SVM_EVTINJ_TYPE_EXEPT |