Re: [RFC PATCH v2 6/9] perf pmu: Probe registers, memory events, rdpmc, and branch skid

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf pmu: 64-bit operand in rv32 asm constraint in run_skid_branch_loop()
--

commit 1d20e4dd9d51fee9a24330f6bf13bda0cca94b25
Author: Ian Rogers <irogers@google.com>

perf pmu: Probe registers, memory events, rdpmc, and branch skid

This commit extends perf_pmu__probe() in tools/perf/util/pmu.c to probe
supported user and interrupt sampling register masks, memory profiling
events and sample formats, user-space mmap counter reading (rdpmc) and PMC
bit-width, and empirically measured branch instruction skid across
precision levels.

> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 83c8d6f392ba9..15a21f8c65ed4 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -3356,6 +3356,739 @@ static void probe_sample_types(struct perf_pmu *pmu,

[ ... ]

> +static noinline bool run_skid_branch_loop(u64 iters, uintptr_t *out_s_ip,
> +					  uintptr_t *out_b_ip,
> +					  uintptr_t *out_e_ip,
> +					  int *out_insn_sz, int *out_nop_cnt)
> +{

[ ... ]

> +#elif defined(__riscv)
> +	uintptr_t s_ip, b_ip, e_ip;
> +
> +	asm volatile(
> +		".option push\n\t"
> +		".option norvc\n\t"
> +		"la %[s_ip], 1f\n\t"
> +		"la %[b_ip], 2f\n\t"
> +		"la %[e_ip], 3f\n\t"
> +		".balign 64\n"
> +		"1:\n\t"
> +		".rept 512\n\t"
> +		"nop\n\t"
> +		".endr\n\t"
> +		"addi %[cnt], %[cnt], -1\n\t"
> +		"2:\n\t"
> +		"bnez %[cnt], 1b\n"
> +		"3:\n\t"
> +		".option pop\n\t"
> +		: [cnt] "+r" (iters),

[Severity: Low]
Will this fail to build on 32-bit RISC-V (__riscv_xlen == 32) when
compiling with Clang?

In run_skid_branch_loop(), the loop counter parameter iters is declared as
a 64-bit u64. Unlike the x86 and ARM blocks which are guarded by 64-bit
target macros (__x86_64__ and __aarch64__), #elif defined(__riscv) matches
both 32-bit and 64-bit RISC-V targets.

On 32-bit RISC-V, general-purpose registers are 32 bits wide, so binding
the 64-bit u64 variable iters to the single-register constraint "+r"
causes Clang to reject the inline assembly with a register size mismatch
error.

Should iters be changed to unsigned long (or uintptr_t), or should the
preprocessor guard check defined(__riscv) && __riscv_xlen == 64?

> +		  [s_ip] "=&r" (s_ip),
> +		  [b_ip] "=&r" (b_ip),
> +		  [e_ip] "=&r" (e_ip)
> +		:
> +		: "memory"
> +	);

[ ... ]

> +}

[ ... ]

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017185442.2137623-1-irogers@google.com?part=6