[PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Sean Christopherson]

Add a helper to get a pointer to the corresponding VMCB field given an LBR
MSR index, and use it to dedup the handling in svm_{g,s}et_msr().

No functional change intended.

Suggested-by: Yosry Ahmed <yosry@kernel.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c | 48 +++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3407deac90bd..88999b6fc8a3 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2750,6 +2750,23 @@ static int svm_get_feature_msr(u32 msr, u64 *data)
 	return 0;
 }
 
+static __always_inline u64 *svm_vmcb_lbr(struct vcpu_svm *svm, u32 msr)
+{
+	switch (msr) {
+	case MSR_IA32_LASTBRANCHFROMIP:
+		return &svm->vmcb->save.br_from;
+	case MSR_IA32_LASTBRANCHTOIP:
+		return &svm->vmcb->save.br_to;
+	case MSR_IA32_LASTINTFROMIP:
+		return &svm->vmcb->save.last_excp_from;
+	case MSR_IA32_LASTINTTOIP:
+		return &svm->vmcb->save.last_excp_to;
+	default:
+		break;
+	}
+	BUILD_BUG();
+}
+
 static bool sev_es_prevent_msr_access(struct kvm_vcpu *vcpu,
 				      struct msr_data *msr_info)
 {
@@ -2826,16 +2843,10 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = lbrv ? svm->vmcb->save.dbgctl : 0;
 		break;
 	case MSR_IA32_LASTBRANCHFROMIP:
-		msr_info->data = lbrv ? svm->vmcb->save.br_from : 0;
-		break;
 	case MSR_IA32_LASTBRANCHTOIP:
-		msr_info->data = lbrv ? svm->vmcb->save.br_to : 0;
-		break;
 	case MSR_IA32_LASTINTFROMIP:
-		msr_info->data = lbrv ? svm->vmcb->save.last_excp_from : 0;
-		break;
 	case MSR_IA32_LASTINTTOIP:
-		msr_info->data = lbrv ? svm->vmcb->save.last_excp_to : 0;
+		msr_info->data = lbrv ? *svm_vmcb_lbr(svm, msr_info->index) : 0;
 		break;
 	case MSR_VM_HSAVE_PA:
 		msr_info->data = svm->nested.hsave_msr;
@@ -3111,35 +3122,14 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		svm_update_lbrv(vcpu);
 		break;
 	case MSR_IA32_LASTBRANCHFROMIP:
-		if (!lbrv)
-			return KVM_MSR_RET_UNSUPPORTED;
-		if (!msr->host_initiated)
-			return 1;
-		svm->vmcb->save.br_from = data;
-		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
-		break;
 	case MSR_IA32_LASTBRANCHTOIP:
-		if (!lbrv)
-			return KVM_MSR_RET_UNSUPPORTED;
-		if (!msr->host_initiated)
-			return 1;
-		svm->vmcb->save.br_to = data;
-		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
-		break;
 	case MSR_IA32_LASTINTFROMIP:
-		if (!lbrv)
-			return KVM_MSR_RET_UNSUPPORTED;
-		if (!msr->host_initiated)
-			return 1;
-		svm->vmcb->save.last_excp_from = data;
-		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
-		break;
 	case MSR_IA32_LASTINTTOIP:
 		if (!lbrv)
 			return KVM_MSR_RET_UNSUPPORTED;
 		if (!msr->host_initiated)
 			return 1;
-		svm->vmcb->save.last_excp_to = data;
+		*svm_vmcb_lbr(svm, ecx) = data;
 		vmcb_mark_dirty(svm->vmcb, VMCB_LBR);
 		break;
 	case MSR_VM_HSAVE_PA:

base-commit: 5128b972fb2801ad9aca54d990a75611ab5283a9
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Yosry Ahmed]

On Tue, Mar 10, 2026 at 3:04 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Add a helper to get a pointer to the corresponding VMCB field given an LBR
> MSR index, and use it to dedup the handling in svm_{g,s}et_msr().
>
> No functional change intended.
>
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

LGTM:

Reviewed-by: Yosry Ahmed <yosry@kernel.org>

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Yosry Ahmed]

