[PATCH v4 0/2] KVM: SVM: Make VMGEXIT GHCB exit codes more readable

[PATCH v4 0/2] KVM: SVM: Make VMGEXIT GHCB exit codes more readable

[Melody Wang]

Hi all,

This patchset addresses all of review comments from Sean. All feedback is appreciated.

Thanks,
Melody

Changelog:
v3:

Here are two patches to make VMGEXIT GHCB exit codes more readable. All
feedback is appreciated.

Melody Wang (2):
  KVM: SVM: Convert plain error code numbers to defines
  KVM: SVM: Provide helpers to set the error code

 arch/x86/include/asm/sev-common.h |  8 +++++++
 arch/x86/kvm/svm/sev.c            | 36 ++++++++++++++++---------------
 arch/x86/kvm/svm/svm.c            |  6 +-----
 arch/x86/kvm/svm/svm.h            | 29 +++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v4 1/2] KVM: SVM: Convert plain error code numbers to defines

[Melody Wang]

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 50f5666938c0..ac166af9a422 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_NO_ACTION		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 943bd074a5d3..59a0d8292f87 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3420,7 +3420,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. */
@@ -3564,7 +3564,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;
@@ -4111,7 +4111,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 */
 }
@@ -4304,7 +4304,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_NO_ACTION);
 	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 0);
 
 	exit_code = kvm_ghcb_get_sw_exit_code(control);
@@ -4354,7 +4354,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);
 		}
 
@@ -4384,7 +4384,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 21dacd312779..0de2bf132056 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 |
-- 
2.34.1

[PATCH v4 2/2] KVM: SVM: Provide helpers to set the error code

[Melody Wang]

Provide helpers to set the error code when converting VMGEXIT SW_EXITINFO1 and
SW_EXITINFO2 codes from plain numbers to proper defines. Add comments for
better code readability.

No functionality changed.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Melody Wang <huibo.wang@amd.com>
---
 arch/x86/kvm/svm/sev.c | 36 +++++++++++++++++++-----------------
 arch/x86/kvm/svm/svm.c |  6 +-----
 arch/x86/kvm/svm/svm.h | 29 +++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 59a0d8292f87..6e2a0f0b4753 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3420,8 +3420,7 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
 		dump_ghcb(svm);
 	}
 
-	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);
+	svm_vmgexit_bad_input(svm, reason);
 
 	/* Resume the guest to "return" the error code. */
 	return 1;
@@ -3564,8 +3563,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, GHCB_HV_RESP_MALFORMED_INPUT);
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_SCRATCH_AREA);
+	svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_SCRATCH_AREA);
 
 	return 1;
 }
@@ -3658,7 +3656,14 @@ static void snp_complete_psc(struct vcpu_svm *svm, u64 psc_ret)
 	svm->sev_es.psc_inflight = 0;
 	svm->sev_es.psc_idx = 0;
 	svm->sev_es.psc_2m = false;
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, psc_ret);
+
+	/*
+	 * A value of zero in SW_EXITINFO1 does not guarantee that
+	 * all operations have completed or completed successfully.
+	 * PSC requests always get a "no action" response, with a
+	 * PSC-specific return code in SW_EXITINFO2.
+	 */
+	svm_vmgexit_no_action(svm, psc_ret);
 }
 
 static void __snp_complete_one_psc(struct vcpu_svm *svm)
@@ -4055,7 +4060,7 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 		goto out_unlock;
 	}
 
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(0, fw_err));
+	svm_vmgexit_no_action(svm, SNP_GUEST_ERR(0, fw_err));
 
 	ret = 1; /* resume guest */
 
@@ -4111,8 +4116,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, GHCB_HV_RESP_MALFORMED_INPUT);
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
+	svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
 	return 1; /* resume guest */
 }
 
@@ -4304,8 +4308,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
 	if (ret)
 		return ret;
 
-	ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_NO_ACTION);
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 0);
+	svm_vmgexit_success(svm, 0);
 
 	exit_code = kvm_ghcb_get_sw_exit_code(control);
 	switch (exit_code) {
@@ -4349,20 +4352,20 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
 			break;
 		case 1:
 			/* Get AP jump table address */
-			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, sev->ap_jump_table);
+			svm_vmgexit_success(svm, sev->ap_jump_table);
 			break;
 		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, GHCB_HV_RESP_MALFORMED_INPUT);
-			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
+			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
 		}
 
 		ret = 1;
 		break;
 	}
 	case SVM_VMGEXIT_HV_FEATURES:
-		ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_HV_FT_SUPPORTED);
+		/* Get hypervisor supported features */
+		svm_vmgexit_success(svm, GHCB_HV_FT_SUPPORTED);
 
 		ret = 1;
 		break;
@@ -4384,8 +4387,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, GHCB_HV_RESP_MALFORMED_INPUT);
-			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
+			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
 		}
 
 		ret = 1;
