linux-kernel - Re: [RESEND PATCH 1/2] KVM: X86: Add support for the emulation of DR6_BUS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0de5aad4-231b-e55b-2f2d-e121954742f9@intel.com>
Date:   Thu, 28 Jan 2021 15:41:46 +0800
From:   Xiaoyao Li <xiaoyao.li@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Chenyi Qiang <chenyi.qiang@...el.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH 1/2] KVM: X86: Add support for the emulation of
 DR6_BUS_LOCK bit

On 1/28/2021 3:25 PM, Paolo Bonzini wrote:
> On 28/01/21 08:17, Xiaoyao Li wrote:
>>>
>>> "Active low" means that the bit is usually 1 and goes to 0 when the 
>>> condition (such as RTM or bus lock) happens.  For almost all those 
>>> DR6 bits the value is in fact always 1, but if they are defined in 
>>> the future it will require no code change.
>>
>> Why not keep use DR6_INIT, or DR6_RESET_VALUE? or any other better name.
>>
>> It's just the default clear value of DR6 that no debug condition is hit.
> 
> I preferred "DR6_ACTIVE_LOW" because the value is used only once or 
> twice to initialize dr6, and many times to invert those bits.  For example:
> 
> vcpu->arch.dr6 &= ~DR_TRAP_BITS;
> vcpu->arch.dr6 |= DR6_ACTIVE_LOW;
> vcpu->arch.dr6 |= payload;
> vcpu->arch.dr6 ^= payload & DR6_ACTIVE_LOW;
> 
> payload = vcpu->arch.dr6;
> payload &= ~DR6_BT;
> payload ^= DR6_ACTIVE_LOW;
> 
> The name conveys that it's not just the initialization value; it's also 
> the XOR mask between the #DB exit qualification (which we also use as 
> the "payload") and DR6.

Fair enough.