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

["Rui Qi" <qirui.001@bytedance.com>]

On 11/10/26 5:58 AM, Ian Rogers wrote:
> On Mon, Nov 9, 2026 at 4:06 AM Rui Qi <qirui.001@bytedance.com> 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@bytedance.com>
> 
> 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

Hi Ian,

Thanks — v2 posted as "[PATCH v2] perf parse-events: default data
breakpoints to any size on RISC-V".  It exports
default_data_breakpoint_len()
and switches the data-breakpoint assertions to it; the rw checker now takes
the expected length, so the explicit /4 cases still assert
HW_BREAKPOINT_LEN_4.

I ran 'perf test -v 5' on riscv64 with the patched tree: the bp_len
assertions pass.  The only failure left is the pre-existing "wrong config2"
in 5.3, which reproduces without my change as well.

Rui

> 
>> ---
>>  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
>>