@@ -4622,7 +4624,7 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
 		 * Return from an AP Reset Hold VMGEXIT, where the guest will
 		 * set the CS and RIP. Set SW_EXIT_INFO_2 to a non-zero value.
 		 */
-		ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 1);
+		svm_vmgexit_success(svm, 1);
 		break;
 	case AP_RESET_HOLD_MSR_PROTO:
 		/*
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 0de2bf132056..f8c5c78b917e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2977,11 +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, GHCB_HV_RESP_ISSUE_EXCEPTION);
-	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb,
-				X86_TRAP_GP |
-				SVM_EVTINJ_TYPE_EXEPT |
-				SVM_EVTINJ_VALID);
+	svm_vmgexit_inject_exception(svm, X86_TRAP_GP);
 	return 1;
 }
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 43fa6a16eb19..78c8b5fb2bdc 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -588,6 +588,35 @@ static inline bool is_vnmi_enabled(struct vcpu_svm *svm)
 		return false;
 }
 
+static inline void svm_vmgexit_set_return_code(struct vcpu_svm *svm,
+						u64 response, u64 data)
+{
+	ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, response);
+	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, data);
+}
+
+static inline void svm_vmgexit_inject_exception(struct vcpu_svm *svm, u8 vector)
+{
+	u64 data = SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_EXEPT | vector;
+
+	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_ISSUE_EXCEPTION, data);
+}
+
+static inline void svm_vmgexit_bad_input(struct vcpu_svm *svm, u64 suberror)
+{
+	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_MALFORMED_INPUT, suberror);
+}
+
+static inline void svm_vmgexit_success(struct vcpu_svm *svm, u64 data)
+{
+	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_NO_ACTION, data);
+}
+
+static inline void svm_vmgexit_no_action(struct vcpu_svm *svm, u64 data)
+{
+	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_NO_ACTION, data);
+}
+
 /* svm.c */
 #define MSR_INVALID				0xffffffffU
 
-- 
2.34.1

Re: [PATCH v4 2/2] KVM: SVM: Provide helpers to set the error code

[Paluri, PavanKumar]

On 12/30/2024 2:36 PM, Melody Wang wrote:
> Provide helpers to set the error code when converting VMGEXIT SW_EXITINFO1 and
> SW_EXITINFO2 codes from plain numbers to proper defines. Add comments for
> better code readability.
> 
> No functionality changed.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Melody Wang <huibo.wang@amd.com>
> ---
>  arch/x86/kvm/svm/sev.c | 36 +++++++++++++++++++-----------------
>  arch/x86/kvm/svm/svm.c |  6 +-----
>  arch/x86/kvm/svm/svm.h | 29 +++++++++++++++++++++++++++++
>  3 files changed, 49 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 59a0d8292f87..6e2a0f0b4753 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3420,8 +3420,7 @@ static int sev_es_validate_vmgexit(struct vcpu_svm *svm)
>  		dump_ghcb(svm);
>  	}
>  
> -	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);
> +	svm_vmgexit_bad_input(svm, reason);
>  
>  	/* Resume the guest to "return" the error code. */
>  	return 1;
> @@ -3564,8 +3563,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, GHCB_HV_RESP_MALFORMED_INPUT);
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_SCRATCH_AREA);
> +	svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_SCRATCH_AREA);
>  
>  	return 1;
>  }
> @@ -3658,7 +3656,14 @@ static void snp_complete_psc(struct vcpu_svm *svm, u64 psc_ret)
>  	svm->sev_es.psc_inflight = 0;
>  	svm->sev_es.psc_idx = 0;
>  	svm->sev_es.psc_2m = false;
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, psc_ret);
> +
> +	/*
> +	 * A value of zero in SW_EXITINFO1 does not guarantee that
> +	 * all operations have completed or completed successfully.
> +	 * PSC requests always get a "no action" response, with a
> +	 * PSC-specific return code in SW_EXITINFO2.
> +	 */
I feel you should make this more clearer by mentioning "no action"
corresponds to SW_EXITINFO1.

> +	svm_vmgexit_no_action(svm, psc_ret);
>  }
>  
>  static void __snp_complete_one_psc(struct vcpu_svm *svm)
> @@ -4055,7 +4060,7 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
>  		goto out_unlock;
>  	}
>  
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(0, fw_err));
> +	svm_vmgexit_no_action(svm, SNP_GUEST_ERR(0, fw_err));
>  
You missed out on Sean's suggestion to add the following comment.
/*
 * No action is requested from KVM, even if the request failed due to a
 * firmware error.
 */

I feel this comment is necessary since you are calling
svm_vmgexit_no_action() that sheds light on SW_EXITINFO1.

Thanks,
Pavan

