Re: [PATCH 2/3] KVM: SVM: Manually zero/restore DEBUGCTL if LBR virtualization is disabled

[Ravi Bangoria <ravi.bangoria@amd.com>]

Hi Sean,

On 24-Feb-25 11:43 PM, Sean Christopherson wrote:
> Manually zero DEBUGCTL prior to VMRUN if the host's value is non-zero and
> LBR virtualization is disabled, as hardware only context switches DEBUGCTL
> if LBR virtualization is fully enabled.  Running the guest with the host's
> value has likely been mildly problematic for quite some time, e.g. it will
> result in undesirable behavior if host is running with BTF=1.
> 
> But the bug became fatal with the introduction of Bus Lock Trap ("Detect"
> in kernel paralance) support for AMD (commit 408eb7417a92
> ("x86/bus_lock: Add support for AMD")), as a bus lock in the guest will
> trigger an unexpected #DB.
> 
> Note, suppressing the bus lock #DB, i.e. simply resuming the guest without
> injecting a #DB, is not an option.  It wouldn't address the general issue
> with DEBUGCTL, e.g. for things like BTF, and there are other guest-visible
> side effects if BusLockTrap is left enabled.
> 
> If BusLockTrap is disabled, then DR6.BLD is reserved-to-1; any attempts to
> clear it by software are ignored.  But if BusLockTrap is enabled, software
> can clear DR6.BLD:
> 
>   Software enables bus lock trap by setting DebugCtl MSR[BLCKDB] (bit 2)
>   to 1.  When bus lock trap is enabled, ... The processor indicates that
>   this #DB was caused by a bus lock by clearing DR6[BLD] (bit 11).  DR6[11]
>   previously had been defined to be always 1.
> 
> and clearing DR6.BLD is "sticky" in that it's not set (i.e. lowered) by
> other #DBs:
> 
>   All other #DB exceptions leave DR6[BLD] unmodified
> 
> E.g. leaving BusLockTrap enable can confuse a legacy guest that writes '0'
> to reset DR6.

What if guest sets DEBUGCTL[BusLockTrapEn] and runs an application which
causes a bus lock? Guest will receive #DB due to bus lock, even though
guest CPUID says BusLockTrap isn't supported. Should KVM prevent guest
to write to DEBUGCTL[BusLockTrapEn]? Something like:

---
@@ -3168,6 +3168,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
 		if (data & DEBUGCTL_RESERVED_BITS)
 			return 1;
 
+		if ((data & DEBUGCTLMSR_BUS_LOCK_DETECT) &&
+		    !guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
+			return 1;
+
 		svm_get_lbr_vmcb(svm)->save.dbgctl = data;
 		svm_update_lbrv(vcpu);
 		break;
---

Thanks,
Ravi