CoreSight

coresight@lists.linaro.org

4 participants
2845 discussions

Re: [bug report] coresight: etm4x: Change etm4_platform_driver driver for MMIO devices

by Anshuman Khandual

Hello Dan, On 7/29/23 12:50, Dan Carpenter wrote: > Hello Anshuman Khandual, > > The patch 73d779a03a76: "coresight: etm4x: Change > etm4_platform_driver driver for MMIO devices" from Jul 10, 2023 > (linux-next), leads to the following Smatch static checker warning: > > drivers/hwtracing/coresight/coresight-etm4x-core.c:2272 etm4_remove_platform_dev() > error: we previously assumed 'drvdata' could be null (see line 2268) > > drivers/hwtracing/coresight/coresight-etm4x-core.c > 2264 static int __exit etm4_remove_platform_dev(struct platform_device *pdev) > 2265 { > 2266 struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > 2267 > 2268 if (drvdata) > > This check can probably be removed, right? etm4_remove_dev(drvdata) cannot be called if drvdata is NULL. But there might be an implicit assumption that 'drvdata' could never be NULL in this path. > > 2269 etm4_remove_dev(drvdata); > 2270 pm_runtime_disable(&pdev->dev); > 2271 > --> 2272 if (drvdata->pclk) > > Not checked here We could either have the following change which ensures that 'drvdata' is checked again before the clock is handled. diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..9d186af81ea0 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2267,11 +2267,13 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) if (drvdata) etm4_remove_dev(drvdata); - pm_runtime_disable(&pdev->dev); - if (drvdata->pclk) - clk_put(drvdata->pclk); + pm_runtime_disable(&pdev->dev); + if (drvdata) { + if (drvdata->pclk && !IS_ERR(drvdata->pclk)) + clk_put(drvdata->pclk); + } return 0; } OR remove the first drvdata check as suggested. diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 703b6fcbb6a5..6e1ffddbd4be 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -2265,11 +2265,9 @@ static int __exit etm4_remove_platform_dev(struct platform_device *pdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); - if (drvdata) - etm4_remove_dev(drvdata); + etm4_remove_dev(drvdata); pm_runtime_disable(&pdev->dev); - - if (drvdata->pclk) + if (drvdata->pclk && !IS_ERR(drvdata->pclk) clk_put(drvdata->pclk); return 0; > > 2273 clk_put(drvdata->pclk); > 2274 > 2275 return 0; > 2276 } Also adding Suzuki and James for their input. - Anshuman

3 years, 1 month

[PATCH V3 0/4] coresight: trbe: Enable ACPI based devices

by Anshuman Khandual

This series enables detection of ACPI based TRBE devices via a stand alone purpose built representative platform device. But as a pre-requisite this changes coresight_platform_data structure assignment for the TRBE device. This series is based on v6.5-rc4 kernel, is also dependent on the following EDK2 changes posted earlier by Sami. https://edk2.groups.io/g/devel/message/107239 https://edk2.groups.io/g/devel/message/107241 Changes in V3: - Changed ARMV8_TRBE_PDEV_NAME from "arm-trbe-acpi" into "arm,trbe" - Dropped local variable 'matched' - Replaced 'matched' with 'valid gsi' as being already matched once - Moved find_acpi_cpu_topology_hetero_id() outside conditional check Changes in V2: https://lore.kernel.org/all/20230801094052.750416-1-anshuman.khandual@arm.c… - Refactored arm_spe_acpi_register_device() in a separate patch - Renamed trbe_acpi_resources as trbe_resources - Renamed trbe_acpi_dev as trbe_dev Changes in V1: https://lore.kernel.org/all/20230728112733.359620-1-anshuman.khandual@arm.c… Cc: Sami Mujawar <sami.mujawar(a)arm.com> Cc: Catalin Marinas <catalin.marinas(a)arm.com> Cc: Will Deacon <will(a)kernel.org> Cc: Mark Rutland <mark.rutland(a)arm.com> Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> Cc: Mike Leach <mike.leach(a)linaro.org> Cc: Leo Yan <leo.yan(a)linaro.org> Cc: Alexander Shishkin <alexander.shishkin(a)linux.intel.com> Cc: James Clark <james.clark(a)arm.com> Cc: coresight(a)lists.linaro.org Cc: linux-arm-kernel(a)lists.infradead.org Cc: linux-kernel(a)vger.kernel.org Anshuman Khandual (4): arm_pmu: acpi: Refactor arm_spe_acpi_register_device() arm_pmu: acpi: Add a representative platform device for TRBE coresight: trbe: Add a representative coresight_platform_data for TRBE coresight: trbe: Enable ACPI based TRBE devices arch/arm64/include/asm/acpi.h | 3 + drivers/hwtracing/coresight/coresight-trbe.c | 15 +- drivers/hwtracing/coresight/coresight-trbe.h | 1 + drivers/perf/arm_pmu_acpi.c | 142 +++++++++++++------ include/linux/perf/arm_pmu.h | 1 + 5 files changed, 119 insertions(+), 43 deletions(-) -- 2.25.1

