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

[Yosry Ahmed]

On Tue, Oct 20, 2026 at 4:45 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote:
>
> Split the "already up-to-date" check out of the code that writes the LBR
> MSR intercepts, and write the intercepts unconditionally from
> svm_recalc_msr_intercepts(), so that changes to userspace MSR filters are
> reflected in the MSR permissions map.
>
> svm_recalc_msr_intercepts() is invoked in response to
> KVM_REQ_RECALC_INTERCEPTS, which is raised on MSR filter updates.  But
> svm->lbr_msrs_intercepted tracks only the interception KVM wants, not
> what is actually in the bitmap after the filter is applied.  If the LBR
> MSRs are passed through when userspace installs a filter that denies
> them, the early return leaves them passed through, and the guest can
> access them without exiting to userspace.  Conversely, if a filter
> forced interception when LBR virtualization was enabled, relaxing the
> filter leaves the MSRs needlessly intercepted.  Either way, the bitmap
> remains stale until the next change to LBR_CTL.
>
> Full intercept recalculations are rare (e.g. on RESET/INIT, EFER.SVME
> toggles, and CPUID, PERF_CAPABILITIES, and MSR filter updates), and
> never happen on nested VMRUN or #VMEXIT, so writing the intercepts there
> does not reintroduce the redundant per-VMRUN bitmap updates that the
> check was added to avoid.  And because svm_recalc_msr_intercepts()
> already unconditionally writes the intercepts for many other MSRs (and
> thus marks Hyper-V's nested enlightenments dirty), writing the LBR MSR
> intercepts there doesn't introduce any new invalidations.
>
> Add a comment to clarify that lbr_msrs_intercepted reflects KVM's
> desired interception, not necessarily the state of the bitmap.
>
> Fixes: 3fa05f96fc08 ("KVM: SVM: Fix redundant updates of LBR MSR intercepts")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> ---
>  arch/x86/kvm/svm/svm.c | 28 +++++++++++++++++++++++-----
>  arch/x86/kvm/svm/svm.h |  6 ++++++
>  2 files changed, 29 insertions(+), 5 deletions(-)
>
> 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?

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?

>  }
>
> +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);
> +
> +       /*
> +        * Skip redundant updates from svm_update_lbrv(), e.g. on every nested
> +        * VMRUN and #VMEXIT.  Note, svm_recalc_msr_intercepts() updates the
> +        * bitmap unconditionally, as userspace MSR filters may have changed.
> +        */
> +       if (intercept == svm->lbr_msrs_intercepted)
> +               return;
> +
> +       svm_set_lbr_msr_intercepts(vcpu);
> +}
> +
>  void svm_vcpu_free_msrpm(void *msrpm)
>  {
>         __free_pages(virt_to_page(msrpm), get_order(MSRPM_SIZE));
> @@ -795,8 +808,13 @@ static void svm_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
>         svm_disable_intercept_for_msr(vcpu, MSR_SYSCALL_MASK, MSR_TYPE_RW);
>  #endif
>
> +       /*
> +        * Unconditionally refresh the LBR MSR intercepts, as userspace MSR
> +        * filters may have changed, in which case the bitmap may be stale even
> +        * if KVM's desired interception hasn't changed.
> +        */
>         if (lbrv)
> -               svm_recalc_lbr_msr_intercepts(vcpu);
> +               svm_set_lbr_msr_intercepts(vcpu);
>
>         if (cpu_feature_enabled(X86_FEATURE_IBPB))
>                 svm_set_intercept_for_msr(vcpu, MSR_IA32_PRED_CMD, MSR_TYPE_W,
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 84f19026d3e8..62e08d6d4706 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -364,6 +364,12 @@ struct vcpu_svm {
>
>         bool avic_irq_window;
>         bool x2avic_msrs_intercepted;
> +
> +       /*
> +        * True if KVM wants the LBR MSRs to be intercepted.  Note, userspace MSR
> +        * filters may force interception regardless, i.e. this doesn't
> +        * necessarily reflect the state of the MSR bitmap.
> +        */
>         bool lbr_msrs_intercepted;
>
>         /* Guest GIF value, used when vGIF is not enabled */
> --
> 2.56.0.212.g129ede20d9-goog
>