* [PATCH V4 0/2] arm64/debug: Drop redundant DBG_MDSCR_* macros
@ 2025-06-12 3:35 Anshuman Khandual
2025-06-12 3:35 ` [PATCH V4 1/2] " Anshuman Khandual
2025-06-12 3:35 ` [PATCH V4 2/2] KVM: selftests: Change MDSCR_EL1 register holding variables as uint64_t Anshuman Khandual
0 siblings, 2 replies; 6+ messages in thread
From: Anshuman Khandual @ 2025-06-12 3:35 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Rutland,
Ada Couprie Diaz, Marc Zyngier, Oliver Upton, Joey Gouly,
linux-kernel, kvmarm, kvm
MDSCR_EL1 has already been defined in tools sysreg format and hence can be
used in all debug monitor related call paths. Subsequently all DBG_MDSCR_*
macros become redundant and hence can be dropped off completely. While here
convert all variables handling MDSCR_EL1 register as u64 which reflects its
true width as well.
This series applies on v6.16-rc1
Changes in V4:
- Updated the commit message in [PATCH 1/2] explainig rationale for changing
mdscr variable as u64 per Mark
Changes in V3:
https://lore.kernel.org/all/20250610053128.4118784-1-anshuman.khandual@arm.com/
- Split out the self test changes into a separate patch per Mark
- Added RB tag from Ada
Changes in V2:
https://lore.kernel.org/all/20250508044752.234543-1-anshuman.khandual@arm.com/
- Changed reg, val width to u64 in cortex_a76_erratum_1463225_svc_handler() per Ada
- Changed mdscr register width to uint64_t in enable_monitor_debug_exceptions() and
install_ss() per Ada
Changes in V1:
https://lore.kernel.org/all/20250417105253.3188976-1-anshuman.khandual@arm.com/
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ada Couprie Diaz <ada.coupriediaz@arm.com>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Cc: kvmarm@lists.linux.dev
Cc: kvm@vger.kernel.org
Anshuman Khandual (2):
arm64/debug: Drop redundant DBG_MDSCR_* macros
KVM: selftests: Change MDSCR_EL1 register holding variables as uint64_t
arch/arm64/include/asm/assembler.h | 4 ++--
arch/arm64/include/asm/debug-monitors.h | 6 -----
arch/arm64/kernel/debug-monitors.c | 22 +++++++++----------
arch/arm64/kernel/entry-common.c | 4 ++--
.../selftests/kvm/arm64/debug-exceptions.c | 4 ++--
5 files changed, 17 insertions(+), 23 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH V4 1/2] arm64/debug: Drop redundant DBG_MDSCR_* macros 2025-06-12 3:35 [PATCH V4 0/2] arm64/debug: Drop redundant DBG_MDSCR_* macros Anshuman Khandual @ 2025-06-12 3:35 ` Anshuman Khandual 2025-06-12 8:17 ` Marc Zyngier 2025-06-12 3:35 ` [PATCH V4 2/2] KVM: selftests: Change MDSCR_EL1 register holding variables as uint64_t Anshuman Khandual 1 sibling, 1 reply; 6+ messages in thread From: Anshuman Khandual @ 2025-06-12 3:35 UTC (permalink / raw) To: linux-arm-kernel Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Rutland, Ada Couprie Diaz, Marc Zyngier, Oliver Upton, Joey Gouly, linux-kernel, kvmarm, kvm MDSCR_EL1 has already been defined in tools sysreg format and hence can be used in all debug monitor related call paths. But using generated sysreg definitions causes build warnings because there is a mismatch between mdscr variable (u32) and GENMASK() based masks (long unsigned int). Convert all variables handling MDSCR_EL1 register as u64 which also reflects its true width as well. -------------------------------------------------------------------------- arch/arm64/kernel/debug-monitors.c: In function ‘disable_debug_monitors’: arch/arm64/kernel/debug-monitors.c:108:13: warning: conversion from ‘long unsigned int’ to ‘u32’ {aka ‘unsigned int’} changes value from ‘18446744073709518847’ to ‘4294934527’ [-Woverflow] 108 | disable = ~MDSCR_EL1_MDE; | ^ -------------------------------------------------------------------------- Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: linux-arm-kernel@lists.infradead.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- arch/arm64/include/asm/assembler.h | 4 ++-- arch/arm64/include/asm/debug-monitors.h | 6 ------ arch/arm64/kernel/debug-monitors.c | 22 +++++++++++----------- arch/arm64/kernel/entry-common.c | 4 ++-- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index ad63457a05c5..f229d96616e5 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -53,7 +53,7 @@ .macro disable_step_tsk, flgs, tmp tbz \flgs, #TIF_SINGLESTEP, 9990f mrs \tmp, mdscr_el1 - bic \tmp, \tmp, #DBG_MDSCR_SS + bic \tmp, \tmp, #MDSCR_EL1_SS msr mdscr_el1, \tmp isb // Take effect before a subsequent clear of DAIF.D 9990: @@ -63,7 +63,7 @@ .macro enable_step_tsk, flgs, tmp tbz \flgs, #TIF_SINGLESTEP, 9990f mrs \tmp, mdscr_el1 - orr \tmp, \tmp, #DBG_MDSCR_SS + orr \tmp, \tmp, #MDSCR_EL1_SS msr mdscr_el1, \tmp 9990: .endm diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h index 8f6ba31b8658..1f37dd01482b 100644 --- a/arch/arm64/include/asm/debug-monitors.h +++ b/arch/arm64/include/asm/debug-monitors.h @@ -13,14 +13,8 @@ #include <asm/ptrace.h> /* Low-level stepping controls. */ -#define DBG_MDSCR_SS (1 << 0) #define DBG_SPSR_SS (1 << 21) -/* MDSCR_EL1 enabling bits */ -#define DBG_MDSCR_KDE (1 << 13) -#define DBG_MDSCR_MDE (1 << 15) -#define DBG_MDSCR_MASK ~(DBG_MDSCR_KDE | DBG_MDSCR_MDE) - #define DBG_ESR_EVT(x) (((x) >> 27) & 0x7) /* AArch64 */ diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c index 58f047de3e1c..08f1d02507cd 100644 --- a/arch/arm64/kernel/debug-monitors.c +++ b/arch/arm64/kernel/debug-monitors.c @@ -34,7 +34,7 @@ u8 debug_monitors_arch(void) /* * MDSCR access routines. */ -static void mdscr_write(u32 mdscr) +static void mdscr_write(u64 mdscr) { unsigned long flags; flags = local_daif_save(); @@ -43,7 +43,7 @@ static void mdscr_write(u32 mdscr) } NOKPROBE_SYMBOL(mdscr_write); -static u32 mdscr_read(void) +static u64 mdscr_read(void) { return read_sysreg(mdscr_el1); } @@ -79,16 +79,16 @@ static DEFINE_PER_CPU(int, kde_ref_count); void enable_debug_monitors(enum dbg_active_el el) { - u32 mdscr, enable = 0; + u64 mdscr, enable = 0; WARN_ON(preemptible()); if (this_cpu_inc_return(mde_ref_count) == 1) - enable = DBG_MDSCR_MDE; + enable = MDSCR_EL1_MDE; if (el == DBG_ACTIVE_EL1 && this_cpu_inc_return(kde_ref_count) == 1) - enable |= DBG_MDSCR_KDE; + enable |= MDSCR_EL1_KDE; if (enable && debug_enabled) { mdscr = mdscr_read(); @@ -100,16 +100,16 @@ NOKPROBE_SYMBOL(enable_debug_monitors); void disable_debug_monitors(enum dbg_active_el el) { - u32 mdscr, disable = 0; + u64 mdscr, disable = 0; WARN_ON(preemptible()); if (this_cpu_dec_return(mde_ref_count) == 0) - disable = ~DBG_MDSCR_MDE; + disable = ~MDSCR_EL1_MDE; if (el == DBG_ACTIVE_EL1 && this_cpu_dec_return(kde_ref_count) == 0) - disable &= ~DBG_MDSCR_KDE; + disable &= ~MDSCR_EL1_KDE; if (disable) { mdscr = mdscr_read(); @@ -415,7 +415,7 @@ void kernel_enable_single_step(struct pt_regs *regs) { WARN_ON(!irqs_disabled()); set_regs_spsr_ss(regs); - mdscr_write(mdscr_read() | DBG_MDSCR_SS); + mdscr_write(mdscr_read() | MDSCR_EL1_SS); enable_debug_monitors(DBG_ACTIVE_EL1); } NOKPROBE_SYMBOL(kernel_enable_single_step); @@ -423,7 +423,7 @@ NOKPROBE_SYMBOL(kernel_enable_single_step); void kernel_disable_single_step(void) { WARN_ON(!irqs_disabled()); - mdscr_write(mdscr_read() & ~DBG_MDSCR_SS); + mdscr_write(mdscr_read() & ~MDSCR_EL1_SS); disable_debug_monitors(DBG_ACTIVE_EL1); } NOKPROBE_SYMBOL(kernel_disable_single_step); @@ -431,7 +431,7 @@ NOKPROBE_SYMBOL(kernel_disable_single_step); int kernel_active_single_step(void) { WARN_ON(!irqs_disabled()); - return mdscr_read() & DBG_MDSCR_SS; + return mdscr_read() & MDSCR_EL1_SS; } NOKPROBE_SYMBOL(kernel_active_single_step); diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index 7c1970b341b8..171f93f2494b 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -344,7 +344,7 @@ static DEFINE_PER_CPU(int, __in_cortex_a76_erratum_1463225_wa); static void cortex_a76_erratum_1463225_svc_handler(void) { - u32 reg, val; + u64 reg, val; if (!unlikely(test_thread_flag(TIF_SINGLESTEP))) return; @@ -354,7 +354,7 @@ static void cortex_a76_erratum_1463225_svc_handler(void) __this_cpu_write(__in_cortex_a76_erratum_1463225_wa, 1); reg = read_sysreg(mdscr_el1); - val = reg | DBG_MDSCR_SS | DBG_MDSCR_KDE; + val = reg | MDSCR_EL1_SS | MDSCR_EL1_KDE; write_sysreg(val, mdscr_el1); asm volatile("msr daifclr, #8"); isb(); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V4 1/2] arm64/debug: Drop redundant DBG_MDSCR_* macros 2025-06-12 3:35 ` [PATCH V4 1/2] " Anshuman Khandual @ 2025-06-12 8:17 ` Marc Zyngier 2025-06-12 10:24 ` Anshuman Khandual 0 siblings, 1 reply; 6+ messages in thread From: Marc Zyngier @ 2025-06-12 8:17 UTC (permalink / raw) To: Anshuman Khandual Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland, Ada Couprie Diaz, Oliver Upton, Joey Gouly, linux-kernel, kvmarm, kvm On Thu, 12 Jun 2025 04:35:46 +0100, Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > MDSCR_EL1 has already been defined in tools sysreg format and hence can be > used in all debug monitor related call paths. But using generated sysreg > definitions causes build warnings because there is a mismatch between mdscr > variable (u32) and GENMASK() based masks (long unsigned int). Convert all > variables handling MDSCR_EL1 register as u64 which also reflects its true > width as well. > > -------------------------------------------------------------------------- > arch/arm64/kernel/debug-monitors.c: In function ‘disable_debug_monitors’: > arch/arm64/kernel/debug-monitors.c:108:13: warning: conversion from ‘long > unsigned int’ to ‘u32’ {aka ‘unsigned int’} changes value from > ‘18446744073709518847’ to ‘4294934527’ [-Woverflow] > 108 | disable = ~MDSCR_EL1_MDE; > | ^ > -------------------------------------------------------------------------- > > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > --- > arch/arm64/include/asm/assembler.h | 4 ++-- > arch/arm64/include/asm/debug-monitors.h | 6 ------ > arch/arm64/kernel/debug-monitors.c | 22 +++++++++++----------- > arch/arm64/kernel/entry-common.c | 4 ++-- > 4 files changed, 15 insertions(+), 21 deletions(-) > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > index ad63457a05c5..f229d96616e5 100644 > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@ -53,7 +53,7 @@ > .macro disable_step_tsk, flgs, tmp > tbz \flgs, #TIF_SINGLESTEP, 9990f > mrs \tmp, mdscr_el1 > - bic \tmp, \tmp, #DBG_MDSCR_SS > + bic \tmp, \tmp, #MDSCR_EL1_SS > msr mdscr_el1, \tmp > isb // Take effect before a subsequent clear of DAIF.D > 9990: > @@ -63,7 +63,7 @@ > .macro enable_step_tsk, flgs, tmp > tbz \flgs, #TIF_SINGLESTEP, 9990f > mrs \tmp, mdscr_el1 > - orr \tmp, \tmp, #DBG_MDSCR_SS > + orr \tmp, \tmp, #MDSCR_EL1_SS > msr mdscr_el1, \tmp > 9990: > .endm > diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h > index 8f6ba31b8658..1f37dd01482b 100644 > --- a/arch/arm64/include/asm/debug-monitors.h > +++ b/arch/arm64/include/asm/debug-monitors.h > @@ -13,14 +13,8 @@ > #include <asm/ptrace.h> > > /* Low-level stepping controls. */ > -#define DBG_MDSCR_SS (1 << 0) > #define DBG_SPSR_SS (1 << 21) > > -/* MDSCR_EL1 enabling bits */ > -#define DBG_MDSCR_KDE (1 << 13) > -#define DBG_MDSCR_MDE (1 << 15) > -#define DBG_MDSCR_MASK ~(DBG_MDSCR_KDE | DBG_MDSCR_MDE) > - > #define DBG_ESR_EVT(x) (((x) >> 27) & 0x7) > > /* AArch64 */ > diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c > index 58f047de3e1c..08f1d02507cd 100644 > --- a/arch/arm64/kernel/debug-monitors.c > +++ b/arch/arm64/kernel/debug-monitors.c > @@ -34,7 +34,7 @@ u8 debug_monitors_arch(void) > /* > * MDSCR access routines. > */ > -static void mdscr_write(u32 mdscr) > +static void mdscr_write(u64 mdscr) > { > unsigned long flags; > flags = local_daif_save(); > @@ -43,7 +43,7 @@ static void mdscr_write(u32 mdscr) > } > NOKPROBE_SYMBOL(mdscr_write); > > -static u32 mdscr_read(void) > +static u64 mdscr_read(void) > { > return read_sysreg(mdscr_el1); > } > @@ -79,16 +79,16 @@ static DEFINE_PER_CPU(int, kde_ref_count); > > void enable_debug_monitors(enum dbg_active_el el) > { > - u32 mdscr, enable = 0; > + u64 mdscr, enable = 0; > > WARN_ON(preemptible()); > > if (this_cpu_inc_return(mde_ref_count) == 1) > - enable = DBG_MDSCR_MDE; > + enable = MDSCR_EL1_MDE; > > if (el == DBG_ACTIVE_EL1 && > this_cpu_inc_return(kde_ref_count) == 1) > - enable |= DBG_MDSCR_KDE; > + enable |= MDSCR_EL1_KDE; > > if (enable && debug_enabled) { > mdscr = mdscr_read(); > @@ -100,16 +100,16 @@ NOKPROBE_SYMBOL(enable_debug_monitors); > > void disable_debug_monitors(enum dbg_active_el el) > { > - u32 mdscr, disable = 0; > + u64 mdscr, disable = 0; > > WARN_ON(preemptible()); > > if (this_cpu_dec_return(mde_ref_count) == 0) > - disable = ~DBG_MDSCR_MDE; > + disable = ~MDSCR_EL1_MDE; > > if (el == DBG_ACTIVE_EL1 && > this_cpu_dec_return(kde_ref_count) == 0) > - disable &= ~DBG_MDSCR_KDE; > + disable &= ~MDSCR_EL1_KDE; > > if (disable) { > mdscr = mdscr_read(); > @@ -415,7 +415,7 @@ void kernel_enable_single_step(struct pt_regs *regs) > { > WARN_ON(!irqs_disabled()); > set_regs_spsr_ss(regs); > - mdscr_write(mdscr_read() | DBG_MDSCR_SS); > + mdscr_write(mdscr_read() | MDSCR_EL1_SS); > enable_debug_monitors(DBG_ACTIVE_EL1); > } > NOKPROBE_SYMBOL(kernel_enable_single_step); > @@ -423,7 +423,7 @@ NOKPROBE_SYMBOL(kernel_enable_single_step); > void kernel_disable_single_step(void) > { > WARN_ON(!irqs_disabled()); > - mdscr_write(mdscr_read() & ~DBG_MDSCR_SS); > + mdscr_write(mdscr_read() & ~MDSCR_EL1_SS); > disable_debug_monitors(DBG_ACTIVE_EL1); > } > NOKPROBE_SYMBOL(kernel_disable_single_step); > @@ -431,7 +431,7 @@ NOKPROBE_SYMBOL(kernel_disable_single_step); > int kernel_active_single_step(void) > { > WARN_ON(!irqs_disabled()); > - return mdscr_read() & DBG_MDSCR_SS; > + return mdscr_read() & MDSCR_EL1_SS; > } > NOKPROBE_SYMBOL(kernel_active_single_step); > > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 7c1970b341b8..171f93f2494b 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -344,7 +344,7 @@ static DEFINE_PER_CPU(int, __in_cortex_a76_erratum_1463225_wa); > > static void cortex_a76_erratum_1463225_svc_handler(void) > { > - u32 reg, val; > + u64 reg, val; > > if (!unlikely(test_thread_flag(TIF_SINGLESTEP))) > return; > @@ -354,7 +354,7 @@ static void cortex_a76_erratum_1463225_svc_handler(void) > > __this_cpu_write(__in_cortex_a76_erratum_1463225_wa, 1); > reg = read_sysreg(mdscr_el1); > - val = reg | DBG_MDSCR_SS | DBG_MDSCR_KDE; > + val = reg | MDSCR_EL1_SS | MDSCR_EL1_KDE; > write_sysreg(val, mdscr_el1); > asm volatile("msr daifclr, #8"); > isb(); Whilst you're at it, please also change the open-coded constant in __cpu_setup to MDSCR_EL1_TDCC. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 1/2] arm64/debug: Drop redundant DBG_MDSCR_* macros 2025-06-12 8:17 ` Marc Zyngier @ 2025-06-12 10:24 ` Anshuman Khandual 2025-06-12 10:51 ` Marc Zyngier 0 siblings, 1 reply; 6+ messages in thread From: Anshuman Khandual @ 2025-06-12 10:24 UTC (permalink / raw) To: Marc Zyngier Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland, Ada Couprie Diaz, Oliver Upton, Joey Gouly, linux-kernel, kvmarm, kvm On 12/06/25 1:47 PM, Marc Zyngier wrote: > On Thu, 12 Jun 2025 04:35:46 +0100, > Anshuman Khandual <anshuman.khandual@arm.com> wrote: >> >> MDSCR_EL1 has already been defined in tools sysreg format and hence can be >> used in all debug monitor related call paths. But using generated sysreg >> definitions causes build warnings because there is a mismatch between mdscr >> variable (u32) and GENMASK() based masks (long unsigned int). Convert all >> variables handling MDSCR_EL1 register as u64 which also reflects its true >> width as well. >> >> -------------------------------------------------------------------------- >> arch/arm64/kernel/debug-monitors.c: In function ‘disable_debug_monitors’: >> arch/arm64/kernel/debug-monitors.c:108:13: warning: conversion from ‘long >> unsigned int’ to ‘u32’ {aka ‘unsigned int’} changes value from >> ‘18446744073709518847’ to ‘4294934527’ [-Woverflow] >> 108 | disable = ~MDSCR_EL1_MDE; >> | ^ >> -------------------------------------------------------------------------- >> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Will Deacon <will@kernel.org> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> arch/arm64/include/asm/assembler.h | 4 ++-- >> arch/arm64/include/asm/debug-monitors.h | 6 ------ >> arch/arm64/kernel/debug-monitors.c | 22 +++++++++++----------- >> arch/arm64/kernel/entry-common.c | 4 ++-- >> 4 files changed, 15 insertions(+), 21 deletions(-) >> >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h >> index ad63457a05c5..f229d96616e5 100644 >> --- a/arch/arm64/include/asm/assembler.h >> +++ b/arch/arm64/include/asm/assembler.h >> @@ -53,7 +53,7 @@ >> .macro disable_step_tsk, flgs, tmp >> tbz \flgs, #TIF_SINGLESTEP, 9990f >> mrs \tmp, mdscr_el1 >> - bic \tmp, \tmp, #DBG_MDSCR_SS >> + bic \tmp, \tmp, #MDSCR_EL1_SS >> msr mdscr_el1, \tmp >> isb // Take effect before a subsequent clear of DAIF.D >> 9990: >> @@ -63,7 +63,7 @@ >> .macro enable_step_tsk, flgs, tmp >> tbz \flgs, #TIF_SINGLESTEP, 9990f >> mrs \tmp, mdscr_el1 >> - orr \tmp, \tmp, #DBG_MDSCR_SS >> + orr \tmp, \tmp, #MDSCR_EL1_SS >> msr mdscr_el1, \tmp >> 9990: >> .endm >> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h >> index 8f6ba31b8658..1f37dd01482b 100644 >> --- a/arch/arm64/include/asm/debug-monitors.h >> +++ b/arch/arm64/include/asm/debug-monitors.h >> @@ -13,14 +13,8 @@ >> #include <asm/ptrace.h> >> >> /* Low-level stepping controls. */ >> -#define DBG_MDSCR_SS (1 << 0) >> #define DBG_SPSR_SS (1 << 21) >> >> -/* MDSCR_EL1 enabling bits */ >> -#define DBG_MDSCR_KDE (1 << 13) >> -#define DBG_MDSCR_MDE (1 << 15) >> -#define DBG_MDSCR_MASK ~(DBG_MDSCR_KDE | DBG_MDSCR_MDE) >> - >> #define DBG_ESR_EVT(x) (((x) >> 27) & 0x7) >> >> /* AArch64 */ >> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c >> index 58f047de3e1c..08f1d02507cd 100644 >> --- a/arch/arm64/kernel/debug-monitors.c >> +++ b/arch/arm64/kernel/debug-monitors.c >> @@ -34,7 +34,7 @@ u8 debug_monitors_arch(void) >> /* >> * MDSCR access routines. >> */ >> -static void mdscr_write(u32 mdscr) >> +static void mdscr_write(u64 mdscr) >> { >> unsigned long flags; >> flags = local_daif_save(); >> @@ -43,7 +43,7 @@ static void mdscr_write(u32 mdscr) >> } >> NOKPROBE_SYMBOL(mdscr_write); >> >> -static u32 mdscr_read(void) >> +static u64 mdscr_read(void) >> { >> return read_sysreg(mdscr_el1); >> } >> @@ -79,16 +79,16 @@ static DEFINE_PER_CPU(int, kde_ref_count); >> >> void enable_debug_monitors(enum dbg_active_el el) >> { >> - u32 mdscr, enable = 0; >> + u64 mdscr, enable = 0; >> >> WARN_ON(preemptible()); >> >> if (this_cpu_inc_return(mde_ref_count) == 1) >> - enable = DBG_MDSCR_MDE; >> + enable = MDSCR_EL1_MDE; >> >> if (el == DBG_ACTIVE_EL1 && >> this_cpu_inc_return(kde_ref_count) == 1) >> - enable |= DBG_MDSCR_KDE; >> + enable |= MDSCR_EL1_KDE; >> >> if (enable && debug_enabled) { >> mdscr = mdscr_read(); >> @@ -100,16 +100,16 @@ NOKPROBE_SYMBOL(enable_debug_monitors); >> >> void disable_debug_monitors(enum dbg_active_el el) >> { >> - u32 mdscr, disable = 0; >> + u64 mdscr, disable = 0; >> >> WARN_ON(preemptible()); >> >> if (this_cpu_dec_return(mde_ref_count) == 0) >> - disable = ~DBG_MDSCR_MDE; >> + disable = ~MDSCR_EL1_MDE; >> >> if (el == DBG_ACTIVE_EL1 && >> this_cpu_dec_return(kde_ref_count) == 0) >> - disable &= ~DBG_MDSCR_KDE; >> + disable &= ~MDSCR_EL1_KDE; >> >> if (disable) { >> mdscr = mdscr_read(); >> @@ -415,7 +415,7 @@ void kernel_enable_single_step(struct pt_regs *regs) >> { >> WARN_ON(!irqs_disabled()); >> set_regs_spsr_ss(regs); >> - mdscr_write(mdscr_read() | DBG_MDSCR_SS); >> + mdscr_write(mdscr_read() | MDSCR_EL1_SS); >> enable_debug_monitors(DBG_ACTIVE_EL1); >> } >> NOKPROBE_SYMBOL(kernel_enable_single_step); >> @@ -423,7 +423,7 @@ NOKPROBE_SYMBOL(kernel_enable_single_step); >> void kernel_disable_single_step(void) >> { >> WARN_ON(!irqs_disabled()); >> - mdscr_write(mdscr_read() & ~DBG_MDSCR_SS); >> + mdscr_write(mdscr_read() & ~MDSCR_EL1_SS); >> disable_debug_monitors(DBG_ACTIVE_EL1); >> } >> NOKPROBE_SYMBOL(kernel_disable_single_step); >> @@ -431,7 +431,7 @@ NOKPROBE_SYMBOL(kernel_disable_single_step); >> int kernel_active_single_step(void) >> { >> WARN_ON(!irqs_disabled()); >> - return mdscr_read() & DBG_MDSCR_SS; >> + return mdscr_read() & MDSCR_EL1_SS; >> } >> NOKPROBE_SYMBOL(kernel_active_single_step); >> >> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c >> index 7c1970b341b8..171f93f2494b 100644 >> --- a/arch/arm64/kernel/entry-common.c >> +++ b/arch/arm64/kernel/entry-common.c >> @@ -344,7 +344,7 @@ static DEFINE_PER_CPU(int, __in_cortex_a76_erratum_1463225_wa); >> >> static void cortex_a76_erratum_1463225_svc_handler(void) >> { >> - u32 reg, val; >> + u64 reg, val; >> >> if (!unlikely(test_thread_flag(TIF_SINGLESTEP))) >> return; >> @@ -354,7 +354,7 @@ static void cortex_a76_erratum_1463225_svc_handler(void) >> >> __this_cpu_write(__in_cortex_a76_erratum_1463225_wa, 1); >> reg = read_sysreg(mdscr_el1); >> - val = reg | DBG_MDSCR_SS | DBG_MDSCR_KDE; >> + val = reg | MDSCR_EL1_SS | MDSCR_EL1_KDE; >> write_sysreg(val, mdscr_el1); >> asm volatile("msr daifclr, #8"); >> isb(); > > Whilst you're at it, please also change the open-coded constant in > __cpu_setup to MDSCR_EL1_TDCC. I believe you are suggesting about the following change, will fold in the patch. But I guess 'mov' would still be preferred compared to 'mov_q' as MDSCR_EL1_TDCC is a 32 bit constant (atleast the non zero portion) ? --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -454,7 +454,7 @@ SYM_FUNC_START(__cpu_setup) dsb nsh msr cpacr_el1, xzr // Reset cpacr_el1 - mov x1, #1 << 12 // Reset mdscr_el1 and disable + mov x1, MDSCR_EL1_TDCC // Reset mdscr_el1 and disable msr mdscr_el1, x1 // access to the DCC from EL0 reset_pmuserenr_el0 x1 // Disable PMU access from EL0 reset_amuserenr_el0 x1 // Disable AMU access from EL0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V4 1/2] arm64/debug: Drop redundant DBG_MDSCR_* macros 2025-06-12 10:24 ` Anshuman Khandual @ 2025-06-12 10:51 ` Marc Zyngier 0 siblings, 0 replies; 6+ messages in thread From: Marc Zyngier @ 2025-06-12 10:51 UTC (permalink / raw) To: Anshuman Khandual Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland, Ada Couprie Diaz, Oliver Upton, Joey Gouly, linux-kernel, kvmarm, kvm On Thu, 12 Jun 2025 11:24:04 +0100, Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > > > On 12/06/25 1:47 PM, Marc Zyngier wrote: > > On Thu, 12 Jun 2025 04:35:46 +0100, > > Anshuman Khandual <anshuman.khandual@arm.com> wrote: > >> > >> MDSCR_EL1 has already been defined in tools sysreg format and hence can be > >> used in all debug monitor related call paths. But using generated sysreg > >> definitions causes build warnings because there is a mismatch between mdscr > >> variable (u32) and GENMASK() based masks (long unsigned int). Convert all > >> variables handling MDSCR_EL1 register as u64 which also reflects its true > >> width as well. > >> > >> -------------------------------------------------------------------------- > >> arch/arm64/kernel/debug-monitors.c: In function ‘disable_debug_monitors’: > >> arch/arm64/kernel/debug-monitors.c:108:13: warning: conversion from ‘long > >> unsigned int’ to ‘u32’ {aka ‘unsigned int’} changes value from > >> ‘18446744073709518847’ to ‘4294934527’ [-Woverflow] > >> 108 | disable = ~MDSCR_EL1_MDE; > >> | ^ > >> -------------------------------------------------------------------------- > >> > >> Cc: Catalin Marinas <catalin.marinas@arm.com> > >> Cc: Will Deacon <will@kernel.org> > >> Cc: Mark Rutland <mark.rutland@arm.com> > >> Cc: linux-arm-kernel@lists.infradead.org > >> Cc: linux-kernel@vger.kernel.org > >> Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com> > >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> > >> --- > >> arch/arm64/include/asm/assembler.h | 4 ++-- > >> arch/arm64/include/asm/debug-monitors.h | 6 ------ > >> arch/arm64/kernel/debug-monitors.c | 22 +++++++++++----------- > >> arch/arm64/kernel/entry-common.c | 4 ++-- > >> 4 files changed, 15 insertions(+), 21 deletions(-) > >> > >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > >> index ad63457a05c5..f229d96616e5 100644 > >> --- a/arch/arm64/include/asm/assembler.h > >> +++ b/arch/arm64/include/asm/assembler.h > >> @@ -53,7 +53,7 @@ > >> .macro disable_step_tsk, flgs, tmp > >> tbz \flgs, #TIF_SINGLESTEP, 9990f > >> mrs \tmp, mdscr_el1 > >> - bic \tmp, \tmp, #DBG_MDSCR_SS > >> + bic \tmp, \tmp, #MDSCR_EL1_SS > >> msr mdscr_el1, \tmp > >> isb // Take effect before a subsequent clear of DAIF.D > >> 9990: > >> @@ -63,7 +63,7 @@ > >> .macro enable_step_tsk, flgs, tmp > >> tbz \flgs, #TIF_SINGLESTEP, 9990f > >> mrs \tmp, mdscr_el1 > >> - orr \tmp, \tmp, #DBG_MDSCR_SS > >> + orr \tmp, \tmp, #MDSCR_EL1_SS > >> msr mdscr_el1, \tmp > >> 9990: > >> .endm > >> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h > >> index 8f6ba31b8658..1f37dd01482b 100644 > >> --- a/arch/arm64/include/asm/debug-monitors.h > >> +++ b/arch/arm64/include/asm/debug-monitors.h > >> @@ -13,14 +13,8 @@ > >> #include <asm/ptrace.h> > >> > >> /* Low-level stepping controls. */ > >> -#define DBG_MDSCR_SS (1 << 0) > >> #define DBG_SPSR_SS (1 << 21) > >> > >> -/* MDSCR_EL1 enabling bits */ > >> -#define DBG_MDSCR_KDE (1 << 13) > >> -#define DBG_MDSCR_MDE (1 << 15) > >> -#define DBG_MDSCR_MASK ~(DBG_MDSCR_KDE | DBG_MDSCR_MDE) > >> - > >> #define DBG_ESR_EVT(x) (((x) >> 27) & 0x7) > >> > >> /* AArch64 */ > >> diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c > >> index 58f047de3e1c..08f1d02507cd 100644 > >> --- a/arch/arm64/kernel/debug-monitors.c > >> +++ b/arch/arm64/kernel/debug-monitors.c > >> @@ -34,7 +34,7 @@ u8 debug_monitors_arch(void) > >> /* > >> * MDSCR access routines. > >> */ > >> -static void mdscr_write(u32 mdscr) > >> +static void mdscr_write(u64 mdscr) > >> { > >> unsigned long flags; > >> flags = local_daif_save(); > >> @@ -43,7 +43,7 @@ static void mdscr_write(u32 mdscr) > >> } > >> NOKPROBE_SYMBOL(mdscr_write); > >> > >> -static u32 mdscr_read(void) > >> +static u64 mdscr_read(void) > >> { > >> return read_sysreg(mdscr_el1); > >> } > >> @@ -79,16 +79,16 @@ static DEFINE_PER_CPU(int, kde_ref_count); > >> > >> void enable_debug_monitors(enum dbg_active_el el) > >> { > >> - u32 mdscr, enable = 0; > >> + u64 mdscr, enable = 0; > >> > >> WARN_ON(preemptible()); > >> > >> if (this_cpu_inc_return(mde_ref_count) == 1) > >> - enable = DBG_MDSCR_MDE; > >> + enable = MDSCR_EL1_MDE; > >> > >> if (el == DBG_ACTIVE_EL1 && > >> this_cpu_inc_return(kde_ref_count) == 1) > >> - enable |= DBG_MDSCR_KDE; > >> + enable |= MDSCR_EL1_KDE; > >> > >> if (enable && debug_enabled) { > >> mdscr = mdscr_read(); > >> @@ -100,16 +100,16 @@ NOKPROBE_SYMBOL(enable_debug_monitors); > >> > >> void disable_debug_monitors(enum dbg_active_el el) > >> { > >> - u32 mdscr, disable = 0; > >> + u64 mdscr, disable = 0; > >> > >> WARN_ON(preemptible()); > >> > >> if (this_cpu_dec_return(mde_ref_count) == 0) > >> - disable = ~DBG_MDSCR_MDE; > >> + disable = ~MDSCR_EL1_MDE; > >> > >> if (el == DBG_ACTIVE_EL1 && > >> this_cpu_dec_return(kde_ref_count) == 0) > >> - disable &= ~DBG_MDSCR_KDE; > >> + disable &= ~MDSCR_EL1_KDE; > >> > >> if (disable) { > >> mdscr = mdscr_read(); > >> @@ -415,7 +415,7 @@ void kernel_enable_single_step(struct pt_regs *regs) > >> { > >> WARN_ON(!irqs_disabled()); > >> set_regs_spsr_ss(regs); > >> - mdscr_write(mdscr_read() | DBG_MDSCR_SS); > >> + mdscr_write(mdscr_read() | MDSCR_EL1_SS); > >> enable_debug_monitors(DBG_ACTIVE_EL1); > >> } > >> NOKPROBE_SYMBOL(kernel_enable_single_step); > >> @@ -423,7 +423,7 @@ NOKPROBE_SYMBOL(kernel_enable_single_step); > >> void kernel_disable_single_step(void) > >> { > >> WARN_ON(!irqs_disabled()); > >> - mdscr_write(mdscr_read() & ~DBG_MDSCR_SS); > >> + mdscr_write(mdscr_read() & ~MDSCR_EL1_SS); > >> disable_debug_monitors(DBG_ACTIVE_EL1); > >> } > >> NOKPROBE_SYMBOL(kernel_disable_single_step); > >> @@ -431,7 +431,7 @@ NOKPROBE_SYMBOL(kernel_disable_single_step); > >> int kernel_active_single_step(void) > >> { > >> WARN_ON(!irqs_disabled()); > >> - return mdscr_read() & DBG_MDSCR_SS; > >> + return mdscr_read() & MDSCR_EL1_SS; > >> } > >> NOKPROBE_SYMBOL(kernel_active_single_step); > >> > >> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > >> index 7c1970b341b8..171f93f2494b 100644 > >> --- a/arch/arm64/kernel/entry-common.c > >> +++ b/arch/arm64/kernel/entry-common.c > >> @@ -344,7 +344,7 @@ static DEFINE_PER_CPU(int, __in_cortex_a76_erratum_1463225_wa); > >> > >> static void cortex_a76_erratum_1463225_svc_handler(void) > >> { > >> - u32 reg, val; > >> + u64 reg, val; > >> > >> if (!unlikely(test_thread_flag(TIF_SINGLESTEP))) > >> return; > >> @@ -354,7 +354,7 @@ static void cortex_a76_erratum_1463225_svc_handler(void) > >> > >> __this_cpu_write(__in_cortex_a76_erratum_1463225_wa, 1); > >> reg = read_sysreg(mdscr_el1); > >> - val = reg | DBG_MDSCR_SS | DBG_MDSCR_KDE; > >> + val = reg | MDSCR_EL1_SS | MDSCR_EL1_KDE; > >> write_sysreg(val, mdscr_el1); > >> asm volatile("msr daifclr, #8"); > >> isb(); > > > > Whilst you're at it, please also change the open-coded constant in > > __cpu_setup to MDSCR_EL1_TDCC. > > I believe you are suggesting about the following change, will fold > in the patch. But I guess 'mov' would still be preferred compared > to 'mov_q' as MDSCR_EL1_TDCC is a 32 bit constant (atleast the non > zero portion) ? Digression: I'm not sure why you'd ever consider using mov_q for a single-bit constant, irrespective of where that bit is set. The mov instruction (and all the logical operations taking an immediate as a parameter) can encode any contiguous stream of 1s with an arbitrary rotation. See C6.2.247 and co. > > --- a/arch/arm64/mm/proc.S > +++ b/arch/arm64/mm/proc.S > @@ -454,7 +454,7 @@ SYM_FUNC_START(__cpu_setup) > dsb nsh > > msr cpacr_el1, xzr // Reset cpacr_el1 > - mov x1, #1 << 12 // Reset mdscr_el1 and disable > + mov x1, MDSCR_EL1_TDCC // Reset mdscr_el1 and disable > msr mdscr_el1, x1 // access to the DCC from EL0 > reset_pmuserenr_el0 x1 // Disable PMU access from EL0 > reset_amuserenr_el0 x1 // Disable AMU access from EL0 Yes. M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V4 2/2] KVM: selftests: Change MDSCR_EL1 register holding variables as uint64_t 2025-06-12 3:35 [PATCH V4 0/2] arm64/debug: Drop redundant DBG_MDSCR_* macros Anshuman Khandual 2025-06-12 3:35 ` [PATCH V4 1/2] " Anshuman Khandual @ 2025-06-12 3:35 ` Anshuman Khandual 1 sibling, 0 replies; 6+ messages in thread From: Anshuman Khandual @ 2025-06-12 3:35 UTC (permalink / raw) To: linux-arm-kernel Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mark Rutland, Ada Couprie Diaz, Marc Zyngier, Oliver Upton, Joey Gouly, linux-kernel, kvmarm, kvm, linux-kselftest Change MDSCR_EL1 register holding local variables as uint64_t that reflects its true register width as well. Cc: Marc Zyngier <maz@kernel.org> Cc: Oliver Upton <oliver.upton@linux.dev> Cc: Joey Gouly <joey.gouly@arm.com> Cc: kvm@vger.kernel.org Cc: kvmarm@lists.linux.dev Cc: linux-kernel@vger.kernel.org Cc: linux-kselftest@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Reviewed-by: Ada Couprie Diaz <ada.coupriediaz@arm.com> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> --- tools/testing/selftests/kvm/arm64/debug-exceptions.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/kvm/arm64/debug-exceptions.c b/tools/testing/selftests/kvm/arm64/debug-exceptions.c index c7fb55c9135b..e34963956fbc 100644 --- a/tools/testing/selftests/kvm/arm64/debug-exceptions.c +++ b/tools/testing/selftests/kvm/arm64/debug-exceptions.c @@ -140,7 +140,7 @@ static void enable_os_lock(void) static void enable_monitor_debug_exceptions(void) { - uint32_t mdscr; + uint64_t mdscr; asm volatile("msr daifclr, #8"); @@ -223,7 +223,7 @@ void install_hw_bp_ctx(uint8_t addr_bp, uint8_t ctx_bp, uint64_t addr, static void install_ss(void) { - uint32_t mdscr; + uint64_t mdscr; asm volatile("msr daifclr, #8"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-06-12 10:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-12 3:35 [PATCH V4 0/2] arm64/debug: Drop redundant DBG_MDSCR_* macros Anshuman Khandual 2025-06-12 3:35 ` [PATCH V4 1/2] " Anshuman Khandual 2025-06-12 8:17 ` Marc Zyngier 2025-06-12 10:24 ` Anshuman Khandual 2025-06-12 10:51 ` Marc Zyngier 2025-06-12 3:35 ` [PATCH V4 2/2] KVM: selftests: Change MDSCR_EL1 register holding variables as uint64_t Anshuman Khandual
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox