CoreSight

coresight@lists.linaro.org

8 participants
2600 discussions

Re: [PATCH v1 3/4] coresight: ctcu: Enable byte-cntr for TMC ETR devices

by Suzuki K Poulose

On 10/03/2025 09:04, Jie Gan wrote: > The byte-cntr function provided by the CTCU device is used to transfer data > from the ETR buffer to the userspace. An interrupt is triggered if the data Why do we need a new device to transfer the data to userspace ? > size exceeds the threshold set in the BYTECNTRVAL register. The interrupt > handler counts the number of triggered interruptions and the read function > will read the data from the ETR buffer if the IRQ count is greater than 0. > Each successful read process will decrement the IRQ count by 1. Having an interrupt is good, but see below. > > Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > --- > drivers/hwtracing/coresight/Makefile | 2 +- > .../coresight/coresight-ctcu-byte-cntr.c | 339 ++++++++++++++++++ > .../hwtracing/coresight/coresight-ctcu-core.c | 96 ++++- > drivers/hwtracing/coresight/coresight-ctcu.h | 59 ++- > .../hwtracing/coresight/coresight-tmc-etr.c | 5 +- > drivers/hwtracing/coresight/coresight-tmc.h | 2 + > 6 files changed, 493 insertions(+), 10 deletions(-) > create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > > diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile > index 8e62c3150aeb..c90a06768a18 100644 > --- a/drivers/hwtracing/coresight/Makefile > +++ b/drivers/hwtracing/coresight/Makefile > @@ -52,4 +52,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ > obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o > obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o > obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o > -coresight-ctcu-y := coresight-ctcu-core.o > +coresight-ctcu-y := coresight-ctcu-core.o coresight-ctcu-byte-cntr.o > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > new file mode 100644 > index 000000000000..0d16385663f5 > --- /dev/null > +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > @@ -0,0 +1,339 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/coresight.h> > +#include <linux/device.h> > +#include <linux/dma-mapping.h> > +#include <linux/fs.h> > +#include <linux/interrupt.h> > +#include <linux/of_irq.h> > +#include <linux/property.h> > +#include <linux/uaccess.h> > + > +#include "coresight-ctcu.h" > +#include "coresight-priv.h" > +#include "coresight-tmc.h" > + > +#define BYTE_CNTR_CLASS_STR "byte-cntr-class" > + > +static struct class *byte_cntr_class; > +static dev_t byte_cntr_base; > + > +static irqreturn_t byte_cntr_handler(int irq, void *data) > +{ > + struct ctcu_byte_cntr *byte_cntr_data = (struct ctcu_byte_cntr *)data; > + > + atomic_inc(&byte_cntr_data->irq_cnt); > + wake_up(&byte_cntr_data->wq); > + > + return IRQ_HANDLED; > +} > + > +/* Read the data from ETR's DDR buffer. */ > +static void __ctcu_byte_cntr_read_etr_bytes(struct ctcu_byte_cntr *byte_cntr_data, > + size_t *len, char **bufp) > +{ > + struct tmc_drvdata *tmcdrvdata = dev_get_drvdata(byte_cntr_data->sink->dev.parent); > + size_t actual, bytes = byte_cntr_data->thresh_val; > + struct etr_buf *etr_buf = tmcdrvdata->sysfs_buf; > + long r_offset; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + r_offset = byte_cntr_data->r_offset; > + if (*len >= bytes) > + *len = bytes; > + else if ((r_offset % bytes) + *len > bytes) > + *len = bytes - (r_offset % bytes); > + > + actual = tmc_etr_buf_get_data(etr_buf, r_offset, *len, bufp); > + *len = actual; > + if (actual == bytes || (actual + r_offset) % bytes == 0) > + atomic_dec(&byte_cntr_data->irq_cnt); > +} > + How can you safely read the ETR data while it is running ? You should stop the ETR and then provide the data. You could potentially do things like split the ETR buffer into two halves and switch the half used by the ETR on interrupt and expose this to user. > +/* Flush the remaining data in the ETR buffer after the byte cntr has stopped. */ > +static ssize_t ctcu_flush_etr_buffer(struct ctcu_byte_cntr *byte_cntr_data, size_t len, > + char **bufpp) > +{ > + struct tmc_drvdata *tmcdrvdata = dev_get_drvdata(byte_cntr_data->sink->dev.parent); > + struct etr_buf *etr_buf = tmcdrvdata->sysfs_buf; > + ssize_t read_len = 0, remaining_len; > + long r_offset, w_offset; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + r_offset = byte_cntr_data->r_offset; > + w_offset = byte_cntr_data->w_offset; > + if (w_offset < r_offset) > + remaining_len = tmcdrvdata->size + w_offset - r_offset; > + else > + remaining_len = w_offset - r_offset; > + > + if (remaining_len > len) > + remaining_len = len; > + > + if (remaining_len > 0) > + read_len = tmc_etr_buf_get_data(etr_buf, r_offset, remaining_len, bufpp); > + > + return read_len; > +} > + > +static ssize_t ctcu_byte_cntr_copy_data(char __user *data, size_t len, char *bufp, > + struct ctcu_byte_cntr *byte_cntr_data, > + struct tmc_drvdata *tmcdrvdata) > +{ > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + if (copy_to_user(data, bufp, len)) > + return -EFAULT; > + > + byte_cntr_data->total_size += len; > + if (byte_cntr_data->r_offset + len >= tmcdrvdata->size) > + byte_cntr_data->r_offset = 0; > + else > + byte_cntr_data->r_offset += len; > + > + return len; > +} > + > +/* The read function for /dev/byte-cntr%d */ WHY ? WHY can't the data be exported by the existing /dev/tmc_etr* ? > +static ssize_t ctcu_byte_cntr_read_etr_bytes(struct file *fp, char __user *data, > + size_t len, loff_t *ppos) > +{ > + struct ctcu_byte_cntr *byte_cntr_data = fp->private_data; > + struct tmc_drvdata *tmcdrvdata = dev_get_drvdata(byte_cntr_data->sink->dev.parent); > + char *bufp = NULL; > + ssize_t read_len; > + > + if (!data) > + return -EINVAL; > + > + /* > + * Flush the remaining data in the ETR buffer based on the write > + * offset of the ETR buffer when the byte cntr function has stopped. > + */ > + if (!byte_cntr_data->read_active || !byte_cntr_data->enable) { > + read_len = ctcu_flush_etr_buffer(byte_cntr_data, len, &bufp); > + if (read_len > 0) > + return ctcu_byte_cntr_copy_data(data, read_len, bufp, > + byte_cntr_data, tmcdrvdata); > + > + return -EINVAL; > + } > + > + if (!atomic_read(&byte_cntr_data->irq_cnt)) { > + if (wait_event_interruptible(byte_cntr_data->wq, > + atomic_read(&byte_cntr_data->irq_cnt) > 0 || > + !byte_cntr_data->enable)) > + return -ERESTARTSYS; > + } > + > + __ctcu_byte_cntr_read_etr_bytes(byte_cntr_data, &len, &bufp); > + > + return ctcu_byte_cntr_copy_data(data, len, bufp, byte_cntr_data, tmcdrvdata); > +} Could we not plug this into TMC-ETR read ? NAK for a special file approach. As Mike mentioned you need to make sure we are doing this safely. Suzuki > + > +/* Start the byte-cntr function when the path is enabled. */ > +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct coresight_device *sink = coresight_get_sink(path); > + struct ctcu_byte_cntr *byte_cntr_data; > + int port_num; > + > + if (!sink) > + return; > + > + port_num = ctcu_get_active_port(sink, csdev); > + if (port_num < 0) > + return; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port_num]; > + /* Don't start byte-cntr function when threshold is not set. */ > + if (!byte_cntr_data->thresh_val) > + return; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + atomic_set(&byte_cntr_data->irq_cnt, 0); > + byte_cntr_data->sink = sink; > + byte_cntr_data->enable = true; > +} > + > +/* Stop the byte-cntr function when the path is disabled. */ > +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); > + struct coresight_device *sink = coresight_get_sink(path); > + struct ctcu_byte_cntr *byte_cntr_data; > + struct tmc_drvdata *tmcdrvdata; > + int port_num; > + > + if (!sink) > + return; > + > + port_num = ctcu_get_active_port(sink, csdev); > + if (port_num < 0) > + return; > + > + byte_cntr_data = &drvdata->byte_cntr_data[port_num]; > + tmcdrvdata = dev_get_drvdata(byte_cntr_data->sink->dev.parent); > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + /* Store the w_offset of the ETR buffer when stopping. */ > + byte_cntr_data->w_offset = tmc_get_rwp_offset(tmcdrvdata); > + atomic_set(&byte_cntr_data->irq_cnt, 0); > + byte_cntr_data->read_active = false; > + byte_cntr_data->enable = false; > + /* > + * Wakeup once to force the read function to read the remaining > + * data of the ETR buffer. > + */ > + wake_up(&byte_cntr_data->wq); > +} > + > +static int ctcu_byte_cntr_release(struct inode *in, struct file *fp) > +{ > + struct ctcu_byte_cntr *byte_cntr_data = fp->private_data; > + struct device *dev = &byte_cntr_data->sink->dev; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + atomic_set(&byte_cntr_data->irq_cnt, 0); > + byte_cntr_data->read_active = false; > + disable_irq_wake(byte_cntr_data->byte_cntr_irq); > + dev_dbg(dev, "send data total size: %llu bytes, r_offset: %ld w_offset: %ld\n", > + byte_cntr_data->total_size, byte_cntr_data->r_offset, > + byte_cntr_data->w_offset); > + > + return 0; > +} > + > +static int ctcu_byte_cntr_open(struct inode *in, struct file *fp) > +{ > + struct ctcu_byte_cntr *byte_cntr_data = container_of(in->i_cdev, > + struct ctcu_byte_cntr, c_dev); > + struct tmc_drvdata *tmcdrvdata; > + > + if (byte_cntr_data->read_active) > + return -EBUSY; > + > + if (!byte_cntr_data->thresh_val || !byte_cntr_data->sink || > + (coresight_get_mode(byte_cntr_data->sink) != CS_MODE_SYSFS)) > + return -EINVAL; > + > + guard(raw_spinlock_irqsave)(&byte_cntr_data->spin_lock); > + enable_irq_wake(byte_cntr_data->byte_cntr_irq); > + fp->private_data = byte_cntr_data; > + nonseekable_open(in, fp); > + tmcdrvdata = dev_get_drvdata(byte_cntr_data->sink->dev.parent); > + /* > + * The original r_offset is the w_offset of the ETR buffer at the > + * start of the byte-cntr. > + */ > + byte_cntr_data->r_offset = tmc_get_rwp_offset(tmcdrvdata); > + byte_cntr_data->total_size = 0; > + byte_cntr_data->read_active = true; > + byte_cntr_data->enable = true; > + > + return 0; > +} > + > +static const struct file_operations byte_cntr_fops = { > + .owner = THIS_MODULE, > + .open = ctcu_byte_cntr_open, > + .read = ctcu_byte_cntr_read_etr_bytes, > + .release = ctcu_byte_cntr_release, > +}; > + > +static int ctcu_byte_cntr_register_chardev(struct ctcu_byte_cntr *byte_cntr_data, > + int port_num) > +{ > + struct device *device; > + dev_t devt; > + int ret; > + > + cdev_init(&byte_cntr_data->c_dev, &byte_cntr_fops); > + devt = MKDEV(MAJOR(byte_cntr_base), MINOR(byte_cntr_base) + port_num); > + ret = cdev_add(&byte_cntr_data->c_dev, devt, 1); > + if (ret < 0) > + return -ENOMEM; > + > + device = device_create(byte_cntr_class, NULL, devt, byte_cntr_data, > + byte_cntr_data->name); > + > + if (IS_ERR(device)) > + return -ENOMEM; > + > + return 0; > +} > + > +static void ctcu_byte_cntr_unregister_chardev(struct ctcu_drvdata *drvdata) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + int i; > + > + for (i = 0; i < ETR_MAX_NUM; i++) { > + byte_cntr_data = &drvdata->byte_cntr_data[i]; > + device_destroy(byte_cntr_class, byte_cntr_data->c_dev.dev); > + } > +} > + > +int ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int etr_num) > +{ > + struct ctcu_byte_cntr *byte_cntr_data; > + struct device_node *nd = dev->of_node; > + int byte_cntr_irq, ret, i; > + > + ret = alloc_chrdev_region(&byte_cntr_base, 0, ETR_MAX_NUM, BYTE_CNTR_CLASS_STR); > + if (ret < 0) > + return -ENOMEM; > + > + byte_cntr_class = class_create(BYTE_CNTR_CLASS_STR); > + if (IS_ERR(byte_cntr_class)) { > + unregister_chrdev_region(byte_cntr_base, ETR_MAX_NUM); > + return -ENOMEM; > + } > + > + for (i = 0; i < etr_num; i++) { > + byte_cntr_data = &drvdata->byte_cntr_data[i]; > + byte_cntr_irq = of_irq_get_byname(nd, byte_cntr_data->irq_name); > + if (byte_cntr_irq < 0) { > + ret = byte_cntr_irq; > + goto err_exit; > + } > + > + ret = devm_request_irq(dev, byte_cntr_irq, byte_cntr_handler, > + IRQF_TRIGGER_RISING | IRQF_SHARED, > + dev_name(dev), byte_cntr_data); > + if (ret) { > + dev_err(dev, "Failed to register IRQ for %s\n", > + byte_cntr_data->name); > + goto err_exit; > + } > + > + ret = ctcu_byte_cntr_register_chardev(byte_cntr_data, i); > + if (ret) { > + dev_err(dev, "Failed to register chardev for %s\n", > + byte_cntr_data->name); > + goto err_exit; > + } > + > + byte_cntr_data->byte_cntr_irq = byte_cntr_irq; > + atomic_set(&byte_cntr_data->irq_cnt, 0); > + init_waitqueue_head(&byte_cntr_data->wq); > + } > + > + return 0; > + > +err_exit: > + ctcu_byte_cntr_unregister_chardev(drvdata); > + class_destroy(byte_cntr_class); > + unregister_chrdev_region(byte_cntr_base, ETR_MAX_NUM); > + return ret; > +} > + > +void ctcu_byte_cntr_remove(struct ctcu_drvdata *drvdata) > +{ > + ctcu_byte_cntr_unregister_chardev(drvdata); > + class_destroy(byte_cntr_class); > + unregister_chrdev_region(byte_cntr_base, ETR_MAX_NUM); > +} > diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c > index da35d8b4d579..5782655a5f39 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c > +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c > @@ -46,16 +46,24 @@ DEFINE_CORESIGHT_DEVLIST(ctcu_devs, "ctcu"); > #define CTCU_ATID_REG_BIT(traceid) (traceid % 32) > #define CTCU_ATID_REG_SIZE 0x10 > #define CTCU_ETR0_ATID0 0xf8 > +#define CTCU_ETR0_IRQCTRL 0x6c > #define CTCU_ETR1_ATID0 0x108 > +#define CTCU_ETR1_IRQCTRL 0x70 > > static const struct ctcu_etr_config sa8775p_etr_cfgs[] = { > { > - .atid_offset = CTCU_ETR0_ATID0, > - .port_num = 0, > + .atid_offset = CTCU_ETR0_ATID0, > + .irq_ctrl_offset = CTCU_ETR0_IRQCTRL, > + .irq_name = "etr0", > + .byte_cntr_name = "byte-cntr0", > + .port_num = 0, > }, > { > - .atid_offset = CTCU_ETR1_ATID0, > - .port_num = 1, > + .atid_offset = CTCU_ETR1_ATID0, > + .irq_ctrl_offset = CTCU_ETR1_IRQCTRL, > + .irq_name = "etr1", > + .byte_cntr_name = "byte-cntr1", > + .port_num = 1, > }, > }; > > @@ -64,6 +72,69 @@ static const struct ctcu_config sa8775p_cfgs = { > .num_etr_config = ARRAY_SIZE(sa8775p_etr_cfgs), > }; > > +static ssize_t byte_cntr_val_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + int i, len = 0; > + > + for (i = 0; i < ETR_MAX_NUM; i++) { > + if (drvdata->byte_cntr_data[i].irq_ctrl_offset) > + len += scnprintf(buf + len, PAGE_SIZE - len, "%u ", > + drvdata->byte_cntr_data[i].thresh_val); > + } > + > + len += scnprintf(buf + len, PAGE_SIZE - len, "\n"); > + > + return len; > +} > + > +static ssize_t byte_cntr_val_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t size) > +{ > + struct ctcu_drvdata *drvdata = dev_get_drvdata(dev->parent); > + u32 thresh_vals[ETR_MAX_NUM] = { 0 }; > + u32 irq_ctrl_offset; > + int num, i; > + > + num = sscanf(buf, "%i %i", &thresh_vals[0], &thresh_vals[1]); > + if (num <= 0 || num > ETR_MAX_NUM) > + return -EINVAL; > + > + /* Threshold 0 disables the interruption. */ > + guard(raw_spinlock_irqsave)(&drvdata->spin_lock); > + for (i = 0; i < num; i++) { > + /* A small threshold will result in a large number of interruptions */ > + if (thresh_vals[i] && thresh_vals[i] < 4096) > + return -EINVAL; > + > + if (drvdata->byte_cntr_data[i].irq_ctrl_offset) { > + drvdata->byte_cntr_data[i].thresh_val = thresh_vals[i]; > + irq_ctrl_offset = drvdata->byte_cntr_data[i].irq_ctrl_offset; > + CS_UNLOCK(drvdata->base); > + writel_relaxed(thresh_vals[i], drvdata->base + irq_ctrl_offset); > + CS_LOCK(drvdata->base); > + } > + } > + > + return size; > +} > +static DEVICE_ATTR_RW(byte_cntr_val); > + > +static struct attribute *ctcu_attrs[] = { > + &dev_attr_byte_cntr_val.attr, > + NULL, > +}; > + > +static struct attribute_group ctcu_attr_grp = { > + .attrs = ctcu_attrs, > +}; > + > +static const struct attribute_group *ctcu_attr_grps[] = { > + &ctcu_attr_grp, > + NULL, > +}; > + > static void ctcu_program_atid_register(struct ctcu_drvdata *drvdata, u32 reg_offset, > u8 bit, bool enable) > { > @@ -122,7 +193,7 @@ static int __ctcu_set_etr_traceid(struct coresight_device *csdev, u8 traceid, in > * Searching the sink device from helper's view in case there are multiple helper devices > * connected to the sink device. > */ > -static int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper) > +int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper) > { > struct coresight_platform_data *pdata = helper->pdata; > int i; > @@ -160,6 +231,8 @@ static int ctcu_enable(struct coresight_device *csdev, enum cs_mode mode, void * > { > struct coresight_path *path = (struct coresight_path *)data; > > + ctcu_byte_cntr_start(csdev, path); > + > return ctcu_set_etr_traceid(csdev, path, true); > } > > @@ -167,6 +240,8 @@ static int ctcu_disable(struct coresight_device *csdev, void *data) > { > struct coresight_path *path = (struct coresight_path *)data; > > + ctcu_byte_cntr_stop(csdev, path); > + > return ctcu_set_etr_traceid(csdev, path, false); > } > > @@ -188,7 +263,7 @@ static int ctcu_probe(struct platform_device *pdev) > const struct ctcu_config *cfgs; > struct ctcu_drvdata *drvdata; > void __iomem *base; > - int i; > + int ret, i; > > desc.name = coresight_alloc_device_name(&ctcu_devs, dev); > if (!desc.name) > @@ -217,7 +292,14 @@ static int ctcu_probe(struct platform_device *pdev) > for (i = 0; i < cfgs->num_etr_config; i++) { > etr_cfg = &cfgs->etr_cfgs[i]; > drvdata->atid_offset[i] = etr_cfg->atid_offset; > + drvdata->byte_cntr_data[i].irq_name = etr_cfg->irq_name; > + drvdata->byte_cntr_data[i].name = etr_cfg->byte_cntr_name; > + drvdata->byte_cntr_data[i].irq_ctrl_offset = > + etr_cfg->irq_ctrl_offset; > } > + ret = ctcu_byte_cntr_init(dev, drvdata, cfgs->num_etr_config); > + if (ret < 0) > + dev_warn(dev, "Byte cntr init failed\n"); > } > } > > @@ -229,6 +311,7 @@ static int ctcu_probe(struct platform_device *pdev) > desc.subtype.helper_subtype = CORESIGHT_DEV_SUBTYPE_HELPER_CTCU; > desc.pdata = pdata; > desc.dev = dev; > + desc.groups = ctcu_attr_grps; > desc.ops = &ctcu_ops; > desc.access = CSDEV_ACCESS_IOMEM(base); > > @@ -247,6 +330,7 @@ static void ctcu_remove(struct platform_device *pdev) > { > struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev); > > + ctcu_byte_cntr_remove(drvdata); > coresight_unregister(drvdata->csdev); > } > > diff --git a/drivers/hwtracing/coresight/coresight-ctcu.h b/drivers/hwtracing/coresight/coresight-ctcu.h > index e9594c38dd91..e38535c91090 100644 > --- a/drivers/hwtracing/coresight/coresight-ctcu.h > +++ b/drivers/hwtracing/coresight/coresight-ctcu.h > @@ -5,6 +5,8 @@ > > #ifndef _CORESIGHT_CTCU_H > #define _CORESIGHT_CTCU_H > +#include <linux/cdev.h> > + > #include "coresight-trace-id.h" > > /* Maximum number of supported ETR devices for a single CTCU. */ > @@ -13,10 +15,16 @@ > /** > * struct ctcu_etr_config > * @atid_offset: offset to the ATID0 Register. > - * @port_num: in-port number of CTCU device that connected to ETR. > + * @irq_ctrl_offset: offset to the BYTECNTRVAL register. > + * @irq_name: IRQ name in dt node. > + * @byte_cntr_name: name of the byte cntr device node. > + * @port_num: in-port number of the CTCU device that connected to ETR. > */ > struct ctcu_etr_config { > const u32 atid_offset; > + const u32 irq_ctrl_offset; > + const char *irq_name; > + const char *byte_cntr_name; > const u32 port_num; > }; > > @@ -25,15 +33,64 @@ struct ctcu_config { > int num_etr_config; > }; > > +/** > + * struct ctcu_byte_cntr > + * @c_dev: cdev for byte_cntr. > + * @sink csdev of sink device. > + * @enable: indicates that byte_cntr function is enabled or not. > + * @read_active: indicates that byte-cntr node is opened or not. > + * @thresh_val: threshold to trigger a interruption. > + * @total_size total size of transferred data. > + * @byte_cntr_irq: IRQ number. > + * @irq_cnt: IRQ count. > + * @wq: workqueue of reading ETR data. > + * @read_work: work of reading ETR data. > + * @spin_lock: spinlock of byte cntr data. > + * @r_offset: offset of the pointer where reading begins. > + * @w_offset: offset of the write pointer in the ETR buffer when > + * the byte cntr is stopped. > + * @irq_ctrl_offset: offset to the BYTECNTRVAL Register. > + * @name: the name of byte cntr device node. > + * @irq_name: IRQ name in DT. > + */ > +struct ctcu_byte_cntr { > + struct cdev c_dev; > + struct coresight_device *sink; > + bool enable; > + bool read_active; > + u32 thresh_val; > + u64 total_size; > + int byte_cntr_irq; > + atomic_t irq_cnt; > + wait_queue_head_t wq; > + struct work_struct read_work; > + raw_spinlock_t spin_lock; > + long r_offset; > + long w_offset; > + u32 irq_ctrl_offset; > + const char *name; > + const char *irq_name; > +}; > + > struct ctcu_drvdata { > void __iomem *base; > struct clk *apb_clk; > struct device *dev; > struct coresight_device *csdev; > + struct ctcu_byte_cntr byte_cntr_data[ETR_MAX_NUM]; > raw_spinlock_t spin_lock; > u32 atid_offset[ETR_MAX_NUM]; > /* refcnt for each traceid of each sink */ > u8 traceid_refcnt[ETR_MAX_NUM][CORESIGHT_TRACE_ID_RES_TOP]; > }; > > +/* Generic functions */ > +int ctcu_get_active_port(struct coresight_device *sink, struct coresight_device *helper); > + > +/* Byte-cntr functions */ > +void ctcu_byte_cntr_start(struct coresight_device *csdev, struct coresight_path *path); > +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path); > +int ctcu_byte_cntr_init(struct device *dev, struct ctcu_drvdata *drvdata, int port_num); > +void ctcu_byte_cntr_remove(struct ctcu_drvdata *drvdata); > + > #endif > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index ec636ab1fd75..5dc94e890927 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -1040,14 +1040,15 @@ static void tmc_free_etr_buf(struct etr_buf *etr_buf) > * Returns: The size of the linear data available @pos, with *bufpp > * updated to point to the buffer. > */ > -static ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf, > - u64 offset, size_t len, char **bufpp) > +ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf, > + u64 offset, size_t len, char **bufpp) > { > /* Adjust the length to limit this transaction to end of buffer */ > len = (len < (etr_buf->size - offset)) ? len : etr_buf->size - offset; > > return etr_buf->ops->get_data(etr_buf, (u64)offset, len, bufpp); > } > +EXPORT_SYMBOL_GPL(tmc_etr_buf_get_data); > > static inline s64 > tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset) > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index baedb4dcfc3f..2fc77fd4ea25 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -443,5 +443,7 @@ struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, > enum cs_mode mode, void *data); > extern const struct attribute_group coresight_etr_group; > long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); > +ssize_t tmc_etr_buf_get_data(struct etr_buf *etr_buf, u64 offset, size_t len, > + char **bufpp); > > #endif

9 months, 3 weeks

Re: [PATCH v1 0/4] coresight: ctcu: Enable byte-cntr function for TMC ETR

by Mike Leach

Hi, On Mon, 10 Mar 2025 at 09:05, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > > From: Jie Gan <jie.gan(a)oss.qualcomm.com> > > The byte-cntr function provided by the CTCU device is used to transfer data > from the ETR buffer to the userspace. An interrupt is tiggered if the data > size exceeds the threshold set in the BYTECNTRVAL register. The interrupt > handler counts the number of triggered interruptions and the read function > will read the data from the ETR buffer if the IRQ count is greater than 0. > Each successful read process will decrement the IRQ count by 1. > > The byte cntr function will start when the device node is opened for reading, > and the IRQ count will reset when the byte cntr function has stopped. When > the file node is opened, the w_offset of the ETR buffer will be read and > stored in byte_cntr_data, serving as the original r_offset (indicating > where reading starts) for the byte counter function. > > The work queue for the read operation will wake up once when ETR is stopped, > ensuring that the remaining data in the ETR buffer has been flushed based on > the w_offset read at the time of stopping. > > The following shell commands write threshold to BYTECNTRVAL registers. > > Only enable byte-cntr for ETR0: > echo 0x10000 > /sys/devices/platform/soc(a)0/4001000.ctcu/ctcu0/byte_cntr_val > > Enable byte-cntr for both ETR0 and ETR1(support both hex and decimal values): > echo 0x10000 4096 > /sys/devices/platform/soc(a)0/4001000.ctcu/ctcu0/byte_cntr_val > > Setting the BYTECNTRVAL registers to 0 disables the byte-cntr function. > Disable byte-cntr for ETR0: > echo 0 > /sys/devices/platform/soc(a)0/4001000.ctcu/ctcu0/byte_cntr_val > > Disable byte-cntr for both ETR0 and ETR1: > echo 0 0 > /sys/devices/platform/soc(a)0/4001000.ctcu/ctcu0/byte_cntr_val > > There is a minimum threshold to prevent generating too many interrupts. > The minimum threshold is 4096 bytes. The write process will fail if user try > to set the BYTECNTRVAL registers to a value less than 4096 bytes(except > for 0). > > Finally, the user can read data from the ETR buffer through the byte-cntr file > nodes located under /dev, for example reads data from the ETR0 buffer: > cat /dev/byte-cntr0 > > Way to enable and start byte-cntr for ETR0: > echo 0x10000 > /sys/devices/platform/soc(a)0/4001000.ctcu/ctcu0/byte_cntr_val > echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink > echo 1 > /sys/bus/coresight/devices/etm0/enable_source > cat /dev/byte-cntr0 > There is a significant issue with attempting to drain an ETR buffer while it is live in the way you appear to be doing. You have no way of knowing if the TMC hardware write pointer wraps and overtakes the point where you are currently reading. This could cause data corruption as TMC writes as you are reading, or contention for the buffer that affects the TMC write. Even if those two events do not occur, then the trace capture sequence is corrupted. Take a simple example - suppose we split the buffer into 4 blocks of trace, which are filled by the ETR buffer = 1, 2, 3, 4 Now you suppose you have read 1 & 2 into your userspace buffer / file. file = 1, 2 If there is now some system event that prevents your userspace code from running for a while, then it is possible that the ETR continues, wraps and the buffer is now buffer = 5, 6, 7, 4 Your next two reads will be 7, 4 file = 1, 2, 7, 4 This trace is now corrupt and will cause decode errors. There is no way for the decoder to determine that the interface between blocks 2 & 7 is not correct. If you are fortunate then this issue will cause an actual explicit decode error, if you are less fortunate then decode will continue but in fact be inaccurate, with no obvious way to detect the inaccuracy. We encountered this problem early in the development of the perf data collection. Even though perf was stopping the trace to copy the hardware buffer, it would concatenate unrelated trace blocks into the perf userspace buffer, which initially caused decoding errors. This is now mitigated in perf by marking boundaries and recording indexes of the boundaries, so the tool can reset the decoder at the start of non contiguous blocks. If you do not stop the TMC when draining the ETR buffer, you have no way of determining if this has occurred. Clearly using large buffers, split into smaller blocks can mitigate the possibility of a wrap in this way - but never eliminate it, especially given the extreme rate that trace data can be generated. Regards Mike > Jie Gan (4): > coresight: tmc: Introduce new APIs to get the RWP offset of ETR buffer > dt-bindings: arm: Add an interrupt property for Coresight CTCU > coresight: ctcu: Enable byte-cntr for TMC ETR devices > arm64: dts: qcom: sa8775p: Add interrupts to CTCU device > > .../bindings/arm/qcom,coresight-ctcu.yaml | 17 + > arch/arm64/boot/dts/qcom/sa8775p.dtsi | 5 + > drivers/hwtracing/coresight/Makefile | 2 +- > .../coresight/coresight-ctcu-byte-cntr.c | 339 ++++++++++++++++++ > .../hwtracing/coresight/coresight-ctcu-core.c | 96 ++++- > drivers/hwtracing/coresight/coresight-ctcu.h | 59 ++- > .../hwtracing/coresight/coresight-tmc-etr.c | 45 ++- > drivers/hwtracing/coresight/coresight-tmc.h | 3 + > 8 files changed, 556 insertions(+), 10 deletions(-) > create mode 100644 drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c > > -- > 2.34.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

9 months, 3 weeks

Re: [PATCH v1 1/4] coresight: tmc: Introduce new APIs to get the RWP offset of ETR buffer

by Mike Leach

Hi Jie, On Wed, 12 Mar 2025 at 01:21, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > > > > On 3/12/2025 12:49 AM, Mike Leach wrote: > > Hi, > > > > On Mon, 10 Mar 2025 at 09:04, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > >> > >> The new functions calculate and return the offset to the write pointer of > >> the ETR buffer based on whether the memory mode is SG, flat or reserved. > >> The functions have the RWP offset can directly read data from ETR buffer, > >> enabling the transfer of data to any required location. > >> > >> Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > >> --- > >> .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++ > >> drivers/hwtracing/coresight/coresight-tmc.h | 1 + > >> 2 files changed, 41 insertions(+) > >> > >> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > >> index eda7cdad0e2b..ec636ab1fd75 100644 > >> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > >> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > >> @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) > >> } > >> EXPORT_SYMBOL_GPL(tmc_free_sg_table); > >> > >> +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata) > >> +{ > >> + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr; > >> + u64 rwp; > >> + > > > > It is not valid to read RWP if the TMC is running. It must be in the > > stopped or disabled state - see the specifications for TMC /ETR > > > > It is likely that CSUNLOCK / CSLOCK are needed here too, along with > > the spinlock that protects drvdata > > > > See the code in coresight_tmc_etr.c :- > > > > e.g. in > > > > tmc_update_etr_buffer() > > > > ... > > <take spinlock> > > ... > > CS_UNLOCK(drvdata->base); > > tmc_flush_and_stop(drvdata); // this ensures tmc is stopped and > > flushed to memory - essential to ensure full formatted frame is in > > memory. > > tmc_sync_etr_buf(drvdata); // this function reads rwp. > > CS_LOCK(drvdata->base); > > <release spinlokc> > > > > This type of program flow is common to both sysfs and perf handling of > > TMC buffers. > > Hi Mike, > > I am fully understood your point here. > > The function is designed this way to read the w_offset (which may not be > entirely accurate because the etr buffer is not synced) when the Why would you ever base memory access on a pointer that is not entirely accurate? The manuals for TMC/ETR all state that reads to both RWP and RRP when the ETR is running return unknown values. These cannot be used to access the buffer, or determine how much of the buffer has been used on a running ETR. The ETR specification specifically states that it is not permitted to read the buffer data while the ETR is running, when configured in circular buffer mode - which is the mode used in the TMC-ETR linux drivers. Reading the buffer while ETR is running is only permitted if configured in Software FIFO mode 2 - were the ETR will stop on full and stall incoming trace until some data is read out, signalled to the ETR via the RURP. I also note that you are reading back the etr_buf data without doing any dma_sync operations that the perf and sysfs methods in the driver do, after stopping the tmc. > byte-cntr devnode is opened, aiming to reduce the length of redundant > trace data. In this case, we cannot ensure the TMC is stopped or > disabled. The specification requires that you must ensure the TMC is stopped to read these registers. >The byte-cntr only requires an offset to know where it can > start before the expected trace data gets into ETR buffer. > > The w_offset is also read when the byte-cntr function stops, which > occurs after the TMC is disabled. > > Maybe this is not a good idea and I should read r_offset upon open? > The primary goal for byte-cntr is trying to transfer useful trace data > from the ETR buffer to the userspace, if we start from r_offset, a large > number of redundant trace data which the user does not expect will be > transferred simultaneously. > > It is difficult to justify adding code to a driver that does not correspond to the specification of the hardware device. Regards Mike > > > >> + rwp = tmc_read_rwp(drvdata); > >> + return rwp - paddr; > >> +} > >> + > >> +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) > >> +{ > >> + struct etr_buf *etr_buf = drvdata->sysfs_buf; > >> + struct etr_sg_table *etr_table = etr_buf->private; > >> + struct tmc_sg_table *table = etr_table->sg_table; > >> + long w_offset; > >> + u64 rwp; > >> + > > > > Same comments as above > > > >> + rwp = tmc_read_rwp(drvdata); > >> + w_offset = tmc_sg_get_data_page_offset(table, rwp); > >> + > >> + return w_offset; > >> +} > >> + > >> +/* > >> + * Retrieve the offset to the write pointer of the ETR buffer based on whether > >> + * the memory mode is SG, flat or reserved. > >> + */ > >> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) > >> +{ > >> + struct etr_buf *etr_buf = drvdata->sysfs_buf; > >> + > > > > As this is an exported function, please ensure that the inputs are > > valid - check the pointers > > Sure, will do. > > Thanks, > Jie > > > > > Code to ensure TMC is flushed and stopped could be inserted here. > > > > Regards > > > > Mike > > > >> + if (etr_buf->mode == ETR_MODE_ETR_SG) > >> + return tmc_sg_get_rwp_offset(drvdata); > >> + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV) > >> + return tmc_flat_resrv_get_rwp_offset(drvdata); > >> + else > >> + return -EINVAL; > >> +} > >> +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset); > >> + > >> /* > >> * Alloc pages for the table. Since this will be used by the device, > >> * allocate the pages closer to the device (i.e, dev_to_node(dev) > >> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > >> index b48bc9a01cc0..baedb4dcfc3f 100644 > >> --- a/drivers/hwtracing/coresight/coresight-tmc.h > >> +++ b/drivers/hwtracing/coresight/coresight-tmc.h > >> @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void); > >> struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, > >> enum cs_mode mode, void *data); > >> extern const struct attribute_group coresight_etr_group; > >> +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); > >> > >> #endif > >> -- > >> 2.34.1 > >> > > > > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

9 months, 3 weeks

Re: [PATCH] coresight: configfs: Constify struct config_item_type

by Suzuki K Poulose

On Sun, 11 Aug 2024 11:30:20 +0200, Christophe JAILLET wrote: > 'struct config_item_type' is not modified in this driver. > > These structures are only used with config_group_init_type_name() which > takes a "const struct config_item_type *" as a 3rd argument or with > struct config_group.cg_item.ci_type which is also a "const struct > config_item_type *". > > [...] Applied, thanks! [1/1] coresight: configfs: Constify struct config_item_type https://git.kernel.org/coresight/c/b5060c17 Best regards, -- Suzuki K Poulose <suzuki.poulose(a)arm.com>

9 months, 3 weeks

Re: [PATCH] coresight: configfs: Constify struct config_item_type

by James Clark

On 11/08/2024 10:30 am, Christophe JAILLET wrote: > 'struct config_item_type' is not modified in this driver. > > These structures are only used with config_group_init_type_name() which > takes a "const struct config_item_type *" as a 3rd argument or with > struct config_group.cg_item.ci_type which is also a "const struct > config_item_type *". > > Constifying this structure moves some data to a read-only section, so > increase overall security, especially when the structure holds some > function pointers. > > On a x86_64, with allmodconfig: > Before: > ====== > text data bss dec hex filename > 4904 1376 136 6416 1910 drivers/hwtracing/coresight/coresight-syscfg-configfs.o > > After: > ===== > text data bss dec hex filename > 5264 1120 16 6400 1900 drivers/hwtracing/coresight/coresight-syscfg-configfs.o > > Signed-off-by: Christophe JAILLET <christophe.jaillet(a)wanadoo.fr> > --- > Compile tested-only. > --- > .../hwtracing/coresight/coresight-syscfg-configfs.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > index 433ede94dd63..213b4159b062 100644 > --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c > @@ -160,7 +160,7 @@ static struct configfs_attribute *cscfg_config_view_attrs[] = { > NULL, > }; > > -static struct config_item_type cscfg_config_view_type = { > +static const struct config_item_type cscfg_config_view_type = { > .ct_owner = THIS_MODULE, > .ct_attrs = cscfg_config_view_attrs, > }; > @@ -170,7 +170,7 @@ static struct configfs_attribute *cscfg_config_preset_attrs[] = { > NULL, > }; > > -static struct config_item_type cscfg_config_preset_type = { > +static const struct config_item_type cscfg_config_preset_type = { > .ct_owner = THIS_MODULE, > .ct_attrs = cscfg_config_preset_attrs, > }; > @@ -272,7 +272,7 @@ static struct configfs_attribute *cscfg_feature_view_attrs[] = { > NULL, > }; > > -static struct config_item_type cscfg_feature_view_type = { > +static const struct config_item_type cscfg_feature_view_type = { > .ct_owner = THIS_MODULE, > .ct_attrs = cscfg_feature_view_attrs, > }; > @@ -309,7 +309,7 @@ static struct configfs_attribute *cscfg_param_view_attrs[] = { > NULL, > }; > > -static struct config_item_type cscfg_param_view_type = { > +static const struct config_item_type cscfg_param_view_type = { > .ct_owner = THIS_MODULE, > .ct_attrs = cscfg_param_view_attrs, > }; > @@ -380,7 +380,7 @@ static struct config_group *cscfg_create_feature_group(struct cscfg_feature_desc > return &feat_view->group; > } > > -static struct config_item_type cscfg_configs_type = { > +static const struct config_item_type cscfg_configs_type = { > .ct_owner = THIS_MODULE, > }; > > @@ -414,7 +414,7 @@ void cscfg_configfs_del_config(struct cscfg_config_desc *config_desc) > } > } > > -static struct config_item_type cscfg_features_type = { > +static const struct config_item_type cscfg_features_type = { > .ct_owner = THIS_MODULE, > }; > Reviewed-by: James Clark <james.clark(a)linaro.org>

9 months, 3 weeks

[PATCH v3] coresight: Add a KUnit test for coresight_find_default_sink()

by James Clark

Add a test to confirm that default sink selection skips over an ETF and returns an ETR even if it's further away. This also makes it easier to add new unit tests in the future. Reviewed-by: Leo Yan <leo.yan(a)arm.com> Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v3: - Use CORESIGHT_DEV_SUBTYPE_SOURCE_BUS type instead of the default (CORESIGHT_DEV_SUBTYPE_SOURCE_PROC) so that the test still works even when TRBE sinks are registered. This also removes the need for the fake CPU ID callback. - Link to v2: https://lore.kernel.org/r/20250305-james-cs-kunit-test-v2-1-83ba682b976c@li… Changes in v2: - Let devm free everything rather than doing individual kfrees: "Like with managed drivers, KUnit-managed fake devices are automatically cleaned up when the test finishes, but can be manually cleaned up early with kunit_device_unregister()." - Link to v1: https://lore.kernel.org/r/20250225164639.522741-1-james.clark@linaro.org --- drivers/hwtracing/coresight/Kconfig | 9 +++ drivers/hwtracing/coresight/Makefile | 3 +- drivers/hwtracing/coresight/coresight-core.c | 1 + .../hwtracing/coresight/coresight-kunit-tests.c | 77 ++++++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index ecd7086a5b83..f064e3d172b3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -259,4 +259,13 @@ config CORESIGHT_DUMMY To compile this driver as a module, choose M here: the module will be called coresight-dummy. + +config CORESIGHT_KUNIT_TESTS + tristate "Enable Coresight unit tests" + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable Coresight unit tests. Only useful for development and not + intended for production. + endif diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 8e62c3150aeb..96f0dfedb1bf 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -51,5 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ coresight-cti-sysfs.o obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o -obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o -coresight-ctcu-y := coresight-ctcu-core.o +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index bd0a7edd38c9..b101aa133ceb 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -959,6 +959,7 @@ coresight_find_default_sink(struct coresight_device *csdev) } return csdev->def_sink; } +EXPORT_SYMBOL_GPL(coresight_find_default_sink); static int coresight_remove_sink_ref(struct device *dev, void *data) { diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c new file mode 100644 index 000000000000..87439769207c --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> +#include <kunit/device.h> +#include <linux/coresight.h> + +#include "coresight-priv.h" + +static struct coresight_device *coresight_test_device(struct device *dev) +{ + struct coresight_device *csdev = devm_kcalloc(dev, 1, + sizeof(struct coresight_device), + GFP_KERNEL); + csdev->pdata = devm_kcalloc(dev, 1, + sizeof(struct coresight_platform_data), + GFP_KERNEL); + return csdev; +} + +static void test_default_sink(struct kunit *test) +{ + /* + * ETM -> ETF -> ETR -> CATU + * ^ + * | default + */ + struct device *dev = kunit_device_register(test, "coresight_kunit"); + struct coresight_device *etm = coresight_test_device(dev), + *etf = coresight_test_device(dev), + *etr = coresight_test_device(dev), + *catu = coresight_test_device(dev); + struct coresight_connection conn = {}; + struct coresight_ops_source src_ops = {}; + struct coresight_ops etm_cs_ops = { .source_ops = &src_ops }; + + etm->type = CORESIGHT_DEV_TYPE_SOURCE; + /* + * Don't use CORESIGHT_DEV_SUBTYPE_SOURCE_PROC, that would always return + * a TRBE sink if one is registered. + */ + etm->subtype.source_subtype = CORESIGHT_DEV_SUBTYPE_SOURCE_BUS; + etm->ops = &etm_cs_ops; + etf->type = CORESIGHT_DEV_TYPE_LINKSINK; + etf->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; + etr->type = CORESIGHT_DEV_TYPE_SINK; + etr->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM; + catu->type = CORESIGHT_DEV_TYPE_HELPER; + + conn.src_dev = etm; + conn.dest_dev = etf; + coresight_add_out_conn(dev, etm->pdata, &conn); + + conn.src_dev = etf; + conn.dest_dev = etr; + coresight_add_out_conn(dev, etf->pdata, &conn); + + conn.src_dev = etr; + conn.dest_dev = catu; + coresight_add_out_conn(dev, etr->pdata, &conn); + + KUNIT_ASSERT_PTR_EQ(test, coresight_find_default_sink(etm), etr); +} + +static struct kunit_case coresight_testcases[] = { + KUNIT_CASE(test_default_sink), + {} +}; + +static struct kunit_suite coresight_test_suite = { + .name = "coresight_test_suite", + .test_cases = coresight_testcases, +}; + +kunit_test_suites(&coresight_test_suite); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("James Clark <james.clark(a)linaro.org>"); +MODULE_DESCRIPTION("Arm CoreSight KUnit tests"); --- base-commit: eeafe6a8790ea125252ca2e23c1a2469eaff1d9a change-id: 20250305-james-cs-kunit-test-3af1df2401e6 Best regards, -- James Clark <james.clark(a)linaro.org>

9 months, 3 weeks

Re: [PATCH v1 1/4] coresight: tmc: Introduce new APIs to get the RWP offset of ETR buffer

by Mike Leach

Hi, On Mon, 10 Mar 2025 at 09:04, Jie Gan <quic_jiegan(a)quicinc.com> wrote: > > The new functions calculate and return the offset to the write pointer of > the ETR buffer based on whether the memory mode is SG, flat or reserved. > The functions have the RWP offset can directly read data from ETR buffer, > enabling the transfer of data to any required location. > > Signed-off-by: Jie Gan <quic_jiegan(a)quicinc.com> > --- > .../hwtracing/coresight/coresight-tmc-etr.c | 40 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tmc.h | 1 + > 2 files changed, 41 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index eda7cdad0e2b..ec636ab1fd75 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -267,6 +267,46 @@ void tmc_free_sg_table(struct tmc_sg_table *sg_table) > } > EXPORT_SYMBOL_GPL(tmc_free_sg_table); > > +static long tmc_flat_resrv_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + dma_addr_t paddr = drvdata->sysfs_buf->hwaddr; > + u64 rwp; > + It is not valid to read RWP if the TMC is running. It must be in the stopped or disabled state - see the specifications for TMC /ETR It is likely that CSUNLOCK / CSLOCK are needed here too, along with the spinlock that protects drvdata See the code in coresight_tmc_etr.c :- e.g. in tmc_update_etr_buffer() ... <take spinlock> ... CS_UNLOCK(drvdata->base); tmc_flush_and_stop(drvdata); // this ensures tmc is stopped and flushed to memory - essential to ensure full formatted frame is in memory. tmc_sync_etr_buf(drvdata); // this function reads rwp. CS_LOCK(drvdata->base); <release spinlokc> This type of program flow is common to both sysfs and perf handling of TMC buffers. > + rwp = tmc_read_rwp(drvdata); > + return rwp - paddr; > +} > + > +static long tmc_sg_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf *etr_buf = drvdata->sysfs_buf; > + struct etr_sg_table *etr_table = etr_buf->private; > + struct tmc_sg_table *table = etr_table->sg_table; > + long w_offset; > + u64 rwp; > + Same comments as above > + rwp = tmc_read_rwp(drvdata); > + w_offset = tmc_sg_get_data_page_offset(table, rwp); > + > + return w_offset; > +} > + > +/* > + * Retrieve the offset to the write pointer of the ETR buffer based on whether > + * the memory mode is SG, flat or reserved. > + */ > +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata) > +{ > + struct etr_buf *etr_buf = drvdata->sysfs_buf; > + As this is an exported function, please ensure that the inputs are valid - check the pointers Code to ensure TMC is flushed and stopped could be inserted here. Regards Mike > + if (etr_buf->mode == ETR_MODE_ETR_SG) > + return tmc_sg_get_rwp_offset(drvdata); > + else if (etr_buf->mode == ETR_MODE_FLAT || etr_buf->mode == ETR_MODE_RESRV) > + return tmc_flat_resrv_get_rwp_offset(drvdata); > + else > + return -EINVAL; > +} > +EXPORT_SYMBOL_GPL(tmc_get_rwp_offset); > + > /* > * Alloc pages for the table. Since this will be used by the device, > * allocate the pages closer to the device (i.e, dev_to_node(dev) > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h > index b48bc9a01cc0..baedb4dcfc3f 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc.h > +++ b/drivers/hwtracing/coresight/coresight-tmc.h > @@ -442,5 +442,6 @@ void tmc_etr_remove_catu_ops(void); > struct etr_buf *tmc_etr_get_buffer(struct coresight_device *csdev, > enum cs_mode mode, void *data); > extern const struct attribute_group coresight_etr_group; > +long tmc_get_rwp_offset(struct tmc_drvdata *drvdata); > > #endif > -- > 2.34.1 > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

9 months, 3 weeks

[PATCH v2] coresight: Add a KUnit test for coresight_find_default_sink()

by James Clark

Add a test to confirm that default sink selection skips over an ETF and returns an ETR even if it's further away. This also makes it easier to add new unit tests in the future. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Changes in v2: - Let devm free everything rather than doing individual kfrees: "Like with managed drivers, KUnit-managed fake devices are automatically cleaned up when the test finishes, but can be manually cleaned up early with kunit_device_unregister()." - Link to v1: https://lore.kernel.org/r/20250225164639.522741-1-james.clark@linaro.org --- drivers/hwtracing/coresight/Kconfig | 9 +++ drivers/hwtracing/coresight/Makefile | 3 +- drivers/hwtracing/coresight/coresight-core.c | 1 + .../hwtracing/coresight/coresight-kunit-tests.c | 77 ++++++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig index ecd7086a5b83..f064e3d172b3 100644 --- a/drivers/hwtracing/coresight/Kconfig +++ b/drivers/hwtracing/coresight/Kconfig @@ -259,4 +259,13 @@ config CORESIGHT_DUMMY To compile this driver as a module, choose M here: the module will be called coresight-dummy. + +config CORESIGHT_KUNIT_TESTS + tristate "Enable Coresight unit tests" + depends on KUNIT + default KUNIT_ALL_TESTS + help + Enable Coresight unit tests. Only useful for development and not + intended for production. + endif diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile index 8e62c3150aeb..96f0dfedb1bf 100644 --- a/drivers/hwtracing/coresight/Makefile +++ b/drivers/hwtracing/coresight/Makefile @@ -51,5 +51,4 @@ coresight-cti-y := coresight-cti-core.o coresight-cti-platform.o \ coresight-cti-sysfs.o obj-$(CONFIG_ULTRASOC_SMB) += ultrasoc-smb.o obj-$(CONFIG_CORESIGHT_DUMMY) += coresight-dummy.o -obj-$(CONFIG_CORESIGHT_CTCU) += coresight-ctcu.o -coresight-ctcu-y := coresight-ctcu-core.o +obj-$(CONFIG_CORESIGHT_KUNIT_TESTS) += coresight-kunit-tests.o diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index bd0a7edd38c9..b101aa133ceb 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -959,6 +959,7 @@ coresight_find_default_sink(struct coresight_device *csdev) } return csdev->def_sink; } +EXPORT_SYMBOL_GPL(coresight_find_default_sink); static int coresight_remove_sink_ref(struct device *dev, void *data) { diff --git a/drivers/hwtracing/coresight/coresight-kunit-tests.c b/drivers/hwtracing/coresight/coresight-kunit-tests.c new file mode 100644 index 000000000000..a136af05eaf4 --- /dev/null +++ b/drivers/hwtracing/coresight/coresight-kunit-tests.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> +#include <kunit/device.h> +#include <linux/coresight.h> + +#include "coresight-priv.h" + +static struct coresight_device *coresight_test_device(struct device *dev) +{ + struct coresight_device *csdev = devm_kcalloc(dev, 1, + sizeof(struct coresight_device), + GFP_KERNEL); + csdev->pdata = devm_kcalloc(dev, 1, + sizeof(struct coresight_platform_data), + GFP_KERNEL); + return csdev; +} + +static int coresight_test_cpuid(struct coresight_device *csdev) +{ + return 0; +} + +static void test_default_sink(struct kunit *test) +{ + /* + * ETM -> ETF -> ETR -> CATU + * ^ + * | default + */ + struct device *dev = kunit_device_register(test, "coresight_kunit"); + struct coresight_device *etm = coresight_test_device(dev), + *etf = coresight_test_device(dev), + *etr = coresight_test_device(dev), + *catu = coresight_test_device(dev); + struct coresight_connection conn = {}; + struct coresight_ops_source src_ops = {.cpu_id = coresight_test_cpuid }; + struct coresight_ops etm_cs_ops = { .source_ops = &src_ops }; + + etm->type = CORESIGHT_DEV_TYPE_SOURCE; + etm->ops = &etm_cs_ops; + etf->type = CORESIGHT_DEV_TYPE_LINKSINK; + etf->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER; + etr->type = CORESIGHT_DEV_TYPE_SINK; + etr->subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM; + catu->type = CORESIGHT_DEV_TYPE_HELPER; + + conn.src_dev = etm; + conn.dest_dev = etf; + coresight_add_out_conn(dev, etm->pdata, &conn); + + conn.src_dev = etf; + conn.dest_dev = etr; + coresight_add_out_conn(dev, etf->pdata, &conn); + + conn.src_dev = etr; + conn.dest_dev = catu; + coresight_add_out_conn(dev, etr->pdata, &conn); + + KUNIT_ASSERT_PTR_EQ(test, coresight_find_default_sink(etm), etr); +} + +static struct kunit_case coresight_testcases[] = { + KUNIT_CASE(test_default_sink), + {} +}; + +static struct kunit_suite coresight_test_suite = { + .name = "coresight_test_suite", + .test_cases = coresight_testcases, +}; + +kunit_test_suites(&coresight_test_suite); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("James Clark <james.clark(a)linaro.org>"); +MODULE_DESCRIPTION("Arm CoreSight KUnit tests"); --- base-commit: eeafe6a8790ea125252ca2e23c1a2469eaff1d9a change-id: 20250305-james-cs-kunit-test-3af1df2401e6 Best regards, -- James Clark <james.clark(a)linaro.org>

9 months, 3 weeks

[PATCH] coresight: docs: Remove target sink from examples

by James Clark

Previously the sink had to be specified, but now it auto selects one by default. Including a sink in the examples causes issues when copy pasting the command because it might not work if that sink isn't present. Remove the sink from all the basic examples and create a new section specifically about overriding the default one. Make the text a but more concise now that it's in the advanced section, and similarly for removing the old kernel advice. Signed-off-by: James Clark <james.clark(a)linaro.org> --- Documentation/trace/coresight/coresight.rst | 41 ++++++++----------- .../userspace-api/perf_ring_buffer.rst | 4 +- 2 files changed, 18 insertions(+), 27 deletions(-) diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst index d4f93d6a2d63..806699871b80 100644 --- a/Documentation/trace/coresight/coresight.rst +++ b/Documentation/trace/coresight/coresight.rst @@ -462,44 +462,35 @@ queried by the perf command line tool: cs_etm// [Kernel PMU event] - linaro@linaro-nano:~$ - Regardless of the number of tracers available in a system (usually equal to the amount of processor cores), the "cs_etm" PMU will be listed only once. A Coresight PMU works the same way as any other PMU, i.e the name of the PMU is -listed along with configuration options within forward slashes '/'. Since a -Coresight system will typically have more than one sink, the name of the sink to -work with needs to be specified as an event option. -On newer kernels the available sinks are listed in sysFS under +provided along with configuration options within forward slashes '/' (see +`Config option formats`_). + +Advanced Perf framework usage +----------------------------- + +Sink selection +~~~~~~~~~~~~~~ + +An appropriate sink will be selected automatically for use with Perf, but since +there will typically be more than one sink, the name of the sink to use may be +specified as a special config option prefixed with '@'. + +The available sinks are listed in sysFS under ($SYSFS)/bus/event_source/devices/cs_etm/sinks/:: root@localhost:/sys/bus/event_source/devices/cs_etm/sinks# ls tmc_etf0 tmc_etr0 tpiu0 -On older kernels, this may need to be found from the list of coresight devices, -available under ($SYSFS)/bus/coresight/devices/:: - - root:~# ls /sys/bus/coresight/devices/ - etm0 etm1 etm2 etm3 etm4 etm5 funnel0 - funnel1 funnel2 replicator0 stm0 tmc_etf0 tmc_etr0 tpiu0 root@linaro-nano:~# perf record -e cs_etm/@tmc_etr0/u --per-thread program -As mentioned above in section "Device Naming scheme", the names of the devices could -look different from what is used in the example above. One must use the device names -as it appears under the sysFS. - -The syntax within the forward slashes '/' is important. The '@' character -tells the parser that a sink is about to be specified and that this is the sink -to use for the trace session. - More information on the above and other example on how to use Coresight with the perf tools can be found in the "HOWTO.md" file of the openCSD gitHub repository [#third]_. -Advanced perf framework usage ------------------------------ - AutoFDO analysis using the perf tools ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -508,7 +499,7 @@ perf can be used to record and analyze trace of programs. Execution can be recorded using 'perf record' with the cs_etm event, specifying the name of the sink to record to, e.g:: - perf record -e cs_etm/@tmc_etr0/u --per-thread + perf record -e cs_etm//u --per-thread The 'perf report' and 'perf script' commands can be used to analyze execution, synthesizing instruction and branch events from the instruction trace. @@ -572,7 +563,7 @@ sort example is from the AutoFDO tutorial (https://gcc.gnu.org/wiki/AutoFDO/Tuto Bubble sorting array of 30000 elements 5910 ms - $ perf record -e cs_etm/@tmc_etr0/u --per-thread taskset -c 2 ./sort + $ perf record -e cs_etm//u --per-thread taskset -c 2 ./sort Bubble sorting array of 30000 elements 12543 ms [ perf record: Woken up 35 times to write data ] diff --git a/Documentation/userspace-api/perf_ring_buffer.rst b/Documentation/userspace-api/perf_ring_buffer.rst index bde9d8cbc106..dc71544532ce 100644 --- a/Documentation/userspace-api/perf_ring_buffer.rst +++ b/Documentation/userspace-api/perf_ring_buffer.rst @@ -627,7 +627,7 @@ regular ring buffer. AUX events and AUX trace data are two different things. Let's see an example:: - perf record -a -e cycles -e cs_etm/@tmc_etr0/ -- sleep 2 + perf record -a -e cycles -e cs_etm// -- sleep 2 The above command enables two events: one is the event *cycles* from PMU and another is the AUX event *cs_etm* from Arm CoreSight, both are saved @@ -766,7 +766,7 @@ only record AUX trace data at a specific time point which users are interested in. E.g. below gives an example of how to take snapshots with 1 second interval with Arm CoreSight:: - perf record -e cs_etm/@tmc_etr0/u -S -a program & + perf record -e cs_etm//u -S -a program & PERFPID=$! while true; do kill -USR2 $PERFPID -- 2.34.1

9 months, 3 weeks

[PATCH v2 0/8] Arm CoreSight: Support AUX pause and resume

by Leo Yan

This series is to enable AUX pause and resume on Arm CoreSight. The first patch extracts the trace unit controlling operations to two functions. These two functions will be used by AUX pause and resume. Patches 02 and 03 change the ETMv4 driver to prepare callback functions for AUX pause and resume. Patch 04 changes the ETM perf layer to support AUX pause and resume in a perf session. The patches 05 and 06 offers an extra feature for updating buffer on AUX pause occasion, which can mitigate the trace data lose issue. Patch 07 documents the AUX pause usages with Arm CoreSight. The last patch syncs headers between user space and the kernel. This patch set has been verified on the Hikey960 board and TC platform. The previous one uses ETR and the later uses TRBE as sink. It is suggested to disable CPUIdle (add `nohlt` option in Linux command line) when verifying this series. ETM and funnel drivers are found issues during CPU suspend and resume which will be addressed separately. Changes from v1: - Added validation function pointers in pause and resume APIs (Mike). Leo Yan (8): coresight: etm4x: Extract the trace unit controlling coresight: Introduce pause and resume APIs for source coresight: etm4x: Hook pause and resume callbacks coresight: perf: Support AUX trace pause and resume coresight: etm: Add an attribute for updating buffer coresight: perf: Update buffer on AUX pause Documentation: coresight: Document AUX pause and resume perf cs-etm: Sync kernel coresight-pmu.h header .../trace/coresight/coresight-perf.rst | 50 ++++++ drivers/hwtracing/coresight/coresight-core.c | 20 +++ .../hwtracing/coresight/coresight-etm-perf.c | 94 +++++++++- .../hwtracing/coresight/coresight-etm-perf.h | 2 + .../coresight/coresight-etm4x-core.c | 166 ++++++++++++------ drivers/hwtracing/coresight/coresight-etm4x.h | 2 + drivers/hwtracing/coresight/coresight-priv.h | 2 + include/linux/coresight-pmu.h | 1 + include/linux/coresight.h | 4 + tools/include/linux/coresight-pmu.h | 1 + 10 files changed, 289 insertions(+), 53 deletions(-) -- 2.34.1

9 months, 3 weeks

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight