CoreSight

coresight@lists.linaro.org

1 participants
2095 discussions

Re: [PATCH] perf: cs-etm: Update decoder code for OpenCSD version 1.4

by James Clark

On 24/01/2023 21:50, Ian Rogers wrote: > On Fri, Jan 20, 2023 at 7:47 AM James Clark <james.clark(a)arm.com> wrote: >> >> >> >> On 20/01/2023 15:37, Mike Leach wrote: >>> OpenCSD version 1.4 is released with support for FEAT_ITE. >>> This adds a new packet type, with associated output element ID in >>> the packet type enum - OCSD_GEN_TRC_ELEM_INSTRUMENTATION. >>> >>> As we just ignore this packet in perf, add to the switch statement >>> to avoid the "enum not handled in switch error", but conditionally >>> so as not to break the perf build for older OpenCSD installations. >>> >>> Signed-off-by: Mike Leach <mike.leach(a)linaro.org> >>> --- >>> tools/perf/util/cs-etm-decoder/cs-etm-decoder.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> index fa3aa9c0fb2e..48e7121880a9 100644 >>> --- a/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> +++ b/tools/perf/util/cs-etm-decoder/cs-etm-decoder.c >>> @@ -604,6 +604,9 @@ static ocsd_datapath_resp_t cs_etm_decoder__gen_trace_elem_printer( >>> case OCSD_GEN_TRC_ELEM_CUSTOM: >>> case OCSD_GEN_TRC_ELEM_SYNC_MARKER: >>> case OCSD_GEN_TRC_ELEM_MEMTRANS: >>> +#if (OCSD_VER_NUM >= 0x010400) >>> + case OCSD_GEN_TRC_ELEM_INSTRUMENTATION: >>> +#endif >>> default: >>> break; >>> } >> >> Checked the build with both old and new versions of OpenCSD and it's ok: >> >> Reviewed-by: James Clark <james.clark(a)arm.com> > > Acked-by: Ian Rogers <irogers(a)google.com> > > Thanks, > Ian Hi Arnaldo, Is it ok to merge this change? If anyone is building with the latest OpenCSD they will get a build error on the unhandled switch case, and we just got it in our CI too. I suppose we could disable the warning around this switch, but it's pretty rare to add new packets so might be best to leave it. Thanks James

1 year, 9 months

Re: [PATCH -next] coresight: tpda: fix return value check in tpda_probe()

by Suzuki K Poulose

On 29/01/2023 08:42, Yang Yingliang wrote: > devm_ioremap_resource() never returns NULL pointer, it > will return ERR_PTR() when it fails, so replace the check > with IS_ERR(). Thanks for the patch. > > Fixes: 5b7916625c01 ("Coresight: Add TPDA link driver") > Signed-off-by: Yang Yingliang <yangyingliang(a)huawei.com> > --- > drivers/hwtracing/coresight/coresight-tpda.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index 19c25c9f6157..6313b12880e0 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -145,7 +145,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) > dev_set_drvdata(dev, drvdata); > > base = devm_ioremap_resource(dev, &adev->res); > - if (!base) > + if (IS_ERR(base)) > return -ENOMEM; I have fixed this up to : return PTR_ERR(base); for consistency. Thanks Suzuki

1 year, 9 months

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

by Suzuki K Poulose

On 27/01/2023 17:00, Arnd Bergmann wrote: > On Fri, Jan 27, 2023, at 17:46, Suzuki K Poulose wrote: >> On 26/01/2023 20:37, Arnd Bergmann wrote: >>> On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote: >>>> On 26/01/2023 16:35, Arnd Bergmann wrote: >>>>> From: Arnd Bergmann <arnd(a)arndb.de> >>>> Thanks for the fix, I will queue this. Btw, I did try to >>>> reproduce it locally, but couldn't trigger the warnings, >>>> even with >>>> >>>> CONFIG_WERROR=y >>>> >>>> and all CORESIGHT configs builtin. I see other drivers doing the >>>> same outside coresight too. Just curious to know why is this >>>> any different. Is it specific to "bus" driver (e.g. AMBA) ? >>> >>> The warning comes from postprocessing the object file, it's got >>> nothing to do with the bus type, only with a symbol in .data >>> referencing a symbol in .init.text. Maybe there are some >>> config options that keep the section from getting discarded? >>> Or possibly you only built the files in this directory, but did >>> not get to the final link? >> >> I did a full kernel build. Also, I see a similar issue with the >> coresight-etm4x (by code inspection) driver. Did you not hit that ? >> >> May be there is a config option that is masking it on my end. But >> the case of etm4x driver is puzzling. >> >> $ git grep etm4_remove_amba >> drivers/hwtracing/coresight/coresight-etm4x-core.c >> drivers/hwtracing/coresight/coresight-etm4x-core.c:static void __exit >> etm4_remove_amba(struct amba_device *adev) >> drivers/hwtracing/coresight/coresight-etm4x-core.c: .remove >> = etm4_remove_amba, > > Indeed, that one clearly has the same but, but I have never > observed a warning for it. > > I checked one more thing and found that I only get the warning > for 32-bit Arm builds, but not arm64. Since the etm4x driver > 'depends on ARM64' for its use of asm/sysreg.h, > I never test-built it on 32-bit arm. > > From the git history of arch/arm64/kernel/vmlinux.lds.S, > I can see that arm64 never discards the .exit section, see > commit 07c802bd7c39 ("arm64: vmlinux.lds.S: don't discard > .exit.* sections at link-time"). That makes sense, thanks for getting to the bottom of this. I have pushed it to coresight next. https://git.kernel.org/coresight/c/0c1ccc158bbc Kind regards Suzuki > > Arnd

1 year, 9 months

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

by Suzuki K Poulose

On 26/01/2023 20:37, Arnd Bergmann wrote: > On Thu, Jan 26, 2023, at 19:02, Suzuki K Poulose wrote: >> On 26/01/2023 16:35, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd(a)arndb.de> >> Thanks for the fix, I will queue this. Btw, I did try to >> reproduce it locally, but couldn't trigger the warnings, >> even with >> >> CONFIG_WERROR=y >> >> and all CORESIGHT configs builtin. I see other drivers doing the >> same outside coresight too. Just curious to know why is this >> any different. Is it specific to "bus" driver (e.g. AMBA) ? > > The warning comes from postprocessing the object file, it's got > nothing to do with the bus type, only with a symbol in .data > referencing a symbol in .init.text. Maybe there are some > config options that keep the section from getting discarded? > Or possibly you only built the files in this directory, but did > not get to the final link? I did a full kernel build. Also, I see a similar issue with the coresight-etm4x (by code inspection) driver. Did you not hit that ? May be there is a config option that is masking it on my end. But the case of etm4x driver is puzzling. $ git grep etm4_remove_amba drivers/hwtracing/coresight/coresight-etm4x-core.c drivers/hwtracing/coresight/coresight-etm4x-core.c:static void __exit etm4_remove_amba(struct amba_device *adev) drivers/hwtracing/coresight/coresight-etm4x-core.c: .remove = etm4_remove_amba, Suzuki > > Arnd

1 year, 9 months

Re: [PATCH] Coresight: tpda/tpdm: remove incorrect __exit annotation

by Suzuki K Poulose

Hi Arnd, On 26/01/2023 16:35, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd(a)arndb.de> > > 'remove' callbacks get called whenever a device is unbound from > the driver, which can get triggered from user space. > > Putting it into the __exit section means that the function gets > dropped in for built-in drivers, as pointed out by this build > warning: > > `tpda_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpda.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpda.o > `tpdm_remove' referenced in section `.data' of drivers/hwtracing/coresight/coresight-tpdm.o: defined in discarded section `.exit.text' of drivers/hwtracing/coresight/coresight-tpdm.o > Thanks for the fix, I will queue this. Btw, I did try to reproduce it locally, but couldn't trigger the warnings, even with CONFIG_WERROR=y and all CORESIGHT configs builtin. I see other drivers doing the same outside coresight too. Just curious to know why is this any different. Is it specific to "bus" driver (e.g. AMBA) ? Suzuki > Fixes: 5b7916625c01 ("Coresight: Add TPDA link driver") > Fixes: b3c71626a933 ("Coresight: Add coresight TPDM source driver") > Signed-off-by: Arnd Bergmann <arnd(a)arndb.de> > --- > drivers/hwtracing/coresight/coresight-tpda.c | 2 +- > drivers/hwtracing/coresight/coresight-tpdm.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index 19c25c9f6157..382d648529e7 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -174,7 +174,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) > return 0; > } > > -static void __exit tpda_remove(struct amba_device *adev) > +static void tpda_remove(struct amba_device *adev) > { > struct tpda_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 349a82bb3270..9479a5e8c672 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -223,7 +223,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id) > return 0; > } > > -static void __exit tpdm_remove(struct amba_device *adev) > +static void tpdm_remove(struct amba_device *adev) > { > struct tpdm_drvdata *drvdata = dev_get_drvdata(&adev->dev); >

1 year, 9 months

Re: [PATCH] coresight: tmc-etr: Don't enable ETR when it's not ready

by Suzuki K Poulose

Folks, On 24/01/2023 20:09, Yabin Cui wrote: > Ping for review. And I still can't reproduce it, even if I reduced the > timeout to 2us and tried different workloads. Any suggestions for how > to reproduce it? > I think we should go ahead and fix this in the driver to handle flaky hardware cases. But I would like this to be addressed for all the TMC types, not just ETRs, as Mike pointed out. Thanks Suzuki > Thanks, > Yabin > > On Tue, Jan 10, 2023 at 1:06 PM Yabin Cui <yabinc(a)google.com> wrote: >> >>> Do you have a reproducer for this or some more info? >>> For example is it a regression or has it always been there? And on which >>> platform. >> >> It happens on Pixel 6 and 7. We collect ETM data periodically from some >> internal dogfood devices. The problem has happened several times on >> dogfood devices. But I am still trying to reproduce it locally. >> >> We use the scatter-gather mode of ETR, and allocate a 4M buffer. In userspace, >> we use simpleperf in Android to collect system wide ETM data. What is special >> is, simpleperf disables and reenables perf events every 100ms to flush ETM >> data to perf aux buffer. >> >> Pixel 6 and 7 have hardware monitoring AXI traffic. The hardware finds ETR is >> trying to read from or write to a low invalid address (like 0x2E0000). The >> problem always happens right after the "tmc_etr: timeout while waiting for TMC >> to be Ready" message. And in almost all cases, I can find a "timeout while >> waiting for completion of Manual Flush" message from the previous session. >> >> One log history is below: >> [11484.610008][ C0] coresight tmc_etr0: timeout while waiting for >> completion of Manual Flush >> [11484.610177][ C0] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> [11484.615367][ C0] coresight tmc_etr0: timeout while waiting for >> completion of Manual Flush >> [11484.615534][ C0] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> [12089.486044][ C0] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> AXI error report reading from invalid address >> >> Another log history is below: >> [76709.382650][ C5] coresight tmc_etf1: timeout while waiting for >> TMC to be Ready >> [76709.382852][ C7] coresight tmc_etr0: timeout while waiting for >> completion of Manual Flush >> [76709.382995][ C7] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> [76709.384510][ C7] coresight tmc_etr0: timeout while waiting for >> completion of Manual Flush >> [76709.384649][ C7] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> [76709.384838][ C0] coresight tmc_etr0: timeout while waiting for >> TMC to be Ready >> AIX error report writing to invalid address >> >> It seems if the previous manual flush doesn't finish gracefully, ETR may not be >> ready for the next enable (even after 10min as in the first log). And if we >> continue to enable ETR, an invalid AXI IO may happen. >> >> Thanks, >> Yabin >> >> On Tue, Jan 10, 2023 at 10:04 AM Suzuki K Poulose >> <suzuki.poulose(a)arm.com> wrote: >>> >>> On 10/01/2023 17:48, Mike Leach wrote: >>>> Hi, >>>> >>>> On Tue, 10 Jan 2023 at 09:30, James Clark <james.clark(a)arm.com> wrote: >>>>> >>>>> >>>>> >>>>> On 09/01/2023 23:43, Yabin Cui wrote: >>>>>> Otherwise, it may cause error in AXI bus and result in a kernel panic. >>>>> >>>>> Hi Yabin, >>>>> >>>>> Thanks for the fix. Do you have a reproducer for this or some more info? >>>>> For example is it a regression or has it always been there? And on which >>>>> platform. >>>>> >>>>> Thanks >>>>> James >>>>> >>>>>> >>>>>> Signed-off-by: Yabin Cui <yabinc(a)google.com> >>>>>> --- >>>>>> .../hwtracing/coresight/coresight-tmc-core.c | 4 +++- >>>>>> .../hwtracing/coresight/coresight-tmc-etr.c | 18 +++++++++++++++--- >>>>>> drivers/hwtracing/coresight/coresight-tmc.h | 2 +- >>>>>> 3 files changed, 19 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c >>>>>> index 07abf28ad725..c106d142e632 100644 >>>>>> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c >>>>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c >>>>>> @@ -31,7 +31,7 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb"); >>>>>> DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf"); >>>>>> DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr"); >>>>>> >>>>>> -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) >>>>>> +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) >>>>>> { >>>>>> struct coresight_device *csdev = drvdata->csdev; >>>>>> struct csdev_access *csa = &csdev->access; >>>>>> @@ -40,7 +40,9 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata) >>>>>> if (coresight_timeout(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) { >>>>>> dev_err(&csdev->dev, >>>>>> "timeout while waiting for TMC to be Ready\n"); >>>>>> + return -EBUSY; >>>>>> } >>>>>> + return 0; >>>>>> } >>>>>> >>>>>> void tmc_flush_and_stop(struct tmc_drvdata *drvdata) >>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>>> index 867ad8bb9b0c..2da99dd41ed6 100644 >>>>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>>> @@ -983,15 +983,21 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) >>>>>> etr_buf->ops->sync(etr_buf, rrp, rwp); >>>>>> } >>>>>> >>>>>> -static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) >>>>>> +static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) >>>>>> { >>>>>> u32 axictl, sts; >>>>>> struct etr_buf *etr_buf = drvdata->etr_buf; >>>>>> + int rc = 0; >>>>>> >>>>>> CS_UNLOCK(drvdata->base); >>>>>> >>>>>> /* Wait for TMCSReady bit to be set */ >>>>>> - tmc_wait_for_tmcready(drvdata); >>>>>> + rc = tmc_wait_for_tmcready(drvdata); >>>>>> + if (rc) { >>>>>> + dev_err(&drvdata->csdev->dev, "not ready ETR isn't enabled\n"); >>>>>> + CS_LOCK(drvdata->base); >>>>>> + return rc; >>>>>> + } >>>>>> >>>>>> writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ); >>>>>> writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE); >>>>>> @@ -1032,6 +1038,7 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata) >>>>>> tmc_enable_hw(drvdata); >>>>>> >>>>>> CS_LOCK(drvdata->base); >>>>>> + return rc; >>>>>> } >>>>>> >>>>>> static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, >>>>>> @@ -1060,7 +1067,12 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata, >>>>>> rc = coresight_claim_device(drvdata->csdev); >>>>>> if (!rc) { >>>>>> drvdata->etr_buf = etr_buf; >>>>>> - __tmc_etr_enable_hw(drvdata); >>>>>> + rc = __tmc_etr_enable_hw(drvdata); >>>>>> + if (rc) { >>>>>> + drvdata->etr_buf = NULL; >>>>>> + coresight_disclaim_device(drvdata->csdev); >>>>>> + tmc_etr_disable_catu(drvdata); >>>>>> + } >>>>>> } >>>>>> >>>>>> return rc; >>>>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h >>>>>> index 66959557cf39..01c0382a29c0 100644 >>>>>> --- a/drivers/hwtracing/coresight/coresight-tmc.h >>>>>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h >>>>>> @@ -255,7 +255,7 @@ struct tmc_sg_table { >>>>>> }; >>>>>> >>>>>> /* Generic functions */ >>>>>> -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); >>>>>> +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); >>>>>> void tmc_flush_and_stop(struct tmc_drvdata *drvdata); >>>>>> void tmc_enable_hw(struct tmc_drvdata *drvdata); >>>>>> void tmc_disable_hw(struct tmc_drvdata *drvdata); >>>> >>>> There is no point in waiting for a timeout, and then carrying on even >>>> when it is exceeded. As such this patch seems reasonable. >>>> We should also apply the same principle to the ETF and ETB devices >>>> which use the same tmc_wait_for_tmcready() function. >>> >>> +1 >>> >>> I am fine with pushing this change, as it is doing the right thing. >>> >>>> >>>> However - the concern is that this appears to be happening on starting >>>> the ETR - there should be no outstanding AXI operations that cause the >>>> system to not be ready - as we will either be using this the first >>>> time after reset, or we should have successfully stopped and flushed >>>> the ETR from the previous operation. This warrants further >>>> investigation - should we be extending the timeout - which is already >>>> at a rather generous 100uS, and do we also need to check the MemErr >>>> bit in the status register? >>> >>> It would be good to dump the value of TMC_STATUS to see what is going >>> on. >>> >>>> >>>> As James says, we need details of when and how the problem occurs - >>>> as far as I know it has not been seen on any systems we currently use >>>> (though could have been missed given the current code) >>> >>> +1 >>> >>> Kind regards >>> Suzuki >>> >>> >>>> >>>> Regards >>>> >>>> Mike >>>> >>>> >>>> -- >>>> Mike Leach >>>> Principal Engineer, ARM Ltd. >>>> Manchester Design Centre. UK >>>

1 year, 9 months

Re: [PATCH] coresight: tmc-etr: Don't enable ETR when it's not ready

by James Clark

On 24/01/2023 20:09, Yabin Cui wrote: > Ping for review. And I still can't reproduce it, even if I reduced the > timeout to 2us and tried different workloads. Any suggestions for how > to reproduce it? > Hi Yabin, I just found this in my kernel log files from a few days ago. Jan 19 15:03:35 n1-sdp kernel: [865337.521413] coresight tmc_etr0: timeout while waiting for completion of Manual Flush Jan 19 15:03:35 n1-sdp kernel: [865337.529351] coresight tmc_etr0: timeout while waiting for TMC to be Ready This is on N1SDP, and I've only been running "normal" perf commands like: perf record -e cs_etm//k -- taskset -c 2 sleep 1 perf record -e cs_etm// -- stress -c 2 -t 1 I also haven't been able to reproduce it since James

1 year, 9 months

[PATCH v2] perf: cs-etm: Improve missing sink warning message

by James Clark

Make the sink error message more similar to the event error message that reminds about missing kernel support. The available sinks are also determined by the hardware so mention that too. Also, usually it's not necessary to specify the sink, so add that as a hint. Now the error for a made up sink looks like this: $ perf record -e cs_etm/@abc/ Couldn't find sink "abc" on event cs_etm/@abc/. Missing kernel or device support? Hint: An appropriate sink will be picked automatically if one isn't is specified. For any error other than ENOENT, the same message as before is displayed. Signed-off-by: James Clark <james.clark(a)arm.com> --- V1 -> V2: Keep old error message unless it's ENOENT tools/perf/arch/arm/util/cs-etm.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 481e170cd3f1..7f71c8a237ff 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -283,9 +283,15 @@ static int cs_etm_set_sink_attr(struct perf_pmu *pmu, ret = perf_pmu__scan_file(pmu, path, "%x", &hash); if (ret != 1) { - pr_err("failed to set sink \"%s\" on event %s with %d (%s)\n", - sink, evsel__name(evsel), errno, - str_error_r(errno, msg, sizeof(msg))); + if (errno == ENOENT) + pr_err("Couldn't find sink \"%s\" on event %s\n" + "Missing kernel or device support?\n\n" + "Hint: An appropriate sink will be picked automatically if one isn't specified.\n", + sink, evsel__name(evsel)); + else + pr_err("Failed to set sink \"%s\" on event %s with %d (%s)\n", + sink, evsel__name(evsel), errno, + str_error_r(errno, msg, sizeof(msg))); return ret; } base-commit: 5670ebf54bd26482f57a094c53bdc562c106e0a9 -- 2.39.1

1 year, 9 months

[PATCH] coresight: perf: Output trace id only once

by Suzuki K Poulose

With the dynamic traceid allocation scheme in, we output the AUX_OUTPUT_HWID packet every time event->start() is called. This could cause too many such records in the perf.data, while only one per CPU throughout the life time of the event is required. Make sure we only output it once. Before this patch: $ perf report -D | grep OUTPUT_HW_ID ... AUX_OUTPUT_HW_ID events: 55 (18.3%) After this patch: $ perf report -D | grep OUTPUT_HW_ID ... AUX_OUTPUT_HW_ID events: 5 ( 1.9%) Cc: Mike Leach <mike.leach(a)linaro.org> Cc: James Clark <james.clark(a)arm.com> Cc: Leo Yan <leo.yan(a)linaro.org> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> --- drivers/hwtracing/coresight/coresight-etm-perf.c | 16 ++++++++++++---- drivers/hwtracing/coresight/coresight-etm-perf.h | 2 ++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 12fff661456e..a48c97da8165 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -495,10 +495,18 @@ static void etm_event_start(struct perf_event *event, int flags) if (source_ops(csdev)->enable(csdev, event, CS_MODE_PERF)) goto fail_disable_path; - /* output cpu / trace ID in perf record */ - hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, CS_AUX_HW_ID_CURR_VERSION); - hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, coresight_trace_id_read_cpu_id(cpu)); - perf_report_aux_output_id(event, hw_id); + /* + * output cpu / trace ID in perf record, once for the lifetime + * of the event. + */ + if (!cpumask_test_cpu(cpu, &event_data->aux_hwid_done)) { + cpumask_set_cpu(cpu, &event_data->aux_hwid_done); + hw_id = FIELD_PREP(CS_AUX_HW_ID_VERSION_MASK, + CS_AUX_HW_ID_CURR_VERSION); + hw_id |= FIELD_PREP(CS_AUX_HW_ID_TRACE_ID_MASK, + coresight_trace_id_read_cpu_id(cpu)); + perf_report_aux_output_id(event, hw_id); + } out: /* Tell the perf core the event is alive */ diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.h b/drivers/hwtracing/coresight/coresight-etm-perf.h index 468f7799ab4f..bebbadee2ceb 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.h +++ b/drivers/hwtracing/coresight/coresight-etm-perf.h @@ -48,6 +48,7 @@ struct etm_filters { * struct etm_event_data - Coresight specifics associated to an event * @work: Handle to free allocated memory outside IRQ context. * @mask: Hold the CPU(s) this event was set for. + * @aux_hwid_done: Whether a CPU has emitted the TraceID packet or not. * @snk_config: The sink configuration. * @cfg_hash: The hash id of any coresight config selected. * @path: An array of path, each slot for one CPU. @@ -55,6 +56,7 @@ struct etm_filters { struct etm_event_data { struct work_struct work; cpumask_t mask; + cpumask_t aux_hwid_done; void *snk_config; u32 cfg_hash; struct list_head * __percpu *path; -- 2.34.1

1 year, 9 months

[PATCH] coresight: Fix uninitialised variable use in coresight_disable

by Suzuki K Poulose

Kernel test robot reports: drivers/hwtracing/coresight/coresight-core.c:1176:7: warning: variable 'hash' is used uninitialized whenever switch case is taken [-Wsometimes-uninitialized] case CORESIGHT_DEV_SUBTYPE_SOURCE_PROC: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/hwtracing/coresight/coresight-core.c:1195:24: note: uninitialized use occurs here idr_remove(&path_idr, hash); ^~~~ Fix this by moving the usage of the hash variable to where it actually should have been. Cc: Mao Jinlong <quic_jinlmao(a)quicinc.com> Reported-by: kernel test robot <lkp(a)intel.com> Signed-off-by: Suzuki K Poulose <suzuki.poulose(a)arm.com> --- drivers/hwtracing/coresight/coresight-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index a798008ac56e..d3bf82c0de1d 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1189,13 +1189,13 @@ void coresight_disable(struct coresight_device *csdev) pr_err("Path is not found for %s\n", dev_name(&csdev->dev)); goto out; } + idr_remove(&path_idr, hash); break; default: /* We can't be here */ break; } - idr_remove(&path_idr, hash); coresight_disable_path(path); coresight_release_path(path); -- 2.34.1

1 year, 9 months

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight