Received-SPF: pass (google.com: domain of linux-kernel+bounces-202804-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 5 Jun 2024 08:08:54 -0700
In-Reply-To: <e1c29dd4-2eb9-44fe-abf2-f5ca0e84e2a6@amd.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240429060643.211-1-ravi.bangoria@amd.com> <20240429060643.211-4-ravi.bangoria@amd.com>
 <Zl5jqwWO4FyawPHG@google.com> <e1c29dd4-2eb9-44fe-abf2-f5ca0e84e2a6@amd.com>
Message-ID: <ZmB_hl7coZ_8KA8Q@google.com>
Subject: Re: [PATCH 3/3] KVM SVM: Add Bus Lock Detect support
From: Sean Christopherson <seanjc@google.com>
To: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, 
	dave.hansen@linux.intel.com, pbonzini@redhat.com, thomas.lendacky@amd.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
Content-Type: text/plain; charset="us-ascii"

On Wed, Jun 05, 2024, Ravi Bangoria wrote:
> Hi Sean,
> 
> On 6/4/2024 6:15 AM, Sean Christopherson wrote:
> > On Mon, Apr 29, 2024, Ravi Bangoria wrote:
> >> Upcoming AMD uarch will support Bus Lock Detect. Add support for it
> >> in KVM. Bus Lock Detect is enabled through MSR_IA32_DEBUGCTLMSR and
> >> MSR_IA32_DEBUGCTLMSR is virtualized only if LBR Virtualization is
> >> enabled. Add this dependency in the KVM.
> > 
> > This is woefully incomplete, e.g. db_interception() needs to be updated to decipher
> > whether the #DB is the responsbility of the host or of the guest.
> 
> Can you please elaborate. Are you referring to vcpu->guest_debug thingy?

Yes.  More broadly, all of db_interception().

> > Honestly, I don't see any point in virtualizing this in KVM.  As Jim alluded to,
> > what's far, far more interesting for KVM is "Bus Lock Threshold".  Virtualizing
> > this for the guest would have been nice to have during the initial split-lock #AC
> > support, but now I'm skeptical the complexity is worth the payoff.
> 
> This has a valid usecase of penalizing offending processes. I'm not sure
> how much it's really used in the production though.

Yeah, but split-lock #AC and #DB have existed on Intel for years, and no one has
put in the effort to land KVM support, despite the series getting as far as v9[*].
Some of the problems on Intel were due to the awful FMS-based feature detection,
but those weren't the only hiccups.  E.g. IIRC, we never sorted out what should
happen if both the host and guest want bus-lock #DBs.

Anyways, my point is that, except for SEV-ES+ where there's no good reason NOT to
virtualize Bus Lock Detect, I'm not convinced that it's worth virtualizing bus-lock
#DBs.

[*] https://lore.kernel.org/all/20200509110542.8159-1-xiaoyao.li@intel.com

> > I suppose we could allow it if #DB isn't interecepted, at which point the enabling
> > required is minimal?
> 
> The feature uses DEBUG_CTL MSR, #DB and DR6 register. Do you mean expose
> it when all three are accelerated or just #DB?

I mean that if KVM isn't intercepting #DB, then there's no extra complexity needed
to sort out whether the #DB "belongs" to the host or the guest.  See commit
90cbf6d914ad ("KVM: SEV-ES: Eliminate #DB intercept when DebugSwap enabled").