Re: [PATCH v1 41/41] perf test: Add perf sysinfo shell test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf test: sysinfo.sh fails on archs without userspace perf_regs tables
--

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

perf test: Add perf sysinfo shell test

This commit adds a shell test exercising default perf sysinfo summary
output, --pmus per-PMU output, -j JSON validation, PMU glob filtering,
and -i perf.data inspection.

> diff --git a/tools/perf/tests/shell/sysinfo.sh b/tools/perf/tests/shell/sysinfo.sh
> new file mode 100755
> index 0000000000000..59091b4d750d6
> --- /dev/null
> +++ b/tools/perf/tests/shell/sysinfo.sh
> @@ -0,0 +1,83 @@

[ ... ]

> +test_sysinfo_filter() {
> +  echo "Filtered perf sysinfo output test"
> +  perf sysinfo -o "${sysinfo_output}" software
> +  grep -q "^software (" "${sysinfo_output}"
> +  if grep -q "Modes:.*sampling" "${sysinfo_output}"; then
> +    grep -q "Sample types:.*ip" "${sysinfo_output}"
> +    if grep -q "Sample types:.*regs_user" "${sysinfo_output}"; then
> +      grep -q "User registers:" "${sysinfo_output}"
> +    fi
> +    if grep -q "Sample types:.*regs_intr" "${sysinfo_output}"; then
> +      grep -q "Intr registers:" "${sysinfo_output}"
> +    fi

[Severity: Medium]
Can test_sysinfo_filter() fail under set -e on architectures such as Alpha
and PA-RISC where the kernel enables CONFIG_HAVE_PERF_REGS, but userspace
tools/perf/util/perf_regs.c lacks register mask tables?

During PMU probing in tools/perf/util/pmu.c:probe_sample_types(), the probe
falls back to bit 0 when perf_arch_reg_ip(EM_HOST) returns 0:

	u64 reg_bit = BIT_ULL(perf_arch_reg_ip(EM_HOST)) ?: 1ULL;

On Alpha and PA-RISC, kernel perf_reg_validate() accepts bit 0, so
PERF_SAMPLE_REGS_USER and PERF_SAMPLE_REGS_INTR succeed and print_pmu_text()
outputs regs_user and regs_intr in the "Sample types:" line.

However, when tools/perf/util/pmu.c:probe_pmu_regs_one() queries the
register masks:

	u64 gpr_mask = intr ? perf_intr_reg_mask(EM_HOST, &dummy_abi) :
			      perf_user_reg_mask(EM_HOST, &dummy_abi);

both perf_intr_reg_mask() and perf_user_reg_mask() in
tools/perf/util/perf_regs.c return 0 for unhandled ELF machines like
EM_ALPHA and EM_PARISC, leaving user_regs_mask and intr_regs_mask as 0.
Then tools/perf/builtin-sysinfo.c:print_probed_regs() returns early without
printing "User registers:" or "Intr registers:":

	if (!mask && !simd_mask && !pred_mask)
		return;

Would this cause the inner grep checks for "User registers:" and
"Intr registers:" in test_sysinfo_filter() to fail and abort the test on
those architectures?

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