Re: [PATCH v1 4/7] perf cs-etm: Add PID format into metadata

2021-01-11 Thread Leo Yan
On Mon, Jan 11, 2021 at 09:45:12AM +, Suzuki Kuruppassery Poulose wrote:
> Hi Leo,
> 
> On 1/9/21 7:44 AM, Leo Yan wrote:
> > It's possible for CoreSight to trace PID in either CONTEXTIDR_EL1 or
> > CONTEXTIDR_EL2, the PID format info is used to distinguish the PID
> > is traced in which register.
> > 
> > This patch saves PID format into the metadata when record.
> 
> The patch looks good to me. One minor suggestion below
> 
> > 
> > Signed-off-by: Leo Yan 
> > ---
> >   tools/perf/arch/arm/util/cs-etm.c | 21 +
> >   tools/perf/util/cs-etm.c  |  2 ++
> >   tools/perf/util/cs-etm.h  |  2 ++
> >   3 files changed, 25 insertions(+)
> > 
> > diff --git a/tools/perf/arch/arm/util/cs-etm.c 
> > b/tools/perf/arch/arm/util/cs-etm.c
> > index fad7b6e13ccc..ee78df3b1b07 100644
> > --- a/tools/perf/arch/arm/util/cs-etm.c
> > +++ b/tools/perf/arch/arm/util/cs-etm.c
> > @@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> > struct cs_etm_recording *ptr =
> > container_of(itr, struct cs_etm_recording, itr);
> > struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
> > +   u64 pid_fmt;
> > /* first see what kind of tracer this cpu is affined to */
> > if (cs_etm_is_etmv4(itr, cpu)) {
> > @@ -641,6 +642,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
> >   metadata_etmv4_ro
> >   [CS_ETMV4_TRCAUTHSTATUS]);
> > +   /*
> > +* The PID format will be used when decode the trace data;
> > +* based on it the decoder will make decision for setting
> > +* sample's PID as context_id or VMID.
> > +*/
> > +   pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
> > +   if (!pid_fmt)
> > +   pid_fmt = 1ULL << ETM_OPT_CTXTID;
> > +   info->priv[*offset + CS_ETMV4_PID_FMT] = pid_fmt;
> > +
> 
> Given we do this same step twice here in this function and also in patch 2.
> I am wondering if this could be made into a small helper function ?
> 
> static u64 cs_etm_pmu_format_pid(cs_etm_pm)
> {
>   pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
>   /*
>* An older kernel doesn't expose this, so fall back to using
>* CTXTID.
>*/
>   if (!pid_fmt)
>   pid_fmt = 1ULL << ETM_OPT_CTXTID;
>   return pid_fmt;
> }

Agreed; will follow up this suggestion.

Thanks,
Leo


Re: [PATCH v1 4/7] perf cs-etm: Add PID format into metadata

2021-01-11 Thread Suzuki K Poulose

Hi Leo,

On 1/9/21 7:44 AM, Leo Yan wrote:

It's possible for CoreSight to trace PID in either CONTEXTIDR_EL1 or
CONTEXTIDR_EL2, the PID format info is used to distinguish the PID
is traced in which register.

This patch saves PID format into the metadata when record.


The patch looks good to me. One minor suggestion below



Signed-off-by: Leo Yan 
---
  tools/perf/arch/arm/util/cs-etm.c | 21 +
  tools/perf/util/cs-etm.c  |  2 ++
  tools/perf/util/cs-etm.h  |  2 ++
  3 files changed, 25 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c 
b/tools/perf/arch/arm/util/cs-etm.c
index fad7b6e13ccc..ee78df3b1b07 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+   u64 pid_fmt;
  
  	/* first see what kind of tracer this cpu is affined to */

if (cs_etm_is_etmv4(itr, cpu)) {
@@ -641,6 +642,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
  metadata_etmv4_ro
  [CS_ETMV4_TRCAUTHSTATUS]);
  
+		/*

+* The PID format will be used when decode the trace data;
+* based on it the decoder will make decision for setting
+* sample's PID as context_id or VMID.
+*/
+   pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
+   if (!pid_fmt)
+   pid_fmt = 1ULL << ETM_OPT_CTXTID;
+   info->priv[*offset + CS_ETMV4_PID_FMT] = pid_fmt;
+


Given we do this same step twice here in this function and also in patch 2.
I am wondering if this could be made into a small helper function ?

static u64 cs_etm_pmu_format_pid(cs_etm_pm)
{
pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
/*
 * An older kernel doesn't expose this, so fall back to using
 * CTXTID.
 */
if (!pid_fmt)
pid_fmt = 1ULL << ETM_OPT_CTXTID;
return pid_fmt;
}

