Re: [kvm-unit-tests PATCH 1/2] nVMX: Remove the IA32_DEBUGCTLMSR access in debugctls test

[Chenyi Qiang <chenyi.qiang@intel.com>]

On 8/11/2025 3:29 PM, Xiaoyao Li wrote:
> On 8/11/2025 2:30 PM, Chenyi Qiang wrote:
>> Current debug controls test can pass but will trigger some error
>> messages because it tries to access LBR (bit 0) and BTF (bit 1) in
>> IA32_DEBUGCTLMSR:
>>
>>    kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x407de7 Unhandled WRMSR(0x1d9) = 0x1
>>    kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x0 Unhandled WRMSR(0x1d9) = 0x2
>>    kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40936f Unhandled WRMSR(0x1d9) = 0x3
>>    kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40cf09 Unhandled WRMSR(0x1d9) = 0x1
>>    kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40940d Unhandled WRMSR(0x1d9) = 0x3
>>
>> The IA32_DEBUGCTLMSR value isn't used as a criterion for determining
>> whether the test is passed. It only provides some hints on the expected
>> values with the control of {ENT_LOAD, EXIT_SAVE}_DBGCTLS. The reality is
>> different because KVM only allows the guest to access the valid bits
>> depending on the supported features. Luckily, KVM will exempt BTF and
>> LBR from validity check which makes the test survive.
>>
>> Considering that IA32_DEBUGCTLMSR access is not practically effective
>> and will bring error messages, eliminate the related code and rename
>> the test to specifically address the DR7 check.
> 
> I would expect you explained it more clear, e.g.,
> 
> "debug controls" test was added by commit[0] to verify that "VM-Entry load debug controls" and "VM-Exit save debug controls" are correctly emulated by KVM for nested VMX. But due to the limitation that KVM didn't support MSR_IA32_DEBUGCTL for guest at that time, the test commented out all the value comparison of MSR_IA32_DEBUGCTL and leave it to future when KVM supports the MSR.
> 
> The test doesn't check the functionality of save/restore guest MSR_IA32_DEBUGCTL on vm-exit/-entry, but it keeps the write of MSR_IA32_DEBUGCTL. It leads to
> 
>   kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x407de7 Unhandled WRMSR(0x1d9) = 0x1
>   kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x0 Unhandled WRMSR(0x1d9) = 0x2
>   kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40936f Unhandled WRMSR(0x1d9) = 0x3
>   kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40cf09 Unhandled WRMSR(0x1d9) = 0x1
>   kvm_intel: kvm [18663]: vcpu0, guest rIP: 0x40940d Unhandled WRMSR(0x1d9) = 0x3
> 
> to the kernel log. Though it doesn't break the test, the log confuses people to think something wrong happened in the testcase.
> 
> Current KVM does support some bits of MSR_IA32_DEBUGCTL but they depend on the vcpu model exposed. To simplify the case and eliminate the confusing "Unhandled WRMSR(0x1d9)" log, remove the MSR_IA32_DEBUGCTL logic in the test and make it concentrate only on DR7. Following patch will bring back MSR_IA32_DEBUGCTL separately.
> 
> [0] dc5c01f17b1a ("VMX: Test behavior on set and cleared save/load debug controls")

It looks much clearer. Thanks for the rewrite!