linux-kernel - Re: [PATCH v1 2/3] x86/traps: Initialize DR7 by writing its architectural reset value

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <310B2567-8680-4E1D-B1BB-A56809466ED4@zytor.com>
Date: Fri, 13 Jun 2025 00:59:04 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Xin Li <xin@...or.com>, Peter Zijlstra <peterz@...radead.org>
CC: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        x86@...nel.org, seanjc@...gle.com, pbonzini@...hat.com,
        brgerst@...il.com, tony.luck@...el.com, fenghuay@...dia.com
Subject: Re: [PATCH v1 2/3] x86/traps: Initialize DR7 by writing its architectural reset value

On June 13, 2025 12:51:31 AM PDT, Xin Li <xin@...or.com> wrote:
>On 6/13/2025 12:15 AM, Peter Zijlstra wrote:
>> On Fri, Jun 13, 2025 at 12:01:16AM -0700, Xin Li (Intel) wrote:
>> 
>>> While at it, replace the hardcoded debug register number 7 with the
>>> existing DR_CONTROL macro for clarity.
>> 
>> Yeah, not really a fan of that... IMO that obfuscates the code more than
>> it helps, consider:
>> 
>>> -	get_debugreg(dr7, 7);
>>> +	get_debugreg(dr7, DR_CONTROL);
>
>Actually I kind of agree with you that it may not help, because I had
>thought to rename DR7_RESET_VALUE to DR_CONTROL_RESET_VALUE.
>
>Yes, we should remember DR7 is the control register, however I also hate
>to decode it when looking at the code.
>
>
>> 
>> and:
>> 
>>> -	for (i = 0; i < 8; i++) {
>>> -		/* Ignore db4, db5 */
>>> -		if ((i == 4) || (i == 5))
>>> -			continue;
>>> +	/* Control register first */
>>> +	set_debugreg(DR7_RESET_VALUE, DR_CONTROL);
>>> +	set_debugreg(0, DR_STATUS);
>>>   +	/* Ignore db4, db5 */
>>> +	for (i = DR_FIRSTADDR; i <= DR_LASTADDR; i++)
>> 
>> I had to git-grep DR_{FIRST,LAST}ADDR to double check this was correct :(
>> 
>> Also, you now write them in the order:
>> 
>>    dr7, dr6, /* dr4, dr5 */, dr0, dr1, dr2, dr3
>> 
>> My OCD disagrees with this :-)
>> 
>
>The order of the other debug registers doesn't seem critical, however
>the control debug register should be the first, right?
>
>Here I prefer to use "control register" rather than "dr7" here :)
>
>Thanks!
>    Xin

That's the real issue here, 7 is the control register and 6 is the status register; 4-5 and 8-15 don't even exist. 

But we want to reset the control register first.

Incidentally, do you know the following x86 register sequences in the proper order?

ax, cx, dx, bx, sp, bp, si, di, ...
al, cl, dl, bl, ah, ch, dh, bh
es, cs, ss, ds, fs, gs