Suzuki


[PATCH v1 4/7] perf cs-etm: Add PID format into metadata

2021-01-08 Thread Leo Yan
It's possible for CoreSight to trace PID in either CONTEXTIDR_EL1 or
CONTEXTIDR_EL2, the PID format info is used to distinguish the PID
is traced in which register.

This patch saves PID format into the metadata when record.

Signed-off-by: Leo Yan 
---
 tools/perf/arch/arm/util/cs-etm.c | 21 +
 tools/perf/util/cs-etm.c  |  2 ++
 tools/perf/util/cs-etm.h  |  2 ++
 3 files changed, 25 insertions(+)

diff --git a/tools/perf/arch/arm/util/cs-etm.c 
b/tools/perf/arch/arm/util/cs-etm.c
index fad7b6e13ccc..ee78df3b1b07 100644
--- a/tools/perf/arch/arm/util/cs-etm.c
+++ b/tools/perf/arch/arm/util/cs-etm.c
@@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
struct cs_etm_recording *ptr =
container_of(itr, struct cs_etm_recording, itr);
struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu;
+   u64 pid_fmt;
 
/* first see what kind of tracer this cpu is affined to */
if (cs_etm_is_etmv4(itr, cpu)) {
@@ -641,6 +642,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
  metadata_etmv4_ro
  [CS_ETMV4_TRCAUTHSTATUS]);
 
+   /*
+* The PID format will be used when decode the trace data;
+* based on it the decoder will make decision for setting
+* sample's PID as context_id or VMID.
+*/
+   pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
+   if (!pid_fmt)
+   pid_fmt = 1ULL << ETM_OPT_CTXTID;
+   info->priv[*offset + CS_ETMV4_PID_FMT] = pid_fmt;
+
/* How much space was used */
increment = CS_ETMV4_PRIV_MAX;
} else {
@@ -658,6 +669,16 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,
cs_etm_get_ro(cs_etm_pmu, cpu,
  metadata_etmv3_ro[CS_ETM_ETMIDR]);
 
+   /*
+* The PID format will be used when decode the trace data;
+* based on it the decoder will make decision for setting
+* sample's PID as context_id or VMID.
+*/
+   pid_fmt = perf_pmu__format_bits(_etm_pmu->format, "pid");
+   if (!pid_fmt)
+   pid_fmt = 1ULL << ETM_OPT_CTXTID;
+   info->priv[*offset + CS_ETM_PID_FMT] = pid_fmt;
+
/* How much space was used */
increment = CS_ETM_PRIV_MAX;
}
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 5e284725dceb..763085db29ae 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -2447,6 +2447,7 @@ static const char * const cs_etm_priv_fmts[] = {
[CS_ETM_ETMTRACEIDR]= " ETMTRACEIDR%llx\n",
[CS_ETM_ETMCCER]= " ETMCCER%llx\n",
[CS_ETM_ETMIDR] = " ETMIDR %llx\n",
+   [CS_ETM_PID_FMT]= " PID Format %llx\n",
 };
 
 static const char * const cs_etmv4_priv_fmts[] = {
@@ -2459,6 +2460,7 @@ static const char * const cs_etmv4_priv_fmts[] = {
[CS_ETMV4_TRCIDR2]  = " TRCIDR2%llx\n",
[CS_ETMV4_TRCIDR8]  = " TRCIDR8%llx\n",
[CS_ETMV4_TRCAUTHSTATUS] = "TRCAUTHSTATUS  %llx\n",
+   [CS_ETMV4_PID_FMT]  = " PID Format %llx\n",
 };
 
 static void cs_etm__print_auxtrace_info(__u64 *val, int num)
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
index 4ad925d6d799..8cbbea6100a1 100644
--- a/tools/perf/util/cs-etm.h
+++ b/tools/perf/util/cs-etm.h
@@ -38,6 +38,7 @@ enum {
/* RO, taken from sysFS */
CS_ETM_ETMCCER,
CS_ETM_ETMIDR,
+   CS_ETM_PID_FMT,
CS_ETM_PRIV_MAX,
 };
 
@@ -52,6 +53,7 @@ enum {
CS_ETMV4_TRCIDR2,
CS_ETMV4_TRCIDR8,
CS_ETMV4_TRCAUTHSTATUS,
+   CS_ETMV4_PID_FMT,
CS_ETMV4_PRIV_MAX,
 };
 
-- 
2.25.1