All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Shivansh Dhiman <shivansh.dhiman@amd.com>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>,
	tglx@linutronix.de, mingo@redhat.com,  bp@alien8.de,
	dave.hansen@linux.intel.com, pbonzini@redhat.com,
	 thomas.lendacky@amd.com, jmattson@google.com, hpa@zytor.com,
	 rmk+kernel@armlinux.org.uk, peterz@infradead.org,
	james.morse@arm.com,  lukas.bulwahn@gmail.com,
	arjan@linux.intel.com, j.granados@samsung.com,
	 sibs@chinatelecom.cn, nik.borisov@suse.com,
	michael.roth@amd.com,  nikunj.dadhania@amd.com,
	babu.moger@amd.com, x86@kernel.org,  kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, santosh.shukla@amd.com,
	 ananth.narayan@amd.com, sandipan.das@amd.com,
	manali.shukla@amd.com,  yosry.ahmed@linux.dev
Subject: Re: [PATCH v4 4/4] KVM: SVM: Add Bus Lock Detect support
Date: Tue, 11 Nov 2025 07:15:12 -0800	[thread overview]
Message-ID: <aRNTADUbIGze6Vyt@google.com> (raw)
In-Reply-To: <a704b1f7-a550-4c38-b58d-9bc0783019f1@amd.com>

On Tue, Nov 11, 2025, Shivansh Dhiman wrote:
> On 17-08-2024 05:43, Sean Christopherson wrote:
> > On Thu, Aug 08, 2024, Ravi Bangoria wrote:
> >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> >> index e1b6a16e97c0..9f3d31a5d231 100644
> >> --- a/arch/x86/kvm/svm/svm.c
> >> +++ b/arch/x86/kvm/svm/svm.c
> >> @@ -1047,7 +1047,8 @@ 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) ||
> >> +	u64 dbgctl_buslock_lbr = DEBUGCTLMSR_BUS_LOCK_DETECT | DEBUGCTLMSR_LBR;
> >> +	bool enable_lbrv = (svm_get_lbr_vmcb(svm)->save.dbgctl & dbgctl_buslock_lbr) ||
> >>  			    (is_guest_mode(vcpu) && guest_can_use(vcpu, X86_FEATURE_LBRV) &&
> >>  			    (svm->nested.ctl.virt_ext & LBR_CTL_ENABLE_MASK));
> > 
> > Out of sight, but this leads to calling svm_enable_lbrv() even when the guest
> > just wants to enable BUS_LOCK_DETECT.  Ignoring SEV-ES guests, KVM will intercept
> > writes to DEBUGCTL, so can't KVM defer mucking with the intercepts and
> > svm_copy_lbrs() until the guest actually wants to use LBRs?
> > 
> > Hmm, and I think the existing code is broken.  If L1 passes DEBUGCTL through to
> > L2, then KVM will handles writes to L1's effective value.  And if L1 also passes
> > through the LBRs, then KVM will fail to update the MSR bitmaps for vmcb02.
> > 
> > Ah, it's just a performance issue though, because KVM will still emulate RDMSR.
> > 
> > Ugh, this code is silly.  The LBR MSRs are read-only, yet KVM passes them through
> > for write.
> > 
> > Anyways, I'm thinking something like this?  Note, using msr_write_intercepted()
> > is wrong, because that'll check L2's bitmap if is_guest_mode(), and the idea is
> > to use L1's bitmap as the canary.

...

> ===========================================================
> Issue 1: Interception still enabled after enabling LBRV
> ===========================================================
> Using the 6.16 upstream kernel (unpatched) I ran the KUT tests and they passed
> when run from both the bare metal and from inside a L1 guest. However for L2
> guest, when looking at the logs I found that RDMSR interception of LBR MSRs is
> still enabled despite the LBRV is enabled for the L2 guest. Effectively, the
> reads are emulated instead of being virtualized, which is not the intended
> behaviour. KUT cannot distinguish between emulated and virtualized RDMSR, and
> hence the test passes regardless.

I haven't looked closely at your patch or at Yosry's patches, but I suspect this
was _just_ fixed:

https://lore.kernel.org/all/20251108004524.1600006-1-yosry.ahmed@linux.dev

> ===========================================================
> Issue 2: Basic LBR KUT fails with Sean's implementation
> ===========================================================
> After using your implementation, all KUTs passed on the bare metal. With LBRV
> enabled for L2, RDMSR interception of LBR MSRs is disabled as intended.
> However, when running KUT tests inside an L1 guest, the tests fail.

Same story here: I haven't had cycles to actually look at code, but Yosry also
posted a pile of changes for KUT:

https://lore.kernel.org/all/20251110232642.633672-1-yosry.ahmed@linux.dev

  reply	other threads:[~2025-11-11 15:15 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-08  6:29 [PATCH v4 0/4] x86/cpu: Add Bus Lock Detect support for AMD Ravi Bangoria
2024-08-08  6:29 ` [PATCH v4 1/4] x86/split_lock: Move Split and Bus lock code to a dedicated file Ravi Bangoria
2024-08-08 16:07   ` [tip: x86/splitlock] " tip-bot2 for Ravi Bangoria
2024-08-08  6:29 ` [PATCH v4 2/4] x86/bus_lock: Add support for AMD Ravi Bangoria
2024-08-08 16:07   ` [tip: x86/splitlock] " tip-bot2 for Ravi Bangoria
2024-08-08  6:29 ` [PATCH v4 3/4] KVM: SVM: Don't advertise Bus Lock Detect to guest if SVM support is missing Ravi Bangoria
2024-08-08  6:29 ` [PATCH v4 4/4] KVM: SVM: Add Bus Lock Detect support Ravi Bangoria
2024-08-17  0:13   ` Sean Christopherson
2024-08-20 16:38     ` Ravi Bangoria
2024-09-05 11:40       ` Ravi Bangoria
2024-08-21  5:36     ` Ravi Bangoria
2024-08-21 10:40       ` Ravi Bangoria
2024-08-26 18:31         ` Sean Christopherson
2025-11-11 10:03     ` Shivansh Dhiman
2025-11-11 15:15       ` Sean Christopherson [this message]
2025-11-14  8:50         ` Shivansh Dhiman
2024-08-23 23:47 ` [PATCH v4 0/4] x86/cpu: Add Bus Lock Detect support for AMD Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aRNTADUbIGze6Vyt@google.com \
    --to=seanjc@google.com \
    --cc=ananth.narayan@amd.com \
    --cc=arjan@linux.intel.com \
    --cc=babu.moger@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=j.granados@samsung.com \
    --cc=james.morse@arm.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=manali.shukla@amd.com \
    --cc=michael.roth@amd.com \
    --cc=mingo@redhat.com \
    --cc=nik.borisov@suse.com \
    --cc=nikunj.dadhania@amd.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=sandipan.das@amd.com \
    --cc=santosh.shukla@amd.com \
    --cc=shivansh.dhiman@amd.com \
    --cc=sibs@chinatelecom.cn \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=x86@kernel.org \
    --cc=yosry.ahmed@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.