CoreSight

coresight@lists.linaro.org

2 participants
2639 discussions

Re: [PATCH v2 7/7] perf pmu: Lazily compute default config

by Leo Yan

On Thu, Oct 12, 2023 at 10:56:45AM -0700, Ian Rogers wrote: > The default config is computed during creation of the PMU and may do > things like scanning sysfs, when the PMU may just be used as part of > scanning. Change default_config to perf_event_attr_init_default, a > callback that is used when a default config needs initializing. This > avoids holding onto the memory for a perf_event_attr and copying. > > On a tigerlake laptop running the pmu-scan benchmark: > > Before: > Running 'internals/pmu-scan' benchmark: > Computing performance of sysfs PMU event scan for 100 times > Average core PMU scanning took: 28.780 usec (+- 0.503 usec) > Average PMU scanning took: 283.480 usec (+- 18.471 usec) > Number of openat syscalls: 30,227 > > After: > Running 'internals/pmu-scan' benchmark: > Computing performance of sysfs PMU event scan for 100 times > Average core PMU scanning took: 27.880 usec (+- 0.169 usec) > Average PMU scanning took: 245.260 usec (+- 15.758 usec) > Number of openat syscalls: 28,914 > > Over 3 runs it is a nearly 12% reduction in execution time and a 4.3% > of openat calls. > > Signed-off-by: Ian Rogers <irogers(a)google.com> > Reviewed-by: Adrian Hunter <adrian.hunter(a)intel.com> > --- > tools/perf/arch/arm/util/cs-etm.c | 13 ++------ > tools/perf/arch/arm/util/pmu.c | 4 +-- > tools/perf/arch/arm64/util/arm-spe.c | 45 ++++++++++++++-------------- > tools/perf/arch/x86/util/intel-pt.c | 25 ++++++++-------- > tools/perf/arch/x86/util/pmu.c | 2 +- > tools/perf/util/arm-spe.h | 4 ++- > tools/perf/util/cs-etm.h | 2 +- > tools/perf/util/intel-pt.h | 3 +- > tools/perf/util/parse-events.c | 12 ++++---- > tools/perf/util/pmu.c | 3 +- > tools/perf/util/pmu.h | 7 +++-- > 11 files changed, 58 insertions(+), 62 deletions(-) > > diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c > index b8d6a953fd74..16bba74f048b 100644 > --- a/tools/perf/arch/arm/util/cs-etm.c > +++ b/tools/perf/arch/arm/util/cs-etm.c > @@ -917,16 +917,9 @@ struct auxtrace_record *cs_etm_record_init(int *err) > * (CFG_CHG and evsel__set_config_if_unset()). If no default is set then user > * changes aren't tracked. > */ > -struct perf_event_attr * > -cs_etm_get_default_config(struct perf_pmu *pmu __maybe_unused) > +void > +cs_etm_get_default_config(const struct perf_pmu *pmu __maybe_unused, > + struct perf_event_attr *attr) > { > - struct perf_event_attr *attr; > - > - attr = zalloc(sizeof(struct perf_event_attr)); > - if (!attr) > - return NULL; > - > attr->sample_period = 1; > - > - return attr; > } > diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c > index f25f68f84a94..7f3af3b97f3b 100644 > --- a/tools/perf/arch/arm/util/pmu.c > +++ b/tools/perf/arch/arm/util/pmu.c > @@ -20,12 +20,12 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused) > if (!strcmp(pmu->name, CORESIGHT_ETM_PMU_NAME)) { > /* add ETM default config here */ > pmu->selectable = true; > - pmu->default_config = cs_etm_get_default_config(pmu); > + pmu->perf_event_attr_init_default = cs_etm_get_default_config; > #if defined(__aarch64__) > } else if (strstarts(pmu->name, ARM_SPE_PMU_NAME)) { > pmu->selectable = true; > pmu->is_uncore = false; > - pmu->default_config = arm_spe_pmu_default_config(pmu); > + pmu->perf_event_attr_init_default = arm_spe_pmu_default_config; > } else if (strstarts(pmu->name, HISI_PTT_PMU_NAME)) { > pmu->selectable = true; > #endif > diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c > index 08a76734ccd2..e3acc739bd00 100644 > --- a/tools/perf/arch/arm64/util/arm-spe.c > +++ b/tools/perf/arch/arm64/util/arm-spe.c > @@ -113,6 +113,25 @@ arm_spe_snapshot_resolve_auxtrace_defaults(struct record_opts *opts, > } > } > > +static __u64 arm_spe_pmu__sample_period(const struct perf_pmu *arm_spe_pmu) > +{ > + static __u64 sample_period; > + > + if (sample_period) > + return sample_period; I would like to drop the static 'sample_period' optimization. IIUC, it's possible for some platforms to create more than one arm_spe events (e.g. Arm big.LITTLE system), in that case, it's incorrect to use the single sample period for all of arm_spe events. Otherwise, Arm SPE changes are good for me. Thanks, Leo > + > + /* > + * If kernel driver doesn't advertise a minimum, > + * use max allowable by PMSIDR_EL1.INTERVAL > + */ > + if (perf_pmu__scan_file(arm_spe_pmu, "caps/min_interval", "%llu", > + &sample_period) != 1) { > + pr_debug("arm_spe driver doesn't advertise a min. interval. Using 4096\n"); > + sample_period = 4096; > + } > + return sample_period; > +} > + > static int arm_spe_recording_options(struct auxtrace_record *itr, > struct evlist *evlist, > struct record_opts *opts) > @@ -136,7 +155,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, > return -EINVAL; > } > evsel->core.attr.freq = 0; > - evsel->core.attr.sample_period = arm_spe_pmu->default_config->sample_period; > + evsel->core.attr.sample_period = arm_spe_pmu__sample_period(arm_spe_pmu); > evsel->needs_auxtrace_mmap = true; > arm_spe_evsel = evsel; > opts->full_auxtrace = true; > @@ -495,26 +514,8 @@ struct auxtrace_record *arm_spe_recording_init(int *err, > return &sper->itr; > } > > -struct perf_event_attr > -*arm_spe_pmu_default_config(struct perf_pmu *arm_spe_pmu) > +void > +arm_spe_pmu_default_config(const struct perf_pmu *arm_spe_pmu, struct perf_event_attr *attr) > { > - struct perf_event_attr *attr; > - > - attr = zalloc(sizeof(struct perf_event_attr)); > - if (!attr) { > - pr_err("arm_spe default config cannot allocate a perf_event_attr\n"); > - return NULL; > - } > - > - /* > - * If kernel driver doesn't advertise a minimum, > - * use max allowable by PMSIDR_EL1.INTERVAL > - */ > - if (perf_pmu__scan_file(arm_spe_pmu, "caps/min_interval", "%llu", > - &attr->sample_period) != 1) { > - pr_debug("arm_spe driver doesn't advertise a min. interval. Using 4096\n"); > - attr->sample_period = 4096; > - } > - > - return attr; > + attr->sample_period = arm_spe_pmu__sample_period(arm_spe_pmu); > } > diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c > index 6d6cd8f9133c..fa0c718b9e72 100644 > --- a/tools/perf/arch/x86/util/intel-pt.c > +++ b/tools/perf/arch/x86/util/intel-pt.c > @@ -60,7 +60,7 @@ struct intel_pt_recording { > size_t priv_size; > }; > > -static int intel_pt_parse_terms_with_default(struct perf_pmu *pmu, > +static int intel_pt_parse_terms_with_default(const struct perf_pmu *pmu, > const char *str, > u64 *config) > { > @@ -84,7 +84,7 @@ static int intel_pt_parse_terms_with_default(struct perf_pmu *pmu, > return err; > } > > -static int intel_pt_parse_terms(struct perf_pmu *pmu, const char *str, u64 *config) > +static int intel_pt_parse_terms(const struct perf_pmu *pmu, const char *str, u64 *config) > { > *config = 0; > return intel_pt_parse_terms_with_default(pmu, str, config); > @@ -177,7 +177,7 @@ static int intel_pt_pick_bit(int bits, int target) > return pick; > } > > -static u64 intel_pt_default_config(struct perf_pmu *intel_pt_pmu) > +static u64 intel_pt_default_config(const struct perf_pmu *intel_pt_pmu) > { > char buf[256]; > int mtc, mtc_periods = 0, mtc_period; > @@ -256,18 +256,17 @@ static int intel_pt_parse_snapshot_options(struct auxtrace_record *itr, > return 0; > } > > -struct perf_event_attr * > -intel_pt_pmu_default_config(struct perf_pmu *intel_pt_pmu) > +void intel_pt_pmu_default_config(const struct perf_pmu *intel_pt_pmu, > + struct perf_event_attr *attr) > { > - struct perf_event_attr *attr; > + static u64 config; > + static bool initialized; > > - attr = zalloc(sizeof(struct perf_event_attr)); > - if (!attr) > - return NULL; > - > - attr->config = intel_pt_default_config(intel_pt_pmu); > - > - return attr; > + if (!initialized) { > + config = intel_pt_default_config(intel_pt_pmu); > + initialized = true; > + } > + attr->config = config; > } > > static const char *intel_pt_find_filter(struct evlist *evlist, > diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c > index 949b3e2c67bd..469555ae9b3c 100644 > --- a/tools/perf/arch/x86/util/pmu.c > +++ b/tools/perf/arch/x86/util/pmu.c > @@ -23,7 +23,7 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused) > if (!strcmp(pmu->name, INTEL_PT_PMU_NAME)) { > pmu->auxtrace = true; > pmu->selectable = true; > - pmu->default_config = intel_pt_pmu_default_config(pmu); > + pmu->perf_event_attr_init_default = intel_pt_pmu_default_config; > } > if (!strcmp(pmu->name, INTEL_BTS_PMU_NAME)) { > pmu->auxtrace = true; > diff --git a/tools/perf/util/arm-spe.h b/tools/perf/util/arm-spe.h > index 98d3235781c3..4f4900c18f3e 100644 > --- a/tools/perf/util/arm-spe.h > +++ b/tools/perf/util/arm-spe.h > @@ -27,5 +27,7 @@ struct auxtrace_record *arm_spe_recording_init(int *err, > int arm_spe_process_auxtrace_info(union perf_event *event, > struct perf_session *session); > > -struct perf_event_attr *arm_spe_pmu_default_config(struct perf_pmu *arm_spe_pmu); > +void arm_spe_pmu_default_config(const struct perf_pmu *arm_spe_pmu, > + struct perf_event_attr *attr); > + > #endif > diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h > index 7cca37887917..4696267a32f0 100644 > --- a/tools/perf/util/cs-etm.h > +++ b/tools/perf/util/cs-etm.h > @@ -242,7 +242,7 @@ struct cs_etm_packet_queue { > > int cs_etm__process_auxtrace_info(union perf_event *event, > struct perf_session *session); > -struct perf_event_attr *cs_etm_get_default_config(struct perf_pmu *pmu); > +void cs_etm_get_default_config(const struct perf_pmu *pmu, struct perf_event_attr *attr); > > enum cs_etm_pid_fmt { > CS_ETM_PIDFMT_NONE, > diff --git a/tools/perf/util/intel-pt.h b/tools/perf/util/intel-pt.h > index c7d6068e3a6b..18fd0be52e6c 100644 > --- a/tools/perf/util/intel-pt.h > +++ b/tools/perf/util/intel-pt.h > @@ -42,6 +42,7 @@ struct auxtrace_record *intel_pt_recording_init(int *err); > int intel_pt_process_auxtrace_info(union perf_event *event, > struct perf_session *session); > > -struct perf_event_attr *intel_pt_pmu_default_config(struct perf_pmu *pmu); > +void intel_pt_pmu_default_config(const struct perf_pmu *intel_pt_pmu, > + struct perf_event_attr *attr); > > #endif > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index c56e07bd7dd6..ea5579510b97 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -1418,11 +1418,10 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, > } > fix_raw(&parsed_terms, pmu); > > - if (pmu->default_config) { > - memcpy(&attr, pmu->default_config, sizeof(struct perf_event_attr)); > - } else { > - memset(&attr, 0, sizeof(attr)); > - } > + memset(&attr, 0, sizeof(attr)); > + if (pmu->perf_event_attr_init_default) > + pmu->perf_event_attr_init_default(pmu, &attr); > + > attr.type = pmu->type; > > if (list_empty(&parsed_terms.terms)) { > @@ -1466,7 +1465,8 @@ int parse_events_add_pmu(struct parse_events_state *parse_state, > * When using default config, record which bits of attr->config were > * changed by the user. > */ > - if (pmu->default_config && get_config_chgs(pmu, &parsed_terms, &config_terms)) { > + if (pmu->perf_event_attr_init_default && > + get_config_chgs(pmu, &parsed_terms, &config_terms)) { > parse_events_terms__exit(&parsed_terms); > return -ENOMEM; > } > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 8ef675ea7bdd..a967d25e899b 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -1402,7 +1402,7 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr, > struct parse_events_terms *head_terms, > struct parse_events_error *err) > { > - bool zero = !!pmu->default_config; > + bool zero = !!pmu->perf_event_attr_init_default; > > return perf_pmu__config_terms(pmu, attr, head_terms, zero, err); > } > @@ -2064,7 +2064,6 @@ void perf_pmu__delete(struct perf_pmu *pmu) > > perf_cpu_map__put(pmu->cpus); > > - zfree(&pmu->default_config); > zfree(&pmu->name); > zfree(&pmu->alias_name); > zfree(&pmu->id); > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index 5a05131aa4ce..d2895d415f08 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -92,10 +92,11 @@ struct perf_pmu { > */ > int max_precise; > /** > - * @default_config: Optional default perf_event_attr determined in > - * architecture specific code. > + * @perf_event_attr_init_default: Optional function to default > + * initialize PMU specific parts of the perf_event_attr. > */ > - struct perf_event_attr *default_config; > + void (*perf_event_attr_init_default)(const struct perf_pmu *pmu, > + struct perf_event_attr *attr); > /** > * @cpus: Empty or the contents of either of: > * <sysfs>/bus/event_source/devices/<name>/cpumask. > -- > 2.42.0.655.g421f12c284-goog >

2 years, 4 months

Re: [PATCH v2 3/7] perf arm-spe: Move PMU initialization from default config code

by Leo Yan

On Thu, Oct 12, 2023 at 10:56:41AM -0700, Ian Rogers wrote: > Avoid setting PMU values in arm_spe_pmu_default_config, move to > perf_pmu__arch_init. > > Signed-off-by: Ian Rogers <irogers(a)google.com> > Reviewed-by: Adrian Hunter <adrian.hunter(a)intel.com> > --- > tools/perf/arch/arm/util/pmu.c | 2 ++ > tools/perf/arch/arm64/util/arm-spe.c | 3 --- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/arch/arm/util/pmu.c b/tools/perf/arch/arm/util/pmu.c > index d55d2b15f2e6..f25f68f84a94 100644 > --- a/tools/perf/arch/arm/util/pmu.c > +++ b/tools/perf/arch/arm/util/pmu.c > @@ -23,6 +23,8 @@ void perf_pmu__arch_init(struct perf_pmu *pmu __maybe_unused) > pmu->default_config = cs_etm_get_default_config(pmu); > #if defined(__aarch64__) > } else if (strstarts(pmu->name, ARM_SPE_PMU_NAME)) { > + pmu->selectable = true; > + pmu->is_uncore = false; Arm SPE is not an uncore PMU event. Arm SPE driver registers sysfs node 'cpumask', thus it misleads the pmu_is_uncore() function and it returns 'true' for Arm SPE event. Here we need to explictly set false to correct the 'is_uncore' flag. I did test on my machine, the changes work well for Arm SPE event. Tested-by: Leo Yan <leo.yan(a)linaro.org>

2 years, 4 months

[PATCH v1 RESEND 0/2] perf cs-etm: Improve timestamp tracing

by Leo Yan

This patch series is to improve timestamp handling in per-thread mode. The current code doesn't validate timestamp and always return success for per-thread mode, for a sane implementation, the first patch is to allow validation timestamp tracing in per-thread mode. The second patch is to respect timestamp option "--timestamp" or "-T", when users set this option, the tool will automatically enable hardware timestamp tracing in Arm CoreSight. This patch set is rebased on the latest perf-tools-next branch, and tested on Arm Juno board. And added James Clark's review tags. Leo Yan (2): perf cs-etm: Validate timestamp tracing in per-thread mode perf cs-etm: Respect timestamp option tools/perf/arch/arm/util/cs-etm.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) -- 2.34.1

2 years, 4 months

[PATCH] coresight-tmc-etr: Fix busy error when enable multiple sources

by Mao Jinlong

In enable sink sysfs function, when etr is enabled, it doesn't need to call enable hw function again. [ 91.057233] ------------[ cut here ]------------ [ 91.061983] WARNING: CPU: 6 PID: 145 at drivers/hwtracing/coresight/coresight-tmc-etr.c:1038 tmc_etr_enable_hw+0xc4/0xd0 [ 91.073153] Modules linked in: [ 91.076302] CPU: 6 PID: 145 Comm: sh Tainted: G S W 6.5.0-rc3-g66d5eaa9e5a7 #111 [ 91.085149] Hardware name: Qualcomm Technologies, Inc. SM8450 QRD (DT) [ 91.091847] pstate: 634000c5 (nZCv daIF +PAN -UAO +TCO +DIT -SSBS BTYPE=--) [ 91.099000] pc : tmc_etr_enable_hw+0xc4/0xd0 [ 91.103389] lr : tmc_enable_etr_sink+0x13c/0x294 [ 91.108130] sp : ffff8000815cbb30 [ 91.111537] x29: ffff8000815cbb30 x28: ffff68ce4352a800 x27: 0000000000000000 [ 91.118868] x26: ffff68ce414cc040 x25: ffff68ce414a82e0 x24: 0000000000000000 [ 91.126199] x23: ffff68ce423a1ee0 x22: ffff68ce423a1e80 x21: ffff68ce4352a800 [ 91.133527] x20: 0000000000000001 x19: ffff68ce414aa500 x18: fffffda31cf9c008 [ 91.140857] x17: 0000000000000040 x16: 0000000000000001 x15: fffffc0000000000 [ 91.148188] x14: 00000000f0000080 x13: 0000000000000000 x12: ffff9739c0000000 [ 91.155516] x11: ffffbe9cdf71e400 x10: 0000000000001000 x9 : ffff68d0b29e3b28 [ 91.162845] x8 : fffffda31cf983c8 x7 : fffffda31cf98388 x6 : 00000000ffffffd0 [ 91.170174] x5 : ffff68ce435d8ec0 x4 : ffff68d0b29e2610 x3 : ffff68ce435d8ec0 [ 91.177502] x2 : ffff68ce414aa500 x1 : ffff68ce414aa500 x0 : ffff68ce423a1e80 [ 91.184833] Call trace: [ 91.187349] tmc_etr_enable_hw+0xc4/0xd0 [ 91.191388] tmc_enable_etr_sink+0x13c/0x294 [ 91.195777] coresight_enable_path+0x21c/0x24c [ 91.200347] coresight_enable+0x9c/0x204 [ 91.204379] enable_source_store+0x58/0xa0 [ 91.208595] dev_attr_store+0x18/0x2c [ 91.212361] sysfs_kf_write+0x40/0x54 [ 91.216126] kernfs_fop_write_iter+0x164/0x1dc [ 91.220692] vfs_write+0x3a8/0x460 [ 91.224191] ksys_write+0x6c/0x100 [ 91.227690] __arm64_sys_write+0x1c/0x28 [ 91.231728] invoke_syscall+0x44/0x100 [ 91.235586] el0_svc_common.constprop.1+0x6c/0xe4 [ 91.240418] do_el0_svc+0x38/0x94 [ 91.243833] el0_svc+0x28/0x74 [ 91.246981] el0t_64_sync_handler+0xa0/0xc4 [ 91.251282] el0t_64_sync+0x174/0x178 [ 91.255045] ---[ end trace 0000000000000000 ]--- sh: write error: Device or resource busy Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 66dc5f97a009..e1c7bae9f6ee 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1204,7 +1204,7 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev) static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) { - int ret; + int ret = 0; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); struct etr_buf *sysfs_buf = tmc_etr_get_sysfs_buffer(csdev); @@ -1213,12 +1213,16 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) return PTR_ERR(sysfs_buf); spin_lock_irqsave(&drvdata->spinlock, flags); + if (drvdata->mode == CS_MODE_SYSFS) + goto out; + ret = tmc_etr_enable_hw(drvdata, sysfs_buf); if (!ret) { drvdata->mode = CS_MODE_SYSFS; atomic_inc(&csdev->refcnt); } +out: spin_unlock_irqrestore(&drvdata->spinlock, flags); if (!ret) -- 2.17.1

2 years, 4 months

[PATCH] coresight: etr: Use noncontiguous api instead of noncoherent

by Mao Jinlong

From: Shilpa Suresh <quic_c_sbsure(a)quicinc.com> The iommu support for noncoherent is removed by commit(dma-mapping: remove the {alloc,free}_noncoherent methods 81d88ce55092edf1a1f928efb373f289c6b90efd). Use alloc_noncontiguous function for etr flat buffer allocation. Signed-off-by: Shilpa Suresh <quic_c_sbsure(a)quicinc.com> Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> --- .../hwtracing/coresight/coresight-tmc-etr.c | 47 ++++++++++++++----- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 66dc5f97a009..80e5aa4a16c9 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -24,6 +24,7 @@ struct etr_flat_buf { dma_addr_t daddr; void *vaddr; size_t size; + struct sg_table *sgt; }; /* @@ -608,14 +609,24 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, if (!flat_buf) return -ENOMEM; - flat_buf->vaddr = dma_alloc_noncoherent(real_dev, etr_buf->size, - &flat_buf->daddr, - DMA_FROM_DEVICE, GFP_KERNEL); - if (!flat_buf->vaddr) { + flat_buf->sgt = dma_alloc_noncontiguous(real_dev, etr_buf->size, + DMA_FROM_DEVICE, GFP_KERNEL, 0); + if (!flat_buf->sgt) { kfree(flat_buf); return -ENOMEM; } + flat_buf->daddr = sg_dma_address(flat_buf->sgt->sgl); + flat_buf->vaddr = dma_vmap_noncontiguous(real_dev, etr_buf->size, + flat_buf->sgt); + if (!flat_buf->vaddr) { + dma_free_noncontiguous(real_dev, etr_buf->size, + flat_buf->sgt, + DMA_FROM_DEVICE); + flat_buf->sgt = NULL; + return -ENOMEM; + } + flat_buf->size = etr_buf->size; flat_buf->dev = &drvdata->csdev->dev; etr_buf->hwaddr = flat_buf->daddr; @@ -631,9 +642,12 @@ static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) if (flat_buf && flat_buf->daddr) { struct device *real_dev = flat_buf->dev->parent; - dma_free_noncoherent(real_dev, etr_buf->size, - flat_buf->vaddr, flat_buf->daddr, + dma_vunmap_noncontiguous(real_dev, flat_buf->vaddr); + dma_free_noncontiguous(real_dev, etr_buf->size, + flat_buf->sgt, DMA_FROM_DEVICE); + flat_buf->vaddr = NULL; + flat_buf->sgt = NULL; } kfree(flat_buf); } @@ -642,6 +656,9 @@ static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp) { struct etr_flat_buf *flat_buf = etr_buf->private; struct device *real_dev = flat_buf->dev->parent; + s64 buf_len; + int i; + struct scatterlist *sg; /* * Adjust the buffer to point to the beginning of the trace data @@ -658,13 +675,19 @@ static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp) * the only reason why we would get a wrap around is when the buffer * is full. Sync the entire buffer in one go for this case. */ + if (etr_buf->offset + etr_buf->len > etr_buf->size) - dma_sync_single_for_cpu(real_dev, flat_buf->daddr, - etr_buf->size, DMA_FROM_DEVICE); - else - dma_sync_single_for_cpu(real_dev, - flat_buf->daddr + etr_buf->offset, - etr_buf->len, DMA_FROM_DEVICE); + dma_sync_sgtable_for_cpu(real_dev, flat_buf->sgt, + DMA_FROM_DEVICE); + else { + buf_len = etr_buf->len; + for_each_sg(flat_buf->sgt->sgl, sg, flat_buf->sgt->orig_nents, i) { + dma_sync_sg_for_cpu(real_dev, sg, 1, DMA_FROM_DEVICE); + buf_len -= sg->length; + if (buf_len <= 0) + break; + } + } } static ssize_t tmc_etr_get_data_flat_buf(struct etr_buf *etr_buf, -- 2.17.1

2 years, 4 months

[PATCH 0/3] Fixed some issues and cleanup of ultrasoc-smb

by Junhao He

Fix the two issues listed below and code cleanup a) Fixed the BUG of atomic-sleep b) Fixed uninitialized before use buf_hw_base Junhao He (3): coresight: ultrasoc-smb: fix sleep while close preempt in enable_smb coresight: ultrasoc-smb: simplify the code for check to_copy valid coresight: ultrasoc-smb: fix uninitialized before use buf_hw_base drivers/hwtracing/coresight/ultrasoc-smb.c | 49 +++++++++------------- drivers/hwtracing/coresight/ultrasoc-smb.h | 6 +-- 2 files changed, 23 insertions(+), 32 deletions(-) -- 2.33.0

2 years, 4 months

RE: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

by Linu Cherian

Hi Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > Sent: Tuesday, October 3, 2023 12:02 PM > To: Linu Cherian <lcherian(a)marvell.com>; suzuki.poulose(a)arm.com; > mike.leach(a)linaro.org; james.clark(a)arm.com; leo.yan(a)linaro.org > Cc: linux-arm-kernel(a)lists.infradead.org; coresight(a)lists.linaro.org; linux- > kernel(a)vger.kernel.org; robh+dt(a)kernel.org; > krzysztof.kozlowski+dt(a)linaro.org; conor+dt(a)kernel.org; > devicetree(a)vger.kernel.org; Sunil Kovvuri Goutham > <sgoutham(a)marvell.com>; George Cherian <gcherian(a)marvell.com> > Subject: Re: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add > "memory-region" property > > On 03/10/2023 06:33, Linu Cherian wrote: > > Hi Krzysztof, > > > >> -----Original Message----- > >> From: Krzysztof Kozlowski <krzysztof.kozlowski(a)linaro.org> > >> Sent: Saturday, September 30, 2023 8:59 PM > >> To: Linu Cherian <lcherian(a)marvell.com>; suzuki.poulose(a)arm.com; > >> mike.leach(a)linaro.org; james.clark(a)arm.com; leo.yan(a)linaro.org > >> Cc: linux-arm-kernel(a)lists.infradead.org; coresight(a)lists.linaro.org; > >> linux- kernel(a)vger.kernel.org; robh+dt(a)kernel.org; > >> krzysztof.kozlowski+dt(a)linaro.org; conor+dt(a)kernel.org; > >> devicetree(a)vger.kernel.org; Sunil Kovvuri Goutham > >> <sgoutham(a)marvell.com>; George Cherian <gcherian(a)marvell.com> > >> Subject: [EXT] Re: [PATCH 1/7] dt-bindings: arm: coresight-tmc: Add > >> "memory-region" property > >> > >> External Email > >> > >> --------------------------------------------------------------------- > >> - On 29/09/2023 15:37, Linu Cherian wrote: > >>> memory-region 0: Reserved trace buffer memory > >>> > >>> TMC ETR: When available, use this reserved memory region for > >>> trace data capture. Same region is used for trace data > >>> retention after a panic or watchdog reset. > >>> > >>> TMC ETF: When available, use this reserved memory region for > >>> trace data retention synced from internal SRAM after a panic or > >>> watchdog reset. > >>> > >>> memory-region 1: Reserved meta data memory > >>> > >>> TMC ETR, ETF: When available, use this memory for register > >>> snapshot retention synced from hardware registers after a panic > >>> or watchdog reset. > >>> > >>> Signed-off-by: Linu Cherian <lcherian(a)marvell.com> > >>> --- > >> > >> Where is the changelog? This is supposed to be v4 or something later. > >> Please, keep proper versioning and changelog. > > > > Sure, will add the changelog from next version onwards. > > > > Yeah, the last version was RFC v3 and the RFC tag has been dropped from > this version onwards. > > Hence started this version with V1. > > v1 says it is the first version, but you already had three others. > Please keep continuous version log, regardless whether you call it RFC or RFT > or RFsomething. Okay. Will continue with Patch V5 from next series and add necessary notes in cover letter. > > > > >> > >>> .../bindings/arm/arm,coresight-tmc.yaml | 19 > +++++++++++++++++++ > >>> 1 file changed, 19 insertions(+) > >>> > > Best regards, > Krzysztof

2 years, 4 months

[PATCH] MAINTAINERS: coresight: Add filename catchall for Coresight

by James Clark

There are a few files missing from the list like test_arm_coresight.sh and arm-coresight.txt. These could be picked up just with a name match. Signed-off-by: James Clark <james.clark(a)arm.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 90f13281d297..3ee45066b7e4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2066,6 +2066,7 @@ F: tools/perf/arch/arm/util/pmu.c F: tools/perf/tests/shell/coresight/* F: tools/perf/util/cs-etm-decoder/* F: tools/perf/util/cs-etm.* +N: coresight ARM/CORTINA SYSTEMS GEMINI ARM ARCHITECTURE M: Hans Ulli Kroll <ulli.kroll(a)googlemail.com> -- 2.34.1

2 years, 4 months

Re: [PATCH v2] perf cs-etm: Fix incorrect or missing decoder for raw trace

by James Clark

On 04/10/2023 01:32, Besar Wicaksono wrote: > The decoder creation for raw trace uses metadata from the first CPU. > On per-cpu mode, this metadata is incorrectly used for every decoder. > On per-process/per-thread traces, the first CPU is CPU0. If CPU0 trace > is not enabled, its metadata will be marked unused and the decoder is > not created. Perf report dump skips the decoding part because the > decoder is missing. > > To fix this, use metadata of the CPU associated with sample object. > > Signed-off-by: Besar Wicaksono <bwicaksono(a)nvidia.com> > --- > > Changes from v1: > * Update commit message > * Add fallback to CPU-0 metadata if sample CPU id is not available > * Preserve cs_etm__set_trace_param_* arguments and just breakdown the index > parameter into trace-param and metadata indexes > Thanks to Mike and James for the feedback. > v1: https://lore.kernel.org/lkml/20230919224553.1658-1-bwicaksono@nvidia.com/T/… > > --- > tools/perf/util/cs-etm.c | 106 ++++++++++++++++++++++++--------------- > 1 file changed, 65 insertions(+), 41 deletions(-) > There is a trivial conflict on the perf-tools-next repo that it's probably worth rebasing for, otherwise: Reviewed-by: James Clark <james.clark(a)arm.com> Also should probably include linux-perf-users(a)vger.kernel.org as tools changes actually go through the perf tree rather than the coresight one. > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index 1419b40dfbe8..3abe68a9981e 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -293,22 +293,31 @@ static int cs_etm__metadata_set_trace_id(u8 trace_chan_id, u64 *cpu_metadata) > }) > > /* > - * Get a metadata for a specific cpu from an array. > + * Get a metadata index for a specific cpu from an array. > * > */ > -static u64 *get_cpu_data(struct cs_etm_auxtrace *etm, int cpu) > +static int get_cpu_data_idx(struct cs_etm_auxtrace *etm, int cpu) > { > int i; > - u64 *metadata = NULL; > > for (i = 0; i < etm->num_cpu; i++) { > if (etm->metadata[i][CS_ETM_CPU] == (u64)cpu) { > - metadata = etm->metadata[i]; > - break; > + return i; > } > } > > - return metadata; > + return -1; > +} > + > +/* > + * Get a metadata for a specific cpu from an array. > + * > + */ > +static u64 *get_cpu_data(struct cs_etm_auxtrace *etm, int cpu) > +{ > + int idx = get_cpu_data_idx(etm, cpu); > + > + return (idx != -1) ? etm->metadata[idx] : NULL; > } > > /* > @@ -651,66 +660,80 @@ static void cs_etm__packet_dump(const char *pkt_string) > } > > static void cs_etm__set_trace_param_etmv3(struct cs_etm_trace_params *t_params, > - struct cs_etm_auxtrace *etm, int idx, > - u32 etmidr) > + struct cs_etm_auxtrace *etm, int t_idx, > + int m_idx, u32 etmidr) > { > u64 **metadata = etm->metadata; > > - t_params[idx].protocol = cs_etm__get_v7_protocol_version(etmidr); > - t_params[idx].etmv3.reg_ctrl = metadata[idx][CS_ETM_ETMCR]; > - t_params[idx].etmv3.reg_trc_id = metadata[idx][CS_ETM_ETMTRACEIDR]; > + t_params[t_idx].protocol = cs_etm__get_v7_protocol_version(etmidr); > + t_params[t_idx].etmv3.reg_ctrl = metadata[m_idx][CS_ETM_ETMCR]; > + t_params[t_idx].etmv3.reg_trc_id = metadata[m_idx][CS_ETM_ETMTRACEIDR]; > } > > static void cs_etm__set_trace_param_etmv4(struct cs_etm_trace_params *t_params, > - struct cs_etm_auxtrace *etm, int idx) > + struct cs_etm_auxtrace *etm, int t_idx, > + int m_idx) > { > u64 **metadata = etm->metadata; > > - t_params[idx].protocol = CS_ETM_PROTO_ETMV4i; > - t_params[idx].etmv4.reg_idr0 = metadata[idx][CS_ETMV4_TRCIDR0]; > - t_params[idx].etmv4.reg_idr1 = metadata[idx][CS_ETMV4_TRCIDR1]; > - t_params[idx].etmv4.reg_idr2 = metadata[idx][CS_ETMV4_TRCIDR2]; > - t_params[idx].etmv4.reg_idr8 = metadata[idx][CS_ETMV4_TRCIDR8]; > - t_params[idx].etmv4.reg_configr = metadata[idx][CS_ETMV4_TRCCONFIGR]; > - t_params[idx].etmv4.reg_traceidr = metadata[idx][CS_ETMV4_TRCTRACEIDR]; > + t_params[t_idx].protocol = CS_ETM_PROTO_ETMV4i; > + t_params[t_idx].etmv4.reg_idr0 = metadata[m_idx][CS_ETMV4_TRCIDR0]; > + t_params[t_idx].etmv4.reg_idr1 = metadata[m_idx][CS_ETMV4_TRCIDR1]; > + t_params[t_idx].etmv4.reg_idr2 = metadata[m_idx][CS_ETMV4_TRCIDR2]; > + t_params[t_idx].etmv4.reg_idr8 = metadata[m_idx][CS_ETMV4_TRCIDR8]; > + t_params[t_idx].etmv4.reg_configr = metadata[m_idx][CS_ETMV4_TRCCONFIGR]; > + t_params[t_idx].etmv4.reg_traceidr = metadata[m_idx][CS_ETMV4_TRCTRACEIDR]; > } > > static void cs_etm__set_trace_param_ete(struct cs_etm_trace_params *t_params, > - struct cs_etm_auxtrace *etm, int idx) > + struct cs_etm_auxtrace *etm, int t_idx, > + int m_idx) > { > u64 **metadata = etm->metadata; > > - t_params[idx].protocol = CS_ETM_PROTO_ETE; > - t_params[idx].ete.reg_idr0 = metadata[idx][CS_ETE_TRCIDR0]; > - t_params[idx].ete.reg_idr1 = metadata[idx][CS_ETE_TRCIDR1]; > - t_params[idx].ete.reg_idr2 = metadata[idx][CS_ETE_TRCIDR2]; > - t_params[idx].ete.reg_idr8 = metadata[idx][CS_ETE_TRCIDR8]; > - t_params[idx].ete.reg_configr = metadata[idx][CS_ETE_TRCCONFIGR]; > - t_params[idx].ete.reg_traceidr = metadata[idx][CS_ETE_TRCTRACEIDR]; > - t_params[idx].ete.reg_devarch = metadata[idx][CS_ETE_TRCDEVARCH]; > + t_params[t_idx].protocol = CS_ETM_PROTO_ETE; > + t_params[t_idx].ete.reg_idr0 = metadata[m_idx][CS_ETE_TRCIDR0]; > + t_params[t_idx].ete.reg_idr1 = metadata[m_idx][CS_ETE_TRCIDR1]; > + t_params[t_idx].ete.reg_idr2 = metadata[m_idx][CS_ETE_TRCIDR2]; > + t_params[t_idx].ete.reg_idr8 = metadata[m_idx][CS_ETE_TRCIDR8]; > + t_params[t_idx].ete.reg_configr = metadata[m_idx][CS_ETE_TRCCONFIGR]; > + t_params[t_idx].ete.reg_traceidr = metadata[m_idx][CS_ETE_TRCTRACEIDR]; > + t_params[t_idx].ete.reg_devarch = metadata[m_idx][CS_ETE_TRCDEVARCH]; > } > > static int cs_etm__init_trace_params(struct cs_etm_trace_params *t_params, > struct cs_etm_auxtrace *etm, > + bool formatted, > + int sample_cpu, > int decoders) > { > - int i; > + int t_idx, m_idx; > u32 etmidr; > u64 architecture; > > - for (i = 0; i < decoders; i++) { > - architecture = etm->metadata[i][CS_ETM_MAGIC]; > + for (t_idx = 0; t_idx < decoders; t_idx++) { > + if (formatted) > + m_idx = t_idx; > + else { > + m_idx = get_cpu_data_idx(etm, sample_cpu); > + if (m_idx == -1) { > + pr_warning("CS_ETM: unknown CPU, falling back to first metadata\n"); > + m_idx = 0; > + } > + } > + > + architecture = etm->metadata[m_idx][CS_ETM_MAGIC]; > > switch (architecture) { > case __perf_cs_etmv3_magic: > - etmidr = etm->metadata[i][CS_ETM_ETMIDR]; > - cs_etm__set_trace_param_etmv3(t_params, etm, i, etmidr); > + etmidr = etm->metadata[m_idx][CS_ETM_ETMIDR]; > + cs_etm__set_trace_param_etmv3(t_params, etm, t_idx, m_idx, etmidr); > break; > case __perf_cs_etmv4_magic: > - cs_etm__set_trace_param_etmv4(t_params, etm, i); > + cs_etm__set_trace_param_etmv4(t_params, etm, t_idx, m_idx); > break; > case __perf_cs_ete_magic: > - cs_etm__set_trace_param_ete(t_params, etm, i); > + cs_etm__set_trace_param_ete(t_params, etm, t_idx, m_idx); > break; > default: > return -EINVAL; > @@ -1026,7 +1049,7 @@ static u32 cs_etm__mem_access(struct cs_etm_queue *etmq, u8 trace_chan_id, > } > > static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > - bool formatted) > + bool formatted, int sample_cpu) > { > struct cs_etm_decoder_params d_params; > struct cs_etm_trace_params *t_params = NULL; > @@ -1051,7 +1074,7 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > if (!t_params) > goto out_free; > > - if (cs_etm__init_trace_params(t_params, etm, decoders)) > + if (cs_etm__init_trace_params(t_params, etm, formatted, sample_cpu, decoders)) > goto out_free; > > /* Set decoder parameters to decode trace packets */ > @@ -1091,14 +1114,15 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm, > static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > struct auxtrace_queue *queue, > unsigned int queue_nr, > - bool formatted) > + bool formatted, > + int sample_cpu) > { > struct cs_etm_queue *etmq = queue->priv; > > if (list_empty(&queue->head) || etmq) > return 0; > > - etmq = cs_etm__alloc_queue(etm, formatted); > + etmq = cs_etm__alloc_queue(etm, formatted, sample_cpu); > > if (!etmq) > return -ENOMEM; > @@ -2826,7 +2850,7 @@ static int cs_etm__process_auxtrace_event(struct perf_session *session, > * formatted in piped mode (true). > */ > err = cs_etm__setup_queue(etm, &etm->queues.queue_array[idx], > - idx, true); > + idx, true, -1); > if (err) > return err; > > @@ -3032,7 +3056,7 @@ static int cs_etm__queue_aux_fragment(struct perf_session *session, off_t file_o > idx = auxtrace_event->idx; > formatted = !(aux_event->flags & PERF_AUX_FLAG_CORESIGHT_FORMAT_RAW); > return cs_etm__setup_queue(etm, &etm->queues.queue_array[idx], > - idx, formatted); > + idx, formatted, sample->cpu); > } > > /* Wasn't inside this buffer, but there were no parse errors. 1 == 'not found' */

2 years, 4 months

Re: [PATCH 1/3] perf tests test_arm_coresight: Fix the shellcheck warning in latest test_arm_coresight.sh

by James Clark

On 29/09/2023 05:11, Athira Rajeev wrote: > Running shellcheck on tests/shell/test_arm_coresight.sh > throws below warnings: > > In tests/shell/test_arm_coresight.sh line 15: > cs_etm_path=$(find /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit) > ^--^ SC2061: Quote the parameter to -name so the shell won't interpret it. > > In tests/shell/test_arm_coresight.sh line 20: > if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then > ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined > > This warning is observed after commit: > "commit bb350847965d ("perf test: Update cs_etm testcase for Arm ETE")" > > Fixed this issue by using quoting 'cpu*' for SC2061 and > using "&&" in line number 20 for SC2166 warning > > Fixes: bb350847965d ("perf test: Update cs_etm testcase for Arm ETE") > Signed-off-by: Athira Rajeev <atrajeev(a)linux.vnet.ibm.com> > --- > tools/perf/tests/shell/test_arm_coresight.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh > index fe78c4626e45..f2115dfa24a5 100755 > --- a/tools/perf/tests/shell/test_arm_coresight.sh > +++ b/tools/perf/tests/shell/test_arm_coresight.sh > @@ -12,12 +12,12 @@ > glb_err=0 > > cs_etm_dev_name() { > - cs_etm_path=$(find /sys/bus/event_source/devices/cs_etm/ -name cpu* -print -quit) > + cs_etm_path=$(find /sys/bus/event_source/devices/cs_etm/ -name 'cpu*' -print -quit) > trcdevarch=$(cat ${cs_etm_path}/mgmt/trcdevarch) > archhver=$((($trcdevarch >> 12) & 0xf)) > archpart=$(($trcdevarch & 0xfff)) > > - if [ $archhver -eq 5 -a "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then > + if [ $archhver -eq 5 ] && [ "$(printf "0x%X\n" $archpart)" = "0xA13" ] ; then > echo "ete" > else > echo "etm" Reviewed-by: James Clark <james.clark(a)arm.com>

2 years, 4 months

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight