On June 23, 2025 6:19:31 PM PDT, Ethan Zhao <haifeng.zhao@xxxxxxxxxxxxxxx> wrote: > >在 2025/6/24 0:34, Xin Li 写道: >> On 6/22/2025 11:49 PM, Ethan Zhao wrote: >>> >>> 在 2025/6/21 7:15, Xin Li (Intel) 写道: >>>> Initialize DR6 by writing its architectural reset value to avoid >>>> incorrectly zeroing DR6 to clear DR6.BLD at boot time, which leads >>>> to a false bus lock detected warning. >>>> >>>> The Intel SDM says: >>>> >>>> 1) Certain debug exceptions may clear bits 0-3 of DR6. >>>> >>>> 2) BLD induced #DB clears DR6.BLD and any other debug exception >>>> doesn't modify DR6.BLD. >>>> >>>> 3) RTM induced #DB clears DR6.RTM and any other debug exception >>>> sets DR6.RTM. >>>> >>>> To avoid confusion in identifying debug exceptions, debug handlers >>>> should set DR6.BLD and DR6.RTM, and clear other DR6 bits before >>>> returning. >>>> >>>> The DR6 architectural reset value 0xFFFF0FF0, already defined as >>>> macro DR6_RESERVED, satisfies these requirements, so just use it to >>>> reinitialize DR6 whenever needed. >>>> >>>> Since clear_all_debug_regs() no longer zeros all debug registers, >>>> rename it to initialize_debug_regs() to better reflect its current >>>> behavior. >>>> >>>> Since debug_read_clear_dr6() no longer clears DR6, rename it to >>>> debug_read_reset_dr6() to better reflect its current behavior. >>>> >>>> Reported-by: Sohil Mehta <sohil.mehta@xxxxxxxxx> >>>> Link: https://lore.kernel.org/lkml/06e68373-a92b-472e-8fd9- ba548119770c@xxxxxxxxx/ >>>> Fixes: ebb1064e7c2e9 ("x86/traps: Handle #DB for bus lock") >>>> Suggested-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx> >>>> Tested-by: Sohil Mehta <sohil.mehta@xxxxxxxxx> >>>> Reviewed-by: H. Peter Anvin (Intel) <hpa@xxxxxxxxx> >>>> Reviewed-by: Sohil Mehta <sohil.mehta@xxxxxxxxx> >>>> Acked-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> >>>> Signed-off-by: Xin Li (Intel) <xin@xxxxxxxxx> >>>> Cc: stable@xxxxxxxxxxxxxxx >>>> --- >>>> >>>> Changes in v3: >>>> *) Polish initialize_debug_regs() (PeterZ). >>>> *) Rewrite the comment for DR6_RESERVED definition (Sohil and Sean). >>>> *) Collect TB, RB, AB (PeterZ and Sohil). >>>> >>>> Changes in v2: >>>> *) Use debug register index 6 rather than DR_STATUS (PeterZ and Sean). >>>> *) Move this patch the first of the patch set to ease backporting. >>>> --- >>>> arch/x86/include/uapi/asm/debugreg.h | 21 ++++++++++++++++- >>>> arch/x86/kernel/cpu/common.c | 24 ++++++++------------ >>>> arch/x86/kernel/traps.c | 34 +++++++++++++++++----------- >>>> 3 files changed, 51 insertions(+), 28 deletions(-) >>>> >>>> diff --git a/arch/x86/include/uapi/asm/debugreg.h b/arch/x86/include/ uapi/asm/debugreg.h >>>> index 0007ba077c0c..41da492dfb01 100644 >>>> --- a/arch/x86/include/uapi/asm/debugreg.h >>>> +++ b/arch/x86/include/uapi/asm/debugreg.h >>>> @@ -15,7 +15,26 @@ >>>> which debugging register was responsible for the trap. The other bits >>>> are either reserved or not of interest to us. */ >>>> -/* Define reserved bits in DR6 which are always set to 1 */ >>>> +/* >>>> + * Define bits in DR6 which are set to 1 by default. >>>> + * >>>> + * This is also the DR6 architectural value following Power-up, Reset or INIT. >>>> + * >>>> + * Note, with the introduction of Bus Lock Detection (BLD) and Restricted >>>> + * Transactional Memory (RTM), the DR6 register has been modified: >>>> + * >>>> + * 1) BLD flag (bit 11) is no longer reserved to 1 if the CPU supports >>>> + * Bus Lock Detection. The assertion of a bus lock could clear it. >>>> + * >>>> + * 2) RTM flag (bit 16) is no longer reserved to 1 if the CPU supports >>>> + * restricted transactional memory. #DB occurred inside an RTM region >>>> + * could clear it. >>>> + * >>>> + * Apparently, DR6.BLD and DR6.RTM are active low bits. >>>> + * >>>> + * As a result, DR6_RESERVED is an incorrect name now, but it is kept for >>>> + * compatibility. >>>> + */ >>>> #define DR6_RESERVED (0xFFFF0FF0) >>>> #define DR_TRAP0 (0x1) /* db0 */ >>>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c >>>> index 8feb8fd2957a..0f6c280a94f0 100644 >>>> --- a/arch/x86/kernel/cpu/common.c >>>> +++ b/arch/x86/kernel/cpu/common.c >>>> @@ -2243,20 +2243,16 @@ EXPORT_PER_CPU_SYMBOL(__stack_chk_guard); >>>> #endif >>>> #endif >>>> -/* >>>> - * Clear all 6 debug registers: >>>> - */ >>>> -static void clear_all_debug_regs(void) >>>> +static void initialize_debug_regs(void) >>>> { >>>> - int i; >>>> - >>>> - for (i = 0; i < 8; i++) { >>>> - /* Ignore db4, db5 */ >>>> - if ((i == 4) || (i == 5)) >>>> - continue; >>>> - >>>> - set_debugreg(0, i); >>>> - } >>>> + /* Control register first -- to make sure everything is disabled. */ >>> >>> In the Figure 19-1. Debug Registers of SDM section 19.2 DEBUG REGISTERS, >>> >>> bit 10, 12, 14, 15 of DR7 are marked as gray (Reversed) and their value are filled as >>> >>> 1, 0, 0,0 ; should we clear them all here ? I didn't find any other description in the >>> >>> SDM about the result if they are cleaned. of course, this patch doesn't change >>> >>> the behaviour of original DR7 initialization code, no justification needed, >>> >>> just out of curiosity. >> >> This patch is NOT intended to make any actual change to DR7 >> initialization. >> >So far it is okay, I am just curious why these registers were cleared to zero > >but the git log history and SDM doesn't give too much consistent clue. > >That is 16 years old code. > >> Please take a look at the second patch of this patch set. > >Looking. > > >Thanks, > >Ethan > >> >> Thanks! >> Xin >> >>> >>>> + set_debugreg(0, 7); >>>> + set_debugreg(DR6_RESERVED, 6); >>>> + /* dr5 and dr4 don't exist */ >>>> + set_debugreg(0, 3); >>>> + set_debugreg(0, 2); >>>> + set_debugreg(0, 1); >>>> + set_debugreg(0, 0); > Older than that. I believe it dates back from when Linux first got DRx support.
