* [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP
@ 2025-04-21 5:52 Anshuman Khandual
2025-05-05 8:32 ` Anshuman Khandual
2025-05-16 13:59 ` Will Deacon
0 siblings, 2 replies; 5+ messages in thread
From: Anshuman Khandual @ 2025-04-21 5:52 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Anshuman Khandual, Will Deacon, Mark Rutland, Catalin Marinas,
linux-perf-users, linux-kernel
Array elements inside 'struct user_hwdebug_state.dbg_regs[]' are inherently
coupled with maximum breakpoints or watchpoints which could be present on a
platform and which are defined with macros ARM_MAX_[BRP|WRP].
Rather than explicitly trying to keep the array elements in sync with these
macros and then adding a BUILD_BUG_ON() just to ensure continued compliance
, move these two macros into the uapi ptrace header itself thus making them
available both for user space and kernel.
While here also ensure that ARM_MAX_BRP and ARM_MAX_WRP are always the same
via a new BUILD_BUG_ON(). This helps in making sure that user_hwdebug_state
structure remains usable both for breakpoint and watchpoint registers set
via ptrace() system call interface.
Cc: Will Deacon <will@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-perf-users@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This patch applies on v6.15-rc3
arch/arm64/include/asm/hw_breakpoint.h | 7 -------
arch/arm64/include/uapi/asm/ptrace.h | 10 +++++++++-
arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index bd81cf17744a..63c21b515647 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -75,13 +75,6 @@ static inline void decode_ctrl_reg(u32 reg,
#define ARM_KERNEL_STEP_ACTIVE 1
#define ARM_KERNEL_STEP_SUSPEND 2
-/*
- * Limits.
- * Changing these will require modifications to the register accessors.
- */
-#define ARM_MAX_BRP 16
-#define ARM_MAX_WRP 16
-
/* Virtual debug register bases. */
#define AARCH64_DBG_REG_BVR 0
#define AARCH64_DBG_REG_BCR (AARCH64_DBG_REG_BVR + ARM_MAX_BRP)
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 0f39ba4f3efd..8683f541a467 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -99,6 +99,14 @@ struct user_fpsimd_state {
__u32 __reserved[2];
};
+/*
+ * Maximum number of breakpoint and watchpoint registers
+ * on the platform. These macros get used both in kernel
+ * and user space as well.
+ */
+#define ARM_MAX_BRP 16
+#define ARM_MAX_WRP 16
+
struct user_hwdebug_state {
__u32 dbg_info;
__u32 pad;
@@ -106,7 +114,7 @@ struct user_hwdebug_state {
__u64 addr;
__u32 ctrl;
__u32 pad;
- } dbg_regs[16];
+ } dbg_regs[ARM_MAX_BRP]; /* Or ARM_MAX_WRP */
};
/* SVE/FP/SIMD state (NT_ARM_SVE & NT_ARM_SSVE) */
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index 722ac45f9f7b..9bc51682713d 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -981,6 +981,15 @@ static int __init arch_hw_breakpoint_init(void)
{
int ret;
+ /*
+ * Maximum supported breakpoint and watchpoint registers must
+ * always be the same - regardless of actual register numbers
+ * found on a given platform. This is because the user facing
+ * ptrace structure 'user_hwdebug_state' actually depends on
+ * these macros to be the same.
+ */
+ BUILD_BUG_ON(ARM_MAX_BRP != ARM_MAX_WRP);
+
core_num_brps = get_num_brps();
core_num_wrps = get_num_wrps();
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP
2025-04-21 5:52 [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP Anshuman Khandual
@ 2025-05-05 8:32 ` Anshuman Khandual
2025-05-16 13:59 ` Will Deacon
1 sibling, 0 replies; 5+ messages in thread
From: Anshuman Khandual @ 2025-05-05 8:32 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Will Deacon, Mark Rutland, Catalin Marinas, linux-perf-users,
linux-kernel
On 4/21/25 11:22, Anshuman Khandual wrote:
> Array elements inside 'struct user_hwdebug_state.dbg_regs[]' are inherently
> coupled with maximum breakpoints or watchpoints which could be present on a
> platform and which are defined with macros ARM_MAX_[BRP|WRP].
>
> Rather than explicitly trying to keep the array elements in sync with these
> macros and then adding a BUILD_BUG_ON() just to ensure continued compliance
> , move these two macros into the uapi ptrace header itself thus making them
> available both for user space and kernel.
>
> While here also ensure that ARM_MAX_BRP and ARM_MAX_WRP are always the same
> via a new BUILD_BUG_ON(). This helps in making sure that user_hwdebug_state
> structure remains usable both for breakpoint and watchpoint registers set
> via ptrace() system call interface.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This patch applies on v6.15-rc3
Gentle ping, any updates on this.
>
> arch/arm64/include/asm/hw_breakpoint.h | 7 -------
> arch/arm64/include/uapi/asm/ptrace.h | 10 +++++++++-
> arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index bd81cf17744a..63c21b515647 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -75,13 +75,6 @@ static inline void decode_ctrl_reg(u32 reg,
> #define ARM_KERNEL_STEP_ACTIVE 1
> #define ARM_KERNEL_STEP_SUSPEND 2
>
> -/*
> - * Limits.
> - * Changing these will require modifications to the register accessors.
> - */
> -#define ARM_MAX_BRP 16
> -#define ARM_MAX_WRP 16
> -
> /* Virtual debug register bases. */
> #define AARCH64_DBG_REG_BVR 0
> #define AARCH64_DBG_REG_BCR (AARCH64_DBG_REG_BVR + ARM_MAX_BRP)
> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> index 0f39ba4f3efd..8683f541a467 100644
> --- a/arch/arm64/include/uapi/asm/ptrace.h
> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> @@ -99,6 +99,14 @@ struct user_fpsimd_state {
> __u32 __reserved[2];
> };
>
> +/*
> + * Maximum number of breakpoint and watchpoint registers
> + * on the platform. These macros get used both in kernel
> + * and user space as well.
> + */
> +#define ARM_MAX_BRP 16
> +#define ARM_MAX_WRP 16
> +
> struct user_hwdebug_state {
> __u32 dbg_info;
> __u32 pad;
> @@ -106,7 +114,7 @@ struct user_hwdebug_state {
> __u64 addr;
> __u32 ctrl;
> __u32 pad;
> - } dbg_regs[16];
> + } dbg_regs[ARM_MAX_BRP]; /* Or ARM_MAX_WRP */
> };
>
> /* SVE/FP/SIMD state (NT_ARM_SVE & NT_ARM_SSVE) */
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index 722ac45f9f7b..9bc51682713d 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -981,6 +981,15 @@ static int __init arch_hw_breakpoint_init(void)
> {
> int ret;
>
> + /*
> + * Maximum supported breakpoint and watchpoint registers must
> + * always be the same - regardless of actual register numbers
> + * found on a given platform. This is because the user facing
> + * ptrace structure 'user_hwdebug_state' actually depends on
> + * these macros to be the same.
> + */
> + BUILD_BUG_ON(ARM_MAX_BRP != ARM_MAX_WRP);
> +
> core_num_brps = get_num_brps();
> core_num_wrps = get_num_wrps();
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP
2025-04-21 5:52 [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP Anshuman Khandual
2025-05-05 8:32 ` Anshuman Khandual
@ 2025-05-16 13:59 ` Will Deacon
2025-05-30 3:49 ` Anshuman Khandual
1 sibling, 1 reply; 5+ messages in thread
From: Will Deacon @ 2025-05-16 13:59 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas, linux-perf-users,
linux-kernel
On Mon, Apr 21, 2025 at 11:22:12AM +0530, Anshuman Khandual wrote:
> Array elements inside 'struct user_hwdebug_state.dbg_regs[]' are inherently
> coupled with maximum breakpoints or watchpoints which could be present on a
> platform and which are defined with macros ARM_MAX_[BRP|WRP].
>
> Rather than explicitly trying to keep the array elements in sync with these
> macros and then adding a BUILD_BUG_ON() just to ensure continued compliance
> , move these two macros into the uapi ptrace header itself thus making them
> available both for user space and kernel.
>
> While here also ensure that ARM_MAX_BRP and ARM_MAX_WRP are always the same
> via a new BUILD_BUG_ON(). This helps in making sure that user_hwdebug_state
> structure remains usable both for breakpoint and watchpoint registers set
> via ptrace() system call interface.
>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-perf-users@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> This patch applies on v6.15-rc3
>
> arch/arm64/include/asm/hw_breakpoint.h | 7 -------
> arch/arm64/include/uapi/asm/ptrace.h | 10 +++++++++-
> arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index bd81cf17744a..63c21b515647 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -75,13 +75,6 @@ static inline void decode_ctrl_reg(u32 reg,
> #define ARM_KERNEL_STEP_ACTIVE 1
> #define ARM_KERNEL_STEP_SUSPEND 2
>
> -/*
> - * Limits.
> - * Changing these will require modifications to the register accessors.
> - */
> -#define ARM_MAX_BRP 16
> -#define ARM_MAX_WRP 16
> -
> /* Virtual debug register bases. */
> #define AARCH64_DBG_REG_BVR 0
> #define AARCH64_DBG_REG_BCR (AARCH64_DBG_REG_BVR + ARM_MAX_BRP)
> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> index 0f39ba4f3efd..8683f541a467 100644
> --- a/arch/arm64/include/uapi/asm/ptrace.h
> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> @@ -99,6 +99,14 @@ struct user_fpsimd_state {
> __u32 __reserved[2];
> };
>
> +/*
> + * Maximum number of breakpoint and watchpoint registers
> + * on the platform. These macros get used both in kernel
> + * and user space as well.
> + */
> +#define ARM_MAX_BRP 16
> +#define ARM_MAX_WRP 16
> +
> struct user_hwdebug_state {
> __u32 dbg_info;
> __u32 pad;
> @@ -106,7 +114,7 @@ struct user_hwdebug_state {
> __u64 addr;
> __u32 ctrl;
> __u32 pad;
> - } dbg_regs[16];
> + } dbg_regs[ARM_MAX_BRP]; /* Or ARM_MAX_WRP */
> };
>
> /* SVE/FP/SIMD state (NT_ARM_SVE & NT_ARM_SSVE) */
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index 722ac45f9f7b..9bc51682713d 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -981,6 +981,15 @@ static int __init arch_hw_breakpoint_init(void)
> {
> int ret;
>
> + /*
> + * Maximum supported breakpoint and watchpoint registers must
> + * always be the same - regardless of actual register numbers
> + * found on a given platform. This is because the user facing
> + * ptrace structure 'user_hwdebug_state' actually depends on
> + * these macros to be the same.
> + */
> + BUILD_BUG_ON(ARM_MAX_BRP != ARM_MAX_WRP);
Sorry, but I don't understand why this patch is an improvement over what
we have.
Will
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP
2025-05-16 13:59 ` Will Deacon
@ 2025-05-30 3:49 ` Anshuman Khandual
2025-05-30 12:46 ` Will Deacon
0 siblings, 1 reply; 5+ messages in thread
From: Anshuman Khandual @ 2025-05-30 3:49 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas, linux-perf-users,
linux-kernel
On 5/16/25 19:29, Will Deacon wrote:
> On Mon, Apr 21, 2025 at 11:22:12AM +0530, Anshuman Khandual wrote:
>> Array elements inside 'struct user_hwdebug_state.dbg_regs[]' are inherently
>> coupled with maximum breakpoints or watchpoints which could be present on a
>> platform and which are defined with macros ARM_MAX_[BRP|WRP].
>>
>> Rather than explicitly trying to keep the array elements in sync with these
>> macros and then adding a BUILD_BUG_ON() just to ensure continued compliance
>> , move these two macros into the uapi ptrace header itself thus making them
>> available both for user space and kernel.
>>
>> While here also ensure that ARM_MAX_BRP and ARM_MAX_WRP are always the same
>> via a new BUILD_BUG_ON(). This helps in making sure that user_hwdebug_state
>> structure remains usable both for breakpoint and watchpoint registers set
>> via ptrace() system call interface.
>>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-perf-users@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>> This patch applies on v6.15-rc3
>>
>> arch/arm64/include/asm/hw_breakpoint.h | 7 -------
>> arch/arm64/include/uapi/asm/ptrace.h | 10 +++++++++-
>> arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
>> 3 files changed, 18 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
>> index bd81cf17744a..63c21b515647 100644
>> --- a/arch/arm64/include/asm/hw_breakpoint.h
>> +++ b/arch/arm64/include/asm/hw_breakpoint.h
>> @@ -75,13 +75,6 @@ static inline void decode_ctrl_reg(u32 reg,
>> #define ARM_KERNEL_STEP_ACTIVE 1
>> #define ARM_KERNEL_STEP_SUSPEND 2
>>
>> -/*
>> - * Limits.
>> - * Changing these will require modifications to the register accessors.
>> - */
>> -#define ARM_MAX_BRP 16
>> -#define ARM_MAX_WRP 16
>> -
>> /* Virtual debug register bases. */
>> #define AARCH64_DBG_REG_BVR 0
>> #define AARCH64_DBG_REG_BCR (AARCH64_DBG_REG_BVR + ARM_MAX_BRP)
>> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
>> index 0f39ba4f3efd..8683f541a467 100644
>> --- a/arch/arm64/include/uapi/asm/ptrace.h
>> +++ b/arch/arm64/include/uapi/asm/ptrace.h
>> @@ -99,6 +99,14 @@ struct user_fpsimd_state {
>> __u32 __reserved[2];
>> };
>>
>> +/*
>> + * Maximum number of breakpoint and watchpoint registers
>> + * on the platform. These macros get used both in kernel
>> + * and user space as well.
>> + */
>> +#define ARM_MAX_BRP 16
>> +#define ARM_MAX_WRP 16
>> +
>> struct user_hwdebug_state {
>> __u32 dbg_info;
>> __u32 pad;
>> @@ -106,7 +114,7 @@ struct user_hwdebug_state {
>> __u64 addr;
>> __u32 ctrl;
>> __u32 pad;
>> - } dbg_regs[16];
>> + } dbg_regs[ARM_MAX_BRP]; /* Or ARM_MAX_WRP */
>> };
>>
>> /* SVE/FP/SIMD state (NT_ARM_SVE & NT_ARM_SSVE) */
>> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
>> index 722ac45f9f7b..9bc51682713d 100644
>> --- a/arch/arm64/kernel/hw_breakpoint.c
>> +++ b/arch/arm64/kernel/hw_breakpoint.c
>> @@ -981,6 +981,15 @@ static int __init arch_hw_breakpoint_init(void)
>> {
>> int ret;
>>
>> + /*
>> + * Maximum supported breakpoint and watchpoint registers must
>> + * always be the same - regardless of actual register numbers
>> + * found on a given platform. This is because the user facing
>> + * ptrace structure 'user_hwdebug_state' actually depends on
>> + * these macros to be the same.
>> + */
>> + BUILD_BUG_ON(ARM_MAX_BRP != ARM_MAX_WRP);
>
> Sorry, but I don't understand why this patch is an improvement over what
> we have.
This is an improvement because
- user_hwdebug_state.dbg_regs[16] is hard coded for size without context
requiring explicit sync up when ARM_MAX_WRP/BRP changes to 64 later on
with FEAT_Debugv8p9. Defining the array size in terms of ARM_MAX_WRP/
BRP provides required context and also avoids the need for an explicit
hard coded sync up when those macros change.
- user_hwdebug_state.dbg_regs[16] gets used both for breakpoint registers
and watchpoint registers. Hence there is an inherent assumption that
ARM_MAX_BRP == ARM_MAX_WRP which should be ensured with a corresponding
assert.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP
2025-05-30 3:49 ` Anshuman Khandual
@ 2025-05-30 12:46 ` Will Deacon
0 siblings, 0 replies; 5+ messages in thread
From: Will Deacon @ 2025-05-30 12:46 UTC (permalink / raw)
To: Anshuman Khandual
Cc: linux-arm-kernel, Mark Rutland, Catalin Marinas, linux-perf-users,
linux-kernel
On Fri, May 30, 2025 at 09:19:53AM +0530, Anshuman Khandual wrote:
>
>
> On 5/16/25 19:29, Will Deacon wrote:
> > On Mon, Apr 21, 2025 at 11:22:12AM +0530, Anshuman Khandual wrote:
> >> Array elements inside 'struct user_hwdebug_state.dbg_regs[]' are inherently
> >> coupled with maximum breakpoints or watchpoints which could be present on a
> >> platform and which are defined with macros ARM_MAX_[BRP|WRP].
> >>
> >> Rather than explicitly trying to keep the array elements in sync with these
> >> macros and then adding a BUILD_BUG_ON() just to ensure continued compliance
> >> , move these two macros into the uapi ptrace header itself thus making them
> >> available both for user space and kernel.
> >>
> >> While here also ensure that ARM_MAX_BRP and ARM_MAX_WRP are always the same
> >> via a new BUILD_BUG_ON(). This helps in making sure that user_hwdebug_state
> >> structure remains usable both for breakpoint and watchpoint registers set
> >> via ptrace() system call interface.
> >>
> >> Cc: Will Deacon <will@kernel.org>
> >> Cc: Mark Rutland <mark.rutland@arm.com>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-perf-users@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >> This patch applies on v6.15-rc3
> >>
> >> arch/arm64/include/asm/hw_breakpoint.h | 7 -------
> >> arch/arm64/include/uapi/asm/ptrace.h | 10 +++++++++-
> >> arch/arm64/kernel/hw_breakpoint.c | 9 +++++++++
> >> 3 files changed, 18 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> >> index bd81cf17744a..63c21b515647 100644
> >> --- a/arch/arm64/include/asm/hw_breakpoint.h
> >> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> >> @@ -75,13 +75,6 @@ static inline void decode_ctrl_reg(u32 reg,
> >> #define ARM_KERNEL_STEP_ACTIVE 1
> >> #define ARM_KERNEL_STEP_SUSPEND 2
> >>
> >> -/*
> >> - * Limits.
> >> - * Changing these will require modifications to the register accessors.
> >> - */
> >> -#define ARM_MAX_BRP 16
> >> -#define ARM_MAX_WRP 16
> >> -
> >> /* Virtual debug register bases. */
> >> #define AARCH64_DBG_REG_BVR 0
> >> #define AARCH64_DBG_REG_BCR (AARCH64_DBG_REG_BVR + ARM_MAX_BRP)
> >> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> >> index 0f39ba4f3efd..8683f541a467 100644
> >> --- a/arch/arm64/include/uapi/asm/ptrace.h
> >> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> >> @@ -99,6 +99,14 @@ struct user_fpsimd_state {
> >> __u32 __reserved[2];
> >> };
> >>
> >> +/*
> >> + * Maximum number of breakpoint and watchpoint registers
> >> + * on the platform. These macros get used both in kernel
> >> + * and user space as well.
> >> + */
> >> +#define ARM_MAX_BRP 16
> >> +#define ARM_MAX_WRP 16
> >> +
> >> struct user_hwdebug_state {
> >> __u32 dbg_info;
> >> __u32 pad;
> >> @@ -106,7 +114,7 @@ struct user_hwdebug_state {
> >> __u64 addr;
> >> __u32 ctrl;
> >> __u32 pad;
> >> - } dbg_regs[16];
> >> + } dbg_regs[ARM_MAX_BRP]; /* Or ARM_MAX_WRP */
> >> };
> >>
> >> /* SVE/FP/SIMD state (NT_ARM_SVE & NT_ARM_SSVE) */
> >> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> >> index 722ac45f9f7b..9bc51682713d 100644
> >> --- a/arch/arm64/kernel/hw_breakpoint.c
> >> +++ b/arch/arm64/kernel/hw_breakpoint.c
> >> @@ -981,6 +981,15 @@ static int __init arch_hw_breakpoint_init(void)
> >> {
> >> int ret;
> >>
> >> + /*
> >> + * Maximum supported breakpoint and watchpoint registers must
> >> + * always be the same - regardless of actual register numbers
> >> + * found on a given platform. This is because the user facing
> >> + * ptrace structure 'user_hwdebug_state' actually depends on
> >> + * these macros to be the same.
> >> + */
> >> + BUILD_BUG_ON(ARM_MAX_BRP != ARM_MAX_WRP);
> >
> > Sorry, but I don't understand why this patch is an improvement over what
> > we have.
> This is an improvement because
>
> - user_hwdebug_state.dbg_regs[16] is hard coded for size without context
> requiring explicit sync up when ARM_MAX_WRP/BRP changes to 64 later on
> with FEAT_Debugv8p9. Defining the array size in terms of ARM_MAX_WRP/
> BRP provides required context and also avoids the need for an explicit
> hard coded sync up when those macros change.
>
> - user_hwdebug_state.dbg_regs[16] gets used both for breakpoint registers
> and watchpoint registers. Hence there is an inherent assumption that
> ARM_MAX_BRP == ARM_MAX_WRP which should be ensured with a corresponding
> assert.
Sorry, but I still don't see it. Plus, this is a UAPI header so I think
it's no bad thing to require an "explicit sync up", assuming we can
actually change this structure at all.
Let's leave the code as it is, please.
Will
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-30 12:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-21 5:52 [PATCH] arm64/ptrace: Make user_hwdebug_state.dbg_regs[] array size as ARM_MAX_BRP Anshuman Khandual
2025-05-05 8:32 ` Anshuman Khandual
2025-05-16 13:59 ` Will Deacon
2025-05-30 3:49 ` Anshuman Khandual
2025-05-30 12:46 ` Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).