Re: [PATCH v4 1/5] KVM: SVM: Refactor svm_update_lbrv()

[Sean Christopherson <seanjc@google.com>]

The shortlog is effectively "do work".  Be more precise.

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> Rewrite the enable_lbrv computation in svm_update_lbrv() as a series of
> 'if' statements. Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled(),
> expose it, and use it instead of open-coding the nested LBRV check.

This is super duper obviously two separate and *barely* related patches.

> No functional change intended.
> 
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
> Reviewed-by: Yosry Ahmed <yosry@kernel.org>
> ---
> Changelog:
> v3 -> v4:
>  * Rename nested_vmcb12_has_lbrv() to nested_lbrv_enabled() (Yosry).

I don't like this change.  Unlike nested_vgif_enabled(), this is *only* checking
the effective vmcb12.  Once the guest_cpu_cap_has() code is moved elsewhere, I
don't see any point in keeping the helper.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ef69a51ab27f..e9f2456982d4 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -880,9 +880,13 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	bool current_enable_lbrv = svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR;
> -	bool enable_lbrv = (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR) ||
> -			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
> -			    (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR));
> +	bool enable_lbrv = false;
> +
> +	if (svm->vmcb->save.dbgctl & DEBUGCTLMSR_LBR)
> +		enable_lbrv = true;
> +
> +	if (is_guest_mode(vcpu) && nested_lbrv_enabled(vcpu))
> +		enable_lbrv = true;

This is begging for short-circuit logic (which was kinda the point of the existing
code).  An if-elif is silly, and I agree that squeezing everything into variable
initialization is hard to read, so I think we should do:

static bool svm_need_lbr_virtualization(struct kvm_vcpu *vcpu)
{
	struct vcpu_svm *svm = to_svm(vcpu);

	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
		return true;

	return is_guest_mode(vcpu) &&
	       (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
}


		bool enable_lbrv = svm_need_lbr_virtualization(vcpu);