2023-05-16 02:55:34

by Inochi Amaoto

[permalink] [raw]
Subject: [PATCH 1/2] perf tools riscv: Allow get_cpuid return empty MARCH and MIMP

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 <[email protected]>
---
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;
}
}

- if (!mvendorid || !marchid || !mimpid)
+ if (!mvendorid)
goto free;

if (asprintf(&cpuid, "%s-%s-%s", mvendorid, marchid, mimpid) < 0)
--
2.40.1



2023-05-16 07:54:14

by Nikita Shubin

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools riscv: Allow get_cpuid return empty MARCH and MIMP

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 <[email protected]>
> ---
>  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)

2023-05-16 09:52:32

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools riscv: Allow get_cpuid return empty MARCH and MIMP

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

2023-05-16 11:46:21

by Nikita Shubin

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools riscv: Allow get_cpuid return empty MARCH and MIMP

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv


2023-05-17 05:22:49

by Inochi Amaoto

[permalink] [raw]
Subject: Re: [PATCH 1/2] perf tools riscv: Allow get_cpuid return empty MARCH and MIMP

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