3 years, 1 month

Re: [PATCH V3 1/4] arm_pmu: acpi: Refactor arm_spe_acpi_register_device()

by Anshuman Khandual

On 8/4/23 22:09, Will Deacon wrote: > On Thu, Aug 03, 2023 at 11:43:27AM +0530, Anshuman Khandual wrote: >> >> >> On 8/3/23 11:26, Anshuman Khandual wrote: >>> + /* >>> + * Sanity check all the GICC tables for the same interrupt >>> + * number. For now, only support homogeneous ACPI machines. >>> + */ >>> + for_each_possible_cpu(cpu) { >>> + struct acpi_madt_generic_interrupt *gicc; >>> + >>> + gicc = acpi_cpu_get_madt_gicc(cpu); >>> + if (gicc->header.length < len) >>> + return gsi ? -ENXIO : 0; >>> + >>> + this_gsi = parse_gsi(gicc); >>> + if (!this_gsi) >>> + return gsi ? -ENXIO : 0; >> >> Hello Will, >> >> Moved parse_gsi() return code checking to its original place just to >> make it similar in semantics to existing 'gicc->header.length check'. >> If 'gsi' is valid i.e atleast a single cpu has been probed, return >> -ENXIO indicating mismatch, otherwise just return 0. > > Wouldn't that still be the case without the check in this hunk? We'd run > into the homogeneous check and return -ENXIO from there, no? Although the return code will be the same i.e -ENXIO, but not for the same reason. this_gsi = parse_gsi(gicc); if (!this_gsi) return gsi ? -ENXIO : 0; This returns 0 when IRQ could not be parsed for the first cpu, but returns -ENXIO for subsequent cpus. Although return code -ENXIO here still indicates IRQ parsing to have failed. } else if (hetid != this_hetid || gsi != this_gsi) { pr_warn("ACPI: %s: must be homogeneous\n", pdev->name); return -ENXIO; } This returns -ENXIO when there is a IRQ mismatch. But if the above check is not there, -ENXIO return code here could not be classified into IRQ parse problem or mismatch without looking into the IRQ value.

3 years, 1 month

Re: [RFC PATCH 0/3] coresight: Support exclude_guest with Feat_TRF and nVHE

by Suzuki K Poulose

