Re: [kvm-unit-tests PATCH 2/2] nVMX: Test IA32_DEBUGCTLMSR behavior on set and cleared save/load debug controls

[Sean Christopherson <seanjc@google.com>]

On Mon, Aug 11, 2025, Xiaoyao Li wrote:
> On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
> > Besides the existing DR7 test on debug controls, introduce a similar
> > separate test for IA32_DEBUGCTLMSR.
> > 
> > Previously, the IA32_DEBUGCTLMSR was combined with the DR7 test. However,
> > it attempted to access the LBR and BTF bits in the MSR which can be
> > invalid. Although KVM will exempt these two bits from validity check,
> > they will be cleared and resulted in the unexpected MSR value.
> 
> Initially, LBR (bit 0) and BTF(bit 1) have been allowed to write by the
> guest but the value are dropped, as the workaround to not break some OS that
> writes to the MSR unconditionally.
> 
> BTF never gets supported by KVM. While LBR gained support but it depends on
> PMU and LBR being exposed to the guest, which requires additional parameters
> to the test configuration.
> 
> On the other hand, DEBUGCTLMSR_BUS_LOCK_DETECT chosen by this patch doesn't
> require additional parameter, but it requires the hardware support of the
> feature. IIRC, it needs to be SPR and later. So it has less coverage of
> hardwares than LBR.
> 
> I'm not sure the preference of Sean/Paolo. Let's see what they would say.

I would say add dedicated tests (for BUS_LOCK_DETECT and LBR), and get any coverage
for KVM's handling of DEBUGCTL reads and writes as a side effect.

> > In this new test, access a valid bit (DEBUGCTLMSR_BUS_LOCK_DETECT, bit 2)
> > based on the enumration of Bus Lock Detect.
> > 
> > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
> > ---
> >   x86/vmx_tests.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 88 insertions(+)
> > 
> > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> > index 1832bda3..9a2e598f 100644
> > --- a/x86/vmx_tests.c
> > +++ b/x86/vmx_tests.c
> > @@ -1944,6 +1944,92 @@ static int dbgctls_dr7_exit_handler(union exit_reason exit_reason)
> >   	return VMX_TEST_VMEXIT;
> >   }
> > +static int dbgctls_msr_init(struct vmcs *vmcs)
> > +{
> > +	/* Check for DEBUGCTLMSR_BUS_LOCK_DETECT(bit 2) in IA32_DEBUGCTLMSR */
> > +	if (!(cpuid(7).c & (1 << 24))) {

	if (this_cpu_has(X86_FEATURE_BUS_LOCK_DETECT))

> > +		report_skip("%s : \"Bus Lock Detect\" not supported", __func__);
> > +		return VMX_TEST_VMSKIP;
> > +	}
> > +
> > +	msr_bmp_init();
> > +	wrmsr(MSR_IA32_DEBUGCTLMSR, 0x0);
> > +	vmcs_write(GUEST_DEBUGCTL, 0x4);

Please add #defines, i.e. don't use open coded magic literals.

> > +	vmcs_write(ENT_CONTROLS, vmcs_read(ENT_CONTROLS) | ENT_LOAD_DBGCTLS);
> > +	vmcs_write(EXI_CONTROLS, vmcs_read(EXI_CONTROLS) | EXI_SAVE_DBGCTLS);