Re: [PATCH] perf tools: Add arm64 version of get_cpuid()

[John Garry]

On 12/02/2020 13:40, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 07, 2020 at 09:13:43AM +0000, John Garry escreveu:
> > On 13/12/2019 13:54, John Garry wrote:
> >
> > Hi Arnaldo,
> >
> > Do we need some reviews on this? Or was it missed/still catching up?
>
> Got lost in the holidays, devconf.cz, vacations, sorry, picking it up
> now, together with a Tested-by by Shaokun Zhang, some issues with the
> formatting of the patch:
>
> - Avoid starting lines with '#' as those will vanish when I use 'git am'

ah, so this must be why people use, for example, '/include "..."' in 
commit logs

> - Separate the commit log message from the diff using a '---' at the
>    begining of the line, otherwise 'git am' fails

apologizes, I must have cut that by mistake

> I fixed those up now, will test with my build containers, thanks.

Thanks

> - Arnaldo
>   
> > Cheers,
> > John
> >
> > > Add an arm64 version of get_cpuid(), which is used for various annotation
> > > and headers - for example, I now get the CPUID in "perf report --header",
> > > as shown in this snippet:
> > >
> > > # hostname : ubuntu
> > > # os release : 5.5.0-rc1-dirty
> > > # perf version : 5.5.rc1.gbf8a13dc9851
> > > # arch : aarch64
> > > # nrcpus online : 96
> > > # nrcpus avail : 96
> > > # cpuid : 0x00000000480fd010
> > >
> > > Since much of the code to read the MIDR is already in get_cpuid_str(),
> > > factor out this code.
> > >
> > > Signed-off-by: John Garry <john.garry@xxxxxxxxxx>
> > >
> > > diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
> > > index a32e4b72a98f..d730666ab95d 100644
> > > --- a/tools/perf/arch/arm64/util/header.c
> > > +++ b/tools/perf/arch/arm64/util/header.c
> > > @@ -1,8 +1,10 @@
> > >    #include <stdio.h>
> > >    #include <stdlib.h>
> > >    #include <perf/cpumap.h>
> > > +#include <util/cpumap.h>
> > >    #include <internal/cpumap.h>
> > >    #include <api/fs/fs.h>
> > > +#include <errno.h>
> > >    #include "debug.h"
> > >    #include "header.h"
> > > @@ -12,26 +14,21 @@
> > >    #define MIDR_VARIANT_SHIFT      20
> > >    #define MIDR_VARIANT_MASK       (0xf << MIDR_VARIANT_SHIFT)
> > > -char *get_cpuid_str(struct perf_pmu *pmu)
> > > +static int _get_cpuid(char *buf, size_t sz, struct perf_cpu_map *cpus)
> > >    {
> > > -	char *buf = NULL;
> > > -	char path[PATH_MAX];
> > >    	const char *sysfs = sysfs__mountpoint();
> > > -	int cpu;
> > >    	u64 midr = 0;
> > > -	struct perf_cpu_map *cpus;
> > > -	FILE *file;
> > > +	int cpu;
> > > -	if (!sysfs || !pmu || !pmu->cpus)
> > > -		return NULL;
> > > +	if (!sysfs || sz < MIDR_SIZE)
> > > +		return EINVAL;
> > > -	buf = malloc(MIDR_SIZE);
> > > -	if (!buf)
> > > -		return NULL;
> > > +	cpus = perf_cpu_map__get(cpus);
> > > -	/* read midr from list of cpus mapped to this pmu */
> > > -	cpus = perf_cpu_map__get(pmu->cpus);
> > >    	for (cpu = 0; cpu < perf_cpu_map__nr(cpus); cpu++) {
> > > +		char path[PATH_MAX];
> > > +		FILE *file;
> > > +
> > >    		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d"MIDR,
> > >    				sysfs, cpus->map[cpu]);
> > > @@ -57,12 +54,48 @@ char *get_cpuid_str(struct perf_pmu *pmu)
> > >    		break;
> > >    	}
> > > -	if (!midr) {
> > > +	perf_cpu_map__put(cpus);
> > > +
> > > +	if (!midr)
> > > +		return EINVAL;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int get_cpuid(char *buf, size_t sz)
> > > +{
> > > +	struct perf_cpu_map *cpus = perf_cpu_map__new(NULL);
> > > +	int ret;
> > > +
> > > +	if (!cpus)
> > > +		return EINVAL;
> > > +
> > > +	ret = _get_cpuid(buf, sz, cpus);
> > > +
> > > +	perf_cpu_map__put(cpus);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +char *get_cpuid_str(struct perf_pmu *pmu)
> > > +{
> > > +	char *buf = NULL;
> > > +	int res;
> > > +
> > > +	if (!pmu || !pmu->cpus)
> > > +		return NULL;
> > > +
> > > +	buf = malloc(MIDR_SIZE);
> > > +	if (!buf)
> > > +		return NULL;
> > > +
> > > +	/* read midr from list of cpus mapped to this pmu */
> > > +	res = _get_cpuid(buf, MIDR_SIZE, pmu->cpus);
> > > +	if (res) {
> > >    		pr_err("failed to get cpuid string for PMU %s\n", pmu->name);
> > >    		free(buf);
> > >    		buf = NULL;
> > >    	}
> > > -	perf_cpu_map__put(cpus);
> > >    	return buf;
> > >    }