Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH] perf parse-events: default data breakpoints to any size on RISC-V
@ 2026-11-09 12:03 Rui Qi
  2026-11-09 12:15 ` sashiko-bot
  2026-11-09 21:58 ` Ian Rogers
  0 siblings, 2 replies; 4+ messages in thread
From: Rui Qi @ 2026-11-09 12:03 UTC (permalink / raw)
  To: linux-perf-users
  Cc: linux-kernel, linux-riscv, peterz, mingo, acme, namhyung, Rui Qi

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf parse-events: default data breakpoints to any size on RISC-V
  2026-11-09 12:03 [PATCH] perf parse-events: default data breakpoints to any size on RISC-V Rui Qi
@ 2026-11-09 12:15 ` sashiko-bot
  2026-11-09 21:58 ` Ian Rogers
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-09 12:15 UTC (permalink / raw)
  To: Rui Qi, linux-perf-users

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] perf parse-events: RISC-V bp_len=0 default breaks
  parse-events unit tests
- [Low] perf parse-events: default_data_breakpoint_len() uses unmerged
  bp_len=0 ABI
--

commit bc358ce307d173b53ca7362db7fd768d0758ca0f
Author: Rui Qi <qirui.001@bytedance.com>

perf parse-events: default data breakpoints to any size on RISC-V

This commit adds default_data_breakpoint_len() to default unspecified data
breakpoint lengths to 0 on RISC-V while keeping HW_BREAKPOINT_LEN_4 on
other architectures.

> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index f3ba70d4f76b2..b72e4b0da8a15 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
> +}

[Severity: Low]
Does default_data_breakpoint_len() rely on a kernel ABI that is not yet in
mainline?

Mainline arch/riscv does not select HAVE_HW_BREAKPOINT or implement
hw_breakpoint_arch_parse(), and include/uapi/linux/hw_breakpoint.h only
defines HW_BREAKPOINT_LEN_1 through HW_BREAKPOINT_LEN_8 to describe a
watched byte range (as relied upon by test__wp_modify() in
tools/perf/tests/wp.c).  All in-tree hw_breakpoint_arch_parse()
implementations reject attr->bp_len == 0 with -EINVAL, so baking bp_len = 0
into tools/perf before the RISC-V kernel hw_breakpoint support and UAPI are
merged upstream risks breaking default data breakpoints if the upstream
implementation follows standard byte-range semantics.

>  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();
>  	}

[Severity: Medium]
Does this regression break the "Parse event definition strings" unit test
on RISC-V?

When commit 70b27c756f95 ("perf parse-events: Add default_breakpoint_len
helper") introduced default_breakpoint_len() for instruction breakpoints,
it exported the helper in tools/perf/util/parse-events.h and updated
test__checkevent_breakpoint_x() in tools/perf/tests/parse-events.c.  Here,
test__checkevent_breakpoint(), test__checkevent_breakpoint_r(),
test__checkevent_breakpoint_w(), and test__checkevent_breakpoint_rw() still
assert that evsel->core.attr.bp_len equals HW_BREAKPOINT_LEN_4:

tools/perf/tests/parse-events.c:test__checkevent_breakpoint() {
    ...
	TEST_ASSERT_EVSEL("wrong bp_len", HW_BREAKPOINT_LEN_4 == evsel->core.attr.bp_len, evsel);
    ...
}

On RISC-V, parsing data breakpoints without an explicit length (such as
mem:0, mem:0:r, mem:0:w, and mem:0:rw) now sets attr.bp_len to 0, causing
all 16 un-sized data breakpoint test cases to fail.  Exporting
default_data_breakpoint_len() for those checks (while keeping the explicit
HW_BREAKPOINT_LEN_4 check for test__checkevent_breakpoint_len_rw_modifier(),
which tests mem:0/4:rw:u and delegates to test__checkevent_breakpoint_rw())
would keep the test suite passing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109120320.75053-2-qirui.001@bytedance.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf parse-events: default data breakpoints to any size on RISC-V
  2026-11-09 12:03 [PATCH] perf parse-events: default data breakpoints to any size on RISC-V Rui Qi
  2026-11-09 12:15 ` sashiko-bot
@ 2026-11-09 21:58 ` Ian Rogers
  2026-11-11  6:23   ` Rui Qi
  1 sibling, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2026-11-09 21:58 UTC (permalink / raw)
  To: Rui Qi
  Cc: linux-perf-users, linux-kernel, linux-riscv, peterz, mingo, acme,
	namhyung

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf parse-events: default data breakpoints to any size on RISC-V
  2026-11-09 21:58 ` Ian Rogers
@ 2026-11-11  6:23   ` Rui Qi
  0 siblings, 0 replies; 4+ messages in thread
From: Rui Qi @ 2026-11-11  6:23 UTC (permalink / raw)
  To: Ian Rogers
  Cc: linux-perf-users, linux-kernel, linux-riscv, peterz, mingo, acme,
	namhyung

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-11  6:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 12:03 [PATCH] perf parse-events: default data breakpoints to any size on RISC-V Rui Qi
2026-11-09 12:15 ` sashiko-bot
2026-11-09 21:58 ` Ian Rogers
2026-11-11  6:23   ` Rui Qi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox