[PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()

[PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()

[Yosry Ahmed]

svm_update_lbrv() is called when MSR_IA32_DEBUGCTLMSR is updated, and on
nested transitions where LBRV is used. It checks whether LBRV enablement
needs to be changed in the current VMCB, and if it does, it also
recalculate intercepts to LBR MSRs.

However, there are cases where intercepts need to be updated even when
LBRV enablement doesn't. Example scenario:
- L1 has MSR_IA32_DEBUGCTLMSR cleared.
- L1 runs L2 without LBR_CTL_ENABLE (no LBRV).
- L2 sets DEBUGCTLMSR_LBR in MSR_IA32_DEBUGCTLMSR, svm_update_lbrv()
  sets LBR_CTL_ENABLE in VMCB02 and disables intercepts to LBR MSRs.
- L2 exits to L1, svm_update_lbrv() is not called on this transition.
- L1 clears MSR_IA32_DEBUGCTLMSR, svm_update_lbrv() finds that
  LBR_CTL_ENABLE is already cleared in VMCB01 and does nothing.
- Intercepts remain disabled, L1 reads to LBR MSRs read the host MSRs.

Fix it by always recalculating intercepts in svm_update_lbrv().

Fixes: 1d5a1b5860ed ("KVM: x86: nSVM: correctly virtualize LBR msrs when L2 is running")
Cc: stable@vger.kernel.org

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/svm.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d25c56b30b4e2..26ab75ecf1c67 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -806,25 +806,29 @@ void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
 	vmcb_mark_dirty(to_vmcb, VMCB_LBR);
 }
 
-void svm_enable_lbrv(struct kvm_vcpu *vcpu)
+static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	svm->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
-	svm_recalc_lbr_msr_intercepts(vcpu);
 
 	/* Move the LBR msrs to the vmcb02 so that the guest can see them. */
 	if (is_guest_mode(vcpu))
 		svm_copy_lbrs(svm->vmcb, svm->vmcb01.ptr);
 }
 
-static void svm_disable_lbrv(struct kvm_vcpu *vcpu)
+void svm_enable_lbrv(struct kvm_vcpu *vcpu)
+{
+	__svm_enable_lbrv(vcpu);
+	svm_recalc_lbr_msr_intercepts(vcpu);
+}
+
+static void __svm_disable_lbrv(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	KVM_BUG_ON(sev_es_guest(vcpu->kvm), vcpu->kvm);
 	svm->vmcb->control.virt_ext &= ~LBR_CTL_ENABLE_MASK;
-	svm_recalc_lbr_msr_intercepts(vcpu);
 
 	/*
 	 * Move the LBR msrs back to the vmcb01 to avoid copying them
@@ -853,13 +857,18 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
 			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
 			    (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK));
 
-	if (enable_lbrv == current_enable_lbrv)
-		return;
+	if (enable_lbrv && !current_enable_lbrv)
+		__svm_enable_lbrv(vcpu);
+	else if (!enable_lbrv && current_enable_lbrv)
+		__svm_disable_lbrv(vcpu);
 
-	if (enable_lbrv)
-		svm_enable_lbrv(vcpu);
-	else
-		svm_disable_lbrv(vcpu);
+	/*
+	 * During nested transitions, it is possible that the current VMCB has
+	 * LBR_CTL set, but the previous LBR_CTL had it cleared (or vice versa).
+	 * In this case, even though LBR_CTL does not need an update, intercepts
+	 * do, so always recalculate the intercepts here.
+	 */
+	svm_recalc_lbr_msr_intercepts(vcpu);
 }
 
 void disable_nmi_singlestep(struct vcpu_svm *svm)
-- 
2.51.2.1041.gc1ab5b90ca-goog

Re: [PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()

[Yosry Ahmed]

On Sat, Nov 08, 2025 at 12:45:20AM +0000, Yosry Ahmed wrote:
> svm_update_lbrv() is called when MSR_IA32_DEBUGCTLMSR is updated, and on
> nested transitions where LBRV is used. It checks whether LBRV enablement
> needs to be changed in the current VMCB, and if it does, it also
> recalculate intercepts to LBR MSRs.
> 
> However, there are cases where intercepts need to be updated even when
> LBRV enablement doesn't. Example scenario:
> - L1 has MSR_IA32_DEBUGCTLMSR cleared.
> - L1 runs L2 without LBR_CTL_ENABLE (no LBRV).
> - L2 sets DEBUGCTLMSR_LBR in MSR_IA32_DEBUGCTLMSR, svm_update_lbrv()
>   sets LBR_CTL_ENABLE in VMCB02 and disables intercepts to LBR MSRs.
> - L2 exits to L1, svm_update_lbrv() is not called on this transition.
> - L1 clears MSR_IA32_DEBUGCTLMSR, svm_update_lbrv() finds that
>   LBR_CTL_ENABLE is already cleared in VMCB01 and does nothing.
> - Intercepts remain disabled, L1 reads to LBR MSRs read the host MSRs.
> 
> Fix it by always recalculating intercepts in svm_update_lbrv().

This actually breaks hyperv_svm_test, because svm_update_lbrv() is
called on every nested transition, calling
svm_recalc_lbr_msr_intercepts() -> svm_set_intercept_for_msr() and
setting svm->nested.force_msr_bitmap_recalc to true.

This breaks the hyperv optimization in nested_svm_vmrun_msrpm() AFAICT.

I think there are two ways to fix this:
- Add another bool to svm->nested to track LBR intercepts, and only call
  svm_set_intercept_for_msr() if the intercepts need to be updated.

- Update svm_set_intercept_for_msr() itself to do nothing if the
  intercepts do not need to be changed, which is more clutter but
  applies to other callers as well so could shave cycles elsewhere (see
  below).

Sean, Paolo, any preferences?

Here's what updating svm_set_intercept_for_msr() looks like:

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2fbb0b88c6a3e..b1afafc8b37c0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -664,24 +664,36 @@ void svm_set_intercept_for_msr(struct kvm_vcpu *vcpu, u32 msr, int type, bool se
 {
        struct vcpu_svm *svm = to_svm(vcpu);
        void *msrpm = svm->msrpm;
+       bool recalc = false;
+       bool already_set;

        /* Don't disable interception for MSRs userspace wants to handle. */
        if (type & MSR_TYPE_R) {
-               if (!set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_READ))
+               already_set = svm_test_msr_bitmap_read(msrpm, msr);
+               if (!set && already_set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_READ)) {
                        svm_clear_msr_bitmap_read(msrpm, msr);
-               else
+                       recalc = true;
+               } else if (set && !already_set) {
                        svm_set_msr_bitmap_read(msrpm, msr);
+                       recalc = true;
+               }
        }

        if (type & MSR_TYPE_W) {
-               if (!set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_WRITE))
+               already_set = svm_test_msr_bitmap_write(msrpm, msr);
+               if (!set && already_set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_WRITE)) {
                        svm_clear_msr_bitmap_write(msrpm, msr);
-               else
+                       recalc = true;
+               } else if (set && !already_set) {
                        svm_set_msr_bitmap_write(msrpm, msr);
+                       recalc = true;
+               }
        }

