Re: [PATCH] KVM: SVM: Add a helper to get LBR field pointer to dedup MSR accesses
From: Sean Christopherson
Date: Fri Mar 13 2026 - 16:17:51 EST
On Fri, Mar 13, 2026, Yosry Ahmed wrote:
> On Tue, Mar 10, 2026 at 3:04 PM Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx>
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> > 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).