This patch series is a following up for the previous version which was
delivered by Suzuki [1]. Below gives the background info for why we
need this patch series, directly quotes the description in the cover
letter of the previous version:
"With the Virtualization Host Extensions, the kernel can run at EL2.
In this case the pid is written to CONTEXTIDR_EL2 instead of the
CONTEXTIDR_EL1. Thus the normal coresight tracing will be unable
to detect the PID of the thread generating the trace by looking
at the CONTEXTIDR_EL1. Thus, depending on the kernel EL, we must
switch to tracing the correct CONTEXTIDR register.
With VHE, we must set the TRCCONFIGR.VMID and TRCCONFIGR.VMID_OPT
to include the CONTEXTIDR_EL2 as the VMID in the trace. This
requires the perf tool to detect the changes in the TRCCONFIGR and
use the VMID / CID field for the PID. The challenge here is for
the perf tool to detect the kernel behavior.
Instead of the previously proposed invasive approaches, this set
implements a less intrusive mechanism, by playing with the
perf_event.attribute.config bits."
Same as the previous series, this series keeps the same implementation
for two introduced format bits:
- contextid_in_vmid -> Is only supported when the VMID tracing
and CONTEXTIDR_EL2 both are supported. When requested the perf
etm4x backend sets (TRCCONFIGR.VMID | TRCCONFIGR.VMID_OPT).
As per ETMv4.4 TRM, when the core supports VHE, the CONTEXTIDR_EL2
tracing is mandatory. (See the field TRCID2.VMIDOPT)
- pid -> Is an alias for the correct config to enable PID tracing
on any kernel.
i.e, in EL1 kernel -> pid == contextid
EL2 kernel -> pid == contextid_in_vmid
With this, the perf tool is also updated to request the "pid"
tracing whenever available, falling back to "contextid" if it
is unavailable.
Comparing against the old version, this patch series uses the metadata
to save PID format; after add new item into metadata, it introduces
backward compatibility issue. To allow backward compatibility, this
series calculates per CPU metadata array size and avoid to use the
defined macro, so can always know the correct array size based on the
info stored in perf data file. Finally, the PID format stored in
metadata is passed to decoder and guide the decoder to set PID from
CONTEXTIDR_EL1 or VMID.
This patch series has been tested on Arm Juno-r2 board, with testing
two perf data files: one data file is recorded by the latest perf tool
after applied this patch series, and another data file is recorded by
old perf tool without this patch series, so this can prove the tool is
backward compatible.
Changes from RFC:
* Added comments to clarify cases requested (Leo);
* Explain the change to generic flags for cs_etm_set_option() in the
commit description;
* Stored PID format in metadata and passed it to decoder (Leo);
* Enhanced cs-etm for backward compatibility (Denis Nikitin).
[1] https://archive.armlinux.org.uk/lurker/message/20201110.183310.24406f33.en.html
Leo Yan (4):
perf cs-etm: Calculate per CPU metadata array size
perf cs-etm: Add PID format into metadata
perf cs-etm: Fixup PID_FMT when it is zero
perf cs-etm: Add helper cs_etm__get_pid_fmt()
Suzuki K Poulose (3):
coresight: etm-perf: Add support for PID tracing for kernel at EL2
perf cs_etm: Use pid tracing explicitly instead of contextid
perf cs-etm: Detect pid in VMID for kernel running at EL2
.../hwtracing/coresight/coresight-etm-perf.c | 14 +++
.../coresight/coresight-etm4x-core.c | 9 ++
include/linux/coresight-pmu.h | 11 ++-
tools/include/linux/coresight-pmu.h | 11 ++-
tools/perf/arch/arm/util/cs-etm.c | 89 +++++++++++++++----
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++-
tools/perf/util/cs-etm.c | 61 ++++++++++++-
tools/perf/util/cs-etm.h | 3 +
8 files changed, 198 insertions(+), 32 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Suzuki K Poulose <suzuki.poulose@arm.com>
When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.
i.e, perf record -e cs_etm/pid/u --
will always do the "pid" tracing, independent of the kernel EL.
Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Cc: Mike Leach <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
include/linux/coresight-pmu.h | 11 +++++++----
3 files changed, 30 insertions(+), 4 deletions(-)
@@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src);/* ETMv3.5/PTM's ETMCR is 'config' */PMU_FORMAT_ATTR(cycacc,"config:"__stringify(ETM_OPT_CYCACC));PMU_FORMAT_ATTR(contextid,"config:"__stringify(ETM_OPT_CTXTID));+PMU_FORMAT_ATTR(contextid_in_vmid,"config:"__stringify(ETM_OPT_CTXTID_IN_VMID));PMU_FORMAT_ATTR(timestamp,"config:"__stringify(ETM_OPT_TS));PMU_FORMAT_ATTR(retstack,"config:"__stringify(ETM_OPT_RETSTK));/* Sink ID - same for all ETMs */PMU_FORMAT_ATTR(sinkid,"config2:0-31");+staticssize_tformat_attr_pid_show(structdevice*dev,+structdevice_attribute*attr,+char*page)+{+intpid_fmt=is_kernel_in_hyp_mode()?ETM_OPT_CTXTID_IN_VMID:ETM_OPT_CTXTID;++returnsprintf(page,"config:%d\n",pid_fmt);+}++structdevice_attributeformat_attr_pid=__ATTR(pid,0444,format_attr_pid_show,NULL);+staticstructattribute*etm_config_formats_attr[]={&format_attr_cycacc.attr,&format_attr_contextid.attr,+&format_attr_contextid_in_vmid.attr,+&format_attr_pid.attr,&format_attr_timestamp.attr,&format_attr_retstack.attr,&format_attr_sinkid.attr,
@@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,/* bit[6], Context ID tracing bit */config->cfg|=BIT(ETM4_CFG_BIT_CTXTID);+/* Do not enable VMID tracing if we are not running in EL2 */+if(attr->config&BIT(ETM_OPT_CTXTID_IN_VMID)){+if(!is_kernel_in_hyp_mode()){+ret=-EINVAL;+gotoout;+}+config->cfg|=BIT(ETM4_CFG_BIT_VMID)|BIT(ETM4_CFG_BIT_VMID_OPT);+}+/* return stack - enable if selected and supported */if((attr->config&BIT(ETM_OPT_RETSTK))&&drvdata->retstack)/* bit[12], Return stack enable bit */
From: Suzuki K Poulose <suzuki.poulose@arm.com>
If the kernel is running at EL2, the pid of the task is exposed
via VMID instead of the CONTEXTID. Add support for this in the
perf tool.
By default the perf tool requests contextid and timestamp for
task bound events. Instead of hard coding contextid, switch
to "pid" config exposed by the kernel. While at it, define new
independent macros (rather than using the "config" bits) for
requesting the "pid" and "timestamp" for cs_etm_set_option(),
since the PID config is now dynamic depending on the kernel
exception level.
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
tools/include/linux/coresight-pmu.h | 11 +++--
tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 20 deletions(-)
@@ -86,21 +87,50 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,gotoout;}+pid_fmt=perf_pmu__format_bits(&cs_etm_pmu->format,"pid");/*-*TRCIDR2.CIDSIZE,bit[9-5],indicateswhethercontextIDtracing-*issupported:-*0b00000ContextIDtracingisnotsupported.-*0b00100Maximumof32-bitContextIDsize.-*Allothervaluesarereserved.+*Ifthekerneldoesn'tsupportthe"pid"format(olderkernel),+*fallbacktousingtheCTXTID.*/-val=BMVAL(val,5,9);-if(!val||val!=0x4){+if(!pid_fmt)+pid_fmt=1ULL<<ETM_OPT_CTXTID;++switch(pid_fmt){+case(1ULL<<ETM_OPT_CTXTID):+/*+*TRCIDR2.CIDSIZE,bit[9-5],indicateswhethercontextID+*tracingissupported:+*0b00000ContextIDtracingisnotsupported.+*0b00100Maximumof32-bitContextIDsize.+*Allothervaluesarereserved.+*/+val=BMVAL(val,5,9);+if(!val||val!=0x4){+err=-EINVAL;+gotoout;+}+break;+case(1ULL<<ETM_OPT_CTXTID_IN_VMID):+/*+*TRCIDR2.VMIDOPT[30:29]!=0and+*TRCIDR2.VMIDSIZE[14:10]==0b00100(32bitvirtualcontextid)+*Wecan'tsupportCONTEXTIDRinVMIDifthesizeofthe+*virtualcontextidis<32bit.+*AnyvalueofVMIDSIZE>=4(i.e,>32bit)isfineforus.+*/+if(!BMVAL(val,29,30)||BMVAL(val,10,14)<4){+err=-EINVAL;+gotoout;+}+break;+default:err=-EINVAL;gotoout;}+/* All good, let the kernel know */-evsel->core.attr.config|=(1<<ETM_OPT_CTXTID);+evsel->core.attr.config|=pid_fmt;err=0;out:
@@ -169,17 +203,17 @@ static int cs_etm_set_option(struct auxtrace_record *itr,!cpu_map__has(online_cpus,i))continue;-if(option&ETM_OPT_CTXTID){-err=cs_etm_set_context_id(itr,evsel,i);+if(option&ETM_SET_OPT_PID){+err=cs_etm_set_pid(itr,evsel,i);if(err)gotoout;}-if(option&ETM_OPT_TS){+if(option&ETM_SET_OPT_TS){err=cs_etm_set_timestamp(itr,evsel,i);if(err)gotoout;}-if(option&~(ETM_OPT_CTXTID|ETM_OPT_TS))+if(option&~(ETM_SET_OPT_MASK))/* Nothing else is currently supported */gotoout;}
@@ -406,7 +440,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr,evsel__set_sample_bit(cs_etm_evsel,CPU);err=cs_etm_set_option(itr,cs_etm_evsel,-ETM_OPT_CTXTID|ETM_OPT_TS);+ETM_SET_OPT_PID|ETM_SET_OPT_TS);if(err)gotoout;}
The metadata array can be extended over time and the tool, if using the
predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
size to copy data, it can cause compatible issue within different
versions of perf tool.
E.g. we recorded a data file with an old version tool, afterwards if
use the new version perf tool to parse the file, since the metadata
array has been extended and the macro CS_ETMV4_PRIV_MAX has been
altered, if use it to parse the perf data with old format, this will
lead to mismatch.
To maintain backward compatibility, this patch calculates per CPU
metadata array size on the runtime, the calculation is based on the
info stored in the data file so that it's reliable.
Signed-off-by: Leo Yan <redacted>
---
tools/perf/util/cs-etm.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
@@ -2497,6 +2497,7 @@ int cs_etm__process_auxtrace_info(union perf_event *event,inti,j,k;u64*ptr,*hdr=NULL;u64**metadata=NULL;+intmetadata_cpu_array_size;/**sizeof(auxtrace_info_event::type)+
@@ -2544,6 +2545,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,gotoerr_free_traceid_list;}+/*+*Themetadataisatwodimensionalarray,thefirstdimensionusesCPU+*numberasindexandtheseconddimensionisthemetadataarrayper+*CPU.Sincethemetadataarraycanbeextendedovertime,the+*predefinedmacros(CS_ETM_PRIV_MAXorCS_ETMV4_PRIV_MAX)might+*mismatchwithindifferentversionsoftool,thiscanleadtocopy+*wrongdata.Tomaintainbackwardcompatibility,calculateCPU's+*metadataarraysizeontheruntime.+*/+metadata_cpu_array_size=+(auxtrace_info->header.size-+sizeof(structperf_record_auxtrace_info))/num_cpu/sizeof(u64);+/**Themetadataisstoredintheauxtrace_infosectionandencodes*theconfigurationoftheARMembeddedtracemacrocellwhichis
@@ -2558,12 +2572,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETM_PRIV_MAX;k++)+for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];-i+=CS_ETM_PRIV_MAX;+i+=metadata_cpu_array_size;}elseif(ptr[i]==__perf_cs_etmv4_magic){metadata[j]=zalloc(sizeof(*metadata[j])*CS_ETMV4_PRIV_MAX);
@@ -2571,12 +2585,12 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETMV4_PRIV_MAX;k++)+for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];-i+=CS_ETMV4_PRIV_MAX;+i+=metadata_cpu_array_size;}/* Get an RB node for this CPU */
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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 <redacted>
---
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(+)
@@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,structcs_etm_recording*ptr=container_of(itr,structcs_etm_recording,itr);structperf_pmu*cs_etm_pmu=ptr->cs_etm_pmu;+u64pid_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]);+/*+*ThePIDformatwillbeusedwhendecodethetracedata;+*basedonitthedecoderwillmakedecisionforsetting+*sample'sPIDascontext_idorVMID.+*/+pid_fmt=perf_pmu__format_bits(&cs_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]);+/*+*ThePIDformatwillbeusedwhendecodethetracedata;+*basedonitthedecoderwillmakedecisionforsetting+*sample'sPIDascontext_idorVMID.+*/+pid_fmt=perf_pmu__format_bits(&cs_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;}
This patch adds helper function cs_etm__get_pid_fmt(), by passing
parameter "traceID", it returns the corresponding PID format.
Signed-off-by: Leo Yan <redacted>
---
tools/perf/util/cs-etm.c | 18 ++++++++++++++++++
tools/perf/util/cs-etm.h | 1 +
2 files changed, 19 insertions(+)
@@ -157,6 +157,24 @@ int cs_etm__get_cpu(u8 trace_chan_id, int *cpu)return0;}+intcs_etm__get_pid_fmt(u8trace_chan_id,u64*pid_fmt)+{+structint_node*inode;+u64*metadata;++inode=intlist__find(traceid_list,trace_chan_id);+if(!inode)+return-EINVAL;++metadata=inode->priv;+if(metadata[CS_ETM_MAGIC]==__perf_cs_etmv3_magic)+*pid_fmt=metadata[CS_ETM_PID_FMT];+else+*pid_fmt=metadata[CS_ETMV4_PID_FMT];++return0;+}+voidcs_etm__etmq_set_traceid_queue_timestamp(structcs_etm_queue*etmq,u8trace_chan_id){
If the metadata item CS_ETM_PID_FMT/CS_ETMV4_PID_FMT is zero, this means
the perf data file is recorded with old version tool and the tool has
not extended to support the item.
For this case, this patch fixes up PID_FMT entry to set the value as
BIT(ETM_OPT_CTXTID), this info will be delivered to the decoder to
extract PID from packet's field "context_id".
Signed-off-by: Leo Yan <redacted>
---
tools/perf/util/cs-etm.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
@@ -2577,6 +2578,15 @@ int cs_etm__process_auxtrace_info(union perf_event *event,for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];+/*+*IfthedatainCS_ETM_PID_FMTiszero,meansthe+*informationisn'tstoredinthedatafile,thisis+*becausetheoldperftoolhasn'tyetsupported+*CS_ETM_PID_FMT.Fixuptheitemtooption"CTXTID".+*/+if(!metadata[j][CS_ETM_PID_FMT])+metadata[j][CS_ETM_PID_FMT]=BIT(ETM_OPT_CTXTID);+/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];i+=metadata_cpu_array_size;
@@ -2590,6 +2600,15 @@ int cs_etm__process_auxtrace_info(union perf_event *event,for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];+/*+*IfthedatainCS_ETMV4_PID_FMTiszero,meansthe+*informationisn'tstoredinthedatafile,thisis+*becausetheoldperftoolhasn'tyetsupported+*CS_ETMV4_PID_FMT.Fixuptheitemtooption"CTXTID".+*/+if(!metadata[j][CS_ETMV4_PID_FMT])+metadata[j][CS_ETMV4_PID_FMT]=BIT(ETM_OPT_CTXTID);+/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];i+=metadata_cpu_array_size;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Suzuki K Poulose <suzuki.poulose@arm.com>
The pid of the task could be traced as VMID when the kernel is
running at EL2. Teach the decoder to look for vmid when the
context_id is invalid but we have a valid VMID.
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Co-developed-by: Leo Yan <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-11 07:30:11
On 1/9/21 7:44 AM, Leo Yan wrote:
The metadata array can be extended over time and the tool, if using the
predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
size to copy data, it can cause compatible issue within different
versions of perf tool.
E.g. we recorded a data file with an old version tool, afterwards if
use the new version perf tool to parse the file, since the metadata
array has been extended and the macro CS_ETMV4_PRIV_MAX has been
altered, if use it to parse the perf data with old format, this will
lead to mismatch.
To maintain backward compatibility, this patch calculates per CPU
metadata array size on the runtime, the calculation is based on the
info stored in the data file so that it's reliable.
Signed-off-by: Leo Yan <redacted>
Looks good to me.
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-11 09:46:13
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
@@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,structcs_etm_recording*ptr=container_of(itr,structcs_etm_recording,itr);structperf_pmu*cs_etm_pmu=ptr->cs_etm_pmu;+u64pid_fmt;/* first see what kind of tracer this cpu is affined to */if(cs_etm_is_etmv4(itr,cpu)){
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(&cs_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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-11 09:48:42
On 1/9/21 7:44 AM, Leo Yan wrote:
If the metadata item CS_ETM_PID_FMT/CS_ETMV4_PID_FMT is zero, this means
the perf data file is recorded with old version tool and the tool has
not extended to support the item.
For this case, this patch fixes up PID_FMT entry to set the value as
BIT(ETM_OPT_CTXTID), this info will be delivered to the decoder to
extract PID from packet's field "context_id".
Signed-off-by: Leo Yan <redacted>
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
@@ -2577,6 +2578,15 @@ int cs_etm__process_auxtrace_info(union perf_event *event,for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];+/*+*IfthedatainCS_ETM_PID_FMTiszero,meansthe+*informationisn'tstoredinthedatafile,thisis+*becausetheoldperftoolhasn'tyetsupported+*CS_ETM_PID_FMT.Fixuptheitemtooption"CTXTID".+*/+if(!metadata[j][CS_ETM_PID_FMT])+metadata[j][CS_ETM_PID_FMT]=BIT(ETM_OPT_CTXTID);+/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];i+=metadata_cpu_array_size;
@@ -2590,6 +2600,15 @@ int cs_etm__process_auxtrace_info(union perf_event *event,for(k=0;k<metadata_cpu_array_size;k++)metadata[j][k]=ptr[i+k];+/*+*IfthedatainCS_ETMV4_PID_FMTiszero,meansthe+*informationisn'tstoredinthedatafile,thisis+*becausetheoldperftoolhasn'tyetsupported+*CS_ETMV4_PID_FMT.Fixuptheitemtooption"CTXTID".+*/+if(!metadata[j][CS_ETMV4_PID_FMT])+metadata[j][CS_ETMV4_PID_FMT]=BIT(ETM_OPT_CTXTID);+/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];i+=metadata_cpu_array_size;
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-11 09:56:32
On 1/9/21 7:44 AM, Leo Yan wrote:
This patch adds helper function cs_etm__get_pid_fmt(), by passing
parameter "traceID", it returns the corresponding PID format.
Signed-off-by: Leo Yan <redacted>
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
@@ -157,6 +157,24 @@ int cs_etm__get_cpu(u8 trace_chan_id, int *cpu)return0;}+intcs_etm__get_pid_fmt(u8trace_chan_id,u64*pid_fmt)+{+structint_node*inode;+u64*metadata;++inode=intlist__find(traceid_list,trace_chan_id);+if(!inode)+return-EINVAL;++metadata=inode->priv;+if(metadata[CS_ETM_MAGIC]==__perf_cs_etmv3_magic)+*pid_fmt=metadata[CS_ETM_PID_FMT];+else+*pid_fmt=metadata[CS_ETMV4_PID_FMT];++return0;+}+voidcs_etm__etmq_set_traceid_queue_timestamp(structcs_etm_queue*etmq,u8trace_chan_id){
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-11 10:08:08
Hi Leo
On 1/9/21 7:44 AM, Leo Yan wrote:
From: Suzuki K Poulose <suzuki.poulose@arm.com>
The pid of the task could be traced as VMID when the kernel is
running at EL2. Teach the decoder to look for vmid when the
context_id is invalid but we have a valid VMID.
Thank you again for cleaning up this ! Please see one comment
below.
quoted hunk
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Co-developed-by: Leo Yan <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
@@ -500,13 +501,36 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq,constocsd_generic_trace_elem*elem,constuint8_ttrace_chan_id){-pid_ttid;+pid_ttid=-1;+u64pid_fmt;+intret;-/* Ignore PE_CONTEXT packets that don't have a valid contextID */-if(!elem->context.ctxt_id_valid)+ret=cs_etm__get_pid_fmt(trace_chan_id,&pid_fmt);+if(ret)+returnOCSD_RESP_FATAL_SYS_ERR;
The patch looks fine to me. I am wondering if this can be cached somewhere in
the etmq to avoid doing the search everytime we hit a CID ? Surely for a session,
this woudn't change and thus for the decoder life time.
Cheers
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mike Leach <hidden> Date: 2021-01-11 12:10:06
Hi Leo,
I think there is an issue here in that your modification assumes that
all cpus in the system are of the same ETM type. The original routine
allowed for differing ETM types, thus differing cpu ETM field lengths
between ETMv4 / ETMv3, the field size was used after the relevant
magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
Moving forwards we are seeing the newer FEAT_ETE protocol drivers
appearing on the list, which will ultimately need a new metadata
structure.
We have had discussions within ARM regarding the changing of the
format to be more self describing - which should probably be opened
out to the CS mailing list.
Regards
Mike
On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose [off-list ref] wrote:
On 1/9/21 7:44 AM, Leo Yan wrote:
quoted
The metadata array can be extended over time and the tool, if using the
predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
size to copy data, it can cause compatible issue within different
versions of perf tool.
E.g. we recorded a data file with an old version tool, afterwards if
use the new version perf tool to parse the file, since the metadata
array has been extended and the macro CS_ETMV4_PRIV_MAX has been
altered, if use it to parse the perf data with old format, this will
lead to mismatch.
To maintain backward compatibility, this patch calculates per CPU
metadata array size on the runtime, the calculation is based on the
info stored in the data file so that it's reliable.
Signed-off-by: Leo Yan <redacted>
Looks good to me.
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jan 11, 2021 at 10:07:03AM +0000, Suzuki Kuruppassery Poulose wrote:
Hi Leo
On 1/9/21 7:44 AM, Leo Yan wrote:
quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com>
The pid of the task could be traced as VMID when the kernel is
running at EL2. Teach the decoder to look for vmid when the
context_id is invalid but we have a valid VMID.
Thank you again for cleaning up this ! Please see one comment
below.
Welcome!
quoted
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Co-developed-by: Leo Yan <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++++++++++++---
1 file changed, 28 insertions(+), 4 deletions(-)
@@ -500,13 +501,36 @@ cs_etm_decoder__set_tid(struct cs_etm_queue *etmq,constocsd_generic_trace_elem*elem,constuint8_ttrace_chan_id){-pid_ttid;+pid_ttid=-1;+u64pid_fmt;+intret;-/* Ignore PE_CONTEXT packets that don't have a valid contextID */-if(!elem->context.ctxt_id_valid)+ret=cs_etm__get_pid_fmt(trace_chan_id,&pid_fmt);+if(ret)+returnOCSD_RESP_FATAL_SYS_ERR;
The patch looks fine to me. I am wondering if this can be cached somewhere in
the etmq to avoid doing the search everytime we hit a CID ? Surely for a session,
this woudn't change and thus for the decoder life time.
Good suggestion, will refine for this in next version.
Thanks,
Leo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jan 11, 2021 at 09:45:12AM +0000, Suzuki Kuruppassery Poulose wrote:
Hi Leo,
On 1/9/21 7:44 AM, Leo Yan wrote:
quoted
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
@@ -613,6 +613,7 @@ static void cs_etm_get_metadata(int cpu, u32 *offset,structcs_etm_recording*ptr=container_of(itr,structcs_etm_recording,itr);structperf_pmu*cs_etm_pmu=ptr->cs_etm_pmu;+u64pid_fmt;/* first see what kind of tracer this cpu is affined to */if(cs_etm_is_etmv4(itr,cpu)){
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(&cs_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;
}
Hi Mike,
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
Hi Leo,
I think there is an issue here in that your modification assumes that
all cpus in the system are of the same ETM type. The original routine
allowed for differing ETM types, thus differing cpu ETM field lengths
between ETMv4 / ETMv3, the field size was used after the relevant
magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
Thanks for pointing out this.
Moving forwards we are seeing the newer FEAT_ETE protocol drivers
appearing on the list, which will ultimately need a new metadata
structure.
We have had discussions within ARM regarding the changing of the
format to be more self describing - which should probably be opened
out to the CS mailing list.
I think here have two options. One option is I think we can use
__perf_cs_etmv3_magic/__perf_cs_etmv4_magic as indicator for the
starting of next metadata array; when copy the metadata, always check
the next item in the buffer, if it's __perf_cs_etmv3_magic or
__perf_cs_etmv4_magic, will break loop and start copying metadata
array for next CPU. The suggested change is pasted in below.
Another option is I drop patches 03,05/07 in the series and leave the
backward compatibility fixing for a saperate patch series with self
describing method. Especially, if you think the first option will
introduce trouble for enabling self describing later, then I am happy
to drop patches 03,05.
How about you think for this?
Thanks,
Leo
---8<---
@@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETM_PRIV_MAX;k++)+for(k=0;k<CS_ETM_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];-i+=CS_ETM_PRIV_MAX;+i+=k;}elseif(ptr[i]==__perf_cs_etmv4_magic){metadata[j]=zalloc(sizeof(*metadata[j])*CS_ETMV4_PRIV_MAX);
@@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETMV4_PRIV_MAX;k++)+for(k=0;k<CS_ETMV4_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];-i+=CS_ETMV4_PRIV_MAX;+i+=k;}/* Get an RB node for this CPU */
From: Mike Leach <hidden> Date: 2021-01-11 16:23:35
Hi Leo,
On Sat, 9 Jan 2021 at 07:44, Leo Yan [off-list ref] wrote:
quoted hunk
From: Suzuki K Poulose <suzuki.poulose@arm.com>
When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.
i.e, perf record -e cs_etm/pid/u --
will always do the "pid" tracing, independent of the kernel EL.
Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Cc: Mike Leach <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
include/linux/coresight-pmu.h | 11 +++++++----
3 files changed, 30 insertions(+), 4 deletions(-)
@@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src);/* ETMv3.5/PTM's ETMCR is 'config' */PMU_FORMAT_ATTR(cycacc,"config:"__stringify(ETM_OPT_CYCACC));PMU_FORMAT_ATTR(contextid,"config:"__stringify(ETM_OPT_CTXTID));+PMU_FORMAT_ATTR(contextid_in_vmid,"config:"__stringify(ETM_OPT_CTXTID_IN_VMID));PMU_FORMAT_ATTR(timestamp,"config:"__stringify(ETM_OPT_TS));PMU_FORMAT_ATTR(retstack,"config:"__stringify(ETM_OPT_RETSTK));/* Sink ID - same for all ETMs */PMU_FORMAT_ATTR(sinkid,"config2:0-31");+staticssize_tformat_attr_pid_show(structdevice*dev,+structdevice_attribute*attr,+char*page)+{+intpid_fmt=is_kernel_in_hyp_mode()?ETM_OPT_CTXTID_IN_VMID:ETM_OPT_CTXTID;++returnsprintf(page,"config:%d\n",pid_fmt);+}++structdevice_attributeformat_attr_pid=__ATTR(pid,0444,format_attr_pid_show,NULL);+staticstructattribute*etm_config_formats_attr[]={&format_attr_cycacc.attr,&format_attr_contextid.attr,+&format_attr_contextid_in_vmid.attr,+&format_attr_pid.attr,&format_attr_timestamp.attr,&format_attr_retstack.attr,&format_attr_sinkid.attr,
@@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,/* bit[6], Context ID tracing bit */config->cfg|=BIT(ETM4_CFG_BIT_CTXTID);+/* Do not enable VMID tracing if we are not running in EL2 */+if(attr->config&BIT(ETM_OPT_CTXTID_IN_VMID)){+if(!is_kernel_in_hyp_mode()){+ret=-EINVAL;+gotoout;+}+config->cfg|=BIT(ETM4_CFG_BIT_VMID)|BIT(ETM4_CFG_BIT_VMID_OPT);+}+/* return stack - enable if selected and supported */if((attr->config&BIT(ETM_OPT_RETSTK))&&drvdata->retstack)/* bit[12], Return stack enable bit */
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Leo,
Suzuki and Mike have pointed out a few things to modify and there was a couple
of kernel bot warnings to address as well. As such I will wait for your next
revision before looking at this set.
Thanks,
Mathieu
On Sat, Jan 09, 2021 at 03:44:28PM +0800, Leo Yan wrote:
This patch series is a following up for the previous version which was
delivered by Suzuki [1]. Below gives the background info for why we
need this patch series, directly quotes the description in the cover
letter of the previous version:
"With the Virtualization Host Extensions, the kernel can run at EL2.
In this case the pid is written to CONTEXTIDR_EL2 instead of the
CONTEXTIDR_EL1. Thus the normal coresight tracing will be unable
to detect the PID of the thread generating the trace by looking
at the CONTEXTIDR_EL1. Thus, depending on the kernel EL, we must
switch to tracing the correct CONTEXTIDR register.
With VHE, we must set the TRCCONFIGR.VMID and TRCCONFIGR.VMID_OPT
to include the CONTEXTIDR_EL2 as the VMID in the trace. This
requires the perf tool to detect the changes in the TRCCONFIGR and
use the VMID / CID field for the PID. The challenge here is for
the perf tool to detect the kernel behavior.
Instead of the previously proposed invasive approaches, this set
implements a less intrusive mechanism, by playing with the
perf_event.attribute.config bits."
Same as the previous series, this series keeps the same implementation
for two introduced format bits:
- contextid_in_vmid -> Is only supported when the VMID tracing
and CONTEXTIDR_EL2 both are supported. When requested the perf
etm4x backend sets (TRCCONFIGR.VMID | TRCCONFIGR.VMID_OPT).
As per ETMv4.4 TRM, when the core supports VHE, the CONTEXTIDR_EL2
tracing is mandatory. (See the field TRCID2.VMIDOPT)
- pid -> Is an alias for the correct config to enable PID tracing
on any kernel.
i.e, in EL1 kernel -> pid == contextid
EL2 kernel -> pid == contextid_in_vmid
With this, the perf tool is also updated to request the "pid"
tracing whenever available, falling back to "contextid" if it
is unavailable.
Comparing against the old version, this patch series uses the metadata
to save PID format; after add new item into metadata, it introduces
backward compatibility issue. To allow backward compatibility, this
series calculates per CPU metadata array size and avoid to use the
defined macro, so can always know the correct array size based on the
info stored in perf data file. Finally, the PID format stored in
metadata is passed to decoder and guide the decoder to set PID from
CONTEXTIDR_EL1 or VMID.
This patch series has been tested on Arm Juno-r2 board, with testing
two perf data files: one data file is recorded by the latest perf tool
after applied this patch series, and another data file is recorded by
old perf tool without this patch series, so this can prove the tool is
backward compatible.
Changes from RFC:
* Added comments to clarify cases requested (Leo);
* Explain the change to generic flags for cs_etm_set_option() in the
commit description;
* Stored PID format in metadata and passed it to decoder (Leo);
* Enhanced cs-etm for backward compatibility (Denis Nikitin).
[1] https://archive.armlinux.org.uk/lurker/message/20201110.183310.24406f33.en.html
Leo Yan (4):
perf cs-etm: Calculate per CPU metadata array size
perf cs-etm: Add PID format into metadata
perf cs-etm: Fixup PID_FMT when it is zero
perf cs-etm: Add helper cs_etm__get_pid_fmt()
Suzuki K Poulose (3):
coresight: etm-perf: Add support for PID tracing for kernel at EL2
perf cs_etm: Use pid tracing explicitly instead of contextid
perf cs-etm: Detect pid in VMID for kernel running at EL2
.../hwtracing/coresight/coresight-etm-perf.c | 14 +++
.../coresight/coresight-etm4x-core.c | 9 ++
include/linux/coresight-pmu.h | 11 ++-
tools/include/linux/coresight-pmu.h | 11 ++-
tools/perf/arch/arm/util/cs-etm.c | 89 +++++++++++++++----
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++-
tools/perf/util/cs-etm.c | 61 ++++++++++++-
tools/perf/util/cs-etm.h | 3 +
8 files changed, 198 insertions(+), 32 deletions(-)
--
2.25.1
On Mon, Jan 11, 2021 at 04:22:39PM +0000, Mike Leach wrote:
Hi Leo,
On Sat, 9 Jan 2021 at 07:44, Leo Yan [off-list ref] wrote:
quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com>
When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.
i.e, perf record -e cs_etm/pid/u --
will always do the "pid" tracing, independent of the kernel EL.
Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Cc: Mike Leach <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
include/linux/coresight-pmu.h | 11 +++++++----
3 files changed, 30 insertions(+), 4 deletions(-)
@@ -30,14 +30,28 @@ static DEFINE_PER_CPU(struct coresight_device *, csdev_src);/* ETMv3.5/PTM's ETMCR is 'config' */PMU_FORMAT_ATTR(cycacc,"config:"__stringify(ETM_OPT_CYCACC));PMU_FORMAT_ATTR(contextid,"config:"__stringify(ETM_OPT_CTXTID));+PMU_FORMAT_ATTR(contextid_in_vmid,"config:"__stringify(ETM_OPT_CTXTID_IN_VMID));PMU_FORMAT_ATTR(timestamp,"config:"__stringify(ETM_OPT_TS));PMU_FORMAT_ATTR(retstack,"config:"__stringify(ETM_OPT_RETSTK));/* Sink ID - same for all ETMs */PMU_FORMAT_ATTR(sinkid,"config2:0-31");+staticssize_tformat_attr_pid_show(structdevice*dev,+structdevice_attribute*attr,+char*page)+{+intpid_fmt=is_kernel_in_hyp_mode()?ETM_OPT_CTXTID_IN_VMID:ETM_OPT_CTXTID;++returnsprintf(page,"config:%d\n",pid_fmt);+}++structdevice_attributeformat_attr_pid=__ATTR(pid,0444,format_attr_pid_show,NULL);+staticstructattribute*etm_config_formats_attr[]={&format_attr_cycacc.attr,&format_attr_contextid.attr,+&format_attr_contextid_in_vmid.attr,+&format_attr_pid.attr,&format_attr_timestamp.attr,&format_attr_retstack.attr,&format_attr_sinkid.attr,
@@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,/* bit[6], Context ID tracing bit */config->cfg|=BIT(ETM4_CFG_BIT_CTXTID);+/* Do not enable VMID tracing if we are not running in EL2 */+if(attr->config&BIT(ETM_OPT_CTXTID_IN_VMID)){+if(!is_kernel_in_hyp_mode()){+ret=-EINVAL;+gotoout;+}+config->cfg|=BIT(ETM4_CFG_BIT_VMID)|BIT(ETM4_CFG_BIT_VMID_OPT);+}+/* return stack - enable if selected and supported */if((attr->config&BIT(ETM_OPT_RETSTK))&&drvdata->retstack)/* bit[12], Return stack enable bit */
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
On Mon, Jan 11, 2021 at 11:16:46AM -0700, Mathieu Poirier wrote:
Hi Leo,
Suzuki and Mike have pointed out a few things to modify and there was a couple
of kernel bot warnings to address as well. As such I will wait for your next
revision before looking at this set.
Yeah, this is fine for me.
Thanks,
Mathieu
On Sat, Jan 09, 2021 at 03:44:28PM +0800, Leo Yan wrote:
quoted
This patch series is a following up for the previous version which was
delivered by Suzuki [1]. Below gives the background info for why we
need this patch series, directly quotes the description in the cover
letter of the previous version:
"With the Virtualization Host Extensions, the kernel can run at EL2.
In this case the pid is written to CONTEXTIDR_EL2 instead of the
CONTEXTIDR_EL1. Thus the normal coresight tracing will be unable
to detect the PID of the thread generating the trace by looking
at the CONTEXTIDR_EL1. Thus, depending on the kernel EL, we must
switch to tracing the correct CONTEXTIDR register.
With VHE, we must set the TRCCONFIGR.VMID and TRCCONFIGR.VMID_OPT
to include the CONTEXTIDR_EL2 as the VMID in the trace. This
requires the perf tool to detect the changes in the TRCCONFIGR and
use the VMID / CID field for the PID. The challenge here is for
the perf tool to detect the kernel behavior.
Instead of the previously proposed invasive approaches, this set
implements a less intrusive mechanism, by playing with the
perf_event.attribute.config bits."
Same as the previous series, this series keeps the same implementation
for two introduced format bits:
- contextid_in_vmid -> Is only supported when the VMID tracing
and CONTEXTIDR_EL2 both are supported. When requested the perf
etm4x backend sets (TRCCONFIGR.VMID | TRCCONFIGR.VMID_OPT).
As per ETMv4.4 TRM, when the core supports VHE, the CONTEXTIDR_EL2
tracing is mandatory. (See the field TRCID2.VMIDOPT)
- pid -> Is an alias for the correct config to enable PID tracing
on any kernel.
i.e, in EL1 kernel -> pid == contextid
EL2 kernel -> pid == contextid_in_vmid
With this, the perf tool is also updated to request the "pid"
tracing whenever available, falling back to "contextid" if it
is unavailable.
Comparing against the old version, this patch series uses the metadata
to save PID format; after add new item into metadata, it introduces
backward compatibility issue. To allow backward compatibility, this
series calculates per CPU metadata array size and avoid to use the
defined macro, so can always know the correct array size based on the
info stored in perf data file. Finally, the PID format stored in
metadata is passed to decoder and guide the decoder to set PID from
CONTEXTIDR_EL1 or VMID.
This patch series has been tested on Arm Juno-r2 board, with testing
two perf data files: one data file is recorded by the latest perf tool
after applied this patch series, and another data file is recorded by
old perf tool without this patch series, so this can prove the tool is
backward compatible.
Changes from RFC:
* Added comments to clarify cases requested (Leo);
* Explain the change to generic flags for cs_etm_set_option() in the
commit description;
* Stored PID format in metadata and passed it to decoder (Leo);
* Enhanced cs-etm for backward compatibility (Denis Nikitin).
[1] https://archive.armlinux.org.uk/lurker/message/20201110.183310.24406f33.en.html
Leo Yan (4):
perf cs-etm: Calculate per CPU metadata array size
perf cs-etm: Add PID format into metadata
perf cs-etm: Fixup PID_FMT when it is zero
perf cs-etm: Add helper cs_etm__get_pid_fmt()
Suzuki K Poulose (3):
coresight: etm-perf: Add support for PID tracing for kernel at EL2
perf cs_etm: Use pid tracing explicitly instead of contextid
perf cs-etm: Detect pid in VMID for kernel running at EL2
.../hwtracing/coresight/coresight-etm-perf.c | 14 +++
.../coresight/coresight-etm4x-core.c | 9 ++
include/linux/coresight-pmu.h | 11 ++-
tools/include/linux/coresight-pmu.h | 11 ++-
tools/perf/arch/arm/util/cs-etm.c | 89 +++++++++++++++----
.../perf/util/cs-etm-decoder/cs-etm-decoder.c | 32 ++++++-
tools/perf/util/cs-etm.c | 61 ++++++++++++-
tools/perf/util/cs-etm.h | 3 +
8 files changed, 198 insertions(+), 32 deletions(-)
--
2.25.1
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
I looked into this suggestion but found it's complex than I assumed.
This config bits are not only used for ETMv3.x / PTM, it's also used
as an configuration interface between user space in Perf and kernel
drivers.
For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable
timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side,
the configuration is directly used ETMv3 (in coresight-etm3x-core.c),
but the configuration bits are converted for ETMv4 in the function
etm4_parse_event_config() [2].
So this is a historical issue, at the early period ETMv3 and ETMv4 can
be compatible with each other for configurations, but after evoluation,
some configs only belong to ETMv4 and cannot be applied on ETMv3
anymore, but we still use ETMv3.5 config bits as the interface between
kernel and userspace.
I'd like suggest we use a saperate patch set to refactor the
configuration bits, e.g. define the arbitrary configuration bits as
interface between user space and kernel, these bits are neutral for any
ETM version, in the kernel ETM drivers need to convert to its own
configuration formats. For this patch, I want to keep current
change as it is, is this okay for you?
Thanks,
Leo
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/arch/arm/util/cs-etm.c?h=v5.11-rc3#n152
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/hwtracing/coresight/coresight-etm4x-core.c?h=v5.11-rc3#n422
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
I looked into this suggestion but found it's complex than I assumed.
This config bits are not only used for ETMv3.x / PTM, it's also used
as an configuration interface between user space in Perf and kernel
drivers.
Exactly. I believe this problematic. We are stuckwith using the ETM3.x/PTM
config bits for the CS_ETM pmu, which is a bit wierd and the allocation of
the config bits are sparse. The problem with changing them now, will break
older perf tools decoding a perf.data from a newer kernel. I believe we are
stuck with this.
I would recommend simply updating the comment to reflect that, thats the
generic CS PMU ABI for configuration which was initially based on ETM3.x.
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
I looked into this suggestion but found it's complex than I assumed.
This config bits are not only used for ETMv3.x / PTM, it's also used
as an configuration interface between user space in Perf and kernel
drivers.
For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable
timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side,
the configuration is directly used ETMv3 (in coresight-etm3x-core.c),
but the configuration bits are converted for ETMv4 in the function
etm4_parse_event_config() [2].
So this is a historical issue, at the early period ETMv3 and ETMv4 can
be compatible with each other for configurations, but after evoluation,
some configs only belong to ETMv4 and cannot be applied on ETMv3
anymore, but we still use ETMv3.5 config bits as the interface between
kernel and userspace.
I was aware that etm3/ptm used these bits as both the options and the
bit values for direct writing to the ETMCR register for ETMv3, and
re-mapped to appropriate register values in ETMv4.
In the past we have re-used etmv3.5 bit definitions ETM_xxx when
appropriate, but where unique to ETM4 we have used a ETM4_xxx naming
convention.
I am not suggesting re-factoring the options completely, just
re-naming this single option to make it clear it is unique to ETM4+.
Looking at the etmv3 driver, at present it does not actually appear to
support contextid tracing - and when it does, both bits 14 and 15 will
be required to be used - as ETMCR defines these bits as ContextID
size.
Should this ever get fixed. then having an overlapping option bit -
that appears to be valid for ETMv3 will be confusing.
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
I looked into this suggestion but found it's complex than I assumed.
This config bits are not only used for ETMv3.x / PTM, it's also used
as an configuration interface between user space in Perf and kernel
drivers.
For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable
timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side,
the configuration is directly used ETMv3 (in coresight-etm3x-core.c),
but the configuration bits are converted for ETMv4 in the function
etm4_parse_event_config() [2].
So this is a historical issue, at the early period ETMv3 and ETMv4 can
be compatible with each other for configurations, but after evoluation,
some configs only belong to ETMv4 and cannot be applied on ETMv3
anymore, but we still use ETMv3.5 config bits as the interface between
kernel and userspace.
I was aware that etm3/ptm used these bits as both the options and the
bit values for direct writing to the ETMCR register for ETMv3, and
re-mapped to appropriate register values in ETMv4.
In the past we have re-used etmv3.5 bit definitions ETM_xxx when
appropriate, but where unique to ETM4 we have used a ETM4_xxx naming
convention.
I am concern this approach is not friendly for extension; for example,
let's say IP ETM5 with defined bit 28 as CTXTID, if add a new option
for it, we need to define macro as:
#define ETM5_OPT_CTXTID 28
This will result in confliction with the existed option ETM_OPT_TS
and it is hard for maintenance for options, since there have different
prefixes (like ETM_OPT_xxx, ETM4_OPT_xxx, ETM5_OPT_xxx, etc).
I'd like to take option as knob to enable or disable hardware
feature; the low level drivers should set the appropriate values for
registers based on different options.
Furthermore, ETM driver should report error when detect any option is
not supported, I.e. ETM3 driver should report failure if user wrongly
set the option ETM_OPT_CTXTID_IN_VMID.
I am not suggesting re-factoring the options completely, just
re-naming this single option to make it clear it is unique to ETM4+.
Here I perfer Suzuki's suggestion to simply refine comments, something
like below:
/*
* Below are bit offsets for perf options, most of them are orignally
* coming from ETMv3.5/PTM's ETMCR config bits (so far except
* ETM_OPT_CTXTID_IN_VMID is only used for ETMv4).
*
* ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
* directly use below macros as config bits.
*/
#define ETM_OPT_CYCACC 12
#define ETM_OPT_CTXTID 14
#define ETM_OPT_CTXTID_IN_VMID 15
#define ETM_OPT_TS 28
#define ETM_OPT_RETSTK 29
Looking at the etmv3 driver, at present it does not actually appear to
support contextid tracing - and when it does, both bits 14 and 15 will
be required to be used - as ETMCR defines these bits as ContextID
size.
Should this ever get fixed.
Good catch! Seems to me, this is a good example that we should
distinguish the definition between Perf options and config bits :)
then having an overlapping option bit -
that appears to be valid for ETMv3 will be confusing.
I hope the the proposed change can avoid the confusion, if have
concern, please let me know.
Thanks a lot for suggestions,
Leo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Mike Leach <hidden> Date: 2021-01-13 00:52:41
Hi Leo,
On Mon, 11 Jan 2021 at 15:06, Leo Yan [off-list ref] wrote:
quoted hunk
Hi Mike,
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
quoted
Hi Leo,
I think there is an issue here in that your modification assumes that
all cpus in the system are of the same ETM type. The original routine
allowed for differing ETM types, thus differing cpu ETM field lengths
between ETMv4 / ETMv3, the field size was used after the relevant
magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
Thanks for pointing out this.
quoted
Moving forwards we are seeing the newer FEAT_ETE protocol drivers
appearing on the list, which will ultimately need a new metadata
structure.
We have had discussions within ARM regarding the changing of the
format to be more self describing - which should probably be opened
out to the CS mailing list.
I think here have two options. One option is I think we can use
__perf_cs_etmv3_magic/__perf_cs_etmv4_magic as indicator for the
starting of next metadata array; when copy the metadata, always check
the next item in the buffer, if it's __perf_cs_etmv3_magic or
__perf_cs_etmv4_magic, will break loop and start copying metadata
array for next CPU. The suggested change is pasted in below.
Another option is I drop patches 03,05/07 in the series and leave the
backward compatibility fixing for a saperate patch series with self
describing method. Especially, if you think the first option will
introduce trouble for enabling self describing later, then I am happy
to drop patches 03,05.
How about you think for this?
Thanks,
Leo
---8<---
@@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETM_PRIV_MAX;k++)+for(k=0;k<CS_ETM_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];-i+=CS_ETM_PRIV_MAX;+i+=k;}elseif(ptr[i]==__perf_cs_etmv4_magic){metadata[j]=zalloc(sizeof(*metadata[j])*CS_ETMV4_PRIV_MAX);
@@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETMV4_PRIV_MAX;k++)+for(k=0;k<CS_ETMV4_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];-i+=CS_ETMV4_PRIV_MAX;+i+=k;}/* Get an RB node for this CPU */
That would be a spot fix for the read /copy case, but will not fix the
print routine which will still bail out on older versions of the
format. (when using perf report --dump).
The "self describing" format I have been looking at will add an
NR_PARAMS value to the common block in the CPU metadata parameter
list, increment the header version to '1' and update the format writer
to use the version 1 format while having the reader understand both v0
and v1 formats.
i..e in cs-etm.h perf I add:
/*
* Update the version for new format.
*
* New version 1 format adds a param count to the per cpu metadata.
* This allows easy adding of new metadata parameters.
* Requires that new params always added after current ones.
* Also allows client reader to handle file versions that are different by
* checking the number of params in the file vs the number expected.
*/
#define CS_HEADER_CURRENT_VERSION 1
/* Beginning of header common to both ETMv3 and V4 */
enum {
CS_ETM_MAGIC,
CS_ETM_CPU,
CS_ETM_NR_PARAMS, /* number of parameters to follow in this block */
};
where in verison 1, NR_PARAMS indicates the total number of params
that follow - so adding new parameters can be added to the metadata
enums and the tool will automatically adjust, and will handle v0
files, plus older and newer files that have differing numbers of
parameters, as long as the parameters are only ever added to the end
of the list.
I have been working on a patch for this today, which took a little
longer than expected as it was a little more complex than expected
(the printing routines in for the --dump command!).
I will post this tomorrow when tested - and if we agree it works it
could be rolled into your set - it would make adding the PID parameter
easier, and ensure that this new format is available for the upcoming
developments.
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Minor issue here - ETMv3.x / PTM cannot trace CXTID in VMID so this
may better be named ETM4_OPT_CTXTID_IN_VMID, rather than be grouped
with the ETM3.5 options?
I looked into this suggestion but found it's complex than I assumed.
This config bits are not only used for ETMv3.x / PTM, it's also used
as an configuration interface between user space in Perf and kernel
drivers.
For example, in the userspace, perf tool sets bit ETM_OPT_TS to enable
timestamp [1], this is same for ETMv3 and ETMv4. In the kernel side,
the configuration is directly used ETMv3 (in coresight-etm3x-core.c),
but the configuration bits are converted for ETMv4 in the function
etm4_parse_event_config() [2].
So this is a historical issue, at the early period ETMv3 and ETMv4 can
be compatible with each other for configurations, but after evoluation,
some configs only belong to ETMv4 and cannot be applied on ETMv3
anymore, but we still use ETMv3.5 config bits as the interface between
kernel and userspace.
I was aware that etm3/ptm used these bits as both the options and the
bit values for direct writing to the ETMCR register for ETMv3, and
re-mapped to appropriate register values in ETMv4.
In the past we have re-used etmv3.5 bit definitions ETM_xxx when
appropriate, but where unique to ETM4 we have used a ETM4_xxx naming
convention.
I am concern this approach is not friendly for extension; for example,
let's say IP ETM5 with defined bit 28 as CTXTID, if add a new option
for it, we need to define macro as:
#define ETM5_OPT_CTXTID 28
This will result in confliction with the existed option ETM_OPT_TS
and it is hard for maintenance for options, since there have different
prefixes (like ETM_OPT_xxx, ETM4_OPT_xxx, ETM5_OPT_xxx, etc).
No it will not - we don't need a new option for CTXTID in a
hypothetical ETM5 - as we use the existing one for ETM3 and map it to
the correct bit, just as ETM4 does.
I'd like to take option as knob to enable or disable hardware
feature; the low level drivers should set the appropriate values for
registers based on different options.
Furthermore, ETM driver should report error when detect any option is
not supported, I.e. ETM3 driver should report failure if user wrongly
set the option ETM_OPT_CTXTID_IN_VMID.
quoted
I am not suggesting re-factoring the options completely, just
re-naming this single option to make it clear it is unique to ETM4+.
Here I perfer Suzuki's suggestion to simply refine comments, something
like below:
/*
* Below are bit offsets for perf options, most of them are orignally
* coming from ETMv3.5/PTM's ETMCR config bits (so far except
* ETM_OPT_CTXTID_IN_VMID is only used for ETMv4).
*
* ETMv3.5/PTM doesn't define ETMCR config bits with prefix "ETM3_" and
* directly use below macros as config bits.
*/
#define ETM_OPT_CYCACC 12
#define ETM_OPT_CTXTID 14
#define ETM_OPT_CTXTID_IN_VMID 15
#define ETM_OPT_TS 28
#define ETM_OPT_RETSTK 29
quoted
Looking at the etmv3 driver, at present it does not actually appear to
support contextid tracing - and when it does, both bits 14 and 15 will
be required to be used - as ETMCR defines these bits as ContextID
size.
Should this ever get fixed.
Good catch! Seems to me, this is a good example that we should
distinguish the definition between Perf options and config bits :)
quoted
then having an overlapping option bit -
that appears to be valid for ETMv3 will be confusing.
I hope the the proposed change can avoid the confusion, if have
concern, please let me know.
Thanks a lot for suggestions,
Leo
If you think that clarification via comment is better than a change of
name then go ahead.
Regards
Mike
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
@@ -2558,12 +2558,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETM_PRIV_MAX;k++)+for(k=0;k<CS_ETM_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETM_ETMTRACEIDR];-i+=CS_ETM_PRIV_MAX;+i+=k;}elseif(ptr[i]==__perf_cs_etmv4_magic){metadata[j]=zalloc(sizeof(*metadata[j])*CS_ETMV4_PRIV_MAX);
@@ -2571,12 +2578,19 @@ int cs_etm__process_auxtrace_info(union perf_event *event,err=-ENOMEM;gotoerr_free_metadata;}-for(k=0;k<CS_ETMV4_PRIV_MAX;k++)+for(k=0;k<CS_ETMV4_PRIV_MAX;k++){metadata[j][k]=ptr[i+k];+if(ptr[i+k+1]==__perf_cs_etmv3_magic||+ptr[i+k+1]==__perf_cs_etmv4_magic){+k++;+break;+}+}+/* The traceID is our handle */idx=metadata[j][CS_ETMV4_TRCTRACEIDR];-i+=CS_ETMV4_PRIV_MAX;+i+=k;}/* Get an RB node for this CPU */
That would be a spot fix for the read /copy case, but will not fix the
print routine which will still bail out on older versions of the
format. (when using perf report --dump).
The "self describing" format I have been looking at will add an
NR_PARAMS value to the common block in the CPU metadata parameter
list, increment the header version to '1' and update the format writer
to use the version 1 format while having the reader understand both v0
and v1 formats.
i..e in cs-etm.h perf I add:
/*
* Update the version for new format.
*
* New version 1 format adds a param count to the per cpu metadata.
* This allows easy adding of new metadata parameters.
* Requires that new params always added after current ones.
* Also allows client reader to handle file versions that are different by
* checking the number of params in the file vs the number expected.
*/
#define CS_HEADER_CURRENT_VERSION 1
/* Beginning of header common to both ETMv3 and V4 */
enum {
CS_ETM_MAGIC,
CS_ETM_CPU,
CS_ETM_NR_PARAMS, /* number of parameters to follow in this block */
};
where in verison 1, NR_PARAMS indicates the total number of params
that follow - so adding new parameters can be added to the metadata
enums and the tool will automatically adjust, and will handle v0
files, plus older and newer files that have differing numbers of
parameters, as long as the parameters are only ever added to the end
of the list.
I have been working on a patch for this today, which took a little
longer than expected as it was a little more complex than expected
(the printing routines in for the --dump command!).
I will post this tomorrow when tested - and if we agree it works it
could be rolled into your set - it would make adding the PID parameter
easier, and ensure that this new format is available for the upcoming
developments.
Thanks for the info. I will look at your patch and see how to fit
with it.
Thanks,
Leo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hey guys,
On Sat, Jan 09, 2021 at 03:44:29PM +0800, Leo Yan wrote:
quoted hunk
From: Suzuki K Poulose <suzuki.poulose@arm.com>
When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.
i.e, perf record -e cs_etm/pid/u --
will always do the "pid" tracing, independent of the kernel EL.
Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Cc: Mike Leach <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
include/linux/coresight-pmu.h | 11 +++++++----
3 files changed, 30 insertions(+), 4 deletions(-)
The same applies here. PMU format bits are options the PMU supports rather than
a representation of the hardware, making bit numbering arbitrary. A such we
don't explicitly need a contextid_in_vmid option. Making the current contextid
variable, the same it was done for 'pid', should be sufficient. Based on the
value carried by contexid, i.e 14 or 15, the perf tools will know where to get
the contextID.
With regards to backward functionality, user who hard code 'config' on the perf
command line won't get the results they want when the kernel is at EL2 anyway.
The kernel, with function is_kernel_in_hyp_mode(), is not an issue.
@@ -477,6 +477,15 @@ static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,/* bit[6], Context ID tracing bit */config->cfg|=BIT(ETM4_CFG_BIT_CTXTID);+/* Do not enable VMID tracing if we are not running in EL2 */+if(attr->config&BIT(ETM_OPT_CTXTID_IN_VMID)){+if(!is_kernel_in_hyp_mode()){+ret=-EINVAL;+gotoout;+}+config->cfg|=BIT(ETM4_CFG_BIT_VMID)|BIT(ETM4_CFG_BIT_VMID_OPT);+}+/* return stack - enable if selected and supported */if((attr->config&BIT(ETM_OPT_RETSTK))&&drvdata->retstack)/* bit[12], Return stack enable bit */
On Sat, Jan 09, 2021 at 03:44:30PM +0800, Leo Yan wrote:
quoted hunk
From: Suzuki K Poulose <suzuki.poulose@arm.com>
If the kernel is running at EL2, the pid of the task is exposed
via VMID instead of the CONTEXTID. Add support for this in the
perf tool.
By default the perf tool requests contextid and timestamp for
task bound events. Instead of hard coding contextid, switch
to "pid" config exposed by the kernel. While at it, define new
independent macros (rather than using the "config" bits) for
requesting the "pid" and "timestamp" for cs_etm_set_option(),
since the PID config is now dynamic depending on the kernel
exception level.
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
tools/include/linux/coresight-pmu.h | 11 +++--
tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 20 deletions(-)
@@ -86,21 +87,50 @@ static int cs_etm_set_context_id(struct auxtrace_record *itr,gotoout;}+pid_fmt=perf_pmu__format_bits(&cs_etm_pmu->format,"pid");
If we go with my suggestion from patch 1 this would become "contextid".
I think what follows below is all good.
quoted hunk
/*
- * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID tracing
- * is supported:
- * 0b00000 Context ID tracing is not supported.
- * 0b00100 Maximum of 32-bit Context ID size.
- * All other values are reserved.
+ * If the kernel doesn't support the "pid" format (older kernel),
+ * fall back to using the CTXTID.
*/
- val = BMVAL(val, 5, 9);
- if (!val || val != 0x4) {
+ if (!pid_fmt)
+ pid_fmt = 1ULL << ETM_OPT_CTXTID;
+
+ switch (pid_fmt) {
+ case (1ULL << ETM_OPT_CTXTID):
+ /*
+ * TRCIDR2.CIDSIZE, bit [9-5], indicates whether contextID
+ * tracing is supported:
+ * 0b00000 Context ID tracing is not supported.
+ * 0b00100 Maximum of 32-bit Context ID size.
+ * All other values are reserved.
+ */
+ val = BMVAL(val, 5, 9);
+ if (!val || val != 0x4) {
+ err = -EINVAL;
+ goto out;
+ }
+ break;
+ case (1ULL << ETM_OPT_CTXTID_IN_VMID):
+ /*
+ * TRCIDR2.VMIDOPT[30:29] != 0 and
+ * TRCIDR2.VMIDSIZE[14:10] == 0b00100 (32bit virtual contextid)
+ * We can't support CONTEXTIDR in VMID if the size of the
+ * virtual context id is < 32bit.
+ * Any value of VMIDSIZE >= 4 (i.e, > 32bit) is fine for us.
+ */
+ if (!BMVAL(val, 29, 30) || BMVAL(val, 10, 14) < 4) {
+ err = -EINVAL;
+ goto out;
+ }
+ break;
+ default:
err = -EINVAL;
goto out;
}
+
/* All good, let the kernel know */
- evsel->core.attr.config |= (1 << ETM_OPT_CTXTID);
+ evsel->core.attr.config |= pid_fmt;
err = 0;
out:
I don't think we need this as part of the current patchset. Back in the days I
thought it would be a good idea to use the ETMv3.5/PTM configuration bits but in
hindsight it wasn't. That's what happens when your crystal ball isn't working.
That being said we can start treating the perf_event_attr::config bits
arbitrarily. Someone pointed out that it might get confusing but I don't think
it is anything that can't be overcomed with good in-line documentation.
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
Hi Leo,
I think there is an issue here in that your modification assumes that
all cpus in the system are of the same ETM type. The original routine
allowed for differing ETM types, thus differing cpu ETM field lengths
between ETMv4 / ETMv3, the field size was used after the relevant
magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
I usually go through an entire patchset before looking at the comments people
have made. In this case Mike and I are coming to the exact same conclusion.
I will look at Mike's patch on Monday.
Moving forwards we are seeing the newer FEAT_ETE protocol drivers
appearing on the list, which will ultimately need a new metadata
structure.
We have had discussions within ARM regarding the changing of the
format to be more self describing - which should probably be opened
out to the CS mailing list.
Regards
Mike
On Mon, 11 Jan 2021 at 07:29, Suzuki K Poulose [off-list ref] wrote:
quoted
On 1/9/21 7:44 AM, Leo Yan wrote:
quoted
The metadata array can be extended over time and the tool, if using the
predefined macro (like CS_ETMV4_PRIV_MAX for ETMv4) as metadata array
size to copy data, it can cause compatible issue within different
versions of perf tool.
E.g. we recorded a data file with an old version tool, afterwards if
use the new version perf tool to parse the file, since the metadata
array has been extended and the macro CS_ETMV4_PRIV_MAX has been
altered, if use it to parse the perf data with old format, this will
lead to mismatch.
To maintain backward compatibility, this patch calculates per CPU
metadata array size on the runtime, the calculation is based on the
info stored in the data file so that it's reliable.
Signed-off-by: Leo Yan <redacted>
Looks good to me.
Acked-by: Suzuki K Poulose <suzuki.poulose@arm.com>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Hi Mathieu,
On Fri, Jan 15, 2021 at 03:46:58PM -0700, Mathieu Poirier wrote:
On Mon, Jan 11, 2021 at 12:09:12PM +0000, Mike Leach wrote:
quoted
Hi Leo,
I think there is an issue here in that your modification assumes that
all cpus in the system are of the same ETM type. The original routine
allowed for differing ETM types, thus differing cpu ETM field lengths
between ETMv4 / ETMv3, the field size was used after the relevant
magic number for the cpu ETM was read.
You have replaced two different sizes - with a single calculated size.
I usually go through an entire patchset before looking at the comments people
have made. In this case Mike and I are coming to the exact same conclusion.
Agreed, now this work depends on Mike's patch for extending metadata
version; otherwise if without Mike's patch, it will cause compability
issue.
Hi Mathieu,
On Fri, Jan 15, 2021 at 03:44:16PM -0700, Mathieu Poirier wrote:
On Sat, Jan 09, 2021 at 03:44:30PM +0800, Leo Yan wrote:
quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com>
If the kernel is running at EL2, the pid of the task is exposed
via VMID instead of the CONTEXTID. Add support for this in the
perf tool.
By default the perf tool requests contextid and timestamp for
task bound events. Instead of hard coding contextid, switch
to "pid" config exposed by the kernel. While at it, define new
independent macros (rather than using the "config" bits) for
requesting the "pid" and "timestamp" for cs_etm_set_option(),
since the PID config is now dynamic depending on the kernel
exception level.
Cc: Mike Leach <redacted>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
tools/include/linux/coresight-pmu.h | 11 +++--
tools/perf/arch/arm/util/cs-etm.c | 68 ++++++++++++++++++++++-------
2 files changed, 59 insertions(+), 20 deletions(-)
In the previous patch, these bits are defined in the kernel's header
include/linux/coresight-pmu.h; at here it defines the same bits in
tool's header.
To be honest, I struggled to understand your suggestions, finally I
think it's feasbile and we can simplify the implementation. I will
try to refine the patch series for the review.
Appreciate for good suggestions!
Leo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Suzuki K Poulose <suzuki.poulose@arm.com> Date: 2021-01-19 07:08:17
Hi Mathieu
On 1/15/21 10:30 PM, Mathieu Poirier wrote:
Hey guys,
On Sat, Jan 09, 2021 at 03:44:29PM +0800, Leo Yan wrote:
quoted
From: Suzuki K Poulose <suzuki.poulose@arm.com>
When the kernel is running at EL2, the PID is stored in CONTEXTIDR_EL2.
So, tracing CONTEXTIDR_EL1 doesn't give us the pid of the process.
Thus we should trace the VMID with VMIDOPT set to trace
CONTEXTIDR_EL2 instead of CONTEXTIDR_EL1. Given that we have an existing
config option "contextid" and this will be useful for tracing
virtual machines (when we get to support virtualization). So instead,
this patch adds a new option, contextid_in_vmid as a separate config.
Thus on an EL2 kernel, we will have two options available for
the perf tool. However, to make it easier for the user to
do pid tracing, we add a new format which will default to
"contextid" (on EL1 kernel) or "contextid_in_vmid" (on EL2
kernel). So that the user doesn't have to bother which EL the
kernel is running.
i.e, perf record -e cs_etm/pid/u --
will always do the "pid" tracing, independent of the kernel EL.
Also, the perf tool will be updated to automatically select
"pid" config instead of the "contextid" for system wide/CPU wide
mode.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Al Grant <redacted>
Cc: Mike Leach <redacted>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Leo Yan <redacted>
---
drivers/hwtracing/coresight/coresight-etm-perf.c | 14 ++++++++++++++
drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 +++++++++
include/linux/coresight-pmu.h | 11 +++++++----
3 files changed, 30 insertions(+), 4 deletions(-)
The same applies here. PMU format bits are options the PMU supports rather than
a representation of the hardware, making bit numbering arbitrary. A such we
don't explicitly need a contextid_in_vmid option. Making the current contextid
variable, the same it was done for 'pid', should be sufficient. Based on the
value carried by contexid, i.e 14 or 15, the perf tools will know where to get
the contextID.
With regards to backward functionality, user who hard code 'config' on the perf
command line won't get the results they want when the kernel is at EL2 anyway.
The kernel, with function is_kernel_in_hyp_mode(), is not an issue.
I did think about that. The reason behind using a new alias is vaguely mentioned
in the description. If a host perf session wants to trace a VM with the contextid_el1,
there is no option for that with "contextid" flipped to trace "contextid_el2".
This is precisely why I preferred keeping both the hardware configurations
and let the kernel choose the right one for the EL, by having an alias.
i.e,
perf record -e cs_etm/contextid,contextid_in_vmid/ vm
could still trace the VM vcpu threads and the CID changes within
the VM. (This is triggered from the host, so VM support is not necessary).
If we decide not to do this, or change the meaning of contextid now
to mean "pid" and change it in the future back to what it really
means for supporting such scenarios above, then we are going to
be back where we are with the proposal.
Suzuki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel