Re: [PATCH 1/4] KVM: SVM: Always update LBR MSR intercepts on full MSR intercept recalc

[Sean Christopherson]

On Tue, Oct 20, 2026, Yosry Ahmed wrote:
> On Tue, Oct 20, 2026 at 4:45 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index fe0cba4731b2..55b3ee840365 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -721,14 +721,11 @@ void *svm_alloc_permissions_map(unsigned long size, gfp_t gfp_mask)
> >         return pm;
> >  }
> >
> > -static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
> > +static void svm_set_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
> >  {
> >         struct vcpu_svm *svm = to_svm(vcpu);
> >         bool intercept = !(svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
> >
> > -       if (intercept == svm->lbr_msrs_intercepted)
> > -               return;
> > -
> >         svm_set_intercept_for_msr(vcpu, MSR_IA32_LASTBRANCHFROMIP, MSR_TYPE_RW, intercept);
> >         svm_set_intercept_for_msr(vcpu, MSR_IA32_LASTBRANCHTOIP, MSR_TYPE_RW, intercept);
> >         svm_set_intercept_for_msr(vcpu, MSR_IA32_LASTINTFROMIP, MSR_TYPE_RW, intercept);
> > @@ -740,6 +737,22 @@ static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
> >         svm->lbr_msrs_intercepted = intercept;
> 
> Hmm I was going to ask if we can set svm->lbr_msrs_intercepted based
> on whether or not svm_set_intercept_for_msr() actually set the
> intercept, so that it also captures intercepts due to MSR filtering. I
> assume that we didn't do this because MSR filters can result in
> different interception status for different LBR MSRs, and we don't
> want to track them individually?

NAK.  In the past, KVM had a bitmap to do exactly that, and it was a trainwreck.

> Hmm even if we do track them individually (e.g. as a bitmap), it would
> add a bit of complexity here because we'll need to plumb that into
> svm_set_intercept_for_msr(), right?
> 
> I suppose if we go with "lbr_msrs_intercepted just means KVM wants to
> intercept", then we should probably rename it accordingly?

Probably, but that can be done on top.  And I'd like to change all of these flags
(all two of them?) to be consistent.

> >  }
> >
> > +static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
> > +{
> > +       struct vcpu_svm *svm = to_svm(vcpu);
> > +       bool intercept = !(svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);

Eww.  The naming is all kinda of funky, and I strongly dislike the duplicate code.
E.g. svm_set_lbr_msr_intercepts() recalcs, it's not a pure "set".  I'd much rather
pass a @force param, e.g.

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e6b8c0423813..2d70fb37103e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -728,12 +728,12 @@ void *svm_alloc_permissions_map(unsigned long size, gfp_t gfp_mask)
 	return pm;
 }
 
-static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
+static void __svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu, bool force)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	bool intercept = !(svm->vmcb->control.misc_ctl2 & SVM_MISC2_ENABLE_V_LBR);
 
-	if (intercept == svm->lbr_msrs_intercepted)
+	if (intercept == svm->lbr_msrs_intercepted && !force)
 		return;
 
 	svm_set_intercept_for_msr(vcpu, MSR_IA32_LASTBRANCHFROMIP, MSR_TYPE_RW, intercept);
@@ -747,6 +747,11 @@ static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
 	svm->lbr_msrs_intercepted = intercept;
 }
 
+static void svm_recalc_lbr_msr_intercepts(struct kvm_vcpu *vcpu)
+{
+	__svm_recalc_lbr_msr_intercepts(vcpu, false);
+}
+
 void svm_vcpu_free_msrpm(void *msrpm)
 {
 	__free_pages(virt_to_page(msrpm), get_order(MSRPM_SIZE));
@@ -803,7 +808,7 @@ static void svm_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
 #endif
 
 	if (lbrv)
-		svm_recalc_lbr_msr_intercepts(vcpu);
+		__svm_recalc_lbr_msr_intercepts(vcpu, true);
 
 	if (cpu_feature_enabled(X86_FEATURE_IBPB))
 		svm_set_intercept_for_msr(vcpu, MSR_IA32_PRED_CMD, MSR_TYPE_W,