On Tue, Jul 9, 2024 at 7:25 PM Ravi Bangoria <ravi.bangoria@xxxxxxx> wrote: > > Sean, > > Apologies for the delay. I was waiting for Bus Lock Threshold patches to be > posted upstream: > > https://lore.kernel.org/r/20240709175145.9986-1-manali.shukla@xxxxxxx > > On 12-Jun-24 7:12 AM, Sean Christopherson wrote: > > On Wed, Jun 05, 2024, Ravi Bangoria wrote: > >> On 6/5/2024 8:38 PM, Sean Christopherson wrote: > >>> 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. > >> > >> I've to check about vcpu->guest_debug part, but keeping that aside, host and > >> guest can use Bus Lock Detect in parallel because, DEBUG_CTL MSR and DR6 > >> register are save/restored in VMCB, hardware cause a VMEXIT_EXCEPTION_1 for > >> guest #DB(when intercepted) and hardware raises #DB on host when it's for the > >> host. > > > > I'm talking about the case where the host wants to do something in response to > > bus locks that occurred in the guest. E.g. if the host is taking punitive action, > > say by stalling the vCPU, then the guest kernel could bypass that behavior by > > enabling bus lock detect itself. > > > > Maybe it's moot point in practice, since it sounds like Bus Lock Threshold will > > be available at the same time. > > > > Ugh, and if we wanted to let the host handle guest-induced #DBs, we'd need code > > to keep Bus Lock Detect enabled in the guest since it resides in DEBUG_CTL. Bah. > > > > So I guess if the vcpu->guest_debug part is fairly straightforward, it probably > > makes to virtualize Bus Lock Detect because the only reason not to virtualize it > > would actually require more work/code in KVM. > > KVM forwards #DB to Qemu when vcpu->guest_debug is set and it's Qemu's > responsibility to re-inject exception when Bus Lock Trap is enabled > inside the guest. I realized that it is broken so I've prepared a > Qemu patch, embedding it at the end. > > > I'd still love to see Bus Lock Threshold support sooner than later though :-) > > With Bus Lock Threshold enabled, I assume the changes introduced by this > patch plus Qemu fix are sufficient to support Bus Lock Trap inside the > guest? In any case, it seems that commit 76ea438b4afc ("KVM: X86: Expose bus lock debug exception to guest") prematurely advertised the presence of X86_FEATURE_BUS_LOCK to userspace on non-Intel platforms. We should probably either accept these changes or fix up that commit. Either way, something should be done for all active branches back to v5.15. > Thanks, > Ravi > > ---><--- > From 0b01859f99c4c5e18323e3f4ac19d1f3e5ad4aee Mon Sep 17 00:00:00 2001 > From: Ravi Bangoria <ravi.bangoria@xxxxxxx> > Date: Thu, 4 Jul 2024 06:48:24 +0000 > Subject: [PATCH] target/i386: Add Bus Lock Detect support > > Upcoming AMD uarch will support Bus Lock Detect (called Bus Lock Trap > in AMD docs). Bus Lock Detect is enumerated with cpuid Fn0000_0007_ECX_x0 > bit [24 / BUSLOCKTRAP]. It can be enabled through MSR_IA32_DEBUGCTLMSR. > When enabled, hardware clears DR6[11] and raises a #DB exception on > occurrence of Bus Lock if CPL > 0. More detail about the feature can be > found in AMD APM[1]. > > Qemu supports remote debugging through host gdb (the "gdbstub" facility) > where some of the remote debugging features like instruction and data > breakpoints relies on the same hardware infrastructure (#DB, DR6 etc.) > that Bus Lock Detect also uses. Instead of handling internally, KVM > forwards #DB to Qemu when remote debugging is ON and #DB is being > intercepted. It's Qemu's responsibility to re-inject the exception to > guest when some of the exception source bits (in DR6) are not being > handled by Qemu remote debug handler. Bus Lock Detect is one such case. > > [1]: AMD64 Architecture Programmer's Manual Pub. 40332, Rev. 4.07 - June > 2023, Vol 2, 13.1.3.6 Bus Lock Trap > https://bugzilla.kernel.org/attachment.cgi?id=304653 > > Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxx> > --- > target/i386/cpu.h | 1 + > target/i386/kvm/kvm.c | 4 ++-- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index c64ef0c1a2..89bcff2fa3 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -271,6 +271,7 @@ typedef enum X86Seg { > | CR4_SMEP_MASK | CR4_SMAP_MASK | CR4_PKE_MASK | CR4_PKS_MASK \ > | CR4_LAM_SUP_MASK)) > > +#define DR6_BLD (1 << 11) > #define DR6_BD (1 << 13) > #define DR6_BS (1 << 14) > #define DR6_BT (1 << 15) > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 6c864e4611..d128d4e5ca 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -5141,14 +5141,14 @@ static int kvm_handle_debug(X86CPU *cpu, > } else if (kvm_find_sw_breakpoint(cs, arch_info->pc)) { > ret = EXCP_DEBUG; > } > - if (ret == 0) { > + if (ret == 0 || !(arch_info->dr6 & DR6_BLD)) { > cpu_synchronize_state(cs); > assert(env->exception_nr == -1); > > /* pass to guest */ > kvm_queue_exception(env, arch_info->exception, > arch_info->exception == EXCP01_DB, > - arch_info->dr6); > + ret == 0 ? arch_info->dr6 ^ DR6_BLD : DR6_BLD); > env->has_error_code = 0; > } > > -- > 2.34.1 >
