Re: [PATCH v2 04/12] KVM: SVM: Enable hardware-assisted LBRv2 virtualization

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: SVM: Missing VMCB_LBR dirty mark when toggling V_LBR in svm_update_lbrv()
--

commit 46b5fce40fe2fd2edafbeb3ed6f81f7158c27cba
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: SVM: Enable hardware-assisted LBRv2 virtualization

This commit enables V_LBR when LBRv2 is active in the guest and passes
the LBRv2 MSRs through to the guest by disabling their intercepts while
branch recording is enabled.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 428c95d7ef21b..69f9ea746bb6f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c

[ ... ]

> @@ -897,6 +935,9 @@ static bool svm_need_lbr_virtualization(struct kvm_vcpu *vcpu)
>  	if (svm->vmcb->save.dbgctl & (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BUS_LOCK_DETECT))
>  		return true;
>  
> +	if (svm_lbrv2_active(vcpu))
> +		return true;
> +
>  	return is_guest_mode(vcpu) &&
>  	       (svm->nested.ctl.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
>  }
> @@ -919,6 +960,7 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>  	 * do, so always recalculate the intercepts here.
>  	 */
>  	svm_recalc_lbr_msr_intercepts(vcpu);
> +	svm_recalc_lbrv2_msr_intercepts(vcpu);
>  }

[Severity: High]
When MSR_AMD_DBG_EXTN_CFG is written and toggles DBG_EXTN_CFG_LBRV2EN,
svm_set_msr() calls svm_update_lbrv():

arch/x86/kvm/svm/svm.c:svm_set_msr() {
	...
	case MSR_AMD_DBG_EXTN_CFG:
		if (!svm_lbrv2_supported(vcpu))
			return KVM_MSR_RET_UNSUPPORTED;

		if (data & DBG_EXTN_CFG_RESERVED_BITS)
			return 1;

		if (svm->vmcb->save.dbg_extn_cfg == data)
			break;

		svm->vmcb->save.dbg_extn_cfg = data;
		svm_update_lbrv(vcpu);
		break;
	...
}

In svm_update_lbrv(), __svm_enable_lbrv() or __svm_disable_lbrv() toggles
SVM_MISC2_ENABLE_V_LBR in svm->vmcb->control.misc_ctl2, and
svm_recalc_lbr_msr_intercepts() updates the legacy LBR MSR intercepts:

arch/x86/kvm/svm/svm.c:svm_update_lbrv() {
	...
	if (enable_lbrv && !current_enable_lbrv)
		__svm_enable_lbrv(vcpu);
	else if (!enable_lbrv && current_enable_lbrv)
		__svm_disable_lbrv(vcpu);
	...
	svm_recalc_lbr_msr_intercepts(vcpu);
	svm_recalc_lbrv2_msr_intercepts(vcpu);
}

Per the AMD APM, VMCB clean bit 10 (VMCB_LBR) covers the LBR virtualization
enable bit (SVM_MISC2_ENABLE_V_LBR) in addition to DBGCTL and the legacy LBR
MSRs. Unlike the MSR_IA32_DEBUGCTLMSR path in svm_set_msr(), neither the
MSR_AMD_DBG_EXTN_CFG handler nor __svm_enable_lbrv() / __svm_disable_lbrv()
marks VMCB_LBR dirty.

After a previous VMRUN has marked VMCB_LBR clean via vmcb_mark_all_clean()
in svm_vcpu_run(), can toggling DBG_EXTN_CFG_LBRV2EN leave VMCB_LBR clean in
svm->vmcb->control.clean, causing the CPU to use a stale cached value for
SVM_MISC2_ENABLE_V_LBR and DBGCTL on VMRUN while KVM stops manually
switching DEBUGCTL and disables the LBR MSR intercepts?

Should __svm_enable_lbrv() and __svm_disable_lbrv() call
vmcb_mark_dirty(svm->vmcb, VMCB_LBR)?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019210532.378628-1-shivansh.dhiman@amd.com?part=4