CoreSight August 2022

coresight@lists.linaro.org

11 participants
28 discussions

[PATCH v3] coresight: etm4x: add CPU hotplug support for probing

by Tamas Zsoldos

etm4x devices cannot be successfully probed when their CPU is offline. For example, when booting with maxcpus=n, ETM probing will fail on CPUs >n, and the probing won't be reattempted once the CPUs come online. This will leave those CPUs unable to make use of ETM. This change adds a mechanism to delay the probing if the corresponding CPU is offline, and to try it again when the CPU comes online. Signed-off-by: Tamas Zsoldos <tamas.zsoldos(a)arm.com> --- Changes since v2: * Fix tear down issue (thanks to Suzuki). --- .../coresight/coresight-etm4x-core.c | 153 +++++++++++++----- 1 file changed, 113 insertions(+), 40 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 87299e99dabb..71ab1d6e72dc 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -66,10 +66,13 @@ static enum cpuhp_state hp_online; struct etm4_init_arg { unsigned int pid; - struct etmv4_drvdata *drvdata; + struct device *dev; struct csdev_access *csa; }; +static DEFINE_PER_CPU(struct etm4_init_arg *, delayed_probe); +static int etm4_probe_cpu(unsigned int cpu); + /* * Check if TRCSSPCICRn(i) is implemented for a given instance. * @@ -1071,7 +1074,7 @@ static void etm4_init_arch_data(void *info) struct csdev_access *csa; int i; - drvdata = init_arg->drvdata; + drvdata = dev_get_drvdata(init_arg->dev); csa = init_arg->csa; /* @@ -1514,7 +1517,7 @@ void etm4_config_trace_mode(struct etmv4_config *config) static int etm4_online_cpu(unsigned int cpu) { if (!etmdrvdata[cpu]) - return 0; + return etm4_probe_cpu(cpu); if (etmdrvdata[cpu]->boot_enable && !etmdrvdata[cpu]->sticky_enable) coresight_enable(etmdrvdata[cpu]->csdev); @@ -1890,48 +1893,20 @@ static void etm4_pm_clear(void) } } -static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +static int etm4_add_coresight_dev(struct etm4_init_arg *init_arg) { int ret; struct coresight_platform_data *pdata = NULL; - struct etmv4_drvdata *drvdata; + struct device *dev = init_arg->dev; + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); struct coresight_desc desc = { 0 }; - struct etm4_init_arg init_arg = { 0 }; u8 major, minor; char *type_name; - drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); if (!drvdata) - return -ENOMEM; - - dev_set_drvdata(dev, drvdata); - - if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) - pm_save_enable = coresight_loses_context_with_cpu(dev) ? - PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER; - - if (pm_save_enable != PARAM_PM_SAVE_NEVER) { - drvdata->save_state = devm_kmalloc(dev, - sizeof(struct etmv4_save_state), GFP_KERNEL); - if (!drvdata->save_state) - return -ENOMEM; - } - - drvdata->base = base; - - spin_lock_init(&drvdata->spinlock); - - drvdata->cpu = coresight_get_cpu(dev); - if (drvdata->cpu < 0) - return drvdata->cpu; - - init_arg.drvdata = drvdata; - init_arg.csa = &desc.access; - init_arg.pid = etm_pid; + return -EINVAL; - if (smp_call_function_single(drvdata->cpu, - etm4_init_arch_data, &init_arg, 1)) - dev_err(dev, "ETM arch init failed\n"); + desc.access = *init_arg->csa; if (!drvdata->arch) return -EINVAL; @@ -2002,6 +1977,68 @@ static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) return 0; } +static int etm4_probe(struct device *dev, void __iomem *base, u32 etm_pid) +{ + struct etmv4_drvdata *drvdata; + struct csdev_access access = { 0 }; + struct etm4_init_arg init_arg = { 0 }; + struct etm4_init_arg *delayed; + + drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + dev_set_drvdata(dev, drvdata); + + if (pm_save_enable == PARAM_PM_SAVE_FIRMWARE) + pm_save_enable = coresight_loses_context_with_cpu(dev) ? + PARAM_PM_SAVE_SELF_HOSTED : PARAM_PM_SAVE_NEVER; + + if (pm_save_enable != PARAM_PM_SAVE_NEVER) { + drvdata->save_state = devm_kmalloc(dev, + sizeof(struct etmv4_save_state), GFP_KERNEL); + if (!drvdata->save_state) + return -ENOMEM; + } + + drvdata->base = base; + + spin_lock_init(&drvdata->spinlock); + + drvdata->cpu = coresight_get_cpu(dev); + if (drvdata->cpu < 0) + return drvdata->cpu; + + init_arg.dev = dev; + init_arg.csa = &access; + init_arg.pid = etm_pid; + + /* + * Serialize against CPUHP callbacks to avoid race condition + * between the smp call and saving the delayed probe. + */ + cpus_read_lock(); + if (smp_call_function_single(drvdata->cpu, + etm4_init_arch_data, &init_arg, 1)) { + /* The CPU was offline, try again once it comes online. */ + delayed = devm_kmalloc(dev, sizeof(*delayed), GFP_KERNEL); + if (!delayed) { + cpus_read_unlock(); + return -ENOMEM; + } + + *delayed = init_arg; + + per_cpu(delayed_probe, drvdata->cpu) = delayed; + + cpus_read_unlock(); + return 0; + } + cpus_read_unlock(); + + return etm4_add_coresight_dev(&init_arg); +} + static int etm4_probe_amba(struct amba_device *adev, const struct amba_id *id) { void __iomem *base; @@ -2040,6 +2077,35 @@ static int etm4_probe_platform_dev(struct platform_device *pdev) return ret; } +static int etm4_probe_cpu(unsigned int cpu) +{ + int ret; + struct etm4_init_arg init_arg; + struct csdev_access access = { 0 }; + struct etm4_init_arg *iap = *this_cpu_ptr(&delayed_probe); + + if (!iap) + return 0; + + init_arg = *iap; + devm_kfree(init_arg.dev, iap); + *this_cpu_ptr(&delayed_probe) = NULL; + + ret = pm_runtime_resume_and_get(init_arg.dev); + if (ret < 0) { + dev_err(init_arg.dev, "Failed to get PM runtime!\n"); + return 0; + } + + init_arg.csa = &access; + etm4_init_arch_data(&init_arg); + + etm4_add_coresight_dev(&init_arg); + + pm_runtime_put(init_arg.dev); + return 0; +} + static struct amba_cs_uci_id uci_id_etm4[] = { { /* ETMv4 UCI data */ @@ -2054,16 +2120,20 @@ static void clear_etmdrvdata(void *info) int cpu = *(int *)info; etmdrvdata[cpu] = NULL; + per_cpu(delayed_probe, cpu) = NULL; } static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) { - etm_perf_symlink(drvdata->csdev, false); + bool fully_registered; /* * Taking hotplug lock here to avoid racing between etm4_remove_dev() * and CPU hotplug call backs. */ cpus_read_lock(); + + fully_registered = (etmdrvdata[drvdata->cpu] == drvdata); + /* * The readers for etmdrvdata[] are CPU hotplug call backs * and PM notification call backs. Change etmdrvdata[i] on @@ -2071,12 +2141,15 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) * inside one call back function. */ if (smp_call_function_single(drvdata->cpu, clear_etmdrvdata, &drvdata->cpu, 1)) - etmdrvdata[drvdata->cpu] = NULL; + clear_etmdrvdata(&drvdata->cpu); cpus_read_unlock(); - cscfg_unregister_csdev(drvdata->csdev); - coresight_unregister(drvdata->csdev); + if (fully_registered) { + etm_perf_symlink(drvdata->csdev, false); + cscfg_unregister_csdev(drvdata->csdev); + coresight_unregister(drvdata->csdev); + } return 0; } -- 2.17.1

