Re: [PATCH] perf parse-events: default data breakpoints to any size on RISC-V

[Ian Rogers]

On Mon, Nov 9, 2026 at 4:06 AM Rui Qi <qirui.001@xxxxxxxxxxxxx> wrote:
>
> When the length of a data breakpoint is not specified on the command
> line, perf defaults it to HW_BREAKPOINT_LEN_4:
>
>   $ perf record -e mem:0x30020:w ./prog
>
> That assumes the length describes a byte range that is compared
> against the address of the access, which is how the x86 debug
> registers and most other architectures work.
>
> RISC-V debug triggers behave differently.  An Sdtrig mcontrol6
> trigger encodes the width of the memory access it should match, and
> it only fires for accesses of exactly that width.  Watching an 8-byte
> variable with the implicit 4-byte length therefore never triggers:
> the perf event is created and enabled, but no event is ever
> delivered:
>
>   $ perf stat -e mem:0x30020:w ./prog          # 8-byte stores
>   0       mem:0x30020:w
>   $ perf stat -e mem:0x30020/8:w ./prog
>   10      mem:0x30020:w
>
> The failure is silent and easily mistaken for a broken hardware
> breakpoint implementation.  Tracing hw_breakpoint_exceptions_notify()
> while the event is active confirms the trigger never fires.
>
> Zero is a valid length for RISC-V data breakpoints: the kernel
> encodes it as size 0, which means "match any size".  That is what a
> user leaving out the length asks for, so use it as the default on
> RISC-V and keep the existing 4-byte default elsewhere.
>
> Tested on a riscv64 kernel with hardware breakpoint support.  With the
> default length, watching an 8-byte and a 4-byte variable both count
> 10 stores, while explicit lengths keep their exact-width behaviour:
> /8 only matches the 8-byte access and /4 only the 4-byte one.
>
> Signed-off-by: Rui Qi <qirui.001@xxxxxxxxxxxxx>

The change makes sense to me, and I may have been the last person to
tweak how this parsing runs. Could you address at least the test
failure issue with something like:

- TEST_ASSERT_EVSEL("wrong bp_len", HW_BREAKPOINT_LEN_4 ==
evsel->core.attr.bp_len, evsel);
+ TEST_ASSERT_EVSEL("wrong bp_len", default_data_breakpoint_len() ==
evsel->core.attr.bp_len, evsel);

and then I'll add my tag.

Thanks,
Ian

> ---
>  tools/perf/util/parse-events.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index f3ba70d4f76b..b72e4b0da8a1 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -598,6 +598,27 @@ size_t default_breakpoint_len(void)
>  #endif
>  }
>
> +/*
> + * Default length for data breakpoints when the user did not specify one.
> + *
> + * Most architectures treat the length as a byte range that is compared
> + * against the address of the access, so 4 bytes is a reasonable default
> + * that matches most accesses.  RISC-V debug triggers (Sdtrig mcontrol6)
> + * encode the width of the access in the trigger instead, and a trigger
> + * only fires when the access has exactly that width.  This holds for
> + * both RV32 and RV64.  With a fixed default, an access of a different
> + * width silently never matches, so pass 0 on RISC-V, which the kernel
> + * translates to "match any size".
> + */
> +static size_t default_data_breakpoint_len(void)
> +{
> +#if defined(__riscv)
> +       return 0;
> +#else
> +       return HW_BREAKPOINT_LEN_4;
> +#endif
> +}
> +
>  static int
>  parse_breakpoint_type(const char *type, struct perf_event_attr *attr)
>  {
> @@ -658,7 +679,7 @@ int parse_events_add_breakpoint(struct parse_events_state *parse_state,
>                 if (attr.bp_type == HW_BREAKPOINT_X)
>                         len = default_breakpoint_len();
>                 else
> -                       len = HW_BREAKPOINT_LEN_4;
> +                       len = default_data_breakpoint_len();
>         }
>
>         attr.bp_len = len;
> --
> 2.52.0
>