On Tue, Mar 10, 2026 at 3:04 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Add a helper to get a pointer to the corresponding VMCB field given an LBR
> MSR index, and use it to dedup the handling in svm_{g,s}et_msr().
>
> No functional change intended.
>
> Suggested-by: Yosry Ahmed <yosry@kernel.org>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/svm/svm.c | 48 +++++++++++++++++-------------------------
>  1 file changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3407deac90bd..88999b6fc8a3 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2750,6 +2750,23 @@ static int svm_get_feature_msr(u32 msr, u64 *data)
>         return 0;
>  }
>
> +static __always_inline u64 *svm_vmcb_lbr(struct vcpu_svm *svm, u32 msr)
> +{
> +       switch (msr) {
> +       case MSR_IA32_LASTBRANCHFROMIP:
> +               return &svm->vmcb->save.br_from;
> +       case MSR_IA32_LASTBRANCHTOIP:
> +               return &svm->vmcb->save.br_to;
> +       case MSR_IA32_LASTINTFROMIP:
> +               return &svm->vmcb->save.last_excp_from;
> +       case MSR_IA32_LASTINTTOIP:
> +               return &svm->vmcb->save.last_excp_to;
> +       default:
> +               break;
> +       }
> +       BUILD_BUG();
> +}

This actually fires for me with clang (from both callsites):

/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/arch/x86/kvm/svm/svm.c:2766:2:
error: call to '__compiletime_assert_1058' declared with 'error'
attribute: BUILD_BUG failed
 2766 |         BUILD_BUG();
      |         ^
/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/build_bug.h:59:21:
note: expanded from macro 'BUILD_BUG'
   59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
      |                     ^
/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/build_bug.h:39:37:
note: expanded from macro 'BUILD_BUG_ON_MSG'
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^
/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:706:2:
note: expanded from macro 'compiletime_assert'
  706 |         _compiletime_assert(condition, msg,
__compiletime_assert_, __COUNTER__)
      |         ^
/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:694:2:
note: expanded from macro '_compiletime_assert'
  694 |         __compiletime_assert(condition, msg, prefix, suffix)
      |         ^
/usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:687:4:
note: expanded from macro '__compiletime_assert'
  687 |                         prefix ## suffix();
         \
      |                         ^
<scratch space>:102:1: note: expanded from here
  102 | __compiletime_assert_1058
      | ^

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Sean Christopherson]

On Fri, Mar 13, 2026, Yosry Ahmed wrote:
> On Tue, Mar 10, 2026 at 3:04 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > Add a helper to get a pointer to the corresponding VMCB field given an LBR
> > MSR index, and use it to dedup the handling in svm_{g,s}et_msr().
> >
> > No functional change intended.
> >
> > Suggested-by: Yosry Ahmed <yosry@kernel.org>
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/svm/svm.c | 48 +++++++++++++++++-------------------------
> >  1 file changed, 19 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 3407deac90bd..88999b6fc8a3 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2750,6 +2750,23 @@ static int svm_get_feature_msr(u32 msr, u64 *data)
> >         return 0;
> >  }
> >
> > +static __always_inline u64 *svm_vmcb_lbr(struct vcpu_svm *svm, u32 msr)
> > +{
> > +       switch (msr) {
> > +       case MSR_IA32_LASTBRANCHFROMIP:
> > +               return &svm->vmcb->save.br_from;
> > +       case MSR_IA32_LASTBRANCHTOIP:
> > +               return &svm->vmcb->save.br_to;
> > +       case MSR_IA32_LASTINTFROMIP:
> > +               return &svm->vmcb->save.last_excp_from;
> > +       case MSR_IA32_LASTINTTOIP:
> > +               return &svm->vmcb->save.last_excp_to;
> > +       default:
> > +               break;
> > +       }
> > +       BUILD_BUG();
> > +}
> 
> This actually fires for me with clang (from both callsites):
> 
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/arch/x86/kvm/svm/svm.c:2766:2:
> error: call to '__compiletime_assert_1058' declared with 'error'
> attribute: BUILD_BUG failed
>  2766 |         BUILD_BUG();
>       |         ^
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/build_bug.h:59:21:
> note: expanded from macro 'BUILD_BUG'
>    59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>       |                     ^
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/build_bug.h:39:37:
> note: expanded from macro 'BUILD_BUG_ON_MSG'
>    39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>       |                                     ^
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:706:2:
> note: expanded from macro 'compiletime_assert'
>   706 |         _compiletime_assert(condition, msg,
> __compiletime_assert_, __COUNTER__)
>       |         ^
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:694:2:
> note: expanded from macro '_compiletime_assert'
>   694 |         __compiletime_assert(condition, msg, prefix, suffix)
>       |         ^
> /usr/local/google/home/yosryahmed/dev/kernel.org/nsvm_fixups/include/linux/compiler_types.h:687:4:
> note: expanded from macro '__compiletime_assert'
>   687 |                         prefix ## suffix();
>          \
>       |                         ^
> <scratch space>:102:1: note: expanded from here
>   102 | __compiletime_assert_1058
>       | ^