-       svm_hv_vmcb_dirty_nested_enlightenments(vcpu);
-       svm->nested.force_msr_bitmap_recalc = true;
+       if (recalc) {
+               svm_hv_vmcb_dirty_nested_enlightenments(vcpu);
+               svm->nested.force_msr_bitmap_recalc = true;
+       }
 }

 void *svm_alloc_permissions_map(unsigned long size, gfp_t gfp_mask)

> 
> Fixes: 1d5a1b5860ed ("KVM: x86: nSVM: correctly virtualize LBR msrs when L2 is running")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  arch/x86/kvm/svm/svm.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d25c56b30b4e2..26ab75ecf1c67 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -806,25 +806,29 @@ void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
>  	vmcb_mark_dirty(to_vmcb, VMCB_LBR);
>  }
>  
> -void svm_enable_lbrv(struct kvm_vcpu *vcpu)
> +static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
>  	svm->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
> -	svm_recalc_lbr_msr_intercepts(vcpu);
>  
>  	/* Move the LBR msrs to the vmcb02 so that the guest can see them. */
>  	if (is_guest_mode(vcpu))
>  		svm_copy_lbrs(svm->vmcb, svm->vmcb01.ptr);
>  }
>  
> -static void svm_disable_lbrv(struct kvm_vcpu *vcpu)
> +void svm_enable_lbrv(struct kvm_vcpu *vcpu)
> +{
> +	__svm_enable_lbrv(vcpu);
> +	svm_recalc_lbr_msr_intercepts(vcpu);
> +}
> +
> +static void __svm_disable_lbrv(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
>  	KVM_BUG_ON(sev_es_guest(vcpu->kvm), vcpu->kvm);
>  	svm->vmcb->control.virt_ext &= ~LBR_CTL_ENABLE_MASK;
> -	svm_recalc_lbr_msr_intercepts(vcpu);
>  
>  	/*
>  	 * Move the LBR msrs back to the vmcb01 to avoid copying them
> @@ -853,13 +857,18 @@ void svm_update_lbrv(struct kvm_vcpu *vcpu)
>  			    (is_guest_mode(vcpu) && guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
>  			    (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK));
>  
> -	if (enable_lbrv == current_enable_lbrv)
> -		return;
> +	if (enable_lbrv && !current_enable_lbrv)
> +		__svm_enable_lbrv(vcpu);
> +	else if (!enable_lbrv && current_enable_lbrv)
> +		__svm_disable_lbrv(vcpu);
>  
> -	if (enable_lbrv)
> -		svm_enable_lbrv(vcpu);
> -	else
> -		svm_disable_lbrv(vcpu);
> +	/*
> +	 * During nested transitions, it is possible that the current VMCB has
> +	 * LBR_CTL set, but the previous LBR_CTL had it cleared (or vice versa).
> +	 * In this case, even though LBR_CTL does not need an update, intercepts
> +	 * do, so always recalculate the intercepts here.
> +	 */
> +	svm_recalc_lbr_msr_intercepts(vcpu);
>  }
>  
>  void disable_nmi_singlestep(struct vcpu_svm *svm)
> -- 
> 2.51.2.1041.gc1ab5b90ca-goog
> 

Re: [PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()

[Yosry Ahmed]

On Tue, Nov 11, 2025 at 03:11:37AM +0000, Yosry Ahmed wrote:
> On Sat, Nov 08, 2025 at 12:45:20AM +0000, Yosry Ahmed wrote:
> > svm_update_lbrv() is called when MSR_IA32_DEBUGCTLMSR is updated, and on
> > nested transitions where LBRV is used. It checks whether LBRV enablement
> > needs to be changed in the current VMCB, and if it does, it also
> > recalculate intercepts to LBR MSRs.
> > 
> > However, there are cases where intercepts need to be updated even when
> > LBRV enablement doesn't. Example scenario:
> > - L1 has MSR_IA32_DEBUGCTLMSR cleared.
> > - L1 runs L2 without LBR_CTL_ENABLE (no LBRV).
> > - L2 sets DEBUGCTLMSR_LBR in MSR_IA32_DEBUGCTLMSR, svm_update_lbrv()
> >   sets LBR_CTL_ENABLE in VMCB02 and disables intercepts to LBR MSRs.
> > - L2 exits to L1, svm_update_lbrv() is not called on this transition.
> > - L1 clears MSR_IA32_DEBUGCTLMSR, svm_update_lbrv() finds that
> >   LBR_CTL_ENABLE is already cleared in VMCB01 and does nothing.
> > - Intercepts remain disabled, L1 reads to LBR MSRs read the host MSRs.
> > 
> > Fix it by always recalculating intercepts in svm_update_lbrv().
> 
> This actually breaks hyperv_svm_test, because svm_update_lbrv() is
> called on every nested transition, calling
> svm_recalc_lbr_msr_intercepts() -> svm_set_intercept_for_msr() and
> setting svm->nested.force_msr_bitmap_recalc to true.
> 
> This breaks the hyperv optimization in nested_svm_vmrun_msrpm() AFAICT.
> 
> I think there are two ways to fix this:
> - Add another bool to svm->nested to track LBR intercepts, and only call
>   svm_set_intercept_for_msr() if the intercepts need to be updated.
> 
> - Update svm_set_intercept_for_msr() itself to do nothing if the
>   intercepts do not need to be changed, which is more clutter but
>   applies to other callers as well so could shave cycles elsewhere (see
>   below).
> 
> Sean, Paolo, any preferences?
> 
> Here's what updating svm_set_intercept_for_msr() looks like:

and that diff breaks userspace_msr_exit_test :)

