Message-ID: <0b74d069-51ed-4e5e-9d76-6b1a60e689df@amd.com>
Date: Wed, 10 Jul 2024 07:55:24 +0530
From: Ravi Bangoria <ravi.bangoria@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com, pbonzini@...hat.com, thomas.lendacky@....com,
 hpa@...or.com, rmk+kernel@...linux.org.uk, peterz@...radead.org,
 james.morse@....com, lukas.bulwahn@...il.com, arjan@...ux.intel.com,
 j.granados@...sung.com, sibs@...natelecom.cn, nik.borisov@...e.com,
 michael.roth@....com, nikunj.dadhania@....com, babu.moger@....com,
 x86@...nel.org, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 santosh.shukla@....com, ananth.narayan@....com, sandipan.das@....com,
 manali.shukla@....com
Subject: Re: [PATCH 3/3] KVM SVM: Add Bus Lock Detect support

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@amd.com

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?

Thanks,
Ravi

---><---
>From 0b01859f99c4c5e18323e3f4ac19d1f3e5ad4aee Mon Sep 17 00:00:00 2001
From: Ravi Bangoria <ravi.bangoria@....com>
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@....com>
---
 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

Powered by blists - more mailing lists