Re: [PATCH 3/7] KVM: SVM: Emulate guest accesses to LBR v2 MSRs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] LBRv2 emulation ignores the `lbrv` module parameter, allowing guest LBRv2 usage even when LBR Virtualization is globally disabled.
- [High] The newly added LBR v2 MSRs are missing from KVM's list of saveable MSRs, causing them to be silently dropped during live migration.
- [High] Missing nested SVM state synchronization for LBRv2 fields causes state corruption and cross-tenant information leaks between L2 guests.
--

--- Patch [3]: [PATCH 3/7] KVM: SVM: Emulate guest accesses to LBR v2 MSRs ---
commit ea17366da9bcd5ccfa5c4c9307a6f68cda7d1255
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: SVM: Emulate guest accesses to LBR v2 MSRs
    
Back the LBR v2 MSRs with their VMCB save-area fields in
svm_{get,set}_msr(). This is required when guest accesses to these
MSRs are intercepted. This allows the guest to retain the values
of these MSRs on VMRUN/#VMEXIT cycle when vLBR is disabled.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index ef69a51ab27f9..13ac62604ca05 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c

[ ... ]

> @@ -744,6 +744,12 @@ static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
>  	svm->lbr_msrs_intercepted = intercept;
>  }
>  
> +static bool svm_lbrv2_supported(struct kvm_vcpu *vcpu)
> +{
> +	return guest_cpu_cap_has(vcpu, X86_FEATURE_AMD_LBR_V2) &&
> +	       kvm_vcpu_has_mediated_pmu(vcpu);
> +}

[Severity: High]
Does this capability check need to respect the global lbrv module parameter?

Unlike the legacy LBR logic which explicitly checks lbrv, this function only
checks the X86_FEATURE_AMD_LBR_V2 capability. 

Could this allow a local guest to bypass the host administrator's explicit
disablement of LBR Virtualization (when loaded with lbrv=0) by directly
accessing LBRv2 MSRs?

[ ... ]

> @@ -2882,6 +2901,21 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_IA32_LASTINTTOIP:
>  		msr_info->data = lbrv ? *svm_vmcb_lbr(svm, msr_info->index) : 0;
>  		break;
> +	case MSR_AMD_DBG_EXTN_CFG:
> +		if (!svm_lbrv2_supported(vcpu))
> +			return KVM_MSR_RET_UNSUPPORTED;
> +		msr_info->data = svm->vmcb->save.dbg_extn_cfg;
> +		break;

[Severity: High]
Are these new LBR v2 MSRs missing from KVM's list of saveable MSRs?

Because the new LBRv2 MSRs (MSR_AMD_DBG_EXTN_CFG, MSR_AMD64_LBR_SELECT, and
the MSR_AMD_SAMP_BR_FROM array) are omitted from msrs_to_save_base in
arch/x86/kvm/msrs.c, they won't be returned to the VMM by
KVM_GET_MSR_INDEX_LIST.

Will this cause the LBRv2 state to be silently dropped during live migration?

[ ... ]

> @@ -3181,6 +3215,40 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  		*svm_vmcb_lbr(svm, ecx) = data;
>  		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
>  		break;
> +	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;
> +		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
> +		svm_update_lbrv(vcpu);
> +		break;

[Severity: High]
Does nested SVM state synchronization need to be updated for these new LBRv2
fields?

Looking at nested virtualization, L2 guest state is synchronized using
svm_copy_lbrs() in arch/x86/kvm/svm/svm.h, which is called during
nested_vmcb02_prepare_save().

Because svm_copy_lbrs() was not updated to copy the new dbg_extn_cfg,
lbr_select, and lbr array fields, vmcb02 can retain a previous L2 guest's
branch records. When a new L2 guest reads its MSRs, it could observe the prior
L2 guest's execution trace.

Additionally, upon nested_svm_vmexit(), wouldn't L2 modifications be dropped
because they aren't copied back to L1's vmcb12?

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