Here's an actually tested diff:

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 2fbb0b88c6a3e..88717429ba9d5 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -664,24 +664,38 @@ void svm_set_intercept_for_msr(struct kvm_vcpu *vcpu, u32 msr, int type, bool se
 {
        struct vcpu_svm *svm = to_svm(vcpu);
        void *msrpm = svm->msrpm;
+       bool recalc = false;
+       bool already_set;

        /* Don't disable interception for MSRs userspace wants to handle. */
        if (type & MSR_TYPE_R) {
-               if (!set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_READ))
+               set = set || !kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_READ);
+               already_set = svm_test_msr_bitmap_read(msrpm, msr);
+
+               if (!set && already_set)
                        svm_clear_msr_bitmap_read(msrpm, msr);
-               else
+               else if (set && !already_set)
                        svm_set_msr_bitmap_read(msrpm, msr);
+
+               recalc |= (set != already_set);
        }

        if (type & MSR_TYPE_W) {
-               if (!set && kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_WRITE))
+               set = set || !kvm_msr_allowed(vcpu, msr, KVM_MSR_FILTER_WRITE);
+               already_set = svm_test_msr_bitmap_write(msrpm, msr);
+
+               if (!set && already_set)
                        svm_clear_msr_bitmap_write(msrpm, msr);
-               else
+               else if (set && !already_set)
                        svm_set_msr_bitmap_write(msrpm, msr);
+
+               recalc |= (set != already_set);
        }

-       svm_hv_vmcb_dirty_nested_enlightenments(vcpu);
-       svm->nested.force_msr_bitmap_recalc = true;
+       if (recalc) {
+               svm_hv_vmcb_dirty_nested_enlightenments(vcpu);
+               svm->nested.force_msr_bitmap_recalc = true;
+       }
 }

 void *svm_alloc_permissions_map(unsigned long size, gfp_t gfp_mask)

---

For the record, I don't want to just use svm_test_msr_bitmap_*() in
svm_update_lbrv() because there is no direct equivalent in older kernels
as far as I can tell, so a backport would be completely inapplicable.

Re: [PATCH 2/6] KVM: nSVM: Always recalculate LBR MSR intercepts in svm_update_lbrv()

[Sean Christopherson]

On Tue, Nov 11, 2025, Yosry Ahmed wrote:
> On Tue, Nov 11, 2025 at 03:11:37AM +0000, Yosry Ahmed wrote:
> > On Sat, Nov 08, 2025 at 12:45:20AM +0000, Yosry Ahmed wrote:
> > > svm_update_lbrv() is called when MSR_IA32_DEBUGCTLMSR is updated, and on
> > > nested transitions where LBRV is used. It checks whether LBRV enablement
> > > needs to be changed in the current VMCB, and if it does, it also
> > > recalculate intercepts to LBR MSRs.
> > > 
> > > However, there are cases where intercepts need to be updated even when
> > > LBRV enablement doesn't. Example scenario:
> > > - L1 has MSR_IA32_DEBUGCTLMSR cleared.
> > > - L1 runs L2 without LBR_CTL_ENABLE (no LBRV).
> > > - L2 sets DEBUGCTLMSR_LBR in MSR_IA32_DEBUGCTLMSR, svm_update_lbrv()
> > >   sets LBR_CTL_ENABLE in VMCB02 and disables intercepts to LBR MSRs.
> > > - L2 exits to L1, svm_update_lbrv() is not called on this transition.
> > > - L1 clears MSR_IA32_DEBUGCTLMSR, svm_update_lbrv() finds that
> > >   LBR_CTL_ENABLE is already cleared in VMCB01 and does nothing.
> > > - Intercepts remain disabled, L1 reads to LBR MSRs read the host MSRs.
> > > 
> > > Fix it by always recalculating intercepts in svm_update_lbrv().
> > 
> > This actually breaks hyperv_svm_test, because svm_update_lbrv() is
> > called on every nested transition, calling
> > svm_recalc_lbr_msr_intercepts() -> svm_set_intercept_for_msr() and
> > setting svm->nested.force_msr_bitmap_recalc to true.
> > 
> > This breaks the hyperv optimization in nested_svm_vmrun_msrpm() AFAICT.
> > 
> > I think there are two ways to fix this:
> > - Add another bool to svm->nested to track LBR intercepts, and only call
> >   svm_set_intercept_for_msr() if the intercepts need to be updated.
> > 
> > - Update svm_set_intercept_for_msr() itself to do nothing if the
> >   intercepts do not need to be changed, which is more clutter but
> >   applies to other callers as well so could shave cycles elsewhere (see
> >   below).
> > 
> > Sean, Paolo, any preferences?
> > 
> > Here's what updating svm_set_intercept_for_msr() looks like:

I am *very* strongly opposed to modifying svm_set_intercept_for_msr() to deal
with whatever mess LBRs has created.  Whatever the problem is (I haven't read
through all of this yet), it needs to be fixed in the LBR code, not in
svm_set_intercept_for_msr().

Recalculating MSR intercepts is supposed to done only as needed, I don't want to
encourage lazy code that works by optimizing paths that should be rare.

[PATCH 3/6] KVM: nSVM: Fix and simplify LBR virtualization handling with nested

[Yosry Ahmed]

The current scheme for handling LBRV when nested is used is very
complicated, especially when L1 does not enable LBRV (i.e. does not set
LBR_CTL_ENABLE_MASK).

To avoid copying LBRs between VMCB01 and VMCB02 on every nested
transition, the current implementation switches between using VMCB01 or
VMCB02 as the source of truth for the LBRs while L2 is running. If L2
enables LBR, VMCB02 is used as the source of truth. When L2 disables
LBR, the LBRs are copied to VMCB01 and VMCB01 is used as the source of
truth. This introduces significant complexity, and incorrect behavior in
some cases.

For example, on a nested #VMEXIT, the LBRs are only copied from VMCB02
to VMCB01 if LBRV is enabled in VMCB01. This is because L2's writes to
MSR_IA32_DEBUGCTLMSR to enable LBR are intercepted and propagated to
VMCB01 instead of VMCB02. However, LBRV is only enabled in VMCB02 when
L2 is running.

This means that if L2 enables LBR and exits to L1, the LBRs will not be
propagated from VMCB02 to VMCB01, because LBRV is disabled in VMCB01.

There is no meaningful difference in CPUID rate in L2 when copying LBRs
on every nested transition vs. the current approach, so do the simple
and correct thing and always copy LBRs between VMCB01 and VMCB02 on
nested transitions (when LBRV is disabled by L1). Drop the conditional
LBRs copying in __svm_{enable/disable}_lbrv() as it is now unnecessary.

VMCB02 becomes the only source of truth for LBRs when L2 is running,
regardless of LBRV being enabled by L1, drop svm_get_lbr_vmcb() and use
svm->vmcb directly in its place.

Fixes: 1d5a1b5860ed ("KVM: x86: nSVM: correctly virtualize LBR msrs when L2 is running")
Cc: stable@vger.kernel.org

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c | 20 ++++++-----------
 arch/x86/kvm/svm/svm.c    | 46 +++++++++------------------------------
 2 files changed, 17 insertions(+), 49 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 71664d54d8b2a..c81005b245222 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -678,11 +678,10 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 		 */
 		svm_copy_lbrs(vmcb02, vmcb12);
 		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
-		svm_update_lbrv(&svm->vcpu);
-
-	} else if (unlikely(vmcb01->control.virt_ext & LBR_CTL_ENABLE_MASK)) {
+	} else {
 		svm_copy_lbrs(vmcb02, vmcb01);
 	}
+	svm_update_lbrv(&svm->vcpu);
 }
 
 static inline bool is_evtinj_soft(u32 evtinj)
@@ -835,11 +834,7 @@ static void nested_vmcb02_prepare_control(struct vcpu_svm *svm,
 			svm->soft_int_next_rip = vmcb12_rip;
 	}
 
-	vmcb02->control.virt_ext            = vmcb01->control.virt_ext &
-					      LBR_CTL_ENABLE_MASK;
-	if (guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV))
-		vmcb02->control.virt_ext  |=
-			(svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK);
+	/* LBR_CTL_ENABLE_MASK is controlled by svm_update_lbrv() */
 
 	if (!nested_vmcb_needs_vls_intercept(svm))
 		vmcb02->control.virt_ext |= VIRTUAL_VMLOAD_VMSAVE_ENABLE_MASK;
@@ -1191,13 +1186,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
 
 	if (unlikely(guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
-		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))) {
+		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK)))
 		svm_copy_lbrs(vmcb12, vmcb02);
-		svm_update_lbrv(vcpu);
-	} else if (unlikely(vmcb01->control.virt_ext & LBR_CTL_ENABLE_MASK)) {
+	else
 		svm_copy_lbrs(vmcb01, vmcb02);
-		svm_update_lbrv(vcpu);
-	}
+
+	svm_update_lbrv(vcpu);
 
 	if (vnmi) {
 		if (vmcb02->control.int_ctl & V_NMI_BLOCKING_MASK)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 26ab75ecf1c67..fc42bcdbb5200 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -808,13 +808,7 @@ void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
 
 static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
 {
-	struct vcpu_svm *svm = to_svm(vcpu);
-
-	svm->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
-
-	/* Move the LBR msrs to the vmcb02 so that the guest can see them. */
-	if (is_guest_mode(vcpu))
-		svm_copy_lbrs(svm->vmcb, svm->vmcb01.ptr);
+	to_svm(vcpu)->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
 }
 
 void svm_enable_lbrv(struct kvm_vcpu *vcpu)
@@ -825,35 +819,15 @@ void svm_enable_lbrv(struct kvm_vcpu *vcpu)
 
 static void __svm_disable_lbrv(struct kvm_vcpu *vcpu)
 {
-	struct vcpu_svm *svm = to_svm(vcpu);
-
 	KVM_BUG_ON(sev_es_guest(vcpu->kvm), vcpu->kvm);
-	svm->vmcb->control.virt_ext &= ~LBR_CTL_ENABLE_MASK;
-
-	/*
-	 * Move the LBR msrs back to the vmcb01 to avoid copying them
-	 * on nested guest entries.
-	 */
-	if (is_guest_mode(vcpu))
-		svm_copy_lbrs(svm->vmcb01.ptr, svm->vmcb);
-}
-
-static struct vmcb *svm_get_lbr_vmcb(struct vcpu_svm *svm)
-{
-	/*
-	 * If LBR virtualization is disabled, the LBR MSRs are always kept in
-	 * vmcb01.  If LBR virtualization is enabled and L1 is running VMs of
-	 * its own, the MSRs are moved between vmcb01 and vmcb02 as needed.
-	 */
-	return svm->vmcb->control.virt_ext & LBR_CTL_ENABLE_MASK ? svm->vmcb :
-								   svm->vmcb01.ptr;
+	to_svm(vcpu)->vmcb->control.virt_ext &= ~LBR_CTL_ENABLE_MASK;
 }
 
 void svm_update_lbrv(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 	bool current_enable_lbrv = svm->vmcb->control.virt_ext & LBR_CTL_ENABLE_MASK;
-	bool enable_lbrv = (svm_get_lbr_vmcb(svm)->save.dbgctl & DEBUGCTLMSR_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.virt_ext & LBR_CTL_ENABLE_MASK));
 
@@ -2738,19 +2712,19 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = svm->tsc_aux;
 		break;
 	case MSR_IA32_DEBUGCTLMSR:
-		msr_info->data = svm_get_lbr_vmcb(svm)->save.dbgctl;
+		msr_info->data = svm->vmcb->save.dbgctl;
 		break;
 	case MSR_IA32_LASTBRANCHFROMIP:
-		msr_info->data = svm_get_lbr_vmcb(svm)->save.br_from;
+		msr_info->data = svm->vmcb->save.br_from;
 		break;
 	case MSR_IA32_LASTBRANCHTOIP:
-		msr_info->data = svm_get_lbr_vmcb(svm)->save.br_to;
+		msr_info->data = svm->vmcb->save.br_to;
 		break;
 	case MSR_IA32_LASTINTFROMIP:
-		msr_info->data = svm_get_lbr_vmcb(svm)->save.last_excp_from;
+		msr_info->data = svm->vmcb->save.last_excp_from;
 		break;
 	case MSR_IA32_LASTINTTOIP:
-		msr_info->data = svm_get_lbr_vmcb(svm)->save.last_excp_to;
+		msr_info->data = svm->vmcb->save.last_excp_to;
 		break;
 	case MSR_VM_HSAVE_PA:
 		msr_info->data = svm->nested.hsave_msr;
@@ -3018,10 +2992,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		if (data & DEBUGCTL_RESERVED_BITS)
 			return 1;
 
-		if (svm_get_lbr_vmcb(svm)->save.dbgctl == data)
+		if (svm->vmcb->save.dbgctl == data)
 			break;
 
-		svm_get_lbr_vmcb(svm)->save.dbgctl = data;
+		svm->vmcb->save.dbgctl = data;
 		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
 		svm_update_lbrv(vcpu);
 		break;
-- 
2.51.2.1041.gc1ab5b90ca-goog

[PATCH 4/6] KVM: SVM: Switch svm_copy_lbrs() to a macro

[Yosry Ahmed]

In preparation for using svm_copy_lbrs() with 'struct vmcb_save_area'
without a containing 'struct vmcb', and later even 'struct
vmcb_save_area_cached', make it a macro. Pull the call to
vmcb_mark_dirty() out to the callers.

Macros are generally not preferred compared to functions, mainly due to
type-safety. However, in this case it seems like having a simple macro
copying a few fields is better than copy-pasting the same 5 lines of
code in different places.

On the bright side, pulling vmcb_mark_dirty() calls to the callers makes
it clear that in one case, vmcb_mark_dirty() was being called on VMCB12.
It is not architecturally defined for the CPU to clear arbitrary clean
bits, and it is not needed, so drop that one call.

Technically fixes the non-architectural behavior of setting the dirty
bit on VMCB12.

Fixes: d20c796ca370 ("KVM: x86: nSVM: implement nested LBR virtualization")
Cc: stable@vger.kernel.org

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c | 16 ++++++++++------
 arch/x86/kvm/svm/svm.c    | 11 -----------
 arch/x86/kvm/svm/svm.h    | 10 +++++++++-
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index c81005b245222..e7861392f2fcd 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -676,10 +676,12 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
 		 * Reserved bits of DEBUGCTL are ignored.  Be consistent with
 		 * svm_set_msr's definition of reserved bits.
 		 */
-		svm_copy_lbrs(vmcb02, vmcb12);
+		svm_copy_lbrs(&vmcb02->save, &vmcb12->save);
+		vmcb_mark_dirty(vmcb02, VMCB_LBR);
 		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
 	} else {
-		svm_copy_lbrs(vmcb02, vmcb01);
+		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
+		vmcb_mark_dirty(vmcb02, VMCB_LBR);
 	}
 	svm_update_lbrv(&svm->vcpu);
 }
@@ -1186,10 +1188,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
 		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
 
 	if (unlikely(guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
-		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK)))
-		svm_copy_lbrs(vmcb12, vmcb02);
-	else
-		svm_copy_lbrs(vmcb01, vmcb02);
+		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))) {
+		svm_copy_lbrs(&vmcb12->save, &vmcb02->save);
+	} else {
+		svm_copy_lbrs(&vmcb01->save, &vmcb02->save);
+		vmcb_mark_dirty(vmcb01, VMCB_LBR);
+	}
 
 	svm_update_lbrv(vcpu);
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index fc42bcdbb5200..9eb112f0e61f0 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -795,17 +795,6 @@ static void svm_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
 	 */
 }
 
-void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
-{
-	to_vmcb->save.dbgctl		= from_vmcb->save.dbgctl;
-	to_vmcb->save.br_from		= from_vmcb->save.br_from;
-	to_vmcb->save.br_to		= from_vmcb->save.br_to;
-	to_vmcb->save.last_excp_from	= from_vmcb->save.last_excp_from;
-	to_vmcb->save.last_excp_to	= from_vmcb->save.last_excp_to;
-
-	vmcb_mark_dirty(to_vmcb, VMCB_LBR);
-}
-
 static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
 {
 	to_svm(vcpu)->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index c2acaa49ee1c5..e510c8183bd87 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -687,8 +687,16 @@ static inline void *svm_vcpu_alloc_msrpm(void)
 	return svm_alloc_permissions_map(MSRPM_SIZE, GFP_KERNEL_ACCOUNT);
 }
 
+#define svm_copy_lbrs(to, from)					\
+({								\
+	(to)->dbgctl		= (from)->dbgctl;		\
+	(to)->br_from		= (from)->br_from;		\
+	(to)->br_to		= (from)->br_to;		\
+	(to)->last_excp_from	= (from)->last_excp_from;	\
+	(to)->last_excp_to	= (from)->last_excp_to;		\
+})
+
 void svm_vcpu_free_msrpm(void *msrpm);
-void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb);
 void svm_enable_lbrv(struct kvm_vcpu *vcpu);
 void svm_update_lbrv(struct kvm_vcpu *vcpu);
 
-- 
2.51.2.1041.gc1ab5b90ca-goog

Re: [PATCH 4/6] KVM: SVM: Switch svm_copy_lbrs() to a macro

[Paolo Bonzini]

On 11/8/25 01:45, Yosry Ahmed wrote:
> In preparation for using svm_copy_lbrs() with 'struct vmcb_save_area'
> without a containing 'struct vmcb', and later even 'struct
> vmcb_save_area_cached', make it a macro. Pull the call to
> vmcb_mark_dirty() out to the callers.

The changes to use `struct vmcb_save_area_cached' are not included in 
this series, so they are irrelevant.

Since I've applied patches 1-3, which fix the worst bugs, there are two 
ways to handle the rest:

* keep the function instead of the macro, while making it take a struct 
vmcb_save_area (and therefore pulling vmcb_mark_dirty() to the callers 
and fixing the bug you mention below).

* you resubmit with the changes to use struct vmcb_save_area_cached, so 
that the commit message makes more sense.

Thanks,

Paolo

> Macros are generally not preferred compared to functions, mainly due to
> type-safety. However, in this case it seems like having a simple macro
> copying a few fields is better than copy-pasting the same 5 lines of
> code in different places.
> 
> On the bright side, pulling vmcb_mark_dirty() calls to the callers makes
> it clear that in one case, vmcb_mark_dirty() was being called on VMCB12.
> It is not architecturally defined for the CPU to clear arbitrary clean
> bits, and it is not needed, so drop that one call.

> Technically fixes the non-architectural behavior of setting the dirty
> bit on VMCB12.
> 
> Fixes: d20c796ca370 ("KVM: x86: nSVM: implement nested LBR virtualization")
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>   arch/x86/kvm/svm/nested.c | 16 ++++++++++------
>   arch/x86/kvm/svm/svm.c    | 11 -----------
>   arch/x86/kvm/svm/svm.h    | 10 +++++++++-
>   3 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index c81005b245222..e7861392f2fcd 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -676,10 +676,12 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
>   		 * Reserved bits of DEBUGCTL are ignored.  Be consistent with
>   		 * svm_set_msr's definition of reserved bits.
>   		 */
> -		svm_copy_lbrs(vmcb02, vmcb12);
> +		svm_copy_lbrs(&vmcb02->save, &vmcb12->save);
> +		vmcb_mark_dirty(vmcb02, VMCB_LBR);
>   		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
>   	} else {
> -		svm_copy_lbrs(vmcb02, vmcb01);
> +		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
> +		vmcb_mark_dirty(vmcb02, VMCB_LBR);
>   	}
>   	svm_update_lbrv(&svm->vcpu);
>   }
> @@ -1186,10 +1188,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
>   		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
>   
>   	if (unlikely(guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
> -		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK)))
> -		svm_copy_lbrs(vmcb12, vmcb02);
> -	else
> -		svm_copy_lbrs(vmcb01, vmcb02);
> +		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))) {
> +		svm_copy_lbrs(&vmcb12->save, &vmcb02->save);
> +	} else {
> +		svm_copy_lbrs(&vmcb01->save, &vmcb02->save);
> +		vmcb_mark_dirty(vmcb01, VMCB_LBR);
> +	}
>   
>   	svm_update_lbrv(vcpu);
>   
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index fc42bcdbb5200..9eb112f0e61f0 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -795,17 +795,6 @@ static void svm_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
>   	 */
>   }
>   
> -void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
> -{
> -	to_vmcb->save.dbgctl		= from_vmcb->save.dbgctl;
> -	to_vmcb->save.br_from		= from_vmcb->save.br_from;
> -	to_vmcb->save.br_to		= from_vmcb->save.br_to;
> -	to_vmcb->save.last_excp_from	= from_vmcb->save.last_excp_from;
> -	to_vmcb->save.last_excp_to	= from_vmcb->save.last_excp_to;
> -
> -	vmcb_mark_dirty(to_vmcb, VMCB_LBR);
> -}
> -
>   static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
>   {
>   	to_svm(vcpu)->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index c2acaa49ee1c5..e510c8183bd87 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -687,8 +687,16 @@ static inline void *svm_vcpu_alloc_msrpm(void)
>   	return svm_alloc_permissions_map(MSRPM_SIZE, GFP_KERNEL_ACCOUNT);
>   }
>   
> +#define svm_copy_lbrs(to, from)					\
> +({								\
> +	(to)->dbgctl		= (from)->dbgctl;		\
> +	(to)->br_from		= (from)->br_from;		\
> +	(to)->br_to		= (from)->br_to;		\
> +	(to)->last_excp_from	= (from)->last_excp_from;	\
> +	(to)->last_excp_to	= (from)->last_excp_to;		\
> +})
> +
>   void svm_vcpu_free_msrpm(void *msrpm);
> -void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb);
>   void svm_enable_lbrv(struct kvm_vcpu *vcpu);
>   void svm_update_lbrv(struct kvm_vcpu *vcpu);
>   
Since

Re: [PATCH 4/6] KVM: SVM: Switch svm_copy_lbrs() to a macro

[Yosry Ahmed]

On Sun, Nov 09, 2025 at 08:59:18AM +0100, Paolo Bonzini wrote:
> On 11/8/25 01:45, Yosry Ahmed wrote:
> > In preparation for using svm_copy_lbrs() with 'struct vmcb_save_area'
> > without a containing 'struct vmcb', and later even 'struct
> > vmcb_save_area_cached', make it a macro. Pull the call to
> > vmcb_mark_dirty() out to the callers.
> 
> The changes to use `struct vmcb_save_area_cached' are not included in this
> series, so they are irrelevant.
> 
> Since I've applied patches 1-3, which fix the worst bugs, there are two ways
> to handle the rest:
> 
> * keep the function instead of the macro, while making it take a struct
> vmcb_save_area (and therefore pulling vmcb_mark_dirty() to the callers and
> fixing the bug you mention below).
> 
> * you resubmit with the changes to use struct vmcb_save_area_cached, so that
> the commit message makes more sense.

