From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 236AFC71134 for ; Tue, 10 Jun 2025 22:48:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Type:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To :Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=KeD8Zp/24xgVlGUQo4vJyZJKB+OmxD0TPDaIjWkeY8A=; b=QpPYTr310e4eJoGcPOSCxFIVnC VxAsxAbSz6Qfu7RVy+bK3x5l6kRZLv7mzFNmtvTeplsq9KS/ZMSN/1fywPNjxKnj+JkPXRHeEKHNn vColbHOwHjhEhuB31auIZ6YI3A6p51L+a5N4mZsNP1Ee4PYEDr7uwpRFUn4s60pUWYCOsN5mZPxKc RReOWjpDSqSoCt6ViUl48RjkMwEMSMtilsie9QLtr86YuExzEq9zfTO+7aG5Gtw7DVkQiUwtNzIMd EyhqF3U5ERo5xcY4A+0u8T/yFk4eBdka5Wb9SQOFka7GIxm7CyS+Z6Rv0OJhWkPs5u0LNwAgnLjv6 VRHEc7vw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uP7lD-00000008Ee4-42Zc; Tue, 10 Jun 2025 22:48:03 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uP2Xz-00000007aXw-0HuC for linux-arm-kernel@lists.infradead.org; Tue, 10 Jun 2025 17:14:04 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A03ED14BF; Tue, 10 Jun 2025 10:13:37 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1F6AF3F66E; Tue, 10 Jun 2025 10:13:56 -0700 (PDT) Date: Tue, 10 Jun 2025 18:13:50 +0100 From: Mark Rutland To: Anshuman Khandual Subject: Re: [PATCH V3 1/2] arm64/debug: Drop redundant DBG_MDSCR_* macros Message-ID: References: <20250610053128.4118784-1-anshuman.khandual@arm.com> <20250610053128.4118784-2-anshuman.khandual@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250610053128.4118784-2-anshuman.khandual@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250610_101403_202389_100C7E1B X-CRM114-Status: GOOD ( 23.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jun 10, 2025 at 11:01:27AM +0530, Anshuman Khandual wrote: > 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. I think that for now it'd be best to *only* change over to the generated MDSCR_EL1_* defintions, and leave the register sizes as-is. Those are logically distinct changes, and AFAICT the latter is a requirement for using extended breakpoints, where it would be clearer to have that change as part of the series adding that support, with an explanation as to why we care. Mark. > > Cc: Catalin Marinas > Cc: Will Deacon > Cc: Mark Rutland > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Reviewed-by: Ada Couprie Diaz > Signed-off-by: Anshuman Khandual > --- > 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 > > /* 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 >