Hrm, I was assuming the compiler would be smart to understand the other cases
are unreachable.  What clang version, and are you doing anything special with
your config?  Even with KASAN and other sanitizers enabled, clang-19 is a-ok with
the code.  And I haven't gotten yelled at by build bots (yet).

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Yosry Ahmed]

> Hrm, I was assuming the compiler would be smart to understand the other cases
> are unreachable.  What clang version, and are you doing anything special with
> your config?

Debian clang version 18.1.8 (20+build1)

Nothing special with my config AFAICT.

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Sean Christopherson]

On Fri, Mar 13, 2026, Yosry Ahmed wrote:
> > Hrm, I was assuming the compiler would be smart to understand the other cases
> > are unreachable.  What clang version, and are you doing anything special with
> > your config?
> 
> Debian clang version 18.1.8 (20+build1)
> 
> Nothing special with my config AFAICT.

Lame.  Given that it's Friday and I'm about to become a pumpkin, and it doesn't
really matter if it's compile-timer assert or not, I'm going to sqaush it to this:

static u64 *svm_vmcb_lbr(struct vcpu_svm *svm, u32 msr)
{
	switch (msr) {
	case MSR_IA32_LASTBRANCHFROMIP:
		return &svm->vmcb->save.br_from;
	case MSR_IA32_LASTBRANCHTOIP:
		return &svm->vmcb->save.br_to;
	case MSR_IA32_LASTINTFROMIP:
		return &svm->vmcb->save.last_excp_from;
	case MSR_IA32_LASTINTTOIP:
		return &svm->vmcb->save.last_excp_to;
	default:
		break;
	}
	KVM_BUG_ON(1, svm->vcpu.kvm);
	return &svm->vmcb->save.br_from;
}

Can you double check that kvm-x86/nested builds for you?  In the process of
rebuilding kvm-x86/next...

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Yosry Ahmed]

On Fri, Mar 13, 2026 at 3:17 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Mar 13, 2026, Yosry Ahmed wrote:
> > > Hrm, I was assuming the compiler would be smart to understand the other cases
> > > are unreachable.  What clang version, and are you doing anything special with
> > > your config?
> >
> > Debian clang version 18.1.8 (20+build1)
> >
> > Nothing special with my config AFAICT.
>
> Lame.  Given that it's Friday and I'm about to become a pumpkin, and it doesn't
> really matter if it's compile-timer assert or not, I'm going to sqaush it to this:
>
> static u64 *svm_vmcb_lbr(struct vcpu_svm *svm, u32 msr)
> {
>         switch (msr) {
>         case MSR_IA32_LASTBRANCHFROMIP:
>                 return &svm->vmcb->save.br_from;
>         case MSR_IA32_LASTBRANCHTOIP:
>                 return &svm->vmcb->save.br_to;
>         case MSR_IA32_LASTINTFROMIP:
>                 return &svm->vmcb->save.last_excp_from;
>         case MSR_IA32_LASTINTTOIP:
>                 return &svm->vmcb->save.last_excp_to;
>         default:
>                 break;
>         }
>         KVM_BUG_ON(1, svm->vcpu.kvm);
>         return &svm->vmcb->save.br_from;
> }
>
> Can you double check that kvm-x86/nested builds for you?  In the process of
> rebuilding kvm-x86/next...

It builds. Thanks for taking care of this.

Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses

[Sean Christopherson]

On Tue, 10 Mar 2026 15:04:14 -0700, Sean Christopherson wrote:
> Add a helper to get a pointer to the corresponding VMCB field given an LBR
> MSR index, and use it to dedup the handling in svm_{g,s}et_msr().
> 
> No functional change intended.

Applied to kvm-x86 nested (on March 13th...).

[1/1] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses
      https://github.com/kvm-x86/linux/commit/0b4a043a5414

--
https://github.com/kvm-x86/linux/tree/next