2 years

[PATCH v3 00/13] coresight: Add new API to allocate trace source ID values

by Mike Leach

The current method for allocating trace source ID values to sources is to use a fixed algorithm for CPU based sources of (cpu_num * 2 + 0x10). The STM is allocated ID 0x1. This fixed algorithm is used in both the CoreSight driver code, and by perf when writing the trace metadata in the AUXTRACE_INFO record. The method needs replacing as currently:- 1. It is inefficient in using available IDs. 2. Does not scale to larger systems with many cores and the algorithm has no limits so will generate invalid trace IDs for cpu number > 44. Additionally requirements to allocate additional system IDs on some systems have been seen. This patch set introduces an API that allows the allocation of trace IDs in a dynamic manner. Architecturally reserved IDs are never allocated, and the system is limited to allocating only valid IDs. Each of the current trace sources ETM3.x, ETM4.x and STM is updated to use the new API. For the ETMx.x devices IDs are allocated on certain events a) When using sysfs, an ID will be allocated on hardware enable, or a read of sysfs TRCTRACEID register and freed when the sysfs reset is written. b) When using perf, ID is allocated on during setup AUX event, and freed on event free. IDs are communicated using the AUX_OUTPUT_HW_ID packet. The ID allocator is notified when perf sessions start and stop so CPU based IDs are kept constant throughout any perf session. Note: This patchset breaks some backward compatibility for perf record and perf report. The version of the AUXTRACE_INFO has been updated to reflect the fact that the trace source IDs are generated differently. This will mean older versions of perf report cannot decode the newer file. Applies to coresight/next [4d45bc82df66] Tested on DB410c Changes since v2: 1) Improved backward compatibility: (requested by James) Using the new version of perf on an old kernel will generate a usable file legacy metadata values are set by the new perf and will be used if mew ID packets are not present in the file. Using an older version of perf / simpleperf on an updated kernel may still work. The trace ID allocator has been updated to use the legacy ID values where possible, so generated file and used trace IDs will match up to the point where the legacy algorithm is broken anyway. 2) Various changes to the ID allocator and ID packet format. (suggested by Suzuki) 3) per CPU ID info in allocator now stored as atomic type to allow a passive read without taking the allocator spinlock. perf flow now allocates and releases ID values in setup_aux / free_event. Device enable and event enable use the passive read to set the allocated values. This simplifies the locking mechanisms on the perf run and fixes issues that arose with locking dependencies. Changes since v1: (after feedback & discussion with Mathieu & Suzuki). 1) API has changed. The global trace ID map is managed internally, so it is no longer passed in to the API functions. 2) perf record does not use sysfs to find the trace IDs. These are now output as AUX_OUTPUT_HW_ID events. The drivers, perf record, and perf report have been updated accordingly to generate and handle these events. Mike Leach (13): coresight: trace-id: Add API to dynamically assign Trace ID values coresight: Remove obsolete Trace ID unniqueness checks coresight: stm: Update STM driver to use Trace ID API coresight: etm4x: Update ETM4 driver to use Trace ID API coresight: etm3x: Update ETM3 driver to use Trace ID API coresight: etmX.X: stm: Remove trace_id() callback coresight: perf: traceid: Add perf notifiers for Trace ID perf: cs-etm: Move mapping of Trace ID and cpu into helper function perf: cs-etm: Update record event to use new Trace ID protocol kernel: events: Export perf_report_aux_output_id() perf: cs-etm: Handle PERF_RECORD_AUX_OUTPUT_HW_ID packet coresight: events: PERF_RECORD_AUX_OUTPUT_HW_ID used for Trace ID coresight: trace-id: Add debug & test macros to Trace ID allocation drivers/hwtracing/coresight/Makefile | 2 +- drivers/hwtracing/coresight/coresight-core.c | 49 +-- .../hwtracing/coresight/coresight-etm-perf.c | 23 ++ drivers/hwtracing/coresight/coresight-etm.h | 3 +- .../coresight/coresight-etm3x-core.c | 92 +++-- .../coresight/coresight-etm3x-sysfs.c | 27 +- .../coresight/coresight-etm4x-core.c | 79 ++++- .../coresight/coresight-etm4x-sysfs.c | 27 +- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + drivers/hwtracing/coresight/coresight-stm.c | 49 +-- .../hwtracing/coresight/coresight-trace-id.c | 266 ++++++++++++++ .../hwtracing/coresight/coresight-trace-id.h | 78 +++++ include/linux/coresight-pmu.h | 35 +- include/linux/coresight.h | 3 - kernel/events/core.c | 1 + tools/include/linux/coresight-pmu.h | 48 ++- tools/perf/arch/arm/util/cs-etm.c | 21 +- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 + tools/perf/util/cs-etm.c | 331 +++++++++++++++--- tools/perf/util/cs-etm.h | 14 +- 20 files changed, 933 insertions(+), 225 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-trace-id.c create mode 100644 drivers/hwtracing/coresight/coresight-trace-id.h -- 2.17.1

