| Message ID | IA1PR20MB4953E8FFED81D5733DDFDA2DBB799@IA1PR20MB4953.namprd20.prod.outlook.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | perf: add T-HEAD C9xx series cpu support | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Series has a cover letter |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD ac9a78681b92 |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 6 and now 6 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 14 this patch: 14 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 28 this patch: 28 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 19 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
> > The T-HEAD C9xx series CPU only has MVENDOR defined, and left MARCH > > and MIMP unimplemented. > > According to the docs you can still read them, but it's hardwired to > 64h0. > > How it's supposed to distinguish c906 and c910 for example ? It is unnecessary to distinguish c9xx, their event index is compatible. The dtb and opensbi will final decide which event can be used. > What does /proc/cpuinfo shows on c9xx ? Why can't we use zeroes ? The content is as follows. processor : 0 hart : 0 isa : rv64imafdc mmu : sv39 uarch : thead,c910 mvendorid : 0x5b7 marchid : 0x0 mimpid : 0x0 The `mvendorid`, `marchid`, `mimpid` are the same across allwinner D1 (C906), T-HEAD th1520 (C910) and the sophgo mango (C920). It seems T-HEAD use MCPUID CSR to store CPU info. But this is not standard and not shown in cpuinfo.
Hello Inochi Amaoto! On Tue, 2023-05-16 at 10:37 +0800, Inochi Amaoto wrote: > The T-HEAD C9xx series CPU only has MVENDOR defined, and left MARCH > and MIMP unimplemented. According to the docs you can still read them, but it's hardwired to 64h0. How it's supposed to distinguish c906 and c910 for example ? > > To make perf support T-HEAD C9xx events. remove the restriction of > the MARCH and MIMP. > > Signed-off-by: Inochi Amaoto <inochiama@outlook.com> > --- > tools/perf/arch/riscv/util/header.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/tools/perf/arch/riscv/util/header.c > b/tools/perf/arch/riscv/util/header.c > index 4a41856938a8..031899c627f6 100644 > --- a/tools/perf/arch/riscv/util/header.c > +++ b/tools/perf/arch/riscv/util/header.c > @@ -55,18 +55,13 @@ static char *_get_cpuid(void) > goto free; > } else if (!strncmp(line, CPUINFO_MARCH, > strlen(CPUINFO_MARCH))) { > marchid = _get_field(line); > - if (!marchid) > - goto free; > } else if (!strncmp(line, CPUINFO_MIMP, > strlen(CPUINFO_MIMP))) { > mimpid = _get_field(line); > - if (!mimpid) > - goto free; > - > break; > } > } What does /proc/cpuinfo shows on c9xx ? Why can't we use zeroes ? > > - if (!mvendorid || !marchid || !mimpid) > + if (!mvendorid) > goto free; > > if (asprintf(&cpuid, "%s-%s-%s", mvendorid, marchid, mimpid) > < 0)
On Tue, 2023-05-16 at 17:43 +0800, Inochi Amaoto wrote: > > > The T-HEAD C9xx series CPU only has MVENDOR defined, and left > > > MARCH > > > and MIMP unimplemented. > > > > According to the docs you can still read them, but it's hardwired > > to > > 64h0. > > > > How it's supposed to distinguish c906 and c910 for example ? > > It is unnecessary to distinguish c9xx, their event index is > compatible. > The dtb and opensbi will final decide which event can be used. > > > What does /proc/cpuinfo shows on c9xx ? Why can't we use zeroes ? > > The content is as follows. > > processor : 0 > hart : 0 > isa : rv64imafdc > mmu : sv39 > uarch : thead,c910 > mvendorid : 0x5b7 > marchid : 0x0 > mimpid : 0x0 Then why do you need first patch then ? marchid, mimpid will never be NULL they "0x0" and "0x0" strings respectively. How have you tested it ? There no way "0x5b7-0x0000000000000000-0x[[:xdigit:]]+" will match "0x5b7-0x0-0x0" which cpuid in your case. Just drop this patch. Anyway "PAGER=cat perf list pmu" gives me an empty list on licheerv. > > The `mvendorid`, `marchid`, `mimpid` are the same across allwinner D1 > (C906), > T-HEAD th1520 (C910) and the sophgo mango (C920). It seems T-HEAD use > MCPUID > CSR to store CPU info. But this is not standard and not shown in > cpuinfo. > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
> Then why do you need first patch then ? > > marchid, mimpid will never be NULL they "0x0" and "0x0" strings > respectively. > > How have you tested it ? > > There no way "0x5b7-0x0000000000000000-0x[[:xdigit:]]+" will match > "0x5b7-0x0-0x0" which cpuid in your case. > > Just drop this patch. > > Anyway "PAGER=cat perf list pmu" gives me an empty list on licheerv. Sorry for this mistake, I mistook the type of the MIMP and MARCH as unsigned long. And I write wrong MARCH id in my test container. Anyway, I agree to drop this patch as there is no need.
diff --git a/tools/perf/arch/riscv/util/header.c b/tools/perf/arch/riscv/util/header.c index 4a41856938a8..031899c627f6 100644 --- a/tools/perf/arch/riscv/util/header.c +++ b/tools/perf/arch/riscv/util/header.c @@ -55,18 +55,13 @@ static char *_get_cpuid(void) goto free; } else if (!strncmp(line, CPUINFO_MARCH, strlen(CPUINFO_MARCH))) { marchid = _get_field(line); - if (!marchid) - goto free; } else if (!strncmp(line, CPUINFO_MIMP, strlen(CPUINFO_MIMP))) { mimpid = _get_field(line); - if (!mimpid) - goto free; - break; } } - if (!mvendorid || !marchid || !mimpid) + if (!mvendorid) goto free; if (asprintf(&cpuid, "%s-%s-%s", mvendorid, marchid, mimpid) < 0)
The T-HEAD C9xx series CPU only has MVENDOR defined, and left MARCH and MIMP unimplemented. To make perf support T-HEAD C9xx events. remove the restriction of the MARCH and MIMP. Signed-off-by: Inochi Amaoto <inochiama@outlook.com> --- tools/perf/arch/riscv/util/header.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)