Messages in this thread | | | Date | Wed, 16 Aug 2023 10:14:33 +0100 | Subject | Re: [PATCH v5 3/6] perf test: Add a test for the new Arm CPU ID comparison behavior | From | James Clark <> |
| |
On 15/08/2023 10:47, John Garry wrote: > On 11/08/2023 15:39, James Clark wrote: >> Now that variant and revision fields are taken into account the behavior >> is slightly more complicated so add a test to ensure that this behaves >> as expected. >> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> tools/perf/arch/arm64/include/arch-tests.h | 3 ++ >> tools/perf/arch/arm64/tests/Build | 1 + >> tools/perf/arch/arm64/tests/arch-tests.c | 4 +++ >> tools/perf/arch/arm64/tests/cpuid-match.c | 38 ++++++++++++++++++++++ >> 4 files changed, 46 insertions(+) >> create mode 100644 tools/perf/arch/arm64/tests/cpuid-match.c >> >> diff --git a/tools/perf/arch/arm64/include/arch-tests.h >> b/tools/perf/arch/arm64/include/arch-tests.h >> index 452b3d904521..474d7cf5afbd 100644 >> --- a/tools/perf/arch/arm64/include/arch-tests.h >> +++ b/tools/perf/arch/arm64/include/arch-tests.h >> @@ -2,6 +2,9 @@ >> #ifndef ARCH_TESTS_H >> #define ARCH_TESTS_H >> +struct test_suite; >> + >> +int test__cpuid_match(struct test_suite *test, int subtest); >> extern struct test_suite *arch_tests[]; >> #endif >> diff --git a/tools/perf/arch/arm64/tests/Build >> b/tools/perf/arch/arm64/tests/Build >> index a61c06bdb757..e337c09e7f56 100644 >> --- a/tools/perf/arch/arm64/tests/Build >> +++ b/tools/perf/arch/arm64/tests/Build >> @@ -2,3 +2,4 @@ perf-y += regs_load.o >> perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o >> perf-y += arch-tests.o >> +perf-y += cpuid-match.o >> diff --git a/tools/perf/arch/arm64/tests/arch-tests.c >> b/tools/perf/arch/arm64/tests/arch-tests.c >> index ad16b4f8f63e..74932e72c727 100644 >> --- a/tools/perf/arch/arm64/tests/arch-tests.c >> +++ b/tools/perf/arch/arm64/tests/arch-tests.c >> @@ -3,9 +3,13 @@ >> #include "tests/tests.h" >> #include "arch-tests.h" >> + >> +DEFINE_SUITE("arm64 CPUID matching", cpuid_match); >> + >> struct test_suite *arch_tests[] = { >> #ifdef HAVE_DWARF_UNWIND_SUPPORT >> &suite__dwarf_unwind, >> #endif >> + &suite__cpuid_match, >> NULL, >> }; >> diff --git a/tools/perf/arch/arm64/tests/cpuid-match.c >> b/tools/perf/arch/arm64/tests/cpuid-match.c >> new file mode 100644 >> index 000000000000..af0871b54ae7 >> --- /dev/null >> +++ b/tools/perf/arch/arm64/tests/cpuid-match.c >> @@ -0,0 +1,38 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +#include <linux/compiler.h> >> + >> +#include "arch-tests.h" >> +#include "tests/tests.h" >> +#include "util/header.h" >> + >> +int test__cpuid_match(struct test_suite *test __maybe_unused, >> + int subtest __maybe_unused) >> +{ >> + /* midr with no leading zeros matches */ >> + if (strcmp_cpuid_str("0x410fd0c0", "0x00000000410fd0c0")) >> + return -1; >> + /* Upper case matches */ >> + if (strcmp_cpuid_str("0x410fd0c0", "0x00000000410FD0C0")) >> + return -1; >> + /* r0p0 = r0p0 matches */ >> + if (strcmp_cpuid_str("0x00000000410fd480", "0x00000000410fd480")) >> + return -1; >> + /* r0p1 > r0p0 matches */ >> + if (strcmp_cpuid_str("0x00000000410fd480", "0x00000000410fd481")) >> + return -1; >> + /* r1p0 > r0p0 matches*/ >> + if (strcmp_cpuid_str("0x00000000410fd480", "0x00000000411fd480")) >> + return -1; >> + /* r0p0 < r0p1 doesn't match */ >> + if (!strcmp_cpuid_str("0x00000000410fd481", "0x00000000410fd480")) >> + return -1; >> + /* r0p0 < r1p0 doesn't match */ >> + if (!strcmp_cpuid_str("0x00000000411fd480", "0x00000000410fd480")) >> + return -1; >> + /* Different CPU doesn't match */ >> + if (!strcmp_cpuid_str("0x00000000410fd4c0", "0x00000000430f0af0")) >> + return -1; >> + >> + return 0; >> +} >> + > > Would it be possible to put this in core test code, since x86 also > supports strcmp_cpuid_str()? >
That's how I started, but Ian suggested to move it to an arch specific folder because that's what it was testing.
We could still add test__cpuid_match() in the x86 folder rather than adding it with #ifdefs, but I don't think it needs to be done here because I haven't touched the x86 code.
> Maybe we would have an structure per arch of cpuids and expected > results, like > > struct cpuid_match { > char *cpuid1; > char *cpuid1; > int expected_result; > }; > > > #ifdef ARM64 > cpuid_match_array[] = { > {"0x410fd0c0", "0x00000000410FD0C0", -1}, > {"0x00000000410fd480", "0x00000000410fd480", -1}, > ... > {} /* sentinel */ > > }; > #else if defined(X86) > cpuid_match_array[] = { > {....} > ... > {} /* sentinel */ > > }; > #else > /* no support */ > #endif > > Thanks, > John
| |