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

[Sean Christopherson <seanjc@google.com>]

On Tue, Feb 25, 2025, Melody Wang wrote:
> @@ -3675,8 +3673,13 @@ 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

"A value of zero" is largely redundant, and somewhat confusing.  There's no '0'
in the below code, so to understand the comment, the reader needs to know that
"no action" is a response code of '0' (and is communicated in SW_EXITINFO1,
though that's much less of a problem).

> +	 * completed or completed successfully.  PSC requests always get a "no action"
> +	 * response in SW_EXITINFO1, with a PSC-specific return code in SW_EXITINFO2.

Please wrap at ~80.  Yes, it's a "soft limit", but preferred KVM style is to
wrap at 80 unless running long makes the code more readable.  For a multi-line
comment, I don't see any reason to wrap in the mid-80s.

This is what I ended up with

	/*
	 * PSC requests always get a "no action" response in SW_EXITINFO1, with
	 * a PSC-specific return code in SW_EXITINFO2 that provides the "real"
	 * return code.  E.g. if the PSC request was interrupted, the need to
	 * retry is communicated via SW_EXITINFO2, not SW_EXITINFO1.
	 */

> +	 */
> +	svm_vmgexit_no_action(svm, psc_ret); }

Malformed change to the closing curly brace.

>  	case SVM_VMGEXIT_HV_FEATURES:
> -		ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, GHCB_HV_FT_SUPPORTED);
> +		/* Get hypervisor supported features */

This doesn't add any value.  If the logic isn't clear, then GHCB_HV_FT_SUPPORTED
needs to be renamed.

> +		svm_vmgexit_success(svm, GHCB_HV_FT_SUPPORTED);
>  
>  		ret = 1;
>  		break;