CoreSight

coresight@lists.linaro.org

3 participants
2723 discussions

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

by German Gomez

Hi, I'm Sending this sysfs addition to the etm4x driver in order to expose the timestamp source of the trace (given by the TRFCR_ELx.TS register). As mentioned in [1/2], having this information is useful for assigning Kernel times to the perf samples. Thanks, German German Gomez (3): coresight: etm4x: Expose default timestamp source in sysfs coresight: etm4x: docs: Add documentation for 'ts_source' sysfs interface perf cs_etm: Store ts_source in AUXTRACE_INFO fields .../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 | 34 ++++++++++ tools/perf/arch/arm/util/cs-etm.c | 64 +++++++++++++++++-- tools/perf/util/cs-etm.c | 61 +++++++++--------- tools/perf/util/cs-etm.h | 13 +++- 7 files changed, 159 insertions(+), 36 deletions(-) -- 2.25.1

4 years

Perf trace decode script

by Tanmay Jagdale

Hi Mike, We would like to know if the perf decode script (cs-trace-disasm.py) mentioned in the OpenCSD HOWTO page is hosted anywhere ? https://github.com/Linaro/OpenCSD/blob/master/HOWTO.md#trace-decoding-with-… Also, does the script support generating instruction trace for both user space and kernel space ? With Regards, Tanmay

4 years

Re: [PATCH v7 2/7] hwtracing: Add trace function support for HiSilicon PCIe Tune and Trace device

by Leo Yan

On Thu, Apr 07, 2022 at 08:58:36PM +0800, Yicong Yang via iommu wrote: > HiSilicon PCIe tune and trace device(PTT) is a PCIe Root Complex integrated > Endpoint(RCiEP) device, providing the capability to dynamically monitor and > tune the PCIe traffic, and trace the TLP headers. > > Add the driver for the device to enable the trace function. Register PMU > device of PTT trace, then users can use trace through perf command. The > driver makes use of perf AUX trace and support following events to > configure the trace: > > - filter: select Root port or Endpoint to trace > - type: select the type of traced TLP headers > - direction: select the direction of traced TLP headers > - format: select the data format of the traced TLP headers > > This patch adds the driver part of PTT trace. The perf command support of > PTT trace is added in the following patch. > > Signed-off-by: Yicong Yang <yangyicong(a)hisilicon.com> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron(a)huawei.com> > --- > drivers/Makefile | 1 + > drivers/hwtracing/Kconfig | 2 + > drivers/hwtracing/ptt/Kconfig | 12 + > drivers/hwtracing/ptt/Makefile | 2 + > drivers/hwtracing/ptt/hisi_ptt.c | 874 +++++++++++++++++++++++++++++++ > drivers/hwtracing/ptt/hisi_ptt.h | 166 ++++++ > 6 files changed, 1057 insertions(+) > create mode 100644 drivers/hwtracing/ptt/Kconfig > create mode 100644 drivers/hwtracing/ptt/Makefile > create mode 100644 drivers/hwtracing/ptt/hisi_ptt.c > create mode 100644 drivers/hwtracing/ptt/hisi_ptt.h > > diff --git a/drivers/Makefile b/drivers/Makefile > index 020780b6b4d2..662d50599467 100644 > --- a/drivers/Makefile > +++ b/drivers/Makefile > @@ -175,6 +175,7 @@ obj-$(CONFIG_USB4) += thunderbolt/ > obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/ > obj-y += hwtracing/intel_th/ > obj-$(CONFIG_STM) += hwtracing/stm/ > +obj-$(CONFIG_HISI_PTT) += hwtracing/ptt/ > obj-$(CONFIG_ANDROID) += android/ > obj-$(CONFIG_NVMEM) += nvmem/ > obj-$(CONFIG_FPGA) += fpga/ > diff --git a/drivers/hwtracing/Kconfig b/drivers/hwtracing/Kconfig > index 13085835a636..911ee977103c 100644 > --- a/drivers/hwtracing/Kconfig > +++ b/drivers/hwtracing/Kconfig > @@ -5,4 +5,6 @@ source "drivers/hwtracing/stm/Kconfig" > > source "drivers/hwtracing/intel_th/Kconfig" > > +source "drivers/hwtracing/ptt/Kconfig" > + > endmenu > diff --git a/drivers/hwtracing/ptt/Kconfig b/drivers/hwtracing/ptt/Kconfig > new file mode 100644 > index 000000000000..8902a6f27563 > --- /dev/null > +++ b/drivers/hwtracing/ptt/Kconfig > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +config HISI_PTT > + tristate "HiSilicon PCIe Tune and Trace Device" > + depends on ARM64 || (COMPILE_TEST && 64BIT) > + depends on PCI && HAS_DMA && HAS_IOMEM && PERF_EVENTS > + help > + HiSilicon PCIe Tune and Trace Device exists as a PCIe RCiEP > + device, and it provides support for PCIe traffic tuning and > + tracing TLP headers to the memory. > + > + This driver can also be built as a module. If so, the module > + will be called hisi_ptt. > diff --git a/drivers/hwtracing/ptt/Makefile b/drivers/hwtracing/ptt/Makefile > new file mode 100644 > index 000000000000..908c09a98161 > --- /dev/null > +++ b/drivers/hwtracing/ptt/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0 > +obj-$(CONFIG_HISI_PTT) += hisi_ptt.o > diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c > new file mode 100644 > index 000000000000..242b41870380 > --- /dev/null > +++ b/drivers/hwtracing/ptt/hisi_ptt.c > @@ -0,0 +1,874 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for HiSilicon PCIe tune and trace device > + * > + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd. > + * Author: Yicong Yang <yangyicong(a)hisilicon.com> > + */ > + > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/delay.h> > +#include <linux/dma-iommu.h> > +#include <linux/dma-mapping.h> > +#include <linux/interrupt.h> > +#include <linux/io.h> > +#include <linux/iommu.h> > +#include <linux/iopoll.h> > +#include <linux/module.h> > +#include <linux/sysfs.h> > +#include <linux/vmalloc.h> > + > +#include "hisi_ptt.h" > + > +static u16 hisi_ptt_get_filter_val(struct pci_dev *pdev) > +{ > + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) > + return BIT(HISI_PCIE_CORE_PORT_ID(PCI_SLOT(pdev->devfn))); > + > + return PCI_DEVID(pdev->bus->number, pdev->devfn); > +} > + > +static bool hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt) > +{ > + u32 val; > + > + return !readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_STS, > + val, val & HISI_PTT_TRACE_IDLE, > + HISI_PTT_WAIT_POLL_INTERVAL_US, > + HISI_PTT_WAIT_TRACE_TIMEOUT_US); > +} > + > +static bool hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt) > +{ > + u32 val; > + > + return !readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS, > + val, !val, HISI_PTT_RESET_POLL_INTERVAL_US, > + HISI_PTT_RESET_TIMEOUT_US); > +} > + > +static void hisi_ptt_free_trace_buf(struct hisi_ptt *hisi_ptt) > +{ > + struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > + struct device *dev = &hisi_ptt->pdev->dev; > + int i; > + > + if (!ctrl->trace_buf) > + return; > + > + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++) { > + if (ctrl->trace_buf[i].addr) > + dmam_free_coherent(dev, HISI_PTT_TRACE_BUF_SIZE, > + ctrl->trace_buf[i].addr, > + ctrl->trace_buf[i].dma); > + } > + > + devm_kfree(dev, ctrl->trace_buf); > + ctrl->trace_buf = NULL; > +} > + > +static int hisi_ptt_alloc_trace_buf(struct hisi_ptt *hisi_ptt) > +{ > + struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > + struct device *dev = &hisi_ptt->pdev->dev; > + int i; > + > + hisi_ptt->trace_ctrl.buf_index = 0; > + > + /* If the trace buffer has already been allocated, zero it. */ > + if (ctrl->trace_buf) { > + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++) > + memset(ctrl->trace_buf[i].addr, 0, HISI_PTT_TRACE_BUF_SIZE); > + return 0; > + } > + > + ctrl->trace_buf = devm_kcalloc(dev, HISI_PTT_TRACE_BUF_CNT, > + sizeof(struct hisi_ptt_dma_buffer), GFP_KERNEL); > + if (!ctrl->trace_buf) > + return -ENOMEM; > + > + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; ++i) { > + ctrl->trace_buf[i].addr = dmam_alloc_coherent(dev, HISI_PTT_TRACE_BUF_SIZE, > + &ctrl->trace_buf[i].dma, > + GFP_KERNEL); > + if (!ctrl->trace_buf[i].addr) { > + hisi_ptt_free_trace_buf(hisi_ptt); > + return -ENOMEM; > + } > + } > + > + return 0; > +} > + > +static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt) > +{ > + writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + hisi_ptt->trace_ctrl.started = false; > +} > + > +static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt) > +{ > + struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > + u32 val; > + int i; > + > + /* Check device idle before start trace */ > + if (!hisi_ptt_wait_trace_hw_idle(hisi_ptt)) { > + pci_err(hisi_ptt->pdev, "Failed to start trace, the device is still busy\n"); > + return -EBUSY; > + } > + > + ctrl->started = true; > + > + /* Reset the DMA before start tracing */ > + val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + val |= HISI_PTT_TRACE_CTRL_RST; > + writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + > + hisi_ptt_wait_dma_reset_done(hisi_ptt); > + > + val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + val &= ~HISI_PTT_TRACE_CTRL_RST; > + writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + > + /* Clear the interrupt status */ > + writel(HISI_PTT_TRACE_INT_STAT_MASK, hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT); > + writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_INT_MASK); > + > + /* Configure the trace DMA buffer */ > + for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++) { > + writel(lower_32_bits(ctrl->trace_buf[i].dma), > + hisi_ptt->iobase + HISI_PTT_TRACE_ADDR_BASE_LO_0 + > + i * HISI_PTT_TRACE_ADDR_STRIDE); > + writel(upper_32_bits(ctrl->trace_buf[i].dma), > + hisi_ptt->iobase + HISI_PTT_TRACE_ADDR_BASE_HI_0 + > + i * HISI_PTT_TRACE_ADDR_STRIDE); > + } > + writel(HISI_PTT_TRACE_BUF_SIZE, hisi_ptt->iobase + HISI_PTT_TRACE_ADDR_SIZE); > + > + /* Set the trace control register */ > + val = FIELD_PREP(HISI_PTT_TRACE_CTRL_TYPE_SEL, ctrl->type); > + val |= FIELD_PREP(HISI_PTT_TRACE_CTRL_RXTX_SEL, ctrl->direction); > + val |= FIELD_PREP(HISI_PTT_TRACE_CTRL_DATA_FORMAT, ctrl->format); > + val |= FIELD_PREP(HISI_PTT_TRACE_CTRL_TARGET_SEL, hisi_ptt->trace_ctrl.filter); > + if (!hisi_ptt->trace_ctrl.is_port) > + val |= HISI_PTT_TRACE_CTRL_FILTER_MODE; > + > + /* Start the Trace */ > + val |= HISI_PTT_TRACE_CTRL_EN; > + writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL); > + > + return 0; > +} > + > +static int hisi_ptt_update_aux(struct hisi_ptt *hisi_ptt, int index, bool stop) > +{ > + struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > + struct perf_output_handle *handle = &ctrl->handle; > + struct perf_event *event = handle->event; > + struct hisi_ptt_pmu_buf *buf; > + void *addr; > + > + buf = perf_get_aux(handle); > + if (!buf || !handle->size) > + return -EINVAL; > + > + addr = ctrl->trace_buf[ctrl->buf_index].addr; > + > + memcpy(buf->base + buf->pos, addr, HISI_PTT_TRACE_BUF_SIZE); > + memset(addr, 0, HISI_PTT_TRACE_BUF_SIZE); I am a bit worry buffer usages, at least for below aspects: The first thing is for memset(), which cleans up the buffer and the buffer size is 4MiB, this means it will consume much CPU time to cleanup the buffer, and trace_buf is mapped as non-cacheable, the performance would be get worse. The second thing is here it always copies the trace data with size HISI_PTT_TRACE_BUF_SIZE, usually, the trace module can provide a read pointer register, so you can know the trace data length based on the delta value between write and read pointers. The last thing is the ctrl->trace_buf[] works as bounce buffer, so it means actually there have an extra copy from bounce buffer to AUX buffer, is it possible to directly output PTT trace data to AUX buffer? Sorry if I bring up duplicate questions and before have the simliar discussion when reviewed the patch. > + buf->pos += HISI_PTT_TRACE_BUF_SIZE; > + > + if (stop) { > + perf_aux_output_end(handle, buf->pos); > + } else if (buf->length - buf->pos < HISI_PTT_TRACE_BUF_SIZE) { > + perf_aux_output_skip(handle, buf->length - buf->pos); > + perf_aux_output_end(handle, buf->pos); > + > + buf = perf_aux_output_begin(handle, event); > + if (!buf) > + return -EINVAL; > + > + buf->pos = handle->head % buf->length; > + if (buf->length - buf->pos < HISI_PTT_TRACE_BUF_SIZE) { > + perf_aux_output_end(handle, 0); > + return -EINVAL; > + } > + } > + > + return 0; > +} > + > +static irqreturn_t hisi_ptt_isr(int irq, void *context) > +{ > + struct hisi_ptt *hisi_ptt = context; > + u32 status, buf_idx; > + > + status = readl(hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT); > + buf_idx = ffs(status) - 1; > + > + /* Clear the interrupt status of buffer @buf_idx */ > + writel(status, hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT); > + > + /* > + * Update the AUX buffer and cache the current buffer index, > + * as we need to know this and save the data when the trace > + * is ended out of the interrupt handler. End the trace > + * if the updating fails. > + */ > + if (hisi_ptt_update_aux(hisi_ptt, buf_idx, false)) > + hisi_ptt_trace_end(hisi_ptt); > + else > + hisi_ptt->trace_ctrl.buf_index = (buf_idx + 1) % HISI_PTT_TRACE_BUF_CNT; > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t hisi_ptt_irq(int irq, void *context) > +{ > + struct hisi_ptt *hisi_ptt = context; > + u32 status; > + > + status = readl(hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT); > + if (!(status & HISI_PTT_TRACE_INT_STAT_MASK)) > + return IRQ_NONE; > + > + return IRQ_WAKE_THREAD; > +} > + > +static void hisi_ptt_irq_free_vectors(void *pdev) > +{ > + pci_free_irq_vectors(pdev); > +} > + > +static int hisi_ptt_register_irq(struct hisi_ptt *hisi_ptt) > +{ > + struct pci_dev *pdev = hisi_ptt->pdev; > + int ret; > + > + ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI); > + if (ret < 0) { > + pci_err(pdev, "failed to allocate irq vector, ret = %d\n", ret); > + return ret; > + } > + > + ret = devm_add_action_or_reset(&pdev->dev, hisi_ptt_irq_free_vectors, pdev); > + if (ret < 0) > + return ret; > + > + ret = devm_request_threaded_irq(&pdev->dev, > + pci_irq_vector(pdev, HISI_PTT_TRACE_DMA_IRQ), > + hisi_ptt_irq, hisi_ptt_isr, 0, > + DRV_NAME, hisi_ptt); > + if (ret) { > + pci_err(pdev, "failed to request irq %d, ret = %d\n", > + pci_irq_vector(pdev, HISI_PTT_TRACE_DMA_IRQ), ret); > + return ret; > + } > + > + return 0; > +} > + > +static int hisi_ptt_init_filters(struct pci_dev *pdev, void *data) > +{ > + struct hisi_ptt_filter_desc *filter; > + struct hisi_ptt *hisi_ptt = data; > + struct list_head *target_list; > + > + target_list = pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT ? > + &hisi_ptt->port_filters : &hisi_ptt->req_filters; > + > + filter = kzalloc(sizeof(*filter), GFP_KERNEL); > + if (!filter) { > + pci_err(hisi_ptt->pdev, "failed to add filter %s\n", pci_name(pdev)); > + return -ENOMEM; > + } > + > + filter->pdev = pdev; > + list_add_tail(&filter->list, target_list); > + > + /* Update the available port mask */ > + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) > + hisi_ptt->port_mask |= hisi_ptt_get_filter_val(pdev); > + > + return 0; > +} > + > +static void hisi_ptt_release_filters(void *data) > +{ > + struct hisi_ptt_filter_desc *filter, *tfilter; > + struct hisi_ptt *hisi_ptt = data; > + > + list_for_each_entry_safe(filter, tfilter, &hisi_ptt->req_filters, list) { > + list_del(&filter->list); > + kfree(filter); > + } > + > + list_for_each_entry_safe(filter, tfilter, &hisi_ptt->port_filters, list) { > + list_del(&filter->list); > + kfree(filter); > + } > +} > + > +static int hisi_ptt_init_ctrls(struct hisi_ptt *hisi_ptt) > +{ > + struct pci_dev *pdev = hisi_ptt->pdev; > + struct pci_bus *bus; > + int ret; > + u32 reg; > + > + INIT_LIST_HEAD(&hisi_ptt->port_filters); > + INIT_LIST_HEAD(&hisi_ptt->req_filters); > + > + /* > + * The device range register provides the information about the > + * root ports which the RCiEP can control and trace. The RCiEP > + * and the root ports it support are on the same PCIe core, with > + * same domain number but maybe different bus number. The device > + * range register will tell us which root ports we can support, > + * Bit[31:16] indicates the upper BDF numbers of the root port, > + * while Bit[15:0] indicates the lower. > + */ > + reg = readl(hisi_ptt->iobase + HISI_PTT_DEVICE_RANGE); > + hisi_ptt->upper = FIELD_GET(HISI_PTT_DEVICE_RANGE_UPPER, reg); > + hisi_ptt->lower = FIELD_GET(HISI_PTT_DEVICE_RANGE_LOWER, reg); > + > + bus = pci_find_bus(pci_domain_nr(pdev->bus), PCI_BUS_NUM(hisi_ptt->upper)); > + if (bus) > + pci_walk_bus(bus, hisi_ptt_init_filters, hisi_ptt); > + > + ret = devm_add_action_or_reset(&pdev->dev, hisi_ptt_release_filters, hisi_ptt); > + if (ret) > + return ret; > + > + hisi_ptt->trace_ctrl.default_cpu = cpumask_first(cpumask_of_node(dev_to_node(&pdev->dev))); > + > + return 0; > +} > + > +#define HISI_PTT_PMU_FILTER_IS_PORT BIT(19) > +#define HISI_PTT_PMU_FILTER_VAL_MASK GENMASK(15, 0) > +#define HISI_PTT_PMU_DIRECTION_MASK GENMASK(23, 20) > +#define HISI_PTT_PMU_TYPE_MASK GENMASK(31, 24) > +#define HISI_PTT_PMU_FORMAT_MASK GENMASK(35, 32) > + > +static ssize_t available_root_port_filters_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(dev_get_drvdata(dev)); > + struct hisi_ptt_filter_desc *filter; > + int pos = 0; > + > + mutex_lock(&hisi_ptt->mutex); > + if (list_empty(&hisi_ptt->port_filters)) { > + pos = sysfs_emit(buf, "\n"); > + goto out; > + } > + > + list_for_each_entry(filter, &hisi_ptt->port_filters, list) > + pos += sysfs_emit_at(buf, pos, "%s 0x%05lx\n", > + pci_name(filter->pdev), > + hisi_ptt_get_filter_val(filter->pdev) | > + HISI_PTT_PMU_FILTER_IS_PORT); > + > +out: > + mutex_unlock(&hisi_ptt->mutex); > + return pos; > +} > +static DEVICE_ATTR_ADMIN_RO(available_root_port_filters); > + > +static ssize_t available_requester_filters_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(dev_get_drvdata(dev)); > + struct hisi_ptt_filter_desc *filter; > + int pos = 0; > + > + mutex_lock(&hisi_ptt->mutex); > + if (list_empty(&hisi_ptt->req_filters)) { > + pos = sysfs_emit(buf, "\n"); > + goto out; > + } > + > + list_for_each_entry(filter, &hisi_ptt->req_filters, list) > + pos += sysfs_emit_at(buf, pos, "%s 0x%05x\n", > + pci_name(filter->pdev), > + hisi_ptt_get_filter_val(filter->pdev)); > + > +out: > + mutex_unlock(&hisi_ptt->mutex); > + return pos; > +} > +static DEVICE_ATTR_ADMIN_RO(available_requester_filters); > + > +PMU_FORMAT_ATTR(filter, "config:0-19"); > +PMU_FORMAT_ATTR(direction, "config:20-23"); > +PMU_FORMAT_ATTR(type, "config:24-31"); > +PMU_FORMAT_ATTR(format, "config:32-35"); > + > +static struct attribute *hisi_ptt_pmu_format_attrs[] = { > + &format_attr_filter.attr, > + &format_attr_direction.attr, > + &format_attr_type.attr, > + &format_attr_format.attr, > + NULL > +}; > + > +static struct attribute_group hisi_ptt_pmu_format_group = { > + .name = "format", > + .attrs = hisi_ptt_pmu_format_attrs, > +}; > + > +static struct attribute *hisi_ptt_pmu_filter_attrs[] = { > + &dev_attr_available_root_port_filters.attr, > + &dev_attr_available_requester_filters.attr, > + NULL > +}; > + > +static struct attribute_group hisi_ptt_pmu_filter_group = { > + .attrs = hisi_ptt_pmu_filter_attrs, > +}; > + > +static const struct attribute_group *hisi_ptt_pmu_groups[] = { > + &hisi_ptt_pmu_format_group, > + &hisi_ptt_pmu_filter_group, > + NULL > +}; > + > +/* > + * Check whether the config is valid or not. Some configs are multi-selectable > + * and can be set simultaneously, while some are single selectable (onehot). > + * Use this function to check the non-onehot configs while > + * hisi_ptt_trace_valid_config_onehot() for the onehot ones. > + */ > +static int hisi_ptt_trace_valid_config(u32 val, const u32 *available_list, u32 list_size) > +{ > + int i; > + > + /* The non-onehot configs cannot be 0. */ > + if (!val) > + return -EINVAL; > + > + /* > + * Walk the available list and clear the valid bits of > + * the config. If there is any resident bit after the > + * walk then the config is invalid. > + */ > + for (i = 0; i < list_size; i++) > + val &= ~available_list[i]; > + > + return val ? -EINVAL : 0; > +} > + > +static int hisi_ptt_trace_valid_config_onehot(u32 val, const u32 *available_list, u32 list_size) > +{ > + int i; > + > + for (i = 0; i < list_size; i++) > + if (val == available_list[i]) > + return 0; > + > + return -EINVAL; > +} > + > +static int hisi_ptt_trace_init_filter(struct hisi_ptt *hisi_ptt, u64 config) > +{ > + unsigned long val, port_mask = hisi_ptt->port_mask; > + struct hisi_ptt_filter_desc *filter; > + int ret = -EINVAL; > + > + hisi_ptt->trace_ctrl.is_port = FIELD_GET(HISI_PTT_PMU_FILTER_IS_PORT, config); > + val = FIELD_GET(HISI_PTT_PMU_FILTER_VAL_MASK, config); > + > + /* > + * Port filters are defined as bit mask. For port filters, check > + * the bits in the @val are within the range of hisi_ptt->port_mask > + * and whether it's empty or not, otherwise user has specified > + * some unsupported root ports. > + * > + * For Requester ID filters, walk the available filter list to see > + * whether we have one matched. > + */ > + if (!hisi_ptt->trace_ctrl.is_port) { > + list_for_each_entry(filter, &hisi_ptt->req_filters, list) > + if (val == hisi_ptt_get_filter_val(filter->pdev)) { > + ret = 0; > + break; > + } > + } else if (bitmap_subset(&val, &port_mask, BITS_PER_LONG)) { > + ret = 0; > + } > + > + if (ret) > + return ret; > + > + hisi_ptt->trace_ctrl.filter = val; > + return 0; > +} > + > +static int hisi_ptt_pmu_event_init(struct perf_event *event) > +{ > + /* > + * The supported value of the direction parameter. See hisi_ptt.rst > + * documentation for more details. > + */ > + static const u32 hisi_ptt_trace_available_direction[] = { > + 0, > + 1, > + 2, > + 3, > + }; > + /* Different types can be set simultaneously */ > + static const u32 hisi_ptt_trace_available_type[] = { > + 1, /* posted_request */ > + 2, /* non-posted_request */ > + 4, /* completion */ > + }; > + static const u32 hisi_ptt_trace_availble_format[] = { > + 0, /* 4DW */ > + 1, /* 8DW */ > + }; > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(event->pmu); > + struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl; > + int ret; > + u32 val; > + > + if (event->attr.type != hisi_ptt->hisi_ptt_pmu.type) > + return -ENOENT; Does PTT support per-task mode? Usually PMU not binding to CPU cannot support per-task mode, if this is the case, please add checking like below (copied code from arm_smmuv3_pmu.c): if (event->cpu < 0) { dev_dbg(dev, "Per-task mode not supported\n"); return -EOPNOTSUPP; } > + > + mutex_lock(&hisi_ptt->mutex); Here really need mutex for protection? I think the question can be extended as "if the mutex is necessary for the whole driver"? The reason is the perf core layer has provided the APIs perf_event_ctx_lock() and perf_event_ctx_unlock() to protect event context, and perf core layer has the mutex 'event->mmap_mutex' to protect setup_aux(). Please note, I think init_event() callback is not protected by lock, so you could see many PMU drivers use init_event() callback to validate if the event parameters (e.g. if can work as group events with other hardware events, support stack track, etc). This means below initialise functions should be moved to add() or setup_aux() callbacks, since these functions have been protected by mutex in perf core layer. > + > + ret = hisi_ptt_trace_init_filter(hisi_ptt, event->attr.config); > + if (ret < 0) > + goto out; > + > + val = FIELD_GET(HISI_PTT_PMU_DIRECTION_MASK, event->attr.config); > + ret = hisi_ptt_trace_valid_config_onehot(val, hisi_ptt_trace_available_direction, > + ARRAY_SIZE(hisi_ptt_trace_available_direction)); > + if (ret < 0) > + goto out; > + ctrl->direction = val; > + > + val = FIELD_GET(HISI_PTT_PMU_TYPE_MASK, event->attr.config); > + ret = hisi_ptt_trace_valid_config(val, hisi_ptt_trace_available_type, > + ARRAY_SIZE(hisi_ptt_trace_available_type)); > + if (ret < 0) > + goto out; > + ctrl->type = val; > + > + val = FIELD_GET(HISI_PTT_PMU_FORMAT_MASK, event->attr.config); > + ret = hisi_ptt_trace_valid_config_onehot(val, hisi_ptt_trace_availble_format, > + ARRAY_SIZE(hisi_ptt_trace_availble_format)); > + if (ret < 0) > + goto out; > + ctrl->format = val; > + > +out: > + mutex_unlock(&hisi_ptt->mutex); > + return ret; > +} > + > +static void *hisi_ptt_pmu_setup_aux(struct perf_event *event, void **pages, > + int nr_pages, bool overwrite) > +{ > + struct hisi_ptt_pmu_buf *buf; > + struct page **pagelist; > + int i; > + > + if (overwrite) { > + dev_warn(event->pmu->dev, "Overwrite mode is not supported\n"); > + return NULL; > + } > + > + /* If the pages size less than buffers, we cannot start trace */ > + if (nr_pages < HISI_PTT_TRACE_TOTAL_BUF_SIZE / PAGE_SIZE) > + return NULL; > + > + buf = kzalloc(sizeof(*buf), GFP_KERNEL); > + if (!buf) > + return NULL; > + > + pagelist = kcalloc(nr_pages, sizeof(*pagelist), GFP_KERNEL); > + if (!pagelist) { > + kfree(buf); > + return NULL; > + } > + > + for (i = 0; i < nr_pages; i++) > + pagelist[i] = virt_to_page(pages[i]); > + > + buf->base = vmap(pagelist, nr_pages, VM_MAP, PAGE_KERNEL); > + if (!buf->base) { > + kfree(pagelist); > + kfree(buf); > + return NULL; > + } > + > + buf->nr_pages = nr_pages; > + buf->length = nr_pages * PAGE_SIZE; > + buf->pos = 0; > + > + kfree(pagelist); > + return buf; > +} > + > +static void hisi_ptt_pmu_free_aux(void *aux) > +{ > + struct hisi_ptt_pmu_buf *buf = aux; > + > + vunmap(buf->base); > + kfree(buf); > +} > + > +static void hisi_ptt_pmu_start(struct perf_event *event, int flags) > +{ > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(event->pmu); > + struct perf_output_handle *handle = &hisi_ptt->trace_ctrl.handle; > + struct hw_perf_event *hwc = &event->hw; > + struct hisi_ptt_pmu_buf *buf; > + int cpu = event->cpu; > + int ret; > + > + hwc->state = 0; > + mutex_lock(&hisi_ptt->mutex); > + if (hisi_ptt->trace_ctrl.started) { > + pci_dbg(hisi_ptt->pdev, "trace has already started\n"); Essentialy this is not a PCI driver, it's good to use dev_dbg() variant in the whole driver. > + goto stop; > + } > + > + if (cpu == -1) > + cpu = hisi_ptt->trace_ctrl.default_cpu; > + > + /* > + * Handle the interrupt on the same cpu which starts the trace to avoid > + * context mismatch. Otherwise we'll trigger the WARN from the perf > + * core in event_function_local(). > + */ > + WARN_ON(irq_set_affinity(pci_irq_vector(hisi_ptt->pdev, HISI_PTT_TRACE_DMA_IRQ), > + cpumask_of(cpu))); Nitpick: Can set the irq affinity in the probe phase? Seems to me, the driver has enough info to set irq affinity in the probe phase, it's not very necessary to defer to set irq affinity at here. > + > + ret = hisi_ptt_alloc_trace_buf(hisi_ptt); > + if (ret) { > + pci_dbg(hisi_ptt->pdev, "alloc trace buf failed, ret = %d\n", ret); > + goto stop; > + } It's not a good idea to allocate buffer in the start(). The reason is start() function might be invoked for multiple times in a perf session. I understand the PTT driver doesn't support per-thread mode and snapshot mode, so the start() callback will be not invoked for multiple times, but you might need to consider if later support snapshot mode. I think it's better to move the buffer allocation into setup_aux(). Will continue to review the perf userspace patch. Thanks, Leo > + > + buf = perf_aux_output_begin(handle, event); > + if (!buf) { > + pci_dbg(hisi_ptt->pdev, "aux output begin failed\n"); > + goto stop; > + } > + > + buf->pos = handle->head % buf->length; > + > + ret = hisi_ptt_trace_start(hisi_ptt); > + if (ret) { > + pci_dbg(hisi_ptt->pdev, "trace start failed, ret = %d\n", ret); > + perf_aux_output_end(handle, 0); > + goto stop; > + } > + > + mutex_unlock(&hisi_ptt->mutex); > + return; > +stop: > + event->hw.state |= PERF_HES_STOPPED; > + mutex_unlock(&hisi_ptt->mutex); > +} > + > +static void hisi_ptt_pmu_stop(struct perf_event *event, int flags) > +{ > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(event->pmu); > + struct hw_perf_event *hwc = &event->hw; > + > + if (hwc->state & PERF_HES_STOPPED) > + return; > + > + mutex_lock(&hisi_ptt->mutex); > + if (hisi_ptt->trace_ctrl.started) { > + hisi_ptt_trace_end(hisi_ptt); > + WARN(!hisi_ptt_wait_trace_hw_idle(hisi_ptt), "Device is still busy"); > + hisi_ptt_update_aux(hisi_ptt, hisi_ptt->trace_ctrl.buf_index, true); > + } > + mutex_unlock(&hisi_ptt->mutex); > + > + hwc->state |= PERF_HES_STOPPED; > + perf_event_update_userpage(event); > + hwc->state |= PERF_HES_UPTODATE; > +} > + > +static int hisi_ptt_pmu_add(struct perf_event *event, int flags) > +{ > + struct hisi_ptt *hisi_ptt = to_hisi_ptt(event->pmu); > + struct hw_perf_event *hwc = &event->hw; > + int cpu = event->cpu; > + > + /* > + * Only allow the default cpu to add the event if user doesn't specify > + * the cpus. > + */ > + if (cpu == -1 && smp_processor_id() != hisi_ptt->trace_ctrl.default_cpu) > + return 0; > + > + hwc->state = PERF_HES_STOPPED | PERF_HES_UPTODATE; > + > + if (flags & PERF_EF_START) { > + hisi_ptt_pmu_start(event, PERF_EF_RELOAD); > + if (hwc->state & PERF_HES_STOPPED) > + return -EINVAL; > + } > + > + return 0; > +} > + > +static void hisi_ptt_pmu_del(struct perf_event *event, int flags) > +{ > + hisi_ptt_pmu_stop(event, PERF_EF_UPDATE); > +} > + > +static int hisi_ptt_register_pmu(struct hisi_ptt *hisi_ptt) > +{ > + u16 core_id, sicl_id; > + char *pmu_name; > + u32 reg; > + > + hisi_ptt->hisi_ptt_pmu = (struct pmu) { > + .module = THIS_MODULE, > + .capabilities = PERF_PMU_CAP_EXCLUSIVE | PERF_PMU_CAP_ITRACE, > + .task_ctx_nr = perf_sw_context, > + .attr_groups = hisi_ptt_pmu_groups, > + .event_init = hisi_ptt_pmu_event_init, > + .setup_aux = hisi_ptt_pmu_setup_aux, > + .free_aux = hisi_ptt_pmu_free_aux, > + .start = hisi_ptt_pmu_start, > + .stop = hisi_ptt_pmu_stop, > + .add = hisi_ptt_pmu_add, > + .del = hisi_ptt_pmu_del, > + }; > + > + reg = readl(hisi_ptt->iobase + HISI_PTT_LOCATION); > + core_id = FIELD_GET(HISI_PTT_CORE_ID, reg); > + sicl_id = FIELD_GET(HISI_PTT_SICL_ID, reg); > + > + pmu_name = devm_kasprintf(&hisi_ptt->pdev->dev, GFP_KERNEL, "hisi_ptt%u_%u", > + sicl_id, core_id); > + if (!pmu_name) > + return -ENOMEM; > + > + return perf_pmu_register(&hisi_ptt->hisi_ptt_pmu, pmu_name, -1); > +} > + > +/* > + * The DMA of PTT trace can only use direct mapping, due to some > + * hardware restriction. Check whether there is an IOMMU or the > + * policy of the IOMMU domain is passthrough, otherwise the trace > + * cannot work. > + * > + * The PTT device is supposed to behind the ARM SMMUv3, which > + * should have passthrough the device by a quirk. > + */ > +static int hisi_ptt_check_iommu_mapping(struct pci_dev *pdev) > +{ > + struct iommu_domain *iommu_domain; > + > + iommu_domain = iommu_get_domain_for_dev(&pdev->dev); > + if (!iommu_domain || iommu_domain->type == IOMMU_DOMAIN_IDENTITY) > + return 0; > + > + return -EOPNOTSUPP; > +} > + > +static int hisi_ptt_probe(struct pci_dev *pdev, > + const struct pci_device_id *id) > +{ > + struct hisi_ptt *hisi_ptt; > + int ret; > + > + ret = hisi_ptt_check_iommu_mapping(pdev); > + if (ret) { > + pci_err(pdev, "requires direct DMA mappings\n"); > + return ret; > + } > + > + hisi_ptt = devm_kzalloc(&pdev->dev, sizeof(*hisi_ptt), GFP_KERNEL); > + if (!hisi_ptt) > + return -ENOMEM; > + > + mutex_init(&hisi_ptt->mutex); > + hisi_ptt->pdev = pdev; > + pci_set_drvdata(pdev, hisi_ptt); > + > + ret = pcim_enable_device(pdev); > + if (ret) { > + pci_err(pdev, "failed to enable device, ret = %d\n", ret); > + return ret; > + } > + > + ret = pcim_iomap_regions(pdev, BIT(2), DRV_NAME); > + if (ret) { > + pci_err(pdev, "failed to remap io memory, ret = %d\n", ret); > + return ret; > + } > + > + hisi_ptt->iobase = pcim_iomap_table(pdev)[2]; > + > + ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64)); > + if (ret) { > + pci_err(pdev, "failed to set 64 bit dma mask, ret = %d\n", ret); > + return ret; > + } > + > + pci_set_master(pdev); > + > + ret = hisi_ptt_register_irq(hisi_ptt); > + if (ret) > + return ret; > + > + ret = hisi_ptt_init_ctrls(hisi_ptt); > + if (ret) { > + pci_err(pdev, "failed to init controls, ret = %d\n", ret); > + return ret; > + } > + > + ret = hisi_ptt_register_pmu(hisi_ptt); > + if (ret) { > + pci_err(pdev, "failed to register PMU device, ret = %d", ret); > + return ret; > + } > + > + return 0; > +} > + > +static void hisi_ptt_remove(struct pci_dev *pdev) > +{ > + struct hisi_ptt *hisi_ptt = pci_get_drvdata(pdev); > + > + /* > + * We have to manually unregister the PMU device rather than make it > + * devres managed to keep order that the PMU device's unregistration > + * is prior to the release of DMA buffers. As the DMA buffers are > + * devm allocated when necessary which is after the registration of > + * the PMU device. > + */ > + perf_pmu_unregister(&hisi_ptt->hisi_ptt_pmu); > +} > + > +static const struct pci_device_id hisi_ptt_id_tbl[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_HUAWEI, 0xa12e) }, > + { } > +}; > +MODULE_DEVICE_TABLE(pci, hisi_ptt_id_tbl); > + > +static struct pci_driver hisi_ptt_driver = { > + .name = DRV_NAME, > + .id_table = hisi_ptt_id_tbl, > + .probe = hisi_ptt_probe, > + .remove = hisi_ptt_remove, > +}; > +module_pci_driver(hisi_ptt_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Yicong Yang <yangyicong(a)hisilicon.com>"); > +MODULE_DESCRIPTION("Driver for HiSilicon PCIe tune and trace device"); > diff --git a/drivers/hwtracing/ptt/hisi_ptt.h b/drivers/hwtracing/ptt/hisi_ptt.h > new file mode 100644 > index 000000000000..29b6d6293d6c > --- /dev/null > +++ b/drivers/hwtracing/ptt/hisi_ptt.h > @@ -0,0 +1,166 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Driver for HiSilicon PCIe tune and trace device > + * > + * Copyright (c) 2022 HiSilicon Technologies Co., Ltd. > + * Author: Yicong Yang <yangyicong(a)hisilicon.com> > + */ > + > +#ifndef _HISI_PTT_H > +#define _HISI_PTT_H > + > +#include <linux/bits.h> > +#include <linux/list.h> > +#include <linux/mutex.h> > +#include <linux/pci.h> > +#include <linux/perf_event.h> > +#include <linux/types.h> > + > +#define DRV_NAME "hisi_ptt" > + > +/* > + * The definition of the device registers and register fields. > + */ > +#define HISI_PTT_TRACE_ADDR_SIZE 0x0800 > +#define HISI_PTT_TRACE_ADDR_BASE_LO_0 0x0810 > +#define HISI_PTT_TRACE_ADDR_BASE_HI_0 0x0814 > +#define HISI_PTT_TRACE_ADDR_STRIDE 0x8 > +#define HISI_PTT_TRACE_CTRL 0x0850 > +#define HISI_PTT_TRACE_CTRL_EN BIT(0) > +#define HISI_PTT_TRACE_CTRL_RST BIT(1) > +#define HISI_PTT_TRACE_CTRL_RXTX_SEL GENMASK(3, 2) > +#define HISI_PTT_TRACE_CTRL_TYPE_SEL GENMASK(7, 4) > +#define HISI_PTT_TRACE_CTRL_DATA_FORMAT BIT(14) > +#define HISI_PTT_TRACE_CTRL_FILTER_MODE BIT(15) > +#define HISI_PTT_TRACE_CTRL_TARGET_SEL GENMASK(31, 16) > +#define HISI_PTT_TRACE_INT_STAT 0x0890 > +#define HISI_PTT_TRACE_INT_STAT_MASK GENMASK(3, 0) > +#define HISI_PTT_TRACE_INT_MASK 0x0894 > +#define HISI_PTT_TRACE_WR_STS 0x08a0 > +#define HISI_PTT_TRACE_WR_STS_WRITE GENMASK(27, 0) > +#define HISI_PTT_TRACE_WR_STS_BUFFER GENMASK(29, 28) > +#define HISI_PTT_TRACE_STS 0x08b0 > +#define HISI_PTT_TRACE_IDLE BIT(0) > +#define HISI_PTT_DEVICE_RANGE 0x0fe0 > +#define HISI_PTT_DEVICE_RANGE_UPPER GENMASK(31, 16) > +#define HISI_PTT_DEVICE_RANGE_LOWER GENMASK(15, 0) > +#define HISI_PTT_LOCATION 0x0fe8 > +#define HISI_PTT_CORE_ID GENMASK(15, 0) > +#define HISI_PTT_SICL_ID GENMASK(31, 16) > + > +/* Parameters of PTT trace DMA part. */ > +#define HISI_PTT_TRACE_DMA_IRQ 0 > +#define HISI_PTT_TRACE_BUF_CNT 4 > +#define HISI_PTT_TRACE_BUF_SIZE SZ_4M > +#define HISI_PTT_TRACE_TOTAL_BUF_SIZE (HISI_PTT_TRACE_BUF_SIZE * \ > + HISI_PTT_TRACE_BUF_CNT) > +/* Wait time for hardware DMA to reset */ > +#define HISI_PTT_RESET_TIMEOUT_US 10UL > +#define HISI_PTT_RESET_POLL_INTERVAL_US 1UL > +/* Poll timeout and interval for waiting hardware work to finish */ > +#define HISI_PTT_WAIT_TRACE_TIMEOUT_US 100UL > +#define HISI_PTT_WAIT_POLL_INTERVAL_US 10UL > + > +#define HISI_PCIE_CORE_PORT_ID(devfn) (PCI_FUNC(devfn) << 1) > + > +/** > + * struct hisi_ptt_dma_buffer - describe a single trace buffer of PTT trace. > + * The detail of the data format is described > + * in the documentation of PTT device. > + * @dma: DMA address of this buffer visible to the device > + * @addr: virtual address of this buffer visible to the cpu > + */ > +struct hisi_ptt_dma_buffer { > + dma_addr_t dma; > + void *addr; > +}; > + > +/** > + * struct hisi_ptt_trace_ctrl - control and status of PTT trace > + * @trace_buf: array of the trace buffers for holding the trace data. > + * the length will be HISI_PTT_TRACE_BUF_CNT. > + * @handle: perf output handle of current trace session > + * @default_cpu: default cpu to start the trace session > + * @buf_index: the index of current using trace buffer > + * @started: current trace status, true for started > + * @is_port: whether we're tracing root port or not > + * @direction: direction of the TLP headers to trace > + * @filter: filter value for tracing the TLP headers > + * @format: format of the TLP headers to trace > + * @type: type of the TLP headers to trace > + */ > +struct hisi_ptt_trace_ctrl { > + struct hisi_ptt_dma_buffer *trace_buf; > + struct perf_output_handle handle; > + int default_cpu; > + u32 buf_index; > + bool started; > + bool is_port; > + u32 direction:2; > + u32 filter:16; > + u32 format:1; > + u32 type:4; > +}; > + > +/** > + * struct hisi_ptt_filter_desc - descriptor of the PTT trace filter > + * @list: entry of this descriptor in the filter list > + * @pdev: pci_dev related to this filter > + */ > +struct hisi_ptt_filter_desc { > + struct list_head list; > + struct pci_dev *pdev; > +}; > + > + > +/** > + * struct hisi_ptt_pmu_buf - descriptor of the AUX buffer of PTT trace > + * @length: size of the AUX buffer > + * @nr_pages: number of pages of the AUX buffer > + * @base: start address of AUX buffer > + * @pos: position in the AUX buffer to commit traced data > + */ > +struct hisi_ptt_pmu_buf { > + size_t length; > + int nr_pages; > + void *base; > + long pos; > +}; > + > +/** > + * struct hisi_ptt - per PTT device data > + * @trace_ctrl: the control information of PTT trace > + * @hisi_ptt_pmu: the pum device of trace > + * @iobase: base IO address of the device > + * @pdev: pci_dev of this PTT device > + * @mutex: mutex to protect the filter list and serialize the perf process. > + * @upper: the upper BDF range of the PCI devices managed by this PTT device > + * @lower: the lower BDF range of the PCI devices managed by this PTT device > + * @port_filters: the filter list of root ports > + * @req_filters: the filter list of requester ID > + * @port_mask: port mask of the managed root ports > + */ > +struct hisi_ptt { > + struct hisi_ptt_trace_ctrl trace_ctrl; > + struct pmu hisi_ptt_pmu; > + void __iomem *iobase; > + struct pci_dev *pdev; > + struct mutex mutex; > + u32 upper; > + u32 lower; > + > + /* > + * The trace TLP headers can either be filtered by certain > + * root port, or by the requester ID. Organize the filters > + * by @port_filters and @req_filters here. The mask of all > + * the valid ports is also cached for doing sanity check > + * of user input. > + */ > + struct list_head port_filters; > + struct list_head req_filters; > + u16 port_mask; > +}; > + > +#define to_hisi_ptt(pmu) container_of(pmu, struct hisi_ptt, hisi_ptt_pmu) > + > +#endif /* _HISI_PTT_H */ > -- > 2.24.0 > > _______________________________________________ > iommu mailing list > iommu(a)lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu

4 years

Re: [PATCH v2] coresight: etm4x: return 0 instead of using local ret variable

by Mathieu Poirier

On Sun, Apr 24, 2022 at 02:57:24PM +0800, Shile Zhang wrote: > > > On 2022/4/22 23:45, Mathieu Poirier wrote: > > On Fri, Apr 22, 2022 at 10:02:39AM +0800, Shile Zhang wrote: > > > The function etm4_remove_dev() always return 0, and the former function > > > etm4_remove has been changed to void in commit 3fd269e74f2fe ("amba: Make > > > the remove callback return void"). But now its changed back to int type > > > for some reason, which is different to the stable branch linux-5.10.y. > > > > Please spend time understanding why function etm4_remove_dev()'s return value > > has been changed back to an "int". From there you will likely come to the > > conclusion that adding the above to the changelog doesn't make sense. > > Sorry, I means "some reason" here actually I cannot find out why. > > 1. From the git log of the file > `drivers/hwtracing/coresight/coresight-etm4x-core.c', only log of > etm4_remove changes to void in commit 3fd269e74f2fe. no any log record when > it change back to int. > 2. from the commit 'git log --pretty="%h %ci %cn %s" > drivers/hwtracing/coresight/coresight-etm4x-core.c' > ... > b8336ad947e19 2021-02-04 17:00:32 +0100 Greg Kroah-Hartman coresight: etm4x: > add AMBA id for Cortex-A55 and Cortex-A75 > 3fd269e74f2fe 2021-02-02 14:25:50 +0100 Uwe Kleine-König amba: Make the > remove callback return void > ... > > The commit 'b8336ad947e19' does not change the etm4_remove: > https://github.com/torvalds/linux/commit/b8336ad947e1913b9bb5cdf4f54b687654… > > But the different between the commit 'b8336ad947e19' and '3fd269e74f2fe' > contains the changes of etm4_remove back to int, as following: > --- > ... > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c > b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 82787cba537d3..8c4b0c46c8f32 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -1680,7 +1680,7 @@ static void clear_etmdrvdata(void *info) > etmdrvdata[cpu] = NULL; > } > > -static void etm4_remove(struct amba_device *adev) > +static int etm4_remove(struct amba_device *adev) > { > struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev); > > @@ -1703,6 +1703,8 @@ static void etm4_remove(struct amba_device *adev) > cpus_read_unlock(); > > coresight_unregister(drvdata->csdev); > + > + return 0; > } > > static const struct amba_id etm4_ids[] = { > @@ -1711,6 +1713,8 @@ static const struct amba_id etm4_ids[] = { > CS_AMBA_ID(0x000bb95a), /* Cortex-A72 */ > CS_AMBA_ID(0x000bb959), /* Cortex-A73 */ > CS_AMBA_UCI_ID(0x000bb9da, uci_id_etm4),/* Cortex-A35 */ > + CS_AMBA_UCI_ID(0x000bbd05, uci_id_etm4),/* Cortex-A55 */ > + CS_AMBA_UCI_ID(0x000bbd0a, uci_id_etm4),/* Cortex-A75 */ > CS_AMBA_UCI_ID(0x000bbd0c, uci_id_etm4),/* Neoverse N1 */ > CS_AMBA_UCI_ID(0x000f0205, uci_id_etm4),/* Qualcomm Kryo */ > CS_AMBA_UCI_ID(0x000f0211, uci_id_etm4),/* Qualcomm Kryo */ > ... > --- > > I really don't know how to check which commit change it back. > Could you please help to give me some guidance? Have a look at "git blame" - it is really useful to know what commit introduced a change. In this case, and using coresight next (c86dd9869128), the command would be: $ git blame -L 2059,2059 drivers/hwtracing/coresight/coresight-etm4x-core.c That will tell you how function etm4_remove_dev() got to return an 'int' again. But this is really about understanding the current code rather than trying to understand the history of it. With commit 3fd269e74f2fe, Uwe was making amba remove callback functions return a 'void'. And this is what we see here[1] in v5.10.112. Note that etm4_remove() is the callback to the amba driver. In the mainline code, the amba callback function is now etm4_remove_amba() and still returns a 'void'. Function etm4_remove() is now etm4_remove_dev() and is called by both etm4_remove_amba() and etm4_remove_platform_dev(), to avoid code duplication. The conclusion is that because etm4_remove_dev() no longer interfaces with the amba subsystem, its return value can be 'int'. But in this case the return value is not used and as such can be changed to a 'void', regardless of commit 3fd269e74f2fe. Mathieu [1]. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drive… > > Thanks! > > > > > > > > > > Just let it return void and return 0 directly in it's caller function > > > etm4_remove_platform_dev. > > > > The only rational for this patch is that etm4_remove_dev() always returns '0'. > > And even if it was to return anything else, the return value it not checked. > > And even if the return value was checked, there is nothing to do about an error > > condition since the driver is being removed. > > > > > > > > Signed-off-by: Shile Zhang <shile.zhang(a)linux.alibaba.com> > > > --- > > > v2: re-work the commit log from Mathieu's suggestion. > > > v1: https://lore.kernel.org/linux-arm-kernel/20220421164217.GB1596562@p14s/T/ > > > --- > > > drivers/hwtracing/coresight/coresight-etm4x-core.c | 11 +++++------ > > > 1 file changed, 5 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > > > index 7f416a12000eb..141f8209a152a 100644 > > > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > > > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > > > @@ -2104,7 +2104,7 @@ static void clear_etmdrvdata(void *info) > > > etmdrvdata[cpu] = NULL; > > > } > > > -static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > > > +static void __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > > > { > > > etm_perf_symlink(drvdata->csdev, false); > > > /* > > > @@ -2125,8 +2125,6 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > > > cscfg_unregister_csdev(drvdata->csdev); > > > coresight_unregister(drvdata->csdev); > > > - > > > - return 0; > > > } > > > static void __exit etm4_remove_amba(struct amba_device *adev) > > > @@ -2139,13 +2137,14 @@ static void __exit etm4_remove_amba(struct amba_device *adev) > > > static int __exit etm4_remove_platform_dev(struct platform_device *pdev) > > > { > > > - int ret = 0; > > > struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > > > if (drvdata) > > > - ret = etm4_remove_dev(drvdata); > > > + etm4_remove_dev(drvdata); > > > + > > > pm_runtime_disable(&pdev->dev); > > > - return ret; > > > + > > > + return 0; > > > } > > > static const struct amba_id etm4_ids[] = { > > > -- > > > 2.33.0.rc2 > > >

4 years

[PATCH v6 00/10] Coresight: Add support for TPDM and TPDA

by Mao Jinlong

This series adds support for the trace performance monitoring and diagnostics hardware (TPDM and TPDA). It is composed of two major elements. a) Changes for original coresight framework to support for TPDM and TPDA. b) Add driver code for TPDM and TPDA. Introduction of changes for original coresight framework Support TPDM as new coresight source. Since only STM and ETM are supported as coresight source originally. TPDM is a newly added coresight source. We need to change the original way of saving coresight path to support more types source for coresight driver. The following patch is to add support more coresight sources. coresight: core: Use IDR for non-cpu bound sources' paths. Introduction of TPDM and TPDA TPDM - The trace performance monitoring and diagnostics monitor or TPDM in short serves as data collection component for various dataset types specified in the QPMDA(Qualcomm performance monitoring and diagnostics architecture) spec. The primary use case of the TPDM is to collect data from different data sources and send it to a TPDA for packetization, timestamping and funneling. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware definitions coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support docs: sysfs: coresight: Add sysfs ABI documentation for TPDM TPDA - The trace performance monitoring and diagnostics aggregator or TPDA in short serves as an arbitration and packetization engine for the performance monitoring and diagnostics network as specified in the QPMDA (Qualcomm performance monitoring and diagnostics architecture) specification. The primary use case of the TPDA is to provide packetization, funneling and timestamping of Monitor data as specified in the QPMDA specification. The following patch is to add driver for TPDA. Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions The last patch of this series is a device tree modification, which add the TPDM and TPDA configuration to device tree for validating. ARM: dts: msm: Add coresight components for SM8250 ARM: dts: msm: Add tpdm mm/prng for sm8250 Once this series patches are applied properly, the tpdm and tpda nodes should be observed at the coresight path /sys/bus/coresight/devices e.g. /sys/bus/coresight/devices # ls -l | grep tpd tpda0 -> ../../../devices/platform/soc(a)0/6004000.tpda/tpda0 tpdm0 -> ../../../devices/platform/soc(a)0/6c08000.mm.tpdm/tpdm0 We can use the commands are similar to the below to validate TPDMs. Enable coresight sink first. echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test The test data will be collected in the coresight sink which is enabled. If rwp register of the sink is keeping updating when do integration_test (by cat tmc_etf0/mgmt/rwp), it means there is data generated from TPDM to sink. There must be a tpda between tpdm and the sink. When there are some other trace event hw components in the same HW block with tpdm, tpdm and these hw components will connect to the coresight funnel. When there is only tpdm trace hw in the HW block, tpdm will connect to tpda directly. +---------------+ +-------------+ | tpdm@6c08000 | |tpdm@684C000 | +-------|-------+ +------|------+ | | +-------|-------+ | | funnel@6c0b000| | +-------|-------+ | | | +-------|-------+ | |funnel@6c2d000 | | +-------|-------+ | | | | +---------------+ | +----- tpda@6004000 -----------+ +-------|-------+ | +-------|-------+ |funnel@6005000 | +---------------+ This patch series depends on patch series "coresight: Add new API to allocate trace source ID values". https://patchwork.kernel.org/project/linux-arm-kernel/cover/20220308205000.… Changes from V5: 1. Update maintainers in tpdm/tpda yaml file. -- Mike Leach <mike.leach(a)linaro.org> 2. Set the .remove function pointer in the amba_driver structure of tpdm/tpda driver. Add tpda_remove function for tpda driver. -- Mike Leach <mike.leach(a)linaro.org> 3. Define datasets of tpdm as unsigned long. -- Mike Leach <mike.leach(a)linaro.org> 4. Move all coresight nodes to sm8250.dtsi. -- Mike Leach <mike.leach(a)linaro.org>;Konrad Dybcio <konrad.dybcio(a)somainline.org> 5. Remove CORESIGHT_TPDM_INTEGRATION_TEST config. -- Mike Leach <mike.leach(a)linaro.org> Mao Jinlong (10): coresight: core: Use IDR for non-cpu bound sources' paths. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware definitions coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support docs: sysfs: coresight: Add sysfs ABI documentation for TPDM Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions ARM: dts: msm: Add coresight components for SM8250 ARM: dts: msm: Add tpdm mm/prng for sm8250 .../testing/sysfs-bus-coresight-devices-tpdm | 13 + .../bindings/arm/coresight-tpda.yaml | 119 ++++ .../bindings/arm/coresight-tpdm.yaml | 99 +++ .../devicetree/bindings/arm/coresight.txt | 7 + MAINTAINERS | 1 + arch/arm64/boot/dts/qcom/sm8250.dtsi | 658 ++++++++++++++++++ drivers/hwtracing/coresight/Kconfig | 24 + drivers/hwtracing/coresight/Makefile | 2 + drivers/hwtracing/coresight/coresight-core.c | 42 +- drivers/hwtracing/coresight/coresight-tpda.c | 201 ++++++ drivers/hwtracing/coresight/coresight-tpda.h | 33 + drivers/hwtracing/coresight/coresight-tpdm.c | 258 +++++++ drivers/hwtracing/coresight/coresight-tpdm.h | 63 ++ include/linux/coresight.h | 1 + 14 files changed, 1509 insertions(+), 12 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h -- 2.17.1

4 years

Re: [PATCH v2] coresight: etm4x: return 0 instead of using local ret variable

by Mathieu Poirier

On Fri, Apr 22, 2022 at 10:02:39AM +0800, Shile Zhang wrote: > The function etm4_remove_dev() always return 0, and the former function > etm4_remove has been changed to void in commit 3fd269e74f2fe ("amba: Make > the remove callback return void"). But now its changed back to int type > for some reason, which is different to the stable branch linux-5.10.y. Please spend time understanding why function etm4_remove_dev()'s return value has been changed back to an "int". From there you will likely come to the conclusion that adding the above to the changelog doesn't make sense. > > Just let it return void and return 0 directly in it's caller function > etm4_remove_platform_dev. The only rational for this patch is that etm4_remove_dev() always returns '0'. And even if it was to return anything else, the return value it not checked. And even if the return value was checked, there is nothing to do about an error condition since the driver is being removed. > > Signed-off-by: Shile Zhang <shile.zhang(a)linux.alibaba.com> > --- > v2: re-work the commit log from Mathieu's suggestion. > v1: https://lore.kernel.org/linux-arm-kernel/20220421164217.GB1596562@p14s/T/ > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 7f416a12000eb..141f8209a152a 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -2104,7 +2104,7 @@ static void clear_etmdrvdata(void *info) > etmdrvdata[cpu] = NULL; > } > > -static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > +static void __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > { > etm_perf_symlink(drvdata->csdev, false); > /* > @@ -2125,8 +2125,6 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > > cscfg_unregister_csdev(drvdata->csdev); > coresight_unregister(drvdata->csdev); > - > - return 0; > } > > static void __exit etm4_remove_amba(struct amba_device *adev) > @@ -2139,13 +2137,14 @@ static void __exit etm4_remove_amba(struct amba_device *adev) > > static int __exit etm4_remove_platform_dev(struct platform_device *pdev) > { > - int ret = 0; > struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > > if (drvdata) > - ret = etm4_remove_dev(drvdata); > + etm4_remove_dev(drvdata); > + > pm_runtime_disable(&pdev->dev); > - return ret; > + > + return 0; > } > > static const struct amba_id etm4_ids[] = { > -- > 2.33.0.rc2 >

4 years

Re: [PATCH] coresight: etm4x: return 0 instead of using local ret variable

by Mathieu Poirier

Hi Shile, On Wed, Apr 20, 2022 at 01:28:31PM +0800, Shile Zhang wrote: > The etm4_remove function (now it's rename to etm4_remove_dev) always > return 0, and it has been changed to void in commit 4fd269e74f2f > ("amba: Make the remove callback return void"). But its weird that the > changes is gone in mainline. which is remained in 5.10.y branch. Commit 4fd269e74f2f is not valid upstream. Changes that don't have a critical impact on user experience or fix a bug aren't backported to longterm kernels. > > Just backport the changes of etm4_remove_dev and return 0 directly in it's > caller function etm4_remove_platform_dev. I'm not sure why the work "backport" is used here since this patch is destine for mainline. > > Signed-off-by: Shile Zhang <shile.zhang(a)linux.alibaba.com> > --- > drivers/hwtracing/coresight/coresight-etm4x-core.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c > index 7f416a12000e..141f8209a152 100644 > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c > @@ -2104,7 +2104,7 @@ static void clear_etmdrvdata(void *info) > etmdrvdata[cpu] = NULL; > } > > -static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > +static void __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > { > etm_perf_symlink(drvdata->csdev, false); > /* > @@ -2125,8 +2125,6 @@ static int __exit etm4_remove_dev(struct etmv4_drvdata *drvdata) > > cscfg_unregister_csdev(drvdata->csdev); > coresight_unregister(drvdata->csdev); > - > - return 0; > } > > static void __exit etm4_remove_amba(struct amba_device *adev) > @@ -2139,13 +2137,14 @@ static void __exit etm4_remove_amba(struct amba_device *adev) > > static int __exit etm4_remove_platform_dev(struct platform_device *pdev) > { > - int ret = 0; > struct etmv4_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > > if (drvdata) > - ret = etm4_remove_dev(drvdata); > + etm4_remove_dev(drvdata); > + > pm_runtime_disable(&pdev->dev); > - return ret; > + > + return 0; I'm fine with the code but the changelog needs to be re-worked. The only rational for this patch is that the return value for function etm4_remove_dev() is never used and as such being removed. Thanks, Mathieu > } > > static const struct amba_id etm4_ids[] = { > -- > 2.33.0.rc2 >

4 years

[PATCH v5 00/10] Coresight: Add support for TPDM and TPDA

by Mao Jinlong

This series adds support for the trace performance monitoring and diagnostics hardware (TPDM and TPDA). It is composed of two major elements. a) Changes for original coresight framework to support for TPDM and TPDA. b) Add driver code for TPDM and TPDA. Introduction of changes for original coresight framework Support TPDM as new coresight source. Since only STM and ETM are supported as coresight source originally. TPDM is a newly added coresight source. We need to change the original way of saving coresight path to support more types source for coresight driver. The following patch is to add support more coresight sources. coresight: core: Use IDR for non-cpu bound sources' paths. Introduction of TPDM and TPDA TPDM - The trace performance monitoring and diagnostics monitor or TPDM in short serves as data collection component for various dataset types specified in the QPMDA(Qualcomm performance monitoring and diagnostics architecture) spec. The primary use case of the TPDM is to collect data from different data sources and send it to a TPDA for packetization, timestamping and funneling. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware definitions coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support docs: sysfs: coresight: Add sysfs ABI documentation for TPDM TPDA - The trace performance monitoring and diagnostics aggregator or TPDA in short serves as an arbitration and packetization engine for the performance monitoring and diagnostics network as specified in the QPMDA (Qualcomm performance monitoring and diagnostics architecture) specification. The primary use case of the TPDA is to provide packetization, funneling and timestamping of Monitor data as specified in the QPMDA specification. The following patch is to add driver for TPDA. Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions The last patch of this series is a device tree modification, which add the TPDM and TPDA configuration to device tree for validating. ARM: dts: msm: Add coresight components for SM8250 ARM: dts: msm: Add tpdm mm/prng for sm8250 Once this series patches are applied properly, the tpdm and tpda nodes should be observed at the coresight path /sys/bus/coresight/devices e.g. /sys/bus/coresight/devices # ls -l | grep tpd tpda0 -> ../../../devices/platform/soc(a)0/6004000.tpda/tpda0 tpdm0 -> ../../../devices/platform/soc(a)0/6c08000.mm.tpdm/tpdm0 We can use the commands are similar to the below to validate TPDMs. Enable coresight sink first. echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink echo 1 > /sys/bus/coresight/devices/tpdm0/enable_source echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test The test data will be collected in the coresight sink which is enabled. If rwp register of the sink is keeping updating when do integration_test (by cat tmc_etf0/mgmt/rwp), it means there is data generated from TPDM to sink. There must be a tpda between tpdm and the sink. When there are some other trace event hw components in the same HW block with tpdm, tpdm and these hw components will connect to the coresight funnel. When there is only tpdm trace hw in the HW block, tpdm will connect to tpda directly. +---------------+ +-------------+ | tpdm@6c08000 | |tpdm@684C000 | +-------|-------+ +------|------+ | | +-------|-------+ | | funnel@6c0b000| | +-------|-------+ | | | +-------|-------+ | |funnel@6c2d000 | | +-------|-------+ | | | | +---------------+ | +----- tpda@6004000 -----------+ +-------|-------+ | +-------|-------+ |funnel@6005000 | +---------------+ This patch series depends on patch series "coresight: Add new API to allocate trace source ID values". https://patchwork.kernel.org/project/linux-arm-kernel/cover/20220308205000.… Changes from V4: 1. Keep the ETM source paths per-CPU and use IDR for other sources' paths. (Suzuki K Poulose <suzuki.poulose(a)arm.com>) Mao Jinlong (10): coresight: core: Use IDR for non-cpu bound sources' paths. Coresight: Add coresight TPDM source driver dt-bindings: arm: Adds CoreSight TPDM hardware definitions coresight-tpdm: Add DSB dataset support coresight-tpdm: Add integration test support docs: sysfs: coresight: Add sysfs ABI documentation for TPDM Coresight: Add TPDA link driver dt-bindings: arm: Adds CoreSight TPDA hardware definitions ARM: dts: msm: Add coresight components for SM8250 ARM: dts: msm: Add tpdm mm/prng for sm8250 .../testing/sysfs-bus-coresight-devices-tpdm | 13 + .../bindings/arm/coresight-tpda.yaml | 119 +++ .../bindings/arm/coresight-tpdm.yaml | 99 +++ .../devicetree/bindings/arm/coresight.txt | 7 + MAINTAINERS | 1 + .../arm64/boot/dts/qcom/sm8250-coresight.dtsi | 708 ++++++++++++++++++ arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 + drivers/hwtracing/coresight/Kconfig | 33 + drivers/hwtracing/coresight/Makefile | 2 + drivers/hwtracing/coresight/coresight-core.c | 42 +- drivers/hwtracing/coresight/coresight-tpda.c | 192 +++++ drivers/hwtracing/coresight/coresight-tpda.h | 32 + drivers/hwtracing/coresight/coresight-tpdm.c | 270 +++++++ drivers/hwtracing/coresight/coresight-tpdm.h | 61 ++ include/linux/coresight.h | 1 + 15 files changed, 1570 insertions(+), 12 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpda.yaml create mode 100644 Documentation/devicetree/bindings/arm/coresight-tpdm.yaml create mode 100644 arch/arm64/boot/dts/qcom/sm8250-coresight.dtsi create mode 100644 drivers/hwtracing/coresight/coresight-tpda.c create mode 100644 drivers/hwtracing/coresight/coresight-tpda.h create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.c create mode 100644 drivers/hwtracing/coresight/coresight-tpdm.h -- 2.17.1

4 years

Re: [PATCH v5 09/10] ARM: dts: msm: Add coresight components for SM8250

by Mike Leach

Hi On Wed, 13 Apr 2022 at 17:45, Jinlong Mao <quic_jinlmao(a)quicinc.com> wrote: > > Hi Konrad, > > Thank you for the review. > > On 4/13/2022 5:58 PM, Konrad Dybcio wrote: > > Hi, > > > > > > I added Bjorn, the linux-arm-msm maintainer to CC as he was missing > > for some reason. > > > > > > On 12/04/2022 14:50, Mao Jinlong wrote: > >> Add coresight device tree for sm8250. STM/ETM are added. > >> > >> Signed-off-by: Tao Zhang <quic_taozha(a)quicinc.com> > >> Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com> > >> --- > >> .../arm64/boot/dts/qcom/sm8250-coresight.dtsi | 526 ++++++++++++++++++ > >> arch/arm64/boot/dts/qcom/sm8250.dtsi | 2 + > >> 2 files changed, 528 insertions(+) > >> create mode 100644 arch/arm64/boot/dts/qcom/sm8250-coresight.dtsi > >> > >> diff --git a/arch/arm64/boot/dts/qcom/sm8250-coresight.dtsi > >> b/arch/arm64/boot/dts/qcom/sm8250-coresight.dtsi > >> new file mode 100644 > >> index 000000000000..1de42fd39248 > >> --- /dev/null > >> +++ b/arch/arm64/boot/dts/qcom/sm8250-coresight.dtsi > >> @@ -0,0 +1,526 @@ > >> +// SPDX-License-Identifier: GPL-2.0 > > > > sm8250.dtsi is BSD-3-Clause. Please consider relicensing. > > > > > >> +/* > >> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights > >> reserved. > >> + */ > >> + > >> +&soc { > >> + > >> + stm@6002000 { > >> + compatible = "arm,coresight-stm", "arm,primecell"; > >> + reg = <0 0x06002000 0 0x1000>, > > > > You don't need to break the line at so few characters. > > > > > >> + <0 0x16280000 0 0x180000>; > >> + reg-names = "stm-base", "stm-stimulus-base"; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + stm_out: endpoint { > >> + remote-endpoint = > >> + <&funnel0_in7>; > > > > Same here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + funnel@6041000 { > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + reg = <0 0x06041000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + funnel0_out: endpoint { > >> + remote-endpoint = > >> + <&merge_funnel_in0>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@7 { > >> + reg = <7>; > >> + funnel0_in7: endpoint { > >> + remote-endpoint = <&stm_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + funnel@6042000 { > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + reg = <0 0x06042000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + funnel2_out: endpoint { > >> + remote-endpoint = > >> + <&merge_funnel_in2>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@2 { > >> + reg = <4>; > >> + funnel2_in5: endpoint { > >> + remote-endpoint = > >> + <&apss_merge_funnel_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + funnel@6b04000 { > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + arm,primecell-periphid = <0x000bb908>; > >> + > >> + reg = <0 0x6b04000 0 0x1000>; > >> + reg-names = "funnel-base"; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + merge_funnel_out: endpoint { > >> + remote-endpoint = > >> + <&etf_in>; > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@7 { > >> + reg = <7>; > >> + funnel_swao_in_funnel_merg: endpoint { > >> + remote-endpoint= > > > > And here. > > > > > >> + <&funnel_merg_out_funnel_swao>; > >> + }; > >> + }; > >> + }; > >> + > >> + }; > >> + > >> + funnel@6045000 { > > > > The nodes are not sorted properly (by address). Please fix that. > > > > > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + reg = <0 0x06045000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + funnel_merg_out_funnel_swao: endpoint { > >> + remote-endpoint = <&funnel_swao_in_funnel_merg>; > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@1 { > >> + reg = <0>; > >> + merge_funnel_in0: endpoint { > >> + remote-endpoint = > >> + <&funnel0_out>; > >> + }; > >> + }; > >> + > >> + port@2 { > >> + reg = <1>; > >> + merge_funnel_in2: endpoint { > >> + remote-endpoint = > >> + <&funnel2_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + replicator@6046000 { > >> + compatible = "arm,coresight-dynamic-replicator", > >> "arm,primecell"; > >> + reg = <0 0x06046000 0 0x1000>; > >> + > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + replicator_out: endpoint { > >> + remote-endpoint = <&etr_in>; > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + port { > >> + replicator_cx_in_swao_out: endpoint { > >> + remote-endpoint = <&replicator_swao_out_cx_in>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + replicator@6b06000 { > >> + compatible = "arm,coresight-dynamic-replicator", > >> "arm,primecell"; > >> + reg = <0 0x06b06000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + replicator_swao_out_cx_in: endpoint { > >> + remote-endpoint = <&replicator_cx_in_swao_out>; > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + port { > >> + replicator_in: endpoint { > >> + remote-endpoint = <&etf_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etf@6b05000 { > >> + compatible = "arm,coresight-tmc", "arm,primecell"; > >> + reg = <0 0x6b05000 0 0x1000>; > > > > Please pad the address to 8 chars. > > > > > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + etf_out: endpoint { > >> + remote-endpoint = > >> + <&replicator_in>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@1 { > >> + reg = <0>; > >> + etf_in: endpoint { > >> + remote-endpoint = > >> + <&merge_funnel_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etr@6048000 { > >> + compatible = "arm,coresight-tmc", "arm,primecell"; > >> + reg = <0 0x06048000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,scatter-gather; > >> + > >> + in-ports { > >> + port { > >> + etr_in: endpoint { > >> + remote-endpoint = > >> + <&replicator_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7040000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07040000 0 0x1000>; > >> + > >> + cpu = <&CPU0>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm0_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in0>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7140000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07140000 0 0x1000>; > >> + > >> + cpu = <&CPU1>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm1_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in1>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7240000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07240000 0 0x1000>; > >> + > >> + cpu = <&CPU2>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm2_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in2>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7340000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07340000 0 0x1000>; > >> + > >> + cpu = <&CPU3>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm3_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in3>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7440000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07440000 0 0x1000>; > >> + > >> + cpu = <&CPU4>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm4_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in4>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7540000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07540000 0 0x1000>; > >> + > >> + cpu = <&CPU5>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm5_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in5>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7640000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07640000 0 0x1000>; > >> + > >> + cpu = <&CPU6>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm6_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in6>; > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + etm@7740000 { > >> + compatible = "arm,coresight-etm4x", "arm,primecell"; > >> + reg = <0 0x07740000 0 0x1000>; > >> + > >> + cpu = <&CPU7>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + arm,coresight-loses-context-with-cpu; > >> + > >> + out-ports { > >> + port { > >> + etm7_out: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_in7>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + funnel@7800000 { > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + reg = <0 0x07800000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + apss_funnel_out: endpoint { > >> + remote-endpoint = > >> + <&apss_merge_funnel_in>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@0 { > >> + reg = <0>; > >> + apss_funnel_in0: endpoint { > >> + remote-endpoint = > >> + <&etm0_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@1 { > >> + reg = <1>; > >> + apss_funnel_in1: endpoint { > >> + remote-endpoint = > >> + <&etm1_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@2 { > >> + reg = <2>; > >> + apss_funnel_in2: endpoint { > >> + remote-endpoint = > >> + <&etm2_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@3 { > >> + reg = <3>; > >> + apss_funnel_in3: endpoint { > >> + remote-endpoint = > >> + <&etm3_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@4 { > >> + reg = <4>; > >> + apss_funnel_in4: endpoint { > >> + remote-endpoint = > >> + <&etm4_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@5 { > >> + reg = <5>; > >> + apss_funnel_in5: endpoint { > >> + remote-endpoint = > >> + <&etm5_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@6 { > >> + reg = <6>; > >> + apss_funnel_in6: endpoint { > >> + remote-endpoint = > >> + <&etm6_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + > >> + port@7 { > >> + reg = <7>; > >> + apss_funnel_in7: endpoint { > >> + remote-endpoint = > >> + <&etm7_out>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + }; > >> + > >> + funnel@7810000 { > >> + compatible = "arm,coresight-dynamic-funnel", "arm,primecell"; > >> + reg = <0 0x07810000 0 0x1000>; > >> + > >> + clocks = <&aoss_qmp>; > >> + clock-names = "apb_pclk"; > >> + > >> + out-ports { > >> + port { > >> + apss_merge_funnel_out: endpoint { > >> + remote-endpoint = > >> + <&funnel2_in5>; > > > > And here. > > > > > >> + }; > >> + }; > >> + }; > >> + > >> + in-ports { > >> + port@1 { > >> + reg = <0>; > >> + apss_merge_funnel_in: endpoint { > >> + remote-endpoint = > >> + <&apss_funnel_out>; > >> + }; > >> + }; > >> + }; > >> + }; > >> +}; > >> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi > >> b/arch/arm64/boot/dts/qcom/sm8250.dtsi > >> index af8f22636436..115623392183 100644 > >> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi > >> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi > >> @@ -5434,3 +5434,5 @@ > >> }; > >> }; > >> }; > >> + > >> +#include "sm8250-coresight.dtsi" > > > > Why should everybody want coresight? It's not enabled on (most) > > production devices and may cause a platform crash when you try to use > > it on such ones. > > > > > > These nodes should probably be added to sm8250.dtsi, all with status = > > "disabled" by default, so that they don't break the devices that do > > not support it due to fuse configuration. > > > I will address your comments above. > I create coresight dtsi because of that there are dozens of coresight > nodes for qualcomm HW. > Add all the nodes in a separate file is easy to maintain. > For disabling all the nodes by default, i will check internally and get > back to you. > If the nodes are "disabled", then the base .dts files for platforms that do include coresight would have to be updated to enable all those nodes - which leaves potential for errors if nodes are accidentally omitted. Because sm8250-coresight.dtsi contains only Coresight devices, would it not be better to include this directly in the base .dts file for platforms that do have Coresight, and omit it from those that do not. Regards Mike > > Thanks > Jinlong Mao > > > > > > Konrad > > -- Mike Leach Principal Engineer, ARM Ltd. Manchester Design Centre. UK

4 years

[PATCH v3 00/15] Make ETM register accesses consistent with sysreg.h

by James Clark

Changes since v2: * Implement Mike's suggestion of not having _SHIFT and using the existing FIELD_GET and FIELD_PREP methods. * Dropped the change to add the new REG_VAL macro because of the above. * FIELD_PREP could be used in some trivial cases, but in some cases the shift is still required but can be calculated with __bf_shf * Improved the commit messages. * The change is still binary equivalent, but requires an extra step mentioned at the end of this cover letter. Applies to coresight/next 3619ee28488 Also available at https://gitlab.arm.com/linux-arm/linux-jc/-/tree/james-cs-register-refactor… To check for binary equivalence follow the same steps in the cover letter of v2, but apply the following change to coresight-priv.h. This is because the existing version of the macros wrap the expression in a new scope {} that flips something in the compiler: #undef FIELD_GET #define FIELD_GET(_mask, _reg) (((_reg) & (_mask)) >> __bf_shf(_mask)) #undef FIELD_PREP #define FIELD_PREP(_mask, _val) (((_val) << __bf_shf(_mask)) & (_mask)) Thanks James James Clark (15): coresight: etm4x: Cleanup TRCIDR0 register accesses coresight: etm4x: Cleanup TRCIDR2 register accesses coresight: etm4x: Cleanup TRCIDR3 register accesses coresight: etm4x: Cleanup TRCIDR4 register accesses coresight: etm4x: Cleanup TRCIDR5 register accesses coresight: etm4x: Cleanup TRCCONFIGR register accesses coresight: etm4x: Cleanup TRCEVENTCTL1R register accesses coresight: etm4x: Cleanup TRCSTALLCTLR register accesses coresight: etm4x: Cleanup TRCVICTLR register accesses coresight: etm3x: Cleanup ETMTECR1 register accesses coresight: etm4x: Cleanup TRCACATRn register accesses coresight: etm4x: Cleanup TRCSSCCRn and TRCSSCSRn register accesses coresight: etm4x: Cleanup TRCSSPCICRn register accesses coresight: etm4x: Cleanup TRCBBCTLR register accesses coresight: etm4x: Cleanup TRCRSCTLRn register accesses .../coresight/coresight-etm3x-core.c | 2 +- .../coresight/coresight-etm3x-sysfs.c | 2 +- .../coresight/coresight-etm4x-core.c | 136 +++++-------- .../coresight/coresight-etm4x-sysfs.c | 180 +++++++++--------- drivers/hwtracing/coresight/coresight-etm4x.h | 122 ++++++++++-- 5 files changed, 244 insertions(+), 198 deletions(-) -- 2.28.0

4 years

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight