Re: [PATCH v2] arm64: hw_breakpoint: warn on invalid breakpoint length

[Leo Yan <leo.yan@arm.com>]

Hi,

On Sat, Dec 13, 2025 at 12:23:35PM +0100, Osama Abdelkader wrote:
> Some tools (e.g. perf) incorrectly assume that breakpoints should be
> sizeof(long), but this is wrong for AArch64 where breakpoints must be
> 4 bytes. Add a warning when we silently fix up the parameter so tool
> developers can get notified.

To be clear, the perf tool has fixed the issue since:

  fa6cc3f93258 ("perf parse-events: Vary default_breakpoint_len on i386 and arm64")

The latest perf can pass the correct length (4) for instruction
breakpoint.

> This addresses the FIXME comment by adding diagnostic output rather
> than breaking existing tools by returning -EINVAL.
> 
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2: fix warning messageline splitting
> ---
>  arch/arm64/kernel/hw_breakpoint.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> index ab76b36dce82..cce306145d78 100644
> --- a/arch/arm64/kernel/hw_breakpoint.c
> +++ b/arch/arm64/kernel/hw_breakpoint.c
> @@ -475,11 +475,13 @@ static int arch_build_bp_info(struct perf_event *bp,
>  				return -EINVAL;
>  		} else if (hw->ctrl.len != ARM_BREAKPOINT_LEN_4) {
>  			/*
> -			 * FIXME: Some tools (I'm looking at you perf) assume
> -			 *	  that breakpoints should be sizeof(long). This
> -			 *	  is nonsense. For now, we fix up the parameter
> -			 *	  but we should probably return -EINVAL instead.

This FIXME is still valid for me.  IMO, if really want to remove the
comment, a reasonable solution is:

  1) Backport two patches (fa6cc3f93258 and 70b27c756f9) onto the
     stable kernel code base for fixing perf tool.

  2) Based on perf tool has fixed the issue on the mainline kernel and
     stable kernels, we can change to return -EINVAL instead.

> +			 * Some tools (e.g. perf) incorrectly assume that
> +			 * breakpoints should be sizeof(long). This is wrong
> +			 * for AArch64 where breakpoints must be 4 bytes.
> +			 * Warn the user and fix up the parameter.
>  			 */
> +			pr_warn_once("hw_breakpoint: invalid AArch64 breakpoint length %d, fixing to 4 bytes\n",
> +				     hw->ctrl.len);

AFAIK, since I started to use Armv5 CPUs, Arm instructions (not Thumb
ones) are always 4-byte length. Not sure if the log is useful or not,
I would leave this to maintainers.

Thanks,
Leo

>  			hw->ctrl.len = ARM_BREAKPOINT_LEN_4;
>  		}
>  	}
> -- 
> 2.43.0
> 
>