2 years, 1 month

[PATCH v4 00/13] coresight: Add new API to allocate trace source ID values

by Mike Leach

The current method for allocating trace source ID values to sources is to use a fixed algorithm for CPU based sources of (cpu_num * 2 + 0x10). The STM is allocated ID 0x1. This fixed algorithm is used in both the CoreSight driver code, and by perf when writing the trace metadata in the AUXTRACE_INFO record. The method needs replacing as currently:- 1. It is inefficient in using available IDs. 2. Does not scale to larger systems with many cores and the algorithm has no limits so will generate invalid trace IDs for cpu number > 44. Additionally requirements to allocate additional system IDs on some systems have been seen. This patch set introduces an API that allows the allocation of trace IDs in a dynamic manner. Architecturally reserved IDs are never allocated, and the system is limited to allocating only valid IDs. Each of the current trace sources ETM3.x, ETM4.x and STM is updated to use the new API. For the ETMx.x devices IDs are allocated on certain events a) When using sysfs, an ID will be allocated on hardware enable, or a read of sysfs TRCTRACEID register and freed when the sysfs reset is written. b) When using perf, ID is allocated on during setup AUX event, and freed on event free. IDs are communicated using the AUX_OUTPUT_HW_ID packet. The ID allocator is notified when perf sessions start and stop so CPU based IDs are kept constant throughout any perf session. Note: This patchset breaks some backward compatibility for perf record and perf report. The version of the AUXTRACE_INFO has been updated to reflect the fact that the trace source IDs are generated differently. This will mean older versions of perf report cannot decode the newer file. Applies to coresight/next [4d45bc82df66] Tested on DB410c Changes since v3: 1) Fixed aarch32 build error in ETM3.x driver. Reported-by: kernel test robot <lkp(a)intel.com> Changes since v2: 1) Improved backward compatibility: (requested by James) Using the new version of perf on an old kernel will generate a usable file legacy metadata values are set by the new perf and will be used if mew ID packets are not present in the file. Using an older version of perf / simpleperf on an updated kernel may still work. The trace ID allocator has been updated to use the legacy ID values where possible, so generated file and used trace IDs will match up to the point where the legacy algorithm is broken anyway. 2) Various changes to the ID allocator and ID packet format. (suggested by Suzuki) 3) per CPU ID info in allocator now stored as atomic type to allow a passive read without taking the allocator spinlock. perf flow now allocates and releases ID values in setup_aux / free_event. Device enable and event enable use the passive read to set the allocated values. This simplifies the locking mechanisms on the perf run and fixes issues that arose with locking dependencies. Changes since v1: (after feedback & discussion with Mathieu & Suzuki). 1) API has changed. The global trace ID map is managed internally, so it is no longer passed in to the API functions. 2) perf record does not use sysfs to find the trace IDs. These are now output as AUX_OUTPUT_HW_ID events. The drivers, perf record, and perf report have been updated accordingly to generate and handle these events. Mike Leach (13): coresight: trace-id: Add API to dynamically assign Trace ID values coresight: Remove obsolete Trace ID unniqueness checks coresight: stm: Update STM driver to use Trace ID API coresight: etm4x: Update ETM4 driver to use Trace ID API coresight: etm3x: Update ETM3 driver to use Trace ID API coresight: etmX.X: stm: Remove trace_id() callback coresight: perf: traceid: Add perf notifiers for Trace ID perf: cs-etm: Move mapping of Trace ID and cpu into helper function perf: cs-etm: Update record event to use new Trace ID protocol kernel: events: Export perf_report_aux_output_id() perf: cs-etm: Handle PERF_RECORD_AUX_OUTPUT_HW_ID packet coresight: events: PERF_RECORD_AUX_OUTPUT_HW_ID used for Trace ID coresight: trace-id: Add debug & test macros to Trace ID allocation drivers/hwtracing/coresight/Makefile | 2 +- drivers/hwtracing/coresight/coresight-core.c | 49 +-- .../hwtracing/coresight/coresight-etm-perf.c | 23 ++ drivers/hwtracing/coresight/coresight-etm.h | 3 +- .../coresight/coresight-etm3x-core.c | 92 +++-- .../coresight/coresight-etm3x-sysfs.c | 27 +- .../coresight/coresight-etm4x-core.c | 79 ++++- .../coresight/coresight-etm4x-sysfs.c | 27 +- drivers/hwtracing/coresight/coresight-etm4x.h | 3 + drivers/hwtracing/coresight/coresight-stm.c | 49 +-- .../hwtracing/coresight/coresight-trace-id.c | 266 ++++++++++++++ .../hwtracing/coresight/coresight-trace-id.h | 78 +++++ include/linux/coresight-pmu.h | 35 +- include/linux/coresight.h | 3 - kernel/events/core.c | 1 + tools/include/linux/coresight-pmu.h | 48 ++- tools/perf/arch/arm/util/cs-etm.c | 21 +- .../perf/util/cs-etm-decoder/cs-etm-decoder.c | 7 + tools/perf/util/cs-etm.c | 331 +++++++++++++++--- tools/perf/util/cs-etm.h | 14 +- 20 files changed, 933 insertions(+), 225 deletions(-) create mode 100644 drivers/hwtracing/coresight/coresight-trace-id.c create mode 100644 drivers/hwtracing/coresight/coresight-trace-id.h -- 2.17.1

2 years, 2 months

[PATCH v9 0/2] Add support for UltraSoc System Memory Buffer

by Junhao He

Add support for UltraSoc System Memory Buffer. Change since v8: - Insert a blank line at the end of the config tag in Kconfig. - Fix the "llseek" initialization. - Link: https://lore.kernel.org/linux-arm-kernel/20220816131634.38195-1-hejunhao3@h… Change since v7: - Use the macros for register bit flags and numbers of resource. - Fix punctuation. - Update the Date tag and the KernelVersion tag in the document. - Link: https://lore.kernel.org/lkml/20220712091353.34540-1-hejunhao3@huawei.com/ Change since v6: - Modify the code style and driver description according to Suzuki's comment. - Modify configuration of "drvdata->reading", to void problems in open/read concurrency scenario. - Rename the macro of "SMB_FLOW_MASK". - Use the "handle->head" to determine the page number and offset. - Link: https://lore.kernel.org/linux-arm-kernel/20220606130223.57354-1-liuqi115@hu… Change since v5: - Address the comments from Suzuki, add some comments in SMB document, and modify configuration of "drvdata->reading", to void problems in multi-core concurrency scenario - Link: https://lore.kernel.org/linux-arm-kernel/20220416083953.52610-1-liuqi115@hu… Change since v4: - Add a simple document of SMB driver according to Suzuki's comment. - Address the comments from Suzuki. - Link: https://lore.kernel.org/linux-arm-kernel/20220128061755.31909-1-liuqi115@hu… Change since v3: - Modify the file header according to community specifications. - Address the comments from Mathieu. - Link: https://lore.kernel.org/linux-arm-kernel/20211118110016.40398-1-liuqi115@hu… Change since v2: - Move ultrasoc driver to drivers/hwtracing/coresight. - Link: https://lists.linaro.org/pipermail/coresight/2021-November/007310.html Change since v1: - Drop the document of UltraSoc according to Mathieu's comment. - Add comments to explain some private hardware settings. - Address the comments from Mathieu. - Link: https://lists.linaro.org/pipermail/coresight/2021-August/006842.html Change since RFC: - Move driver to drivers/hwtracing/coresight/ultrasoc. - Remove ultrasoc-axi-com.c, as AXI-COM doesn't need to be configured in basic tracing function. - Remove ultrasoc.c as SMB does not need to register with the ultrasoc core. - Address the comments from Mathieu and Suzuki. - Link: https://lists.linaro.org/pipermail/coresight/2021-June/006535.html Qi Liu (2): drivers/coresight: Add UltraSoc System Memory Buffer driver Documentation: Add document for UltraSoc SMB drivers .../sysfs-bus-coresight-devices-ultra_smb | 31 + .../trace/coresight/ultrasoc-smb.rst | 80 +++ drivers/hwtracing/coresight/Kconfig | 11 + drivers/hwtracing/coresight/Makefile | 1 + drivers/hwtracing/coresight/ultrasoc-smb.c | 636 ++++++++++++++++++ drivers/hwtracing/coresight/ultrasoc-smb.h | 115 ++++ 6 files changed, 874 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-ultra_smb create mode 100644 Documentation/trace/coresight/ultrasoc-smb.rst create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.c create mode 100644 drivers/hwtracing/coresight/ultrasoc-smb.h -- 2.33.0

2 years, 2 months

Re: [PATCH v4 7/7] configfs: Fix LOCKDEP nesting issues with fragment semaphores

by Mike Leach

Hi Christoph On Thu, 11 Aug 2022 at 16:09, Christoph Hellwig <hch(a)lst.de> wrote: > > On Fri, Aug 05, 2022 at 03:15:53PM +0100, Mike Leach wrote: > > You are correct - in this patchset we are adding the use of a binary > > attribute to load and unload Coresight configurations and features - > > which action has a side effect of adding and removing entries in > > particular sub-directories in our configfs sub-system. > > I don't think configfs was designed to be that dynamic. Especially > the fact that a write to a binary attribute, which is intended to > be just for passthrough to hardware is a bit concerning. > configfs may not have originally been intended to be used in quite this way, but the system is robust and well designed, and the APIs work perfectly well in the this way. The model of user programming a configuration item for kernel backed elements that have a userspace lifetime (as described in configfs.rst) is precisely the model we need to successfully leverage the coresight subsystem. Our requirements are more complex than the ACPI tables or USB gadget drivers that use configfs, as we are potentially configuring multiple coresight devices. What the user sees in the configfs directories for the configurations are only the user adjustable features of these configurations. The rest of the programming is set on the relevant devices. > From your further description it sounds like this binary attribute > is loading structured data interpreted by the kernel, which really > isn't how binary attributes in configfs or sysfs are intended to > be used. If you look at the ACPI configfs, then this is in fact loading an ACPI table structure into the kernel, through a binary attribute, that is validated and added to a list of tables, if it passes the validation. This is also an example of using a binary attribute to pass structured data into the kernel for subsequent interpretation - and this example is why I chose to use binary attributes in this way. The data passed through the coresight 'load' binary attribute is validated, and then only if valid loaded into the coresight subsystem for use when coresight trace is activated and the user decides to use the configuration. (coresight configurations are loaded, but are only used if specifically selected during a trace session). Regards Mike -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