Hi Marc On 04/08/2023 20:09, Marc Zyngier wrote: > On Fri, 04 Aug 2023 11:13:10 +0100, > James Clark <james.clark(a)arm.com> wrote: >> >> Hi, >> >> I'm looking for help in testing this and for feedback on whether it's >> useful to anyone. Testing it requires hardware that has Feat_TRF (v8.4) >> but no TRBE. This is because TRBE usage is disabled in nVHE guests. >> >> I don't currently have any access to any hardware, and the FVP model >> can only do self hosted trace using TRBE. >> >> Currently with nVHE you would always get trace from guests, and >> filtering out isn't possible without this patchset. In comparison, with >> VHE guests, they never generate guest trace without [1]. I think the >> existence of trace rather than lack of could suggest that this change is >> less useful than [1]. Also the restricted set of hardware that it works >> on supports that too. > > It'd be nice to have some sort of feature parity, but it seems like a > vanishingly small target of users having access to an ETM sink. > >> >> Apart from compilation and checking that the exclude guest settings >> are correctly programmed on guest switch, this is untested by me. > > I'll have a look at the series, but none of my HW fits in this > description (my ARMv8.4+ boxes don't have any form of tracing). While I have your attention, we have another series that manages the trace filtering for Guests on VHE, completely within the Coresight etm4x driver here [0]. I personally think, it is good to have the guest filtering for both nVHE and VHE under the KVM control, like we do in this series. I would like your opinion on this. [0] https://lkml.kernel.org/r/20230804085219.260790-1-james.clark@arm.com Suzuki > > Thanks, > > M. >

3 years, 1 month

[RFC PATCH 0/3] coresight: Support exclude_guest with Feat_TRF and nVHE

by James Clark

Hi, I'm looking for help in testing this and for feedback on whether it's useful to anyone. Testing it requires hardware that has Feat_TRF (v8.4) but no TRBE. This is because TRBE usage is disabled in nVHE guests. I don't currently have any access to any hardware, and the FVP model can only do self hosted trace using TRBE. Currently with nVHE you would always get trace from guests, and filtering out isn't possible without this patchset. In comparison, with VHE guests, they never generate guest trace without [1]. I think the existence of trace rather than lack of could suggest that this change is less useful than [1]. Also the restricted set of hardware that it works on supports that too. Apart from compilation and checking that the exclude guest settings are correctly programmed on guest switch, this is untested by me. Applies to kvmarm/next (3b4e3afb2032) [1]: https://lore.kernel.org/linux-arm-kernel/20230804085219.260790-3-james.clar… James Clark (3): arm64: KVM: Add support for exclude_guest and exclude_host for ETM arm64: KVM: Support exclude_guest for Coresight trace in nVHE coresight: Support exclude_guest with Feat_TRF and nVHE arch/arm64/include/asm/kvm_host.h | 10 +++- arch/arm64/kvm/Makefile | 1 + arch/arm64/kvm/arm.c | 1 + arch/arm64/kvm/debug.c | 7 +++ arch/arm64/kvm/etm.c | 48 ++++++++++++++++ arch/arm64/kvm/hyp/nvhe/debug-sr.c | 56 +++++++++++++++++-- .../hwtracing/coresight/coresight-etm-perf.c | 4 ++ include/kvm/etm.h | 43 ++++++++++++++ 8 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/kvm/etm.c create mode 100644 include/kvm/etm.h -- 2.34.1

3 years, 1 month

Re: [PATCH 1/2] arm64/sysreg: Move TRFCR definitions to sysreg

by James Clark

On 04/08/2023 13:10, Mark Brown wrote: > On Fri, Aug 04, 2023 at 09:52:16AM +0100, James Clark wrote: > >> TRFCR_EL2_CX needs to become TRFCR_ELx_CX to avoid unnecessary >> duplication and make the SysregFields block re-usable. > > That field is only present in the EL2 version. I would tend to leave > the registers split for that reason, there's some minor potential for > confusion if people refer to the sysreg file rather than the docs, or > potentially confuse some future automation. However that's not a super > strongly held opinion. > True, the potential for confusion is a good reason to not try to avoid duplication. Probably helps if it is ever auto generated or validated as well. I could update it on the next version. But do I leave all the existing _ELx usages in the code, or change them all to _EL1 (Except CX_EL2)? To leave them as _ELx sysreg would look like this, even though _EL1 would probably be more accurate: SysregFields TRFCR_EL2 Res0 63:7 UnsignedEnum 6:5 TS 0b0001 VIRTUAL 0b0010 GUEST_PHYSICAL 0b0011 PHYSICAL EndEnum Res0 4 Field 3 CX Res0 2 Field 1 E2TRE Field 0 E0TRE EndSysregFields SysregFields TRFCR_ELx Res0 63:7 UnsignedEnum 6:5 TS 0b0001 VIRTUAL 0b0010 GUEST_PHYSICAL 0b0011 PHYSICAL EndEnum Res0 4:2 Field 1 ExTRE Field 0 E0TRE EndSysregFields Sysreg TRFCR_EL1 3 0 1 2 1 Fields TRFCR_ELx EndSysreg Sysreg TRFCR_EL2 3 4 1 2 1 Fields TRFCR_EL2 EndSysreg Sysreg TRFCR_EL12 3 5 1 2 1 Fields TRFCR_ELx EndSysreg > Otherwise this checks out against DDI0601 2023-06: > > Reviewed-by: Mark Brown <broonie(a)kernel.org> Thanks for the review

3 years, 1 month

Re: [PATCH -next] coresight: dummy: simplify the code with module_platform_driver

by Suzuki K Poulose

On Fri, 4 Aug 2023 17:27:09 +0800, Yang Yingliang wrote: > The init/exit() of driver only calls platform_driver_register/unregister, > it can be simpilfied with module_platform_driver. > > Applied, thanks! [1/1] coresight: dummy: simplify the code with module_platform_driver https://git.kernel.org/coresight/c/28a03fae6e524d39fec14fea1ee67b86f4870658 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

3 years, 1 month

[RESEND PATCH] coresight: tmc: Explicit type conversions to prevent integer overflow

by Ruidong Tian

Perf cs_etm session will failed when AUX buffer > 1G. perf record -C 0 -m ,2G -e cs_etm// -- taskset -c 0 ls failed to mmap with 12 (Cannot allocate memory) In coresight tmc driver, "nr_pages << PAGE_SHIFT" will overflow when nr_pages >= 0x80000(correspond to 1G AUX buffer). Explicit convert nr_pages to 64 bit to avoid overflow. Signed-off-by: Ruidong Tian <tianruidong(a)linux.alibaba.com> --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 2 +- drivers/hwtracing/coresight/coresight-tmc.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 766325de0e29..1425ecd1cf78 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1267,7 +1267,7 @@ alloc_etr_buf(struct tmc_drvdata *drvdata, struct perf_event *event, * than the size requested via sysfs. */ if ((nr_pages << PAGE_SHIFT) > drvdata->size) { - etr_buf = tmc_alloc_etr_buf(drvdata, (nr_pages << PAGE_SHIFT), + etr_buf = tmc_alloc_etr_buf(drvdata, ((ssize_t)nr_pages << PAGE_SHIFT), 0, node, NULL); if (!IS_ERR(etr_buf)) goto done; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index b97da39652d2..0ee48c5ba764 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -325,7 +325,7 @@ ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, static inline unsigned long tmc_sg_table_buf_size(struct tmc_sg_table *sg_table) { - return sg_table->data_pages.nr_pages << PAGE_SHIFT; + return (unsigned long)sg_table->data_pages.nr_pages << PAGE_SHIFT; } struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata); -- 2.33.1

3 years, 1 month

[PATCH] coresight: etm: Make cycle count threshold user configurable

by Anshuman Khandual

Cycle counting is enabled, when requested and supported but with a default threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR, representing the minimum interval between cycle count trace packets. This makes cycle threshold user configurable, from the user space via perf event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT, in case no explicit request. As expected it creates a sysfs file as well. /sys/bus/event_source/devices/cs_etm/format/cc_threshold New 'cc_threshold' uses 'event->attr.config3' as no more space is available in 'event->attr.config1' or 'event->attr.config2'. Cc: Suzuki K Poulose <suzuki.poulose(a)arm.com> Cc: Mike Leach <mike.leach(a)linaro.org> Cc: James Clark <james.clark(a)arm.com> Cc: Leo Yan <leo.yan(a)linaro.org> Cc: coresight(a)lists.linaro.org Cc: linux-arm-kernel(a)lists.infradead.org Cc: linux-doc(a)vger.kernel.org Cc: linux-kernel(a)vger.kernel.org Signed-off-by: Anshuman Khandual <anshuman.khandual(a)arm.com> --- Documentation/trace/coresight/coresight.rst | 2 ++ drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++ drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index 4a71ea6cb390..b88d83b59531 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -624,6 +624,8 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/ * - timestamp - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP <coresight-timestamp>` + * - cc_treshold + - Cycle count treshhold value How to use the STM module ------------------------- diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c index 5ca6278baff4..09f75dffae60 100644 --- a/drivers/hwtracing/coresight/coresight-etm-perf.c +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset, "config:0-3"); PMU_FORMAT_ATTR(sinkid, "config2:0-31"); /* config ID - set if a system configuration is selected */ PMU_FORMAT_ATTR(configid, "config2:32-63"); +PMU_FORMAT_ATTR(cc_threshold, "config3:0-11"); /* @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = { &format_attr_preset.attr, &format_attr_configid.attr, &format_attr_branch_broadcast.attr, + &format_attr_cc_threshold.attr, NULL, }; diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 9d186af81ea0..9a2766f68416 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev, struct etmv4_config *config = &drvdata->config; struct perf_event_attr *attr = &event->attr; unsigned long cfg_hash; - int preset; + int preset, cc_threshold; /* Clear configuration from previous run */ memset(config, 0, sizeof(struct etmv4_config)); @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev, if (attr->config & BIT(ETM_OPT_CYCACC)) { config->cfg |= TRCCONFIGR_CCI; /* TRM: Must program this for cycacc to work */ - config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT; + cc_treshold = attr->config3 & ETM_CYC_THRESHOLD_MASK; + if (cc_treshold) { + if (cc_treshold < drvdata->ccitmin) + config->ccctlr = drvdata->ccitmin; + else + config->ccctlr = cc_threshold; + } else { + config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT; + } } if (attr->config & BIT(ETM_OPT_TS)) { /* -- 2.25.1

3 years, 1 month

Re: [PATCH] coresight: etm4x: Don't bother the user with nonessential log message

by James Clark

On 03/08/2023 07:22, Tian Tao wrote: > Each cpu will print the following log when initializing > ETM "coresight etm1: CPU1: etm v4.5 initialized", if there are a lot > of cpus, e.g. 128. there will be a screen full of this log. replace > dev_info with dev_dbg prints only when needed. > > Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 7e307022303a..7b51e8594fd5 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -2033,7 +2033,7 @@ static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) > > etmdrvdata[drvdata->cpu] = drvdata; > > - dev_info(&drvdata->csdev->dev, "CPU%d: %s v%d.%d initialized\n", > + dev_dbg(&drvdata->csdev->dev, "CPU%d: %s v%d.%d initialized\n", > drvdata->cpu, type_name, major, minor); > > if (boot_enable) { Hi Tian, Do you think it's worth keeping a single info printout as well as the per-cpu dbg one? I'm thinking that the only way to get these messages now is to boot with debug printing on, and that is quite noisy. Because it's too late to use debugfs to enable just coresight debug messages after booting. So something like this: dev_info_once(&drvdata->csdev->dev, "%s v%d.%d initialized\n", Also I still get this printed out per-cpu, which is from coresight-syscfg.c. Do you not see this? Might be worth doing the same change there. CSCFG registered etm0 CSCFG registered etm1 CSCFG registered etm2 CSCFG registered etm3 Thanks James

3 years, 1 month

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight