Re: [PATCH v4 3/5] KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed()

[Shivansh Dhiman <shivansh.dhiman@amd.com>]

On 25-09-26 23:09, Sean Christopherson wrote:
> The shortlog is again not precise enough.  With this:>
>   KVM: nSVM: Sanitize nested DR6 using kvm_dr6_fixed
> 
> the reader doesn't actually know what behavior is being modified.  It's also way
> too literal; the shortlog+changelog should strive to describe the change in human-
> friendly words, e.g. in conversational language, not be a play-by-play of the code
> change.  And that matters in this case, because the poorly named kvm_dr6_fixed()
> makes it even hard to understand what is actually happening.
> 
>   KVM: nSVM: Don't assume all active-low bits DR6 are fixed-1

That reads better. I'll keep this mind when posting any new series.

> 
> On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
>> When preparing vmcb02 for nested VMRUN, KVM ORs DR6_ACTIVE_LOW into the
>> guest DR6 to force the fixed bits to 1. DR6_ACTIVE_LOW forces bit 11
>> (DR6_BUS_LOCK) to 1 unconditionally.
>>
>> DR6_BUS_LOCK is active-low (the CPU clears it to 0 to report a bus lock), so
>> forcing it to 1 unconditionally would prevent an L2 from ever observing a
>> bus lock (DR6.BLD == 0) across a nested VMRUN.
>>
>> Use kvm_dr6_fixed() instead, which forces DR6_RTM and DR6_BUS_LOCK based on
> 
> We should kill off DR6_FIXED_1 and rename kvm_dr6_fixed() to kvm_get_dr6_fixed_1()
> as prep patches.
> 
> As above, the changelog is too much of a play-by-play.  The names of the macros
> don't matter, and knowing the exact bit position isn't necessary to describe and
> understand the change.
> 
>   When preparing vmcb02 for nested VMRUN, force only the actual fixed-1 bits
>   instead of setting all active-low bits.  The flaw is currently benign, as
>   the only active-low bits supported by KVM are RTM (Restricted Transactional
>   Memory) and BLD (Bus Lock Detect), neither of which is currently supported
>   on SVM, but that's about to change.  I.e. this will break upcoming Bus Lock
>   Detect support as the guest will never see DR6.BLD=0.

Thanks for the reword.

-Shivansh

> 
>> the guest's CPUID. DR6_RTM is a reserved bit on AMD and is thus always set
>> to 1. DR6_BUS_LOCK is left writable once the guest supports Bus Lock
>> Detect.