I can include patches 4-6 with the respin of the series [1] that has the
changes to use `struct vmcb_save_area_cached`. That series origianlly
had the patch to switch svm_copy_lbrs() to a macro, but I moved it here
to use for the save/restore patch. I was planning to rebase [1] on top
of this series anyway.

There is a hiccup though, I assumed everything would go through Sean's
tree so I planned to respin [1] on top of this series. Otherwise, they
will conflict. With the first 3 patches in your tree, I am not sure how
that would work.

I can respin [1] on top of Sean's kvm-x86/next or kvm-x86/svm, but it
will conflict with the patches you picked up eventually, and I already
have them locally on top of the LBR fixes so it seems like wasted
effort.

Sean, Paolo, how do you want to handle this?

[1]https://lore.kernel.org/kvm/20251104195949.3528411-1-yosry.ahmed@linux.dev/

> 
> Thanks,
> 
> Paolo
> 
> > Macros are generally not preferred compared to functions, mainly due to
> > type-safety. However, in this case it seems like having a simple macro
> > copying a few fields is better than copy-pasting the same 5 lines of
> > code in different places.
> > 
> > On the bright side, pulling vmcb_mark_dirty() calls to the callers makes
> > it clear that in one case, vmcb_mark_dirty() was being called on VMCB12.
> > It is not architecturally defined for the CPU to clear arbitrary clean
> > bits, and it is not needed, so drop that one call.
> 
> > Technically fixes the non-architectural behavior of setting the dirty
> > bit on VMCB12.
> > 
> > Fixes: d20c796ca370 ("KVM: x86: nSVM: implement nested LBR virtualization")
> > Cc: stable@vger.kernel.org
> > 
> > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > ---
> >   arch/x86/kvm/svm/nested.c | 16 ++++++++++------
> >   arch/x86/kvm/svm/svm.c    | 11 -----------
> >   arch/x86/kvm/svm/svm.h    | 10 +++++++++-
> >   3 files changed, 19 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index c81005b245222..e7861392f2fcd 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -676,10 +676,12 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm, struct vmcb *vmcb12
> >   		 * Reserved bits of DEBUGCTL are ignored.  Be consistent with
> >   		 * svm_set_msr's definition of reserved bits.
> >   		 */
> > -		svm_copy_lbrs(vmcb02, vmcb12);
> > +		svm_copy_lbrs(&vmcb02->save, &vmcb12->save);
> > +		vmcb_mark_dirty(vmcb02, VMCB_LBR);
> >   		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
> >   	} else {
> > -		svm_copy_lbrs(vmcb02, vmcb01);
> > +		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
> > +		vmcb_mark_dirty(vmcb02, VMCB_LBR);
> >   	}
> >   	svm_update_lbrv(&svm->vcpu);
> >   }
> > @@ -1186,10 +1188,12 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> >   		kvm_make_request(KVM_REQ_EVENT, &svm->vcpu);
> >   	if (unlikely(guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
> > -		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK)))
> > -		svm_copy_lbrs(vmcb12, vmcb02);
> > -	else
> > -		svm_copy_lbrs(vmcb01, vmcb02);
> > +		     (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK))) {
> > +		svm_copy_lbrs(&vmcb12->save, &vmcb02->save);
> > +	} else {
> > +		svm_copy_lbrs(&vmcb01->save, &vmcb02->save);
> > +		vmcb_mark_dirty(vmcb01, VMCB_LBR);
> > +	}
> >   	svm_update_lbrv(vcpu);
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index fc42bcdbb5200..9eb112f0e61f0 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -795,17 +795,6 @@ static void svm_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
> >   	 */
> >   }
> > -void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
> > -{
> > -	to_vmcb->save.dbgctl		= from_vmcb->save.dbgctl;
> > -	to_vmcb->save.br_from		= from_vmcb->save.br_from;
> > -	to_vmcb->save.br_to		= from_vmcb->save.br_to;
> > -	to_vmcb->save.last_excp_from	= from_vmcb->save.last_excp_from;
> > -	to_vmcb->save.last_excp_to	= from_vmcb->save.last_excp_to;
> > -
> > -	vmcb_mark_dirty(to_vmcb, VMCB_LBR);
> > -}
> > -
> >   static void __svm_enable_lbrv(struct kvm_vcpu *vcpu)
> >   {
> >   	to_svm(vcpu)->vmcb->control.virt_ext |= LBR_CTL_ENABLE_MASK;
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index c2acaa49ee1c5..e510c8183bd87 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -687,8 +687,16 @@ static inline void *svm_vcpu_alloc_msrpm(void)
> >   	return svm_alloc_permissions_map(MSRPM_SIZE, GFP_KERNEL_ACCOUNT);
> >   }
> > +#define svm_copy_lbrs(to, from)					\
> > +({								\
> > +	(to)->dbgctl		= (from)->dbgctl;		\
> > +	(to)->br_from		= (from)->br_from;		\
> > +	(to)->br_to		= (from)->br_to;		\
> > +	(to)->last_excp_from	= (from)->last_excp_from;	\
> > +	(to)->last_excp_to	= (from)->last_excp_to;		\
> > +})
> > +
> >   void svm_vcpu_free_msrpm(void *msrpm);
> > -void svm_copy_lbrs(struct vmcb *to_vmcb, struct vmcb *from_vmcb);
> >   void svm_enable_lbrv(struct kvm_vcpu *vcpu);
> >   void svm_update_lbrv(struct kvm_vcpu *vcpu);
> Since
> 

Re: [PATCH 4/6] KVM: SVM: Switch svm_copy_lbrs() to a macro

[Sean Christopherson]

On Mon, Nov 10, 2025, Yosry Ahmed wrote:
> On Sun, Nov 09, 2025 at 08:59:18AM +0100, Paolo Bonzini wrote:
> > On 11/8/25 01:45, Yosry Ahmed wrote:
> > > In preparation for using svm_copy_lbrs() with 'struct vmcb_save_area'
> > > without a containing 'struct vmcb', and later even 'struct
> > > vmcb_save_area_cached', make it a macro. Pull the call to
> > > vmcb_mark_dirty() out to the callers.
> > 
> > The changes to use `struct vmcb_save_area_cached' are not included in this
> > series, so they are irrelevant.
> > 
> > Since I've applied patches 1-3, which fix the worst bugs, there are two ways
> > to handle the rest:
> > 
> > * keep the function instead of the macro, while making it take a struct
> > vmcb_save_area (and therefore pulling vmcb_mark_dirty() to the callers and
> > fixing the bug you mention below).
> > 
> > * you resubmit with the changes to use struct vmcb_save_area_cached, so that
> > the commit message makes more sense.
> 
> I can include patches 4-6 with the respin of the series [1] that has the
> changes to use `struct vmcb_save_area_cached`. That series origianlly
> had the patch to switch svm_copy_lbrs() to a macro, but I moved it here
> to use for the save/restore patch. I was planning to rebase [1] on top
> of this series anyway.
> 
> There is a hiccup though, I assumed everything would go through Sean's
> tree so I planned to respin [1] on top of this series. Otherwise, they
> will conflict. With the first 3 patches in your tree, I am not sure how
> that would work.
> 
> I can respin [1] on top of Sean's kvm-x86/next or kvm-x86/svm, but it
> will conflict with the patches you picked up eventually, and I already
> have them locally on top of the LBR fixes so it seems like wasted
> effort.
> 
> Sean, Paolo, how do you want to handle this?

Base your patches on kvm/master, assuming there's nothing in kvm-x86/svm that
you need.  I can create a one-off branch, e.g. kvm-x86/lbrv, based on kvm/master
or 6.18-rc6.

[PATCH 5/6] KVM: SVM: Add missing save/restore handling of LBR MSRs

[Yosry Ahmed]

MSR_IA32_DEBUGCTLMSR and LBR MSRs are currently not enumerated by
KVM_GET_MSR_INDEX_LIST, and LBR MSRs cannot be set with KVM_SET_MSRS. So
save/restore is completely broken.

Fix it by adding the MSRs to msrs_to_save_base, and allowing writes to
LBR MSRs from userspace only (as they are read-only MSRs). Additionally,
to correctly restore L1's LBRs while L2 is running, make sure the LBRs
are copied from the captured VMCB01 save area in svm_copy_vmrun_state().

Fixes: 24e09cbf480a ("KVM: SVM: enable LBR virtualization")
Cc: stable@vger.kernel.org

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 arch/x86/kvm/svm/nested.c |  3 +++
 arch/x86/kvm/svm/svm.c    | 20 ++++++++++++++++++++
 arch/x86/kvm/x86.c        |  3 +++
 3 files changed, 26 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index e7861392f2fcd..955de5d90f48e 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1057,6 +1057,9 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
 		to_save->isst_addr = from_save->isst_addr;
 		to_save->ssp = from_save->ssp;
 	}
+
+	if (lbrv)
+		svm_copy_lbrs(to_save, from_save);
 }
 
 void svm_copy_vmloadsave_state(struct vmcb *to_vmcb, struct vmcb *from_vmcb)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9eb112f0e61f0..45faf892a0431 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2988,6 +2988,26 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
 		svm_update_lbrv(vcpu);
 		break;
+	case MSR_IA32_LASTBRANCHFROMIP:
+		if (!msr->host_initiated)
+			return 1;
+		svm->vmcb->save.br_from = data;
+		break;
+	case MSR_IA32_LASTBRANCHTOIP:
+		if (!msr->host_initiated)
+			return 1;
+		svm->vmcb->save.br_to = data;
+		break;
+	case MSR_IA32_LASTINTFROMIP:
+		if (!msr->host_initiated)
+			return 1;
+		svm->vmcb->save.last_excp_from = data;
+		break;
+	case MSR_IA32_LASTINTTOIP:
+		if (!msr->host_initiated)
+			return 1;
+		svm->vmcb->save.last_excp_to = data;
+		break;
 	case MSR_VM_HSAVE_PA:
 		/*
 		 * Old kernels did not validate the value written to
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9c2e28028c2b5..1d36a0f782c49 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -348,6 +348,9 @@ static const u32 msrs_to_save_base[] = {
 	MSR_IA32_U_CET, MSR_IA32_S_CET,
 	MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
 	MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
+	MSR_IA32_DEBUGCTLMSR,
+	MSR_IA32_LASTBRANCHFROMIP, MSR_IA32_LASTBRANCHTOIP,
+	MSR_IA32_LASTINTFROMIP, MSR_IA32_LASTINTTOIP,
 };
 
 static const u32 msrs_to_save_pmu[] = {
-- 
2.51.2.1041.gc1ab5b90ca-goog

Re: [PATCH 5/6] KVM: SVM: Add missing save/restore handling of LBR MSRs

[Yosry Ahmed]

November 7, 2025 at 4:45 PM, "Yosry Ahmed" <yosry.ahmed@linux.dev mailto:yosry.ahmed@linux.dev?to=%22Yosry%20Ahmed%22%20%3Cyosry.ahmed%40linux.dev%3E > wrote:
> 
> MSR_IA32_DEBUGCTLMSR and LBR MSRs are currently not enumerated by
> KVM_GET_MSR_INDEX_LIST, and LBR MSRs cannot be set with KVM_SET_MSRS. So
> save/restore is completely broken.
> 
> Fix it by adding the MSRs to msrs_to_save_base, and allowing writes to
> LBR MSRs from userspace only (as they are read-only MSRs). Additionally,
> to correctly restore L1's LBRs while L2 is running, make sure the LBRs
> are copied from the captured VMCB01 save area in svm_copy_vmrun_state().
> 
> Fixes: 24e09cbf480a ("KVM: SVM: enable LBR virtualization")
> Cc: stable@vger.kernel.org
>

Reported-by: Jim Mattson <jmattson@google.com>

> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
[..]