>  	ret = 1; /* resume guest */
>  
> @@ -4111,8 +4116,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, GHCB_HV_RESP_MALFORMED_INPUT);
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
> +	svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
>  	return 1; /* resume guest */
>  }
>  
> @@ -4304,8 +4308,7 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
>  	if (ret)
>  		return ret;
>  
> -	ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, GHCB_HV_RESP_NO_ACTION);
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 0);
> +	svm_vmgexit_success(svm, 0);
>  
>  	exit_code = kvm_ghcb_get_sw_exit_code(control);
>  	switch (exit_code) {
> @@ -4349,20 +4352,20 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
>  			break;
>  		case 1:
>  			/* Get AP jump table address */
> -			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, sev->ap_jump_table);
> +			svm_vmgexit_success(svm, sev->ap_jump_table);
>  			break;
>  		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, GHCB_HV_RESP_MALFORMED_INPUT);
> -			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
> +			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
>  		}
>  
>  		ret = 1;
>  		break;
>  	}
>  	case SVM_VMGEXIT_HV_FEATURES:
> -		ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_HV_FT_SUPPORTED);
> +		/* Get hypervisor supported features */
> +		svm_vmgexit_success(svm, GHCB_HV_FT_SUPPORTED);
>  
>  		ret = 1;
>  		break;
> @@ -4384,8 +4387,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, GHCB_HV_RESP_MALFORMED_INPUT);
> -			ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_ERR_INVALID_INPUT);
> +			svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
>  		}
>  
>  		ret = 1;
> @@ -4622,7 +4624,7 @@ void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
>  		 * Return from an AP Reset Hold VMGEXIT, where the guest will
>  		 * set the CS and RIP. Set SW_EXIT_INFO_2 to a non-zero value.
>  		 */
> -		ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, 1);
> +		svm_vmgexit_success(svm, 1);
>  		break;
>  	case AP_RESET_HOLD_MSR_PROTO:
>  		/*
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 0de2bf132056..f8c5c78b917e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2977,11 +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, GHCB_HV_RESP_ISSUE_EXCEPTION);
> -	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb,
> -				X86_TRAP_GP |
> -				SVM_EVTINJ_TYPE_EXEPT |
> -				SVM_EVTINJ_VALID);
> +	svm_vmgexit_inject_exception(svm, X86_TRAP_GP);
>  	return 1;
>  }
>  
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 43fa6a16eb19..78c8b5fb2bdc 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -588,6 +588,35 @@ static inline bool is_vnmi_enabled(struct vcpu_svm *svm)
>  		return false;
>  }
>  
> +static inline void svm_vmgexit_set_return_code(struct vcpu_svm *svm,
> +						u64 response, u64 data)
> +{
> +	ghcb_set_sw_exit_info_1(svm->sev_es.ghcb, response);
> +	ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, data);
> +}
> +
> +static inline void svm_vmgexit_inject_exception(struct vcpu_svm *svm, u8 vector)
> +{
> +	u64 data = SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_EXEPT | vector;
> +
> +	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_ISSUE_EXCEPTION, data);
> +}
> +
> +static inline void svm_vmgexit_bad_input(struct vcpu_svm *svm, u64 suberror)
> +{
> +	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_MALFORMED_INPUT, suberror);
> +}
> +
> +static inline void svm_vmgexit_success(struct vcpu_svm *svm, u64 data)
> +{
> +	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_NO_ACTION, data);
> +}
> +
> +static inline void svm_vmgexit_no_action(struct vcpu_svm *svm, u64 data)
> +{
> +	svm_vmgexit_set_return_code(svm, GHCB_HV_RESP_NO_ACTION, data);
> +}
> +
>  /* svm.c */
>  #define MSR_INVALID				0xffffffffU
>  

Re: [PATCH v4 0/2] KVM: SVM: Make VMGEXIT GHCB exit codes more readable

[Tom Lendacky]

On 12/30/24 14:36, Melody Wang wrote:

Just some FYIs when it comes to cover letters.

> Hi all,

No need to put this in a cover letter.

> 
> This patchset addresses all of review comments from Sean. All feedback is appreciated.

This should be part of your change log below.

Here you should have a summary of what the series is about.

> 
> Thanks,
> Melody
> 
> Changelog:
> v3:

You're missing the changes from v3 to v4 and you shouldn't delete the
previous log of changes from v1 to v2 and v2 to v3.

> 
> Here are two patches to make VMGEXIT GHCB exit codes more readable. All
> feedback is appreciated.

This should be at the top. Also, this isn't for exit codes, it is for the
hypervisor return error codes, right?

Thanks,
Tom

> 
> Melody Wang (2):
>   KVM: SVM: Convert plain error code numbers to defines
>   KVM: SVM: Provide helpers to set the error code
> 
>  arch/x86/include/asm/sev-common.h |  8 +++++++
>  arch/x86/kvm/svm/sev.c            | 36 ++++++++++++++++---------------
>  arch/x86/kvm/svm/svm.c            |  6 +-----
>  arch/x86/kvm/svm/svm.h            | 29 +++++++++++++++++++++++++
>  4 files changed, 57 insertions(+), 22 deletions(-)
>