2 years, 2 months

[PATCH v2 0/5] coresight: Reduce duplicated sysfs accessors

by James Clark

Changes since v1: * Keep existing signed offset value types until the very last commit and then remove them all at once * Also split out usages of read_pair() and read32() into separate functions in the last commit * Whitespace fixes * Replaced any touched scnprintf() with sysfs_emit() ====== The intent of this change is to reduce the large number identical of functions created by macros for sysfs accessors. It's possible to re-use the same function but pass in the register to access as an argument. This reduces the size of the coresight modules folder by 244KB. The first two patches are refactors to simplify and remove some dead code, and the second two are the changes to use a shared function. Testing ======= No changes in any of the outputs: cat /sys/bus/coresight/devices/*/mgmt/* > before.txt cat /sys/bus/coresight/devices/*/mgmt/* > after.txt diff before.txt after.txt With the following modules loaded: ls /sys/bus/coresight/devices/ etm0 etm2 funnel0 funnel2 replicator0 tmc_etf0 tmc_etf2 tpiu0 etm1 etm3 funnel1 funnel3 stm0 tmc_etf1 tmc_etr0 James Clark (5): coresight: Remove unused function parameter coresight: Simplify sysfs accessors by using csdev_access abstraction coresight: Re-use same function for similar sysfs register accessors coresight: cti-sysfs: Re-use same functions for similar sysfs register accessors coresight: Make new csdev_access offsets unsigned drivers/hwtracing/coresight/coresight-catu.c | 27 +-- drivers/hwtracing/coresight/coresight-catu.h | 8 +- drivers/hwtracing/coresight/coresight-core.c | 28 +++ .../hwtracing/coresight/coresight-cti-sysfs.c | 213 +++++++----------- drivers/hwtracing/coresight/coresight-etb10.c | 28 +-- .../coresight/coresight-etm3x-sysfs.c | 34 +-- drivers/hwtracing/coresight/coresight-priv.h | 74 +++--- .../coresight/coresight-replicator.c | 10 +- drivers/hwtracing/coresight/coresight-stm.c | 40 +--- .../hwtracing/coresight/coresight-tmc-core.c | 48 ++-- drivers/hwtracing/coresight/coresight-tmc.h | 4 +- include/linux/coresight.h | 23 ++ 12 files changed, 227 insertions(+), 310 deletions(-) -- 2.28.0

2 years, 2 months

Re: [PATCH v3 0/2] coresight: etm4x: Expose default timestamp source in sysfs

by James Clark

On 26/08/2022 20:55, Mathieu Poirier wrote: > On Tue, Aug 23, 2022 at 05:06:48PM +0100, James Clark wrote: >> I've taken over this one from German because he's moved to a different >> team. I gave it a quick check and bumped the version number in the docs >> for the next release, but the month is an estimate. >> >> Thanks >> >> Changes since v2: >> >> * Rebased onto coresight/next (b99ee26a1a) >> * Bumped release version to 6.1 > > I have applied this set. Usually I'd let Suzuki handle it since he is already > familiar with the work but 1) he is currently away and 2) the patchset is fairly > simple. Thanks Mathieu > > Thanks, > Mathieu > >> >> Changes since v1: >> >> * Inline etmv4_to_ts_source() function. >> * Collect review tag from Leo. >> >> German Gomez (2): >> coresight: etm4x: Expose default timestamp source in sysfs >> coresight: etm4x: docs: Add documentation for 'ts_source' sysfs >> interface >> >> .../testing/sysfs-bus-coresight-devices-etm4x | 8 +++++ >> .../coresight/coresight-etm4x-reference.rst | 14 +++++++++ >> arch/arm64/include/asm/sysreg.h | 1 + >> .../coresight/coresight-etm4x-sysfs.c | 29 +++++++++++++++++++ >> 4 files changed, 52 insertions(+) >> >> -- >> 2.28.0 >>

2 years, 2 months

Re: [PATCH v3 2/2] coresight: etm4x: docs: Add documentation for 'ts_source' sysfs interface

by James Clark

On 24/08/2022 03:54, Bagas Sanjaya wrote: > On 8/23/22 23:06, James Clark wrote: >> +:Example: >> + ``$> cat ts_source`` >> + >> + ``$> 1`` >> + > > Shouldn't literal code block be used instead for example snippet > above? > It's consistent with the rest of the file. I think consistency for something like this is more important than accuracy otherwise the new entry would appear out of place. Maybe they should all be changed to a different style, but that would be a separate change unrelated to this set. James

2 years, 2 months

[PATCH v3 0/2] coresight: etm4x: Expose default timestamp source in sysfs

by James Clark

I've taken over this one from German because he's moved to a different team. I gave it a quick check and bumped the version number in the docs for the next release, but the month is an estimate. Thanks Changes since v2: * Rebased onto coresight/next (b99ee26a1a) * Bumped release version to 6.1 Changes since v1: * Inline etmv4_to_ts_source() function. * Collect review tag from Leo. German Gomez (2): coresight: etm4x: Expose default timestamp source in sysfs coresight: etm4x: docs: Add documentation for 'ts_source' sysfs interface .../testing/sysfs-bus-coresight-devices-etm4x | 8 +++++ .../coresight/coresight-etm4x-reference.rst | 14 +++++++++ arch/arm64/include/asm/sysreg.h | 1 + .../coresight/coresight-etm4x-sysfs.c | 29 +++++++++++++++++++ 4 files changed, 52 insertions(+) -- 2.28.0

2 years, 2 months

Re: [PATCH v1 2/4] coresight: Simplify sysfs accessors by using csdev_access abstraction

by James Clark

On 22/08/2022 18:37, Mathieu Poirier wrote: > Hi James, > > On Mon, Jul 25, 2022 at 03:52:19PM +0100, James Clark wrote: >> The coresight_device struct is available in the sysfs accessor, and this >> contains a csdev_access struct which can be used to access registers. >> Use this instead of passing in the type of each drvdata so that a common >> function can be shared between all the cs drivers. >> >> No functional changes. >> >> Signed-off-by: James Clark <james.clark(a)arm.com> >> --- >> drivers/hwtracing/coresight/coresight-catu.c | 18 +++++----- >> drivers/hwtracing/coresight/coresight-etb10.c | 19 +++++------ >> .../coresight/coresight-etm3x-sysfs.c | 23 ++++++------- >> drivers/hwtracing/coresight/coresight-priv.h | 14 ++++---- >> .../coresight/coresight-replicator.c | 7 ++-- >> drivers/hwtracing/coresight/coresight-stm.c | 27 +++++++-------- >> .../hwtracing/coresight/coresight-tmc-core.c | 33 ++++++++----------- >> include/linux/coresight.h | 18 ++++++++++ >> 8 files changed, 79 insertions(+), 80 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c >> index e0740c6dbd54..9d89c4054046 100644 >> --- a/drivers/hwtracing/coresight/coresight-catu.c >> +++ b/drivers/hwtracing/coresight/coresight-catu.c >> @@ -365,16 +365,14 @@ static const struct etr_buf_operations etr_catu_buf_ops = { >> .get_data = catu_get_data_etr_buf, >> }; >> >> -coresight_simple_reg32(struct catu_drvdata, devid, CORESIGHT_DEVID); >> -coresight_simple_reg32(struct catu_drvdata, control, CATU_CONTROL); >> -coresight_simple_reg32(struct catu_drvdata, status, CATU_STATUS); >> -coresight_simple_reg32(struct catu_drvdata, mode, CATU_MODE); >> -coresight_simple_reg32(struct catu_drvdata, axictrl, CATU_AXICTRL); >> -coresight_simple_reg32(struct catu_drvdata, irqen, CATU_IRQEN); >> -coresight_simple_reg64(struct catu_drvdata, sladdr, >> - CATU_SLADDRLO, CATU_SLADDRHI); >> -coresight_simple_reg64(struct catu_drvdata, inaddr, >> - CATU_INADDRLO, CATU_INADDRHI); >> +coresight_simple_reg32(devid, CORESIGHT_DEVID); >> +coresight_simple_reg32(control, CATU_CONTROL); >> +coresight_simple_reg32(status, CATU_STATUS); >> +coresight_simple_reg32(mode, CATU_MODE); >> +coresight_simple_reg32(axictrl, CATU_AXICTRL); >> +coresight_simple_reg32(irqen, CATU_IRQEN); >> +coresight_simple_reg64(sladdr, CATU_SLADDRLO, CATU_SLADDRHI); >> +coresight_simple_reg64(inaddr, CATU_INADDRLO, CATU_INADDRHI); >> >> static struct attribute *catu_mgmt_attrs[] = { >> &dev_attr_devid.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c >> index efa39820acec..405bb3355cb1 100644 >> --- a/drivers/hwtracing/coresight/coresight-etb10.c >> +++ b/drivers/hwtracing/coresight/coresight-etb10.c >> @@ -655,17 +655,14 @@ static const struct file_operations etb_fops = { >> .llseek = no_llseek, >> }; >> >> -#define coresight_etb10_reg(name, offset) \ >> - coresight_simple_reg32(struct etb_drvdata, name, offset) >> - >> -coresight_etb10_reg(rdp, ETB_RAM_DEPTH_REG); >> -coresight_etb10_reg(sts, ETB_STATUS_REG); >> -coresight_etb10_reg(rrp, ETB_RAM_READ_POINTER); >> -coresight_etb10_reg(rwp, ETB_RAM_WRITE_POINTER); >> -coresight_etb10_reg(trg, ETB_TRG); >> -coresight_etb10_reg(ctl, ETB_CTL_REG); >> -coresight_etb10_reg(ffsr, ETB_FFSR); >> -coresight_etb10_reg(ffcr, ETB_FFCR); >> +coresight_simple_reg32(rdp, ETB_RAM_DEPTH_REG); >> +coresight_simple_reg32(sts, ETB_STATUS_REG); >> +coresight_simple_reg32(rrp, ETB_RAM_READ_POINTER); >> +coresight_simple_reg32(rwp, ETB_RAM_WRITE_POINTER); >> +coresight_simple_reg32(trg, ETB_TRG); >> +coresight_simple_reg32(ctl, ETB_CTL_REG); >> +coresight_simple_reg32(ffsr, ETB_FFSR); >> +coresight_simple_reg32(ffcr, ETB_FFCR); >> >> static struct attribute *coresight_etb_mgmt_attrs[] = { >> &dev_attr_rdp.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c >> index 68fcbf4ce7a8..12f8e8176c7e 100644 >> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c >> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c >> @@ -1252,19 +1252,16 @@ static struct attribute *coresight_etm_attrs[] = { >> NULL, >> }; >> >> -#define coresight_etm3x_reg(name, offset) \ >> - coresight_simple_reg32(struct etm_drvdata, name, offset) >> - >> -coresight_etm3x_reg(etmccr, ETMCCR); >> -coresight_etm3x_reg(etmccer, ETMCCER); >> -coresight_etm3x_reg(etmscr, ETMSCR); >> -coresight_etm3x_reg(etmidr, ETMIDR); >> -coresight_etm3x_reg(etmcr, ETMCR); >> -coresight_etm3x_reg(etmtraceidr, ETMTRACEIDR); >> -coresight_etm3x_reg(etmteevr, ETMTEEVR); >> -coresight_etm3x_reg(etmtssvr, ETMTSSCR); >> -coresight_etm3x_reg(etmtecr1, ETMTECR1); >> -coresight_etm3x_reg(etmtecr2, ETMTECR2); >> +coresight_simple_reg32(etmccr, ETMCCR); >> +coresight_simple_reg32(etmccer, ETMCCER); >> +coresight_simple_reg32(etmscr, ETMSCR); >> +coresight_simple_reg32(etmidr, ETMIDR); >> +coresight_simple_reg32(etmcr, ETMCR); >> +coresight_simple_reg32(etmtraceidr, ETMTRACEIDR); >> +coresight_simple_reg32(etmteevr, ETMTEEVR); >> +coresight_simple_reg32(etmtssvr, ETMTSSCR); >> +coresight_simple_reg32(etmtecr1, ETMTECR1); >> +coresight_simple_reg32(etmtecr2, ETMTECR2); >> >> static struct attribute *coresight_etm_mgmt_attrs[] = { >> &dev_attr_etmccr.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h >> index f2458b794ef3..cf8ae768106e 100644 >> --- a/drivers/hwtracing/coresight/coresight-priv.h >> +++ b/drivers/hwtracing/coresight/coresight-priv.h >> @@ -40,23 +40,23 @@ >> #define ETM_MODE_EXCL_KERN BIT(30) >> #define ETM_MODE_EXCL_USER BIT(31) >> >> -#define __coresight_simple_show(type, name, lo_off, hi_off) \ >> +#define __coresight_simple_show(name, lo_off, hi_off) \ >> static ssize_t name##_show(struct device *_dev, \ >> struct device_attribute *attr, char *buf) \ >> { \ >> - type *drvdata = dev_get_drvdata(_dev->parent); \ >> + struct coresight_device *csdev = container_of(_dev, struct coresight_device, dev); \ >> u64 val; \ >> pm_runtime_get_sync(_dev->parent); \ >> - val = coresight_read_reg_pair(drvdata->base, lo_off, hi_off); \ >> + val = csdev_access_relaxed_read_pair(&csdev->access, lo_off, hi_off); \ >> pm_runtime_put_sync(_dev->parent); \ >> return scnprintf(buf, PAGE_SIZE, "0x%llx\n", val); \ >> } \ >> static DEVICE_ATTR_RO(name) >> >> -#define coresight_simple_reg32(type, name, offset) \ >> - __coresight_simple_show(type, name, offset, -1) >> -#define coresight_simple_reg64(type, name, lo_off, hi_off) \ >> - __coresight_simple_show(type, name, lo_off, hi_off) >> +#define coresight_simple_reg32(name, offset) \ >> + __coresight_simple_show(name, offset, -1) >> +#define coresight_simple_reg64(name, lo_off, hi_off) \ >> + __coresight_simple_show(name, lo_off, hi_off) >> >> extern const u32 coresight_barrier_pkt[4]; >> #define CORESIGHT_BARRIER_PKT_SIZE (sizeof(coresight_barrier_pkt)) >> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c >> index b86acbc74cf0..7cffcbb2ec42 100644 >> --- a/drivers/hwtracing/coresight/coresight-replicator.c >> +++ b/drivers/hwtracing/coresight/coresight-replicator.c >> @@ -196,11 +196,8 @@ static const struct coresight_ops replicator_cs_ops = { >> .link_ops = &replicator_link_ops, >> }; >> >> -#define coresight_replicator_reg(name, offset) \ >> - coresight_simple_reg32(struct replicator_drvdata, name, offset) >> - >> -coresight_replicator_reg(idfilter0, REPLICATOR_IDFILTER0); >> -coresight_replicator_reg(idfilter1, REPLICATOR_IDFILTER1); >> +coresight_simple_reg32(idfilter0, REPLICATOR_IDFILTER0); >> +coresight_simple_reg32(idfilter1, REPLICATOR_IDFILTER1); >> >> static struct attribute *replicator_mgmt_attrs[] = { >> &dev_attr_idfilter0.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c >> index bb14a3a8a921..4a31905604fe 100644 >> --- a/drivers/hwtracing/coresight/coresight-stm.c >> +++ b/drivers/hwtracing/coresight/coresight-stm.c >> @@ -634,21 +634,18 @@ static ssize_t traceid_store(struct device *dev, >> } >> static DEVICE_ATTR_RW(traceid); >> >> -#define coresight_stm_reg(name, offset) \ >> - coresight_simple_reg32(struct stm_drvdata, name, offset) >> - >> -coresight_stm_reg(tcsr, STMTCSR); >> -coresight_stm_reg(tsfreqr, STMTSFREQR); >> -coresight_stm_reg(syncr, STMSYNCR); >> -coresight_stm_reg(sper, STMSPER); >> -coresight_stm_reg(spter, STMSPTER); >> -coresight_stm_reg(privmaskr, STMPRIVMASKR); >> -coresight_stm_reg(spscr, STMSPSCR); >> -coresight_stm_reg(spmscr, STMSPMSCR); >> -coresight_stm_reg(spfeat1r, STMSPFEAT1R); >> -coresight_stm_reg(spfeat2r, STMSPFEAT2R); >> -coresight_stm_reg(spfeat3r, STMSPFEAT3R); >> -coresight_stm_reg(devid, CORESIGHT_DEVID); >> +coresight_simple_reg32(tcsr, STMTCSR); >> +coresight_simple_reg32(tsfreqr, STMTSFREQR); >> +coresight_simple_reg32(syncr, STMSYNCR); >> +coresight_simple_reg32(sper, STMSPER); >> +coresight_simple_reg32(spter, STMSPTER); >> +coresight_simple_reg32(privmaskr, STMPRIVMASKR); >> +coresight_simple_reg32(spscr, STMSPSCR); >> +coresight_simple_reg32(spmscr, STMSPMSCR); >> +coresight_simple_reg32(spfeat1r, STMSPFEAT1R); >> +coresight_simple_reg32(spfeat2r, STMSPFEAT2R); >> +coresight_simple_reg32(spfeat3r, STMSPFEAT3R); >> +coresight_simple_reg32(devid, CORESIGHT_DEVID); >> >> static struct attribute *coresight_stm_attrs[] = { >> &dev_attr_hwevent_enable.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c >> index d0276af82494..781d213526b7 100644 >> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c >> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c >> @@ -251,25 +251,20 @@ static enum tmc_mem_intf_width tmc_get_memwidth(u32 devid) >> return memwidth; >> } >> >> -#define coresight_tmc_reg(name, offset) \ >> - coresight_simple_reg32(struct tmc_drvdata, name, offset) >> -#define coresight_tmc_reg64(name, lo_off, hi_off) \ >> - coresight_simple_reg64(struct tmc_drvdata, name, lo_off, hi_off) >> - >> -coresight_tmc_reg(rsz, TMC_RSZ); >> -coresight_tmc_reg(sts, TMC_STS); >> -coresight_tmc_reg(trg, TMC_TRG); >> -coresight_tmc_reg(ctl, TMC_CTL); >> -coresight_tmc_reg(ffsr, TMC_FFSR); >> -coresight_tmc_reg(ffcr, TMC_FFCR); >> -coresight_tmc_reg(mode, TMC_MODE); >> -coresight_tmc_reg(pscr, TMC_PSCR); >> -coresight_tmc_reg(axictl, TMC_AXICTL); >> -coresight_tmc_reg(authstatus, TMC_AUTHSTATUS); >> -coresight_tmc_reg(devid, CORESIGHT_DEVID); >> -coresight_tmc_reg64(rrp, TMC_RRP, TMC_RRPHI); >> -coresight_tmc_reg64(rwp, TMC_RWP, TMC_RWPHI); >> -coresight_tmc_reg64(dba, TMC_DBALO, TMC_DBAHI); >> +coresight_simple_reg32(rsz, TMC_RSZ); >> +coresight_simple_reg32(sts, TMC_STS); >> +coresight_simple_reg32(trg, TMC_TRG); >> +coresight_simple_reg32(ctl, TMC_CTL); >> +coresight_simple_reg32(ffsr, TMC_FFSR); >> +coresight_simple_reg32(ffcr, TMC_FFCR); >> +coresight_simple_reg32(mode, TMC_MODE); >> +coresight_simple_reg32(pscr, TMC_PSCR); >> +coresight_simple_reg32(axictl, TMC_AXICTL); >> +coresight_simple_reg32(authstatus, TMC_AUTHSTATUS); >> +coresight_simple_reg32(devid, CORESIGHT_DEVID); >> +coresight_simple_reg64(rrp, TMC_RRP, TMC_RRPHI); >> +coresight_simple_reg64(rwp, TMC_RWP, TMC_RWPHI); >> +coresight_simple_reg64(dba, TMC_DBALO, TMC_DBAHI); >> >> static struct attribute *coresight_tmc_mgmt_attrs[] = { >> &dev_attr_rsz.attr, >> diff --git a/include/linux/coresight.h b/include/linux/coresight.h >> index 9f445f09fcfe..c1bb93c7c1de 100644 >> --- a/include/linux/coresight.h >> +++ b/include/linux/coresight.h >> @@ -372,6 +372,24 @@ static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa, >> return csa->read(offset, true, false); >> } >> >> +static inline u64 csdev_access_relaxed_read_pair(struct csdev_access *csa, >> + u32 lo_offset, u32 hi_offset) > > Parameters lo_offset and hi_offset are s32 in coresight_read_reg_pair()... Hi Mathieu, I probably should have mentioned this in the commit message. You're right that the previous version used signed values, but the csdev accessors in include/linux/coresight.h all use u32 and I had to add a new one in there for 'csdev_access_relaxed_read_pair()' which would have looked very out of place if it was the only one to used signed values. Because of this I also changed the 'not set' test for hi_offset from '< -1' to '== -1' which would also work with unsigned values. So although it looks different, it is still working the same way as before. I can think of some possible options to make it better: * Have csdev_access_relaxed_read_pair() be the only csdev_access_ function to take signed values * Keep the unsigned type but change the unset value of -1 to be UINT_MAX * Split the accessors into ones that are 64 bit pairs, and ones that are a single read. It's always known when it's defined whether it's a 'pair' or not, so technically this if statement with the 'not set' value isn't actually needed, you just use a different accessor type I was tempted to do the 3rd one during the refactor, but I wanted to keep it more like the original than not. I'm not a fan of the first option, I think that would be confusing to read the code and would look like a mistake. So I'm more in favour of 2 or 3. What are your thoughts? > >> +{ >> + u64 val; >> + >> + if (likely(csa->io_mem)) { >> + val = readl_relaxed(csa->base + lo_offset); >> + val |= (hi_offset == -1) ? 0 : >> + (u64)readl_relaxed(csa->base + hi_offset) << 32; >> + return val; >> + } >> + >> + val = csa->read(lo_offset, true, false); >> + val |= (hi_offset == -1) ? 0 : > > ... and hi_offset can't take on a negative value. This is just shorthand for UINT_MAX. I could change it to be more explicit (option 2 above)? > >> + (u64)csa->read(hi_offset, true, false) << 32; >> + return val; >> +} >> + >> static inline u32 csdev_access_read32(struct csdev_access *csa, u32 offset) >> { >> if (likely(csa->io_mem)) >> -- >> 2.28.0 >>

2 years, 2 months

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight August 2022