At the moment we adjust the buffer pointers for reading the trace data via misc device in the common code for ETF/ETB and ETR. Since we are going to change how we manage the buffer for ETR, let us move the buffer manipulation to the respective driver files, hiding it from the common code. We do so by adding type specific helpers for finding the length of data and the pointer to the buffer, for a given length at a file position.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etf.c | 18 +++++++++++ drivers/hwtracing/coresight/coresight-tmc-etr.c | 34 ++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.c | 41 ++++++++++++++----------- drivers/hwtracing/coresight/coresight-tmc.h | 4 +++ 4 files changed, 79 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index e2513b7..e5edf46 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -120,6 +120,24 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata) CS_LOCK(drvdata->base); }

+/* + * Return the available trace data in the buffer from @pos, with + * a maximum limit of @len, updating the @bufpp on where to + * find it. + */ +ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp) +{ + ssize_t actual = len; + + /* Adjust the len to available size @pos */ + if (pos + actual > drvdata->len) + actual = drvdata->len - pos; + if (actual > 0) + *bufpp = drvdata->buf + pos; + return actual; +} + static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev) { int ret = 0; diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 68fbc8f..d3c2b04 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -69,6 +69,40 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) CS_LOCK(drvdata->base); }

+/* + * Return the available trace data in the buffer @pos, with a maximum + * limit of @len, also updating the @bufpp on where to find it. + */ +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp) +{ + ssize_t actual = len; + char *bufp = drvdata->buf + pos; + char *bufend = (char *)(drvdata->vaddr + drvdata->size); + + /* Adjust the len to available size @pos */ + if (pos + actual > drvdata->len) + actual = drvdata->len - pos; + + if (actual <= 0) + return actual; + + /* + * Since we use a circular buffer, with trace data starting + * @drvdata->buf, possibly anywhere in the buffer @drvdata->vaddr, + * wrap the current @pos to within the buffer. + */ + if (bufp >= bufend) + bufp -= drvdata->size; + /* + * For simplicity, avoid copying over a wrapped around buffer. + */ + if ((bufp + actual) > bufend) + actual = bufend - bufp; + *bufpp = bufp; + return actual; +} + static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) { const u32 *barrier; diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 0ea04f5..93c5bfc 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -131,35 +131,40 @@ static int tmc_open(struct inode *inode, struct file *file) return 0; }

+static inline ssize_t tmc_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp) +{ + switch (drvdata->config_type) { + case TMC_CONFIG_TYPE_ETB: + case TMC_CONFIG_TYPE_ETF: + return tmc_etb_get_sysfs_trace(drvdata, pos, len, bufpp); + case TMC_CONFIG_TYPE_ETR: + return tmc_etr_get_sysfs_trace(drvdata, pos, len, bufpp); + } + + return -EINVAL; +} + static ssize_t tmc_read(struct file *file, char __user *data, size_t len, loff_t *ppos) { + char *bufp; + ssize_t actual; struct tmc_drvdata *drvdata = container_of(file->private_data, struct tmc_drvdata, miscdev); - char *bufp = drvdata->buf + *ppos; + actual = tmc_get_sysfs_trace(drvdata, *ppos, len, &bufp); + if (actual <= 0) + return 0;

- if (*ppos + len > drvdata->len) - len = drvdata->len - *ppos; - - if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) { - if (bufp == (char *)(drvdata->vaddr + drvdata->size)) - bufp = drvdata->vaddr; - else if (bufp > (char *)(drvdata->vaddr + drvdata->size)) - bufp -= drvdata->size; - if ((bufp + len) > (char *)(drvdata->vaddr + drvdata->size)) - len = (char *)(drvdata->vaddr + drvdata->size) - bufp; - } - - if (copy_to_user(data, bufp, len)) { + if (copy_to_user(data, bufp, actual)) { dev_dbg(drvdata->dev, "%s: copy_to_user failed\n", __func__); return -EFAULT; }

- *ppos += len; + *ppos += actual; + dev_dbg(drvdata->dev, "%zu bytes copied\n", actual);

- dev_dbg(drvdata->dev, "%s: %zu bytes copied, %d bytes left\n", - __func__, len, (int)(drvdata->len - *ppos)); - return len; + return actual; }

static int tmc_release(struct inode *inode, struct file *file) diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 8df7a81..73f944d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -183,10 +183,14 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata); extern const struct coresight_ops tmc_etb_cs_ops; extern const struct coresight_ops tmc_etf_cs_ops;

+ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp); /* ETR functions */ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata); int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata); extern const struct coresight_ops tmc_etr_cs_ops; +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, + loff_t pos, size_t len, char **bufpp);

#define TMC_REG_PAIR(name, lo_off, hi_off) \

We don't support ETR in perf mode yet. So, don't even try to enable the hardware, even by mistake.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 28 ++----------------------- 1 file changed, 2 insertions(+), 26 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index c73bcb3..6c5e8d1 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -223,32 +223,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)

static int tmc_enable_etr_sink_perf(struct coresight_device *csdev) { - int ret = 0; - unsigned long flags; - struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - - spin_lock_irqsave(&drvdata->spinlock, flags); - if (drvdata->reading) { - ret = -EINVAL; - goto out; - } - - /* - * In Perf mode there can be only one writer per sink. There - * is also no need to continue if the ETR is already operated - * from sysFS. - */ - if (drvdata->mode != CS_MODE_DISABLED) { - ret = -EINVAL; - goto out; - } - - drvdata->mode = CS_MODE_PERF; - tmc_etr_enable_hw(drvdata); -out: - spin_unlock_irqrestore(&drvdata->spinlock, flags); - - return ret; + /* We don't support perf mode yet ! */ + return -EINVAL; }

static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)

We are about to add the support for ETR builtin scatter-gather mode for dealing with large amount of trace buffers. However, on some of the platforms, using the ETR SG mode can lock up the system due to the way the ETR is connected to the memory subsystem.

In SG mode, the ETR performs READ from the scatter-gather table to fetch the next page and regular WRITE of trace data. If the READ operation doesn't complete(due to the memory subsystem issues, which we have seen on a couple of platforms) the trace WRITE cannot proceed leading to issues. So, we by default do not use the SG mode, unless it is known to be safe on the platform. We define a DT property for the TMC node to specify whether we have a proper SG mode.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Mike Leach mike.leach@linaro.org Cc: Mark Rutland mark.rutland@arm.com Cc: John Horley john.horley@arm.com Cc: Robert Walker robert.walker@arm.com Cc: devicetree@vger.kernel.org Cc: frowand.list@gmail.com Cc: Rob Herring robh@kernel.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Documentation/devicetree/bindings/arm/coresight.txt | 2 ++ drivers/hwtracing/coresight/coresight-tmc.c | 9 ++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt index 15ac8e8..603d3c6 100644 --- a/Documentation/devicetree/bindings/arm/coresight.txt +++ b/Documentation/devicetree/bindings/arm/coresight.txt @@ -86,6 +86,8 @@ its hardware characteristcs. * arm,buffer-size: size of contiguous buffer space for TMC ETR (embedded trace router)

+ * arm,scatter-gather: boolean. Indicates that the TMC-ETR can safely + use the SG mode on this system.

Example:

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 93c5bfc..7d8331d 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -20,6 +20,7 @@ #include <linux/err.h> #include <linux/fs.h> #include <linux/miscdevice.h> +#include <linux/property.h> #include <linux/uaccess.h> #include <linux/slab.h> #include <linux/dma-mapping.h> @@ -304,6 +305,12 @@ const struct attribute_group *coresight_tmc_groups[] = { NULL, };

+static inline bool tmc_etr_can_use_sg(struct tmc_drvdata *drvdata) +{ + return fwnode_property_present(drvdata->dev->fwnode, + "arm,scatter-gather"); +} + /* Detect and initialise the capabilities of a TMC ETR */ static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata, u32 devid, void *dev_caps) @@ -313,7 +320,7 @@ static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata, /* Set the unadvertised capabilities */ tmc_etr_init_caps(drvdata, (u32)(unsigned long)dev_caps);

- if (!(devid & TMC_DEVID_NOSCAT)) + if (!(devid & TMC_DEVID_NOSCAT) && tmc_etr_can_use_sg(drvdata)) tmc_etr_set_cap(drvdata, TMC_ETR_SG);

/* Check if the AXI address width is available */

This patch introduces a generic sg table data structure and associated operations. An SG table can be used to map a set of Data pages where the trace data could be stored by the TMC ETR. The information about the data pages could be stored in different formats, depending on the type of the underlying SG mechanism (e.g, TMC ETR SG vs Coresight CATU). The generic structure provides book keeping of the pages used for the data as well as the table contents. The table should be filled by the user of the infrastructure.

A table can be created by specifying the number of data pages as well as the number of table pages required to hold the pointers, where the latter could be different for different types of tables. The pages are mapped in the appropriate dma data direction mode (i.e, DMA_TO_DEVICE for table pages and DMA_FROM_DEVICE for data pages). The framework can optionally accept a set of allocated data pages (e.g, perf ring buffer) and map them accordingly. The table and data pages are vmap'ed to allow easier access by the drivers. The framework also provides helpers to sync the data written to the pages with appropriate directions.

This will be later used by the TMC ETR SG unit and CATU.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- Changes since v1: - Address code style issues, more comments --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 290 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 50 ++++ 2 files changed, 340 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 9780798..1e844f8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -17,9 +17,299 @@

#include <linux/coresight.h> #include <linux/dma-mapping.h> +#include <linux/slab.h> #include "coresight-priv.h" #include "coresight-tmc.h"

+/* + * tmc_pages_get_offset: Go through all the pages in the tmc_pages + * and map the device address @addr to an offset within the virtual + * contiguous buffer. + */ +static long +tmc_pages_get_offset(struct tmc_pages *tmc_pages, dma_addr_t addr) +{ + int i; + dma_addr_t page_start; + + for (i = 0; i < tmc_pages->nr_pages; i++) { + page_start = tmc_pages->daddrs[i]; + if (addr >= page_start && addr < (page_start + PAGE_SIZE)) + return i * PAGE_SIZE + (addr - page_start); + } + + return -EINVAL; +} + +/* + * tmc_pages_free : Unmap and free the pages used by tmc_pages. + * If the pages were not allocated in tmc_pages_alloc(), we would + * simply drop the refcount. + */ +static void tmc_pages_free(struct tmc_pages *tmc_pages, + struct device *dev, enum dma_data_direction dir) +{ + int i; + + for (i = 0; i < tmc_pages->nr_pages; i++) { + if (tmc_pages->daddrs && tmc_pages->daddrs[i]) + dma_unmap_page(dev, tmc_pages->daddrs[i], + PAGE_SIZE, dir); + if (tmc_pages->pages && tmc_pages->pages[i]) + __free_page(tmc_pages->pages[i]); + } + + kfree(tmc_pages->pages); + kfree(tmc_pages->daddrs); + tmc_pages->pages = NULL; + tmc_pages->daddrs = NULL; + tmc_pages->nr_pages = 0; +} + +/* + * tmc_pages_alloc : Allocate and map pages for a given @tmc_pages. + * If @pages is not NULL, the list of page virtual addresses are + * used as the data pages. The pages are then dma_map'ed for @dev + * with dma_direction @dir. + * + * Returns 0 upon success, else the error number. + */ +static int tmc_pages_alloc(struct tmc_pages *tmc_pages, + struct device *dev, int node, + enum dma_data_direction dir, void **pages) +{ + int i, nr_pages; + dma_addr_t paddr; + struct page *page; + + nr_pages = tmc_pages->nr_pages; + tmc_pages->daddrs = kcalloc(nr_pages, sizeof(*tmc_pages->daddrs), + GFP_KERNEL); + if (!tmc_pages->daddrs) + return -ENOMEM; + tmc_pages->pages = kcalloc(nr_pages, sizeof(*tmc_pages->pages), + GFP_KERNEL); + if (!tmc_pages->pages) { + kfree(tmc_pages->daddrs); + tmc_pages->daddrs = NULL; + return -ENOMEM; + } + + for (i = 0; i < nr_pages; i++) { + if (pages && pages[i]) { + page = virt_to_page(pages[i]); + /* Hold a refcount on the page */ + get_page(page); + } else { + page = alloc_pages_node(node, + GFP_KERNEL | __GFP_ZERO, 0); + } + paddr = dma_map_page(dev, page, 0, PAGE_SIZE, dir); + if (dma_mapping_error(dev, paddr)) + goto err; + tmc_pages->daddrs[i] = paddr; + tmc_pages->pages[i] = page; + } + return 0; +err: + tmc_pages_free(tmc_pages, dev, dir); + return -ENOMEM; +} + +static inline dma_addr_t tmc_sg_table_base_paddr(struct tmc_sg_table *sg_table) +{ + if (WARN_ON(!sg_table->data_pages.pages[0])) + return 0; + return sg_table->table_daddr; +} + +static inline void *tmc_sg_table_base_vaddr(struct tmc_sg_table *sg_table) +{ + if (WARN_ON(!sg_table->data_pages.pages[0])) + return NULL; + return sg_table->table_vaddr; +} + +static inline void * +tmc_sg_table_data_vaddr(struct tmc_sg_table *sg_table) +{ + if (WARN_ON(!sg_table->data_pages.nr_pages)) + return 0; + return sg_table->data_vaddr; +} + +static inline long +tmc_sg_get_data_page_offset(struct tmc_sg_table *sg_table, dma_addr_t addr) +{ + return tmc_pages_get_offset(&sg_table->data_pages, addr); +} + +static inline void tmc_free_table_pages(struct tmc_sg_table *sg_table) +{ + if (sg_table->table_vaddr) + vunmap(sg_table->table_vaddr); + tmc_pages_free(&sg_table->table_pages, sg_table->dev, DMA_TO_DEVICE); +} + +static void tmc_free_data_pages(struct tmc_sg_table *sg_table) +{ + if (sg_table->data_vaddr) + vunmap(sg_table->data_vaddr); + tmc_pages_free(&sg_table->data_pages, sg_table->dev, DMA_FROM_DEVICE); +} + +void tmc_free_sg_table(struct tmc_sg_table *sg_table) +{ + tmc_free_table_pages(sg_table); + tmc_free_data_pages(sg_table); +} + +/* + * 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) + * rather than the CPU node). + */ +static int tmc_alloc_table_pages(struct tmc_sg_table *sg_table) +{ + int rc; + struct tmc_pages *table_pages = &sg_table->table_pages; + + rc = tmc_pages_alloc(table_pages, sg_table->dev, + dev_to_node(sg_table->dev), + DMA_TO_DEVICE, NULL); + if (rc) + return rc; + sg_table->table_vaddr = vmap(table_pages->pages, + table_pages->nr_pages, + VM_MAP, + PAGE_KERNEL); + if (!sg_table->table_vaddr) + rc = -ENOMEM; + else + sg_table->table_daddr = table_pages->daddrs[0]; + return rc; +} + +static int tmc_alloc_data_pages(struct tmc_sg_table *sg_table, void **pages) +{ + int rc; + + /* Allocate data pages on the node requested by the caller */ + rc = tmc_pages_alloc(&sg_table->data_pages, + sg_table->dev, sg_table->node, + DMA_FROM_DEVICE, pages); + if (!rc) { + sg_table->data_vaddr = vmap(sg_table->data_pages.pages, + sg_table->data_pages.nr_pages, + VM_MAP, + PAGE_KERNEL); + if (!sg_table->data_vaddr) + rc = -ENOMEM; + } + return rc; +} + +/* + * tmc_alloc_sg_table: Allocate and setup dma pages for the TMC SG table + * and data buffers. TMC writes to the data buffers and reads from the SG + * Table pages. + * + * @dev - Device to which page should be DMA mapped. + * @node - Numa node for mem allocations + * @nr_tpages - Number of pages for the table entries. + * @nr_dpages - Number of pages for Data buffer. + * @pages - Optional list of virtual address of pages. + */ +struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev, + int node, + int nr_tpages, + int nr_dpages, + void **pages) +{ + long rc; + struct tmc_sg_table *sg_table; + + sg_table = kzalloc(sizeof(*sg_table), GFP_KERNEL); + if (!sg_table) + return ERR_PTR(-ENOMEM); + sg_table->data_pages.nr_pages = nr_dpages; + sg_table->table_pages.nr_pages = nr_tpages; + sg_table->node = node; + sg_table->dev = dev; + + rc = tmc_alloc_data_pages(sg_table, pages); + if (!rc) + rc = tmc_alloc_table_pages(sg_table); + if (rc) { + tmc_free_sg_table(sg_table); + kfree(sg_table); + return ERR_PTR(rc); + } + + return sg_table; +} + +/* + * tmc_sg_table_sync_data_range: Sync the data buffer written + * by the device from @offset upto a @size bytes. + */ +void tmc_sg_table_sync_data_range(struct tmc_sg_table *table, + u64 offset, u64 size) +{ + int i, index, start; + int npages = DIV_ROUND_UP(size, PAGE_SIZE); + struct device *dev = table->dev; + struct tmc_pages *data = &table->data_pages; + + start = offset >> PAGE_SHIFT; + for (i = start; i < (start + npages); i++) { + index = i % data->nr_pages; + dma_sync_single_for_cpu(dev, data->daddrs[index], + PAGE_SIZE, DMA_FROM_DEVICE); + } +} + +/* tmc_sg_sync_table: Sync the page table */ +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table) +{ + int i; + struct device *dev = sg_table->dev; + struct tmc_pages *table_pages = &sg_table->table_pages; + + for (i = 0; i < table_pages->nr_pages; i++) + dma_sync_single_for_device(dev, table_pages->daddrs[i], + PAGE_SIZE, DMA_TO_DEVICE); +} + +/* + * tmc_sg_table_get_data: Get the buffer pointer for data @offset + * in the SG buffer. The @bufpp is updated to point to the buffer. + * Returns : + * the length of linear data available at @offset. + * or + * <= 0 if no data is available. + */ +ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, + u64 offset, size_t len, char **bufpp) +{ + size_t size; + int pg_idx = offset >> PAGE_SHIFT; + int pg_offset = offset & (PAGE_SIZE - 1); + struct tmc_pages *data_pages = &sg_table->data_pages; + + size = tmc_sg_table_buf_size(sg_table); + if (offset >= size) + return -EINVAL; + + /* Make sure we don't go beyond the end */ + len = (len < (size - offset)) ? len : size - offset; + /* Respect the page boundaries */ + len = (len < (PAGE_SIZE - pg_offset)) ? len : (PAGE_SIZE - pg_offset); + if (len > 0) + *bufpp = page_address(data_pages->pages[pg_idx]) + pg_offset; + return len; +} + static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) { u32 axictl, sts; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 73f944d..19a765c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -18,6 +18,7 @@ #ifndef _CORESIGHT_TMC_H #define _CORESIGHT_TMC_H

+#include <linux/dma-mapping.h> #include <linux/miscdevice.h>

#define TMC_RSZ 0x004 @@ -171,6 +172,38 @@ struct tmc_drvdata { u32 etr_caps; };

+/** + * struct tmc_pages - Collection of pages used for SG. + * @nr_pages: Number of pages in the list. + * @daddrs: Array of DMA'able page address. + * @pages: Array pages for the buffer. + */ +struct tmc_pages { + int nr_pages; + dma_addr_t *daddrs; + struct page **pages; +}; + +/* + * struct tmc_sg_table - Generic SG table for TMC + * @dev: Device for DMA allocations + * @table_vaddr: Contiguous Virtual address for PageTable + * @data_vaddr: Contiguous Virtual address for Data Buffer + * @table_daddr: DMA address of the PageTable base + * @node: Node for Page allocations + * @table_pages: List of pages & dma address for Table + * @data_pages: List of pages & dma address for Data + */ +struct tmc_sg_table { + struct device *dev; + void *table_vaddr; + void *data_vaddr; + dma_addr_t table_daddr; + int node; + struct tmc_pages table_pages; + struct tmc_pages data_pages; +}; + /* Generic functions */ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); void tmc_flush_and_stop(struct tmc_drvdata *drvdata); @@ -226,4 +259,21 @@ static inline bool tmc_etr_has_cap(struct tmc_drvdata *drvdata, u32 cap) return !!(drvdata->etr_caps & cap); }

+struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev, + int node, + int nr_tpages, + int nr_dpages, + void **pages); +void tmc_free_sg_table(struct tmc_sg_table *sg_table); +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table); +void tmc_sg_table_sync_data_range(struct tmc_sg_table *table, + u64 offset, u64 size); +ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, + u64 offset, size_t len, char **bufpp); +static inline unsigned long +tmc_sg_table_buf_size(struct tmc_sg_table *sg_table) +{ + return sg_table->data_pages.nr_pages << PAGE_SHIFT; +} + #endif

At the moment we always use contiguous memory for TMC ETR tracing when used from sysfs. The size of the buffer is fixed at boot time and can only be changed by modifiying the DT. With the introduction of SG support we could support really large buffers in that mode. This patch abstracts the buffer used for ETR to switch between a contiguous buffer or a SG table depending on the availability of the memory.

This also enables the sysfs mode to use the ETR in SG mode depending on configured the trace buffer size. Also, since ETR will use the new infrastructure to manage the buffer, we can get rid of some of the members in the tmc_drvdata and clean up the fields a bit.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com --- drivers/hwtracing/coresight/coresight-tmc-etr.c | 450 +++++++++++++++++++----- drivers/hwtracing/coresight/coresight-tmc.h | 57 ++- 2 files changed, 418 insertions(+), 89 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 7ab0fd1..143afba 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -17,10 +17,18 @@

#include <linux/coresight.h> #include <linux/dma-mapping.h> +#include <linux/iommu.h> #include <linux/slab.h> #include "coresight-priv.h" #include "coresight-tmc.h"

+struct etr_flat_buf { + struct device *dev; + dma_addr_t daddr; + void *vaddr; + size_t size; +}; + /* * The TMC ETR SG has a page size of 4K. The SG table contains pointers * to 4KB buffers. However, the OS may use a PAGE_SIZE different from @@ -541,7 +549,7 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) * @size - Total size of the data buffer * @pages - Optional list of page virtual address */ -static struct etr_sg_table __maybe_unused * +static struct etr_sg_table * tmc_init_etr_sg_table(struct device *dev, int node, unsigned long size, void **pages) { @@ -573,16 +581,307 @@ tmc_init_etr_sg_table(struct device *dev, int node, return etr_table; }

+/* + * tmc_etr_alloc_flat_buf: Allocate a contiguous DMA buffer. + */ +static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, + struct etr_buf *etr_buf, int node, + void **pages) +{ + struct etr_flat_buf *flat_buf; + + /* We cannot reuse existing pages for flat buf */ + if (pages) + return -EINVAL; + + flat_buf = kzalloc(sizeof(*flat_buf), GFP_KERNEL); + if (!flat_buf) + return -ENOMEM; + + flat_buf->vaddr = dma_alloc_coherent(drvdata->dev, etr_buf->size, + &flat_buf->daddr, GFP_KERNEL); + if (!flat_buf->vaddr) { + kfree(flat_buf); + return -ENOMEM; + } + + flat_buf->size = etr_buf->size; + flat_buf->dev = drvdata->dev; + etr_buf->hwaddr = flat_buf->daddr; + etr_buf->mode = ETR_MODE_FLAT; + etr_buf->private = flat_buf; + return 0; +} + +static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) +{ + struct etr_flat_buf *flat_buf = etr_buf->private; + + if (flat_buf && flat_buf->daddr) + dma_free_coherent(flat_buf->dev, flat_buf->size, + flat_buf->vaddr, flat_buf->daddr); + kfree(flat_buf); +} + +static void tmc_etr_sync_flat_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp) +{ + /* + * Adjust the buffer to point to the beginning of the trace data + * and update the available trace data. + */ + etr_buf->offset = rrp - etr_buf->hwaddr; + if (etr_buf->full) + etr_buf->len = etr_buf->size; + else + etr_buf->len = rwp - rrp; +} + +static ssize_t tmc_etr_get_data_flat_buf(struct etr_buf *etr_buf, + u64 offset, size_t len, char **bufpp) +{ + struct etr_flat_buf *flat_buf = etr_buf->private; + + *bufpp = (char *)flat_buf->vaddr + offset; + /* + * tmc_etr_buf_get_data already adjusts the length to handle + * buffer wrapping around. + */ + return len; +} + +static const struct etr_buf_operations etr_flat_buf_ops = { + .alloc = tmc_etr_alloc_flat_buf, + .free = tmc_etr_free_flat_buf, + .sync = tmc_etr_sync_flat_buf, + .get_data = tmc_etr_get_data_flat_buf, +}; + +/* + * tmc_etr_alloc_sg_buf: Allocate an SG buf @etr_buf. Setup the parameters + * appropriately. + */ +static int tmc_etr_alloc_sg_buf(struct tmc_drvdata *drvdata, + struct etr_buf *etr_buf, int node, + void **pages) +{ + struct etr_sg_table *etr_table; + + etr_table = tmc_init_etr_sg_table(drvdata->dev, node, + etr_buf->size, pages); + if (IS_ERR(etr_table)) + return -ENOMEM; + etr_buf->hwaddr = etr_table->hwaddr; + etr_buf->mode = ETR_MODE_ETR_SG; + etr_buf->private = etr_table; + return 0; +} + +static void tmc_etr_free_sg_buf(struct etr_buf *etr_buf) +{ + struct etr_sg_table *etr_table = etr_buf->private; + + if (etr_table) { + tmc_free_sg_table(etr_table->sg_table); + kfree(etr_table); + } +} + +static ssize_t tmc_etr_get_data_sg_buf(struct etr_buf *etr_buf, u64 offset, + size_t len, char **bufpp) +{ + struct etr_sg_table *etr_table = etr_buf->private; + + return tmc_sg_table_get_data(etr_table->sg_table, offset, len, bufpp); +} + +static void tmc_etr_sync_sg_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp) +{ + long r_offset, w_offset; + struct etr_sg_table *etr_table = etr_buf->private; + struct tmc_sg_table *table = etr_table->sg_table; + + /* Convert hw address to offset in the buffer */ + r_offset = tmc_sg_get_data_page_offset(table, rrp); + if (r_offset < 0) { + dev_warn(table->dev, + "Unable to map RRP %llx to offset\n", rrp); + etr_buf->len = 0; + return; + } + + w_offset = tmc_sg_get_data_page_offset(table, rwp); + if (w_offset < 0) { + dev_warn(table->dev, + "Unable to map RWP %llx to offset\n", rwp); + etr_buf->len = 0; + return; + } + + etr_buf->offset = r_offset; + if (etr_buf->full) + etr_buf->len = etr_buf->size; + else + etr_buf->len = ((w_offset < r_offset) ? etr_buf->size : 0) + + w_offset - r_offset; + tmc_sg_table_sync_data_range(table, r_offset, etr_buf->len); +} + +static const struct etr_buf_operations etr_sg_buf_ops = { + .alloc = tmc_etr_alloc_sg_buf, + .free = tmc_etr_free_sg_buf, + .sync = tmc_etr_sync_sg_buf, + .get_data = tmc_etr_get_data_sg_buf, +}; + +static const struct etr_buf_operations *etr_buf_ops[] = { + [ETR_MODE_FLAT] = &etr_flat_buf_ops, + [ETR_MODE_ETR_SG] = &etr_sg_buf_ops, +}; + +static inline int tmc_etr_mode_alloc_buf(int mode, + struct tmc_drvdata *drvdata, + struct etr_buf *etr_buf, int node, + void **pages) +{ + int rc; + + switch (mode) { + case ETR_MODE_FLAT: + case ETR_MODE_ETR_SG: + rc = etr_buf_ops[mode]->alloc(drvdata, etr_buf, node, pages); + if (!rc) + etr_buf->ops = etr_buf_ops[mode]; + return rc; + default: + return -EINVAL; + } +} + +/* + * tmc_alloc_etr_buf: Allocate a buffer use by ETR. + * @drvdata : ETR device details. + * @size : size of the requested buffer. + * @flags : Required properties for the buffer. + * @node : Node for memory allocations. + * @pages : An optional list of pages. + */ +static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata, + ssize_t size, int flags, + int node, void **pages) +{ + int rc = -ENOMEM; + bool has_etr_sg, has_iommu; + struct etr_buf *etr_buf; + + has_etr_sg = tmc_etr_has_cap(drvdata, TMC_ETR_SG); + has_iommu = iommu_get_domain_for_dev(drvdata->dev); + + etr_buf = kzalloc(sizeof(*etr_buf), GFP_KERNEL); + if (!etr_buf) + return ERR_PTR(-ENOMEM); + + etr_buf->size = size; + + /* + * If we have to use an existing list of pages, we cannot reliably + * use a contiguous DMA memory (even if we have an IOMMU). Otherwise, + * we use the contiguous DMA memory if at least one of the following + * conditions is true: + * a) The ETR cannot use Scatter-Gather. + * b) we have a backing IOMMU + * c) The requested memory size is smaller (< 1M). + * + * Fallback to available mechanisms. + * + */ + if (!pages && + (!has_etr_sg || has_iommu || size < SZ_1M)) + rc = tmc_etr_mode_alloc_buf(ETR_MODE_FLAT, drvdata, + etr_buf, node, pages); + if (rc && has_etr_sg) + rc = tmc_etr_mode_alloc_buf(ETR_MODE_ETR_SG, drvdata, + etr_buf, node, pages); + if (rc) { + kfree(etr_buf); + return ERR_PTR(rc); + } + + return etr_buf; +} + +static void tmc_free_etr_buf(struct etr_buf *etr_buf) +{ + WARN_ON(!etr_buf->ops || !etr_buf->ops->free); + etr_buf->ops->free(etr_buf); + kfree(etr_buf); +} + +/* + * tmc_etr_buf_get_data: Get the pointer the trace data at @offset + * with a maximum of @len bytes. + * 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) +{ + /* 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); +} + +static inline s64 +tmc_etr_buf_insert_barrier_packet(struct etr_buf *etr_buf, u64 offset) +{ + ssize_t len; + char *bufp; + + len = tmc_etr_buf_get_data(etr_buf, offset, + CORESIGHT_BARRIER_PKT_SIZE, &bufp); + if (WARN_ON(len <= CORESIGHT_BARRIER_PKT_SIZE)) + return -EINVAL; + coresight_insert_barrier_packet(bufp); + return offset + CORESIGHT_BARRIER_PKT_SIZE; +} + +/* + * tmc_sync_etr_buf: Sync the trace buffer availability with drvdata. + * Makes sure the trace data is synced to the memory for consumption. + * @etr_buf->offset will hold the offset to the beginning of the trace data + * within the buffer, with @etr_buf->len bytes to consume. + */ +static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) +{ + struct etr_buf *etr_buf = drvdata->etr_buf; + u64 rrp, rwp; + u32 status; + + rrp = tmc_read_rrp(drvdata); + rwp = tmc_read_rwp(drvdata); + status = readl_relaxed(drvdata->base + TMC_STS); + etr_buf->full = status & TMC_STS_FULL; + + WARN_ON(!etr_buf->ops || !etr_buf->ops->sync); + + etr_buf->ops->sync(etr_buf, rrp, rwp); + + /* Insert barrier packets at the beginning, if there was an overflow */ + if (etr_buf->full) + tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset); +} + static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) { u32 axictl, sts; + struct etr_buf *etr_buf = drvdata->etr_buf;

CS_UNLOCK(drvdata->base);

/* Wait for TMCSReady bit to be set */ tmc_wait_for_tmcready(drvdata);

- writel_relaxed(drvdata->size / 4, drvdata->base + TMC_RSZ); + writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ); writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);

axictl = readl_relaxed(drvdata->base + TMC_AXICTL); @@ -595,16 +894,22 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) axictl |= TMC_AXICTL_ARCACHE_OS; }

+ if (etr_buf->mode == ETR_MODE_ETR_SG) { + if (WARN_ON(!tmc_etr_has_cap(drvdata, TMC_ETR_SG))) + return; + axictl |= TMC_AXICTL_SCT_GAT_MODE; + } + writel_relaxed(axictl, drvdata->base + TMC_AXICTL); - tmc_write_dba(drvdata, drvdata->paddr); + tmc_write_dba(drvdata, etr_buf->hwaddr); /* * If the TMC pointers must be programmed before the session, * we have to set it properly (i.e, RRP/RWP to base address and * STS to "not full"). */ if (tmc_etr_has_cap(drvdata, TMC_ETR_SAVE_RESTORE)) { - tmc_write_rrp(drvdata, drvdata->paddr); - tmc_write_rwp(drvdata, drvdata->paddr); + tmc_write_rrp(drvdata, etr_buf->hwaddr); + tmc_write_rwp(drvdata, etr_buf->hwaddr); sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL; writel_relaxed(sts, drvdata->base + TMC_STS); } @@ -620,63 +925,53 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) }

/* - * Return the available trace data in the buffer @pos, with a maximum - * limit of @len, also updating the @bufpp on where to find it. + * Return the available trace data in the buffer (starts at etr_buf->offset, + * limited by etr_buf->len) from @pos, with a maximum limit of @len, + * also updating the @bufpp on where to find it. Since the trace data + * starts at anywhere in the buffer, depending on the RRP, we adjust the + * @len returned to handle buffer wrapping around. */ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len, char **bufpp) { + s64 offset; ssize_t actual = len; - char *bufp = drvdata->buf + pos; - char *bufend = (char *)(drvdata->vaddr + drvdata->size); - - /* Adjust the len to available size @pos */ - if (pos + actual > drvdata->len) - actual = drvdata->len - pos; + struct etr_buf *etr_buf = drvdata->etr_buf;

+ if (pos + actual > etr_buf->len) + actual = etr_buf->len - pos; if (actual <= 0) return actual;

- /* - * Since we use a circular buffer, with trace data starting - * @drvdata->buf, possibly anywhere in the buffer @drvdata->vaddr, - * wrap the current @pos to within the buffer. - */ - if (bufp >= bufend) - bufp -= drvdata->size; - /* - * For simplicity, avoid copying over a wrapped around buffer. - */ - if ((bufp + actual) > bufend) - actual = bufend - bufp; - *bufpp = bufp; - return actual; + /* Compute the offset from which we read the data */ + offset = etr_buf->offset + pos; + if (offset >= etr_buf->size) + offset -= etr_buf->size; + return tmc_etr_buf_get_data(etr_buf, offset, actual, bufpp); }

-static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) +static struct etr_buf * +tmc_etr_setup_sysfs_buf(struct tmc_drvdata *drvdata) { - u32 val; - u64 rwp; + return tmc_alloc_etr_buf(drvdata, drvdata->size, + 0, cpu_to_node(0), NULL); +}

- rwp = tmc_read_rwp(drvdata); - val = readl_relaxed(drvdata->base + TMC_STS); +static void +tmc_etr_free_sysfs_buf(struct etr_buf *buf) +{ + if (buf) + tmc_free_etr_buf(buf); +}

- /* - * Adjust the buffer to point to the beginning of the trace data - * and update the available trace data. - */ - if (val & TMC_STS_FULL) { - drvdata->buf = drvdata->vaddr + rwp - drvdata->paddr; - drvdata->len = drvdata->size; - coresight_insert_barrier_packet(drvdata->buf); - } else { - drvdata->buf = drvdata->vaddr; - drvdata->len = rwp - drvdata->paddr; - } +static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata) +{ + tmc_sync_etr_buf(drvdata); }

static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) { + CS_UNLOCK(drvdata->base);

tmc_flush_and_stop(drvdata); @@ -685,7 +980,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) * read before the TMC is disabled. */ if (drvdata->mode == CS_MODE_SYSFS) - tmc_etr_dump_hw(drvdata); + tmc_etr_sync_sysfs_buf(drvdata); + tmc_disable_hw(drvdata);

CS_LOCK(drvdata->base); @@ -696,34 +992,31 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) int ret = 0; bool used = false; unsigned long flags; - void __iomem *vaddr = NULL; - dma_addr_t paddr; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + struct etr_buf *new_buf = NULL, *free_buf = NULL;

/* - * If we don't have a buffer release the lock and allocate memory. - * Otherwise keep the lock and move along. + * If we are enabling the ETR from disabled state, we need to make + * sure we have a buffer with the right size. The etr_buf is not reset + * immediately after we stop the tracing in SYSFS mode as we wait for + * the user to collect the data. We may be able to reuse the existing + * buffer, provided the size matches. Any allocation has to be done + * with the lock released. */ spin_lock_irqsave(&drvdata->spinlock, flags); - if (!drvdata->vaddr) { + if (!drvdata->etr_buf || (drvdata->etr_buf->size != drvdata->size)) { spin_unlock_irqrestore(&drvdata->spinlock, flags); - - /* - * Contiguous memory can't be allocated while a spinlock is - * held. As such allocate memory here and free it if a buffer - * has already been allocated (from a previous session). - */ - vaddr = dma_alloc_coherent(drvdata->dev, drvdata->size, - &paddr, GFP_KERNEL); - if (!vaddr) - return -ENOMEM; + /* Allocate memory with the spinlock released */ + free_buf = new_buf = tmc_etr_setup_sysfs_buf(drvdata); + if (IS_ERR(new_buf)) + return PTR_ERR(new_buf);

/* Let's try again */ spin_lock_irqsave(&drvdata->spinlock, flags); }

- if (drvdata->reading) { + if (drvdata->reading || drvdata->mode == CS_MODE_PERF) { ret = -EBUSY; goto out; } @@ -731,21 +1024,20 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) /* * In sysFS mode we can have multiple writers per sink. Since this * sink is already enabled no memory is needed and the HW need not be - * touched. + * touched, even if the buffer size has changed. */ if (drvdata->mode == CS_MODE_SYSFS) goto out;

/* - * If drvdata::buf == NULL, use the memory allocated above. - * Otherwise a buffer still exists from a previous session, so - * simply use that. + * If we don't have a buffer or it doesn't match the requested size, + * use the memory allocated above. Otherwise reuse it. */ - if (drvdata->buf == NULL) { + if (!drvdata->etr_buf || + (new_buf && drvdata->etr_buf->size != new_buf->size)) { used = true; - drvdata->vaddr = vaddr; - drvdata->paddr = paddr; - drvdata->buf = drvdata->vaddr; + free_buf = drvdata->etr_buf; + drvdata->etr_buf = new_buf; }

drvdata->mode = CS_MODE_SYSFS; @@ -754,8 +1046,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) spin_unlock_irqrestore(&drvdata->spinlock, flags);

/* Free memory outside the spinlock if need be */ - if (!used && vaddr) - dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr); + if (free_buf) + tmc_etr_free_sysfs_buf(free_buf);

if (!ret) dev_info(drvdata->dev, "TMC-ETR enabled\n"); @@ -834,8 +1126,8 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) goto out; }

- /* If drvdata::buf is NULL the trace data has been read already */ - if (drvdata->buf == NULL) { + /* If drvdata::etr_buf is NULL the trace data has been read already */ + if (drvdata->etr_buf == NULL) { ret = -EINVAL; goto out; } @@ -854,8 +1146,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) { unsigned long flags; - dma_addr_t paddr; - void __iomem *vaddr = NULL; + struct etr_buf *etr_buf = NULL;

/* config types are set a boot time and never change */ if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) @@ -876,17 +1167,16 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) * The ETR is not tracing and the buffer was just read. * As such prepare to free the trace buffer. */ - vaddr = drvdata->vaddr; - paddr = drvdata->paddr; - drvdata->buf = drvdata->vaddr = NULL; + etr_buf = drvdata->etr_buf; + drvdata->etr_buf = NULL; }

drvdata->reading = false; spin_unlock_irqrestore(&drvdata->spinlock, flags);

/* Free allocated memory out side of the spinlock */ - if (vaddr) - dma_free_coherent(drvdata->dev, drvdata->size, vaddr, paddr); + if (etr_buf) + tmc_free_etr_buf(etr_buf);

return 0; } diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 19a765c..c00643c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -55,6 +55,7 @@ #define TMC_STS_TMCREADY_BIT 2 #define TMC_STS_FULL BIT(0) #define TMC_STS_TRIGGERED BIT(1) + /* * TMC_AXICTL - 0x110 * @@ -134,6 +135,35 @@ enum tmc_mem_intf_width { #define CORESIGHT_SOC_600_ETR_CAPS \ (TMC_ETR_SAVE_RESTORE | TMC_ETR_AXI_ARCACHE)

+enum etr_mode { + ETR_MODE_FLAT, /* Uses contiguous flat buffer */ + ETR_MODE_ETR_SG, /* Uses in-built TMC ETR SG mechanism */ +}; + +struct etr_buf_operations; + +/** + * struct etr_buf - Details of the buffer used by ETR + * @mode : Mode of the ETR buffer, contiguous, Scatter Gather etc. + * @full : Trace data overflow + * @size : Size of the buffer. + * @hwaddr : Address to be programmed in the TMC:DBA{LO,HI} + * @offset : Offset of the trace data in the buffer for consumption. + * @len : Available trace data @buf (may round up to the beginning). + * @ops : ETR buffer operations for the mode. + * @private : Backend specific information for the buf + */ +struct etr_buf { + enum etr_mode mode; + bool full; + ssize_t size; + dma_addr_t hwaddr; + unsigned long offset; + s64 len; + const struct etr_buf_operations *ops; + void *private; +}; + /** * struct tmc_drvdata - specifics associated to an TMC component * @base: memory mapped base address for this component. @@ -141,11 +171,10 @@ enum tmc_mem_intf_width { * @csdev: component vitals needed by the framework. * @miscdev: specifics to handle "/dev/xyz.tmc" entry. * @spinlock: only one at a time pls. - * @buf: area of memory where trace data get sent. - * @paddr: DMA start location in RAM. - * @vaddr: virtual representation of @paddr. - * @size: trace buffer size. - * @len: size of the available trace. + * @buf: Snapshot of the trace data for ETF/ETB. + * @etr_buf: details of buffer used in TMC-ETR + * @len: size of the available trace for ETF/ETB. + * @size: trace buffer size for this TMC (common for all modes). * @mode: how this TMC is being used. * @config_type: TMC variant, must be of type @tmc_config_type. * @memwidth: width of the memory interface databus, in bytes. @@ -160,11 +189,12 @@ struct tmc_drvdata { struct miscdevice miscdev; spinlock_t spinlock; bool reading; - char *buf; - dma_addr_t paddr; - void __iomem *vaddr; - u32 size; + union { + char *buf; /* TMC ETB */ + struct etr_buf *etr_buf; /* TMC ETR */ + }; u32 len; + u32 size; u32 mode; enum tmc_config_type config_type; enum tmc_mem_intf_width memwidth; @@ -172,6 +202,15 @@ struct tmc_drvdata { u32 etr_caps; };

+struct etr_buf_operations { + int (*alloc)(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf, + int node, void **pages); + void (*sync)(struct etr_buf *etr_buf, u64 rrp, u64 rwp); + ssize_t (*get_data)(struct etr_buf *etr_buf, u64 offset, size_t len, + char **bufpp); + void (*free)(struct etr_buf *etr_buf); +}; + /** * struct tmc_pages - Collection of pages used for SG. * @nr_pages: Number of pages in the list.

On 18 May 2018 at 10:39, Suzuki K Poulose suzuki.poulose@arm.com wrote:

Advertise that the scatter-gather is properly integrated on all revisions of Juno board.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Cc: Sudeep Holla sudeep.holla@arm.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Lorenzo Pieralisi lorenzo.pieralisi@arm.com Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

Reviewed-by: Mathieu Poirier mathieu.poirier@linaro.org

---

arch/arm64/boot/dts/arm/juno-base.dtsi | 1 + 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/arm/juno-base.dtsi b/arch/arm64/boot/dts/arm/juno-base.dtsi index eb749c5..6ce9090 100644 --- a/arch/arm64/boot/dts/arm/juno-base.dtsi +++ b/arch/arm64/boot/dts/arm/juno-base.dtsi @@ -198,6 +198,7 @@ clocks = <&soc_smc50mhz>; clock-names = "apb_pclk"; power-domains = <&scpi_devpd 0>;

•           arm,scatter-gather;
            port {
                    etr_in_port: endpoint {
                            slave-mode;
  

-- 2.7.4

On Fri, May 18, 2018 at 05:39:24PM +0100, Suzuki K Poulose wrote:

This patch introduces a generic sg table data structure and associated operations. An SG table can be used to map a set of Data pages where the trace data could be stored by the TMC ETR. The information about the data pages could be stored in different formats, depending on the type of the underlying SG mechanism (e.g, TMC ETR SG vs Coresight CATU). The generic structure provides book keeping of the pages used for the data as well as the table contents. The table should be filled by the user of the infrastructure.

A table can be created by specifying the number of data pages as well as the number of table pages required to hold the pointers, where the latter could be different for different types of tables. The pages are mapped in the appropriate dma data direction mode (i.e, DMA_TO_DEVICE for table pages and DMA_FROM_DEVICE for data pages). The framework can optionally accept a set of allocated data pages (e.g, perf ring buffer) and map them accordingly. The table and data pages are vmap'ed to allow easier access by the drivers. The framework also provides helpers to sync the data written to the pages with appropriate directions.

This will be later used by the TMC ETR SG unit and CATU.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

Changes since v1:

• Address code style issues, more comments

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 290 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 50 ++++ 2 files changed, 340 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 9780798..1e844f8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -17,9 +17,299 @@ #include <linux/coresight.h> #include <linux/dma-mapping.h> +#include <linux/slab.h> #include "coresight-priv.h" #include "coresight-tmc.h" +/*

• • tmc_pages_get_offset: Go through all the pages in the tmc_pages
• • and map the device address @addr to an offset within the virtual
• • contiguous buffer.
• */

+static long +tmc_pages_get_offset(struct tmc_pages *tmc_pages, dma_addr_t addr) +{

• int i;
• dma_addr_t page_start;
• for (i = 0; i < tmc_pages->nr_pages; i++) {

• page_start = tmc_pages->daddrs[i];
  

• if (addr >= page_start && addr < (page_start + PAGE_SIZE))
  

• 	return i * PAGE_SIZE + (addr - page_start);
  

• }
• return -EINVAL;

+}

+/*

• • tmc_pages_free : Unmap and free the pages used by tmc_pages.
• • If the pages were not allocated in tmc_pages_alloc(), we would
• • simply drop the refcount.
• */

+static void tmc_pages_free(struct tmc_pages *tmc_pages,

• 	   struct device *dev, enum dma_data_direction dir)
  

+{

• int i;
• for (i = 0; i < tmc_pages->nr_pages; i++) {

• if (tmc_pages->daddrs && tmc_pages->daddrs[i])
  

• 	dma_unmap_page(dev, tmc_pages->daddrs[i],
  

• 			 PAGE_SIZE, dir);
  

• if (tmc_pages->pages && tmc_pages->pages[i])
  

• 	__free_page(tmc_pages->pages[i]);
  

• }
• kfree(tmc_pages->pages);
• kfree(tmc_pages->daddrs);
• tmc_pages->pages = NULL;
• tmc_pages->daddrs = NULL;
• tmc_pages->nr_pages = 0;

+}

+/*

• • tmc_pages_alloc : Allocate and map pages for a given @tmc_pages.
• • If @pages is not NULL, the list of page virtual addresses are
• • used as the data pages. The pages are then dma_map'ed for @dev
• • with dma_direction @dir.
• • Returns 0 upon success, else the error number.
• */

+static int tmc_pages_alloc(struct tmc_pages *tmc_pages,

• 	   struct device *dev, int node,
  

• 	   enum dma_data_direction dir, void **pages)
  

+{

• int i, nr_pages;
• dma_addr_t paddr;
• struct page *page;
• nr_pages = tmc_pages->nr_pages;
• tmc_pages->daddrs = kcalloc(nr_pages, sizeof(*tmc_pages->daddrs),

• 			 GFP_KERNEL);
  

• if (!tmc_pages->daddrs)

• return -ENOMEM;
  

• tmc_pages->pages = kcalloc(nr_pages, sizeof(*tmc_pages->pages),

• 			 GFP_KERNEL);
  

• if (!tmc_pages->pages) {

• kfree(tmc_pages->daddrs);
  

• tmc_pages->daddrs = NULL;
  

• return -ENOMEM;
  

• }
• for (i = 0; i < nr_pages; i++) {

• if (pages && pages[i]) {
  

• 	page = virt_to_page(pages[i]);
  

• 	/* Hold a refcount on the page */
  

• 	get_page(page);
  

• } else {
  

• 	page = alloc_pages_node(node,
  

• 				GFP_KERNEL | __GFP_ZERO, 0);
  

• }
  

• paddr = dma_map_page(dev, page, 0, PAGE_SIZE, dir);
  

• if (dma_mapping_error(dev, paddr))
  

• 	goto err;
  

• tmc_pages->daddrs[i] = paddr;
  

• tmc_pages->pages[i] = page;
  

• }
• return 0;

+err:

• tmc_pages_free(tmc_pages, dev, dir);
• return -ENOMEM;

+}

+static inline dma_addr_t tmc_sg_table_base_paddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return 0;
  

• return sg_table->table_daddr;

+}

+static inline void *tmc_sg_table_base_vaddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return NULL;
  

• return sg_table->table_vaddr;

+}

The above two functions deal with DMA'able and virtual addresses for the table page buffer. Yet the test in the WARN_ON is done on the data page array. Shouldn't this be sg_table->table_pages.pages[0] instead?

If not please add a comment justifying your position so that someone else looking at the code does't end up thinking the same in a year from now.

+static inline void * +tmc_sg_table_data_vaddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.nr_pages))

• return 0;
  

• return sg_table->data_vaddr;

+}

I see that tmc_sg_table_base_vaddr() and tmc_sg_table_data_vaddr() are both returning the virtual address of the contiguous buffer for table and data respectively. Yet there is a discrepency in the naming convention. I would have expected tmc_sg_table_base_vaddr() and tmc_sg_data_base_vaddr() so that there is a little symmetry between them.

Otherwise looks good to me.

+static inline long +tmc_sg_get_data_page_offset(struct tmc_sg_table *sg_table, dma_addr_t addr) +{

• return tmc_pages_get_offset(&sg_table->data_pages, addr);

+}

+static inline void tmc_free_table_pages(struct tmc_sg_table *sg_table) +{

• if (sg_table->table_vaddr)

• vunmap(sg_table->table_vaddr);
  

• tmc_pages_free(&sg_table->table_pages, sg_table->dev, DMA_TO_DEVICE);

+}

+static void tmc_free_data_pages(struct tmc_sg_table *sg_table) +{

• if (sg_table->data_vaddr)

• vunmap(sg_table->data_vaddr);
  

• tmc_pages_free(&sg_table->data_pages, sg_table->dev, DMA_FROM_DEVICE);

+}

+void tmc_free_sg_table(struct tmc_sg_table *sg_table) +{

• tmc_free_table_pages(sg_table);
• tmc_free_data_pages(sg_table);

+}

+/*

• • 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)
• • rather than the CPU node).
• */

+static int tmc_alloc_table_pages(struct tmc_sg_table *sg_table) +{

• int rc;
• struct tmc_pages *table_pages = &sg_table->table_pages;
• rc = tmc_pages_alloc(table_pages, sg_table->dev,

• 	     dev_to_node(sg_table->dev),
  

• 	     DMA_TO_DEVICE, NULL);
  

• if (rc)

• return rc;
  

• sg_table->table_vaddr = vmap(table_pages->pages,

• 		     table_pages->nr_pages,
  

• 		     VM_MAP,
  

• 		     PAGE_KERNEL);
  

• if (!sg_table->table_vaddr)

• rc = -ENOMEM;
  

• else

• sg_table->table_daddr = table_pages->daddrs[0];
  

• return rc;

+}

+static int tmc_alloc_data_pages(struct tmc_sg_table *sg_table, void **pages) +{

• int rc;
• /* Allocate data pages on the node requested by the caller */
• rc = tmc_pages_alloc(&sg_table->data_pages,

• 	     sg_table->dev, sg_table->node,
  

• 	     DMA_FROM_DEVICE, pages);
  

• if (!rc) {

• sg_table->data_vaddr = vmap(sg_table->data_pages.pages,
  

• 			    sg_table->data_pages.nr_pages,
  

• 			    VM_MAP,
  

• 			    PAGE_KERNEL);
  

• if (!sg_table->data_vaddr)
  

• 	rc = -ENOMEM;
  

• }
• return rc;

+}

+/*

• • tmc_alloc_sg_table: Allocate and setup dma pages for the TMC SG table
• • and data buffers. TMC writes to the data buffers and reads from the SG
• • Table pages.
• • @dev - Device to which page should be DMA mapped.
• • @node - Numa node for mem allocations
• • @nr_tpages - Number of pages for the table entries.
• • @nr_dpages - Number of pages for Data buffer.
• • @pages - Optional list of virtual address of pages.
• */

+struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev,

• 			int node,
  

• 			int nr_tpages,
  

• 			int nr_dpages,
  

• 			void **pages)
  

+{

• long rc;
• struct tmc_sg_table *sg_table;
• sg_table = kzalloc(sizeof(*sg_table), GFP_KERNEL);
• if (!sg_table)

• return ERR_PTR(-ENOMEM);
  

• sg_table->data_pages.nr_pages = nr_dpages;
• sg_table->table_pages.nr_pages = nr_tpages;
• sg_table->node = node;
• sg_table->dev = dev;
• rc = tmc_alloc_data_pages(sg_table, pages);
• if (!rc)

• rc = tmc_alloc_table_pages(sg_table);
  

• if (rc) {

• tmc_free_sg_table(sg_table);
  

• kfree(sg_table);
  

• return ERR_PTR(rc);
  

• }
• return sg_table;

+}

+/*

• • tmc_sg_table_sync_data_range: Sync the data buffer written
• • by the device from @offset upto a @size bytes.
• */

+void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,

• 		  u64 offset, u64 size)
  

+{

• int i, index, start;
• int npages = DIV_ROUND_UP(size, PAGE_SIZE);
• struct device *dev = table->dev;
• struct tmc_pages *data = &table->data_pages;
• start = offset >> PAGE_SHIFT;
• for (i = start; i < (start + npages); i++) {

• index = i % data->nr_pages;
  

• dma_sync_single_for_cpu(dev, data->daddrs[index],
  

• 			PAGE_SIZE, DMA_FROM_DEVICE);
  

• }

+}

+/* tmc_sg_sync_table: Sync the page table */ +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table) +{

• int i;
• struct device *dev = sg_table->dev;
• struct tmc_pages *table_pages = &sg_table->table_pages;
• for (i = 0; i < table_pages->nr_pages; i++)

• dma_sync_single_for_device(dev, table_pages->daddrs[i],
  

• 			   PAGE_SIZE, DMA_TO_DEVICE);
  

+}

+/*

• • tmc_sg_table_get_data: Get the buffer pointer for data @offset
• • in the SG buffer. The @bufpp is updated to point to the buffer.
• • Returns :
• • the length of linear data available at @offset.
• • or
• • <= 0 if no data is available.
• */

+ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table,

• 	      u64 offset, size_t len, char **bufpp)
  

+{

• size_t size;
• int pg_idx = offset >> PAGE_SHIFT;
• int pg_offset = offset & (PAGE_SIZE - 1);
• struct tmc_pages *data_pages = &sg_table->data_pages;
• size = tmc_sg_table_buf_size(sg_table);
• if (offset >= size)

• return -EINVAL;
  

• /* Make sure we don't go beyond the end */
• len = (len < (size - offset)) ? len : size - offset;
• /* Respect the page boundaries */
• len = (len < (PAGE_SIZE - pg_offset)) ? len : (PAGE_SIZE - pg_offset);
• if (len > 0)

• *bufpp = page_address(data_pages->pages[pg_idx]) + pg_offset;
  

• return len;

+}

static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) { u32 axictl, sts; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 73f944d..19a765c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -18,6 +18,7 @@ #ifndef _CORESIGHT_TMC_H #define _CORESIGHT_TMC_H +#include <linux/dma-mapping.h> #include <linux/miscdevice.h> #define TMC_RSZ 0x004 @@ -171,6 +172,38 @@ struct tmc_drvdata { u32 etr_caps; }; +/**

• • struct tmc_pages - Collection of pages used for SG.
• • @nr_pages: Number of pages in the list.
• • @daddrs: Array of DMA'able page address.
• • @pages: Array pages for the buffer.
• */

+struct tmc_pages {

• int nr_pages;
• dma_addr_t *daddrs;
• struct page **pages;

+};

+/*

• • struct tmc_sg_table - Generic SG table for TMC
• • @dev: Device for DMA allocations
• • @table_vaddr: Contiguous Virtual address for PageTable
• • @data_vaddr: Contiguous Virtual address for Data Buffer
• • @table_daddr: DMA address of the PageTable base
• • @node: Node for Page allocations
• • @table_pages: List of pages & dma address for Table
• • @data_pages: List of pages & dma address for Data
• */

+struct tmc_sg_table {

• struct device *dev;
• void *table_vaddr;
• void *data_vaddr;
• dma_addr_t table_daddr;
• int node;
• struct tmc_pages table_pages;
• struct tmc_pages data_pages;

+};

/* Generic functions */ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata); void tmc_flush_and_stop(struct tmc_drvdata *drvdata); @@ -226,4 +259,21 @@ static inline bool tmc_etr_has_cap(struct tmc_drvdata *drvdata, u32 cap) return !!(drvdata->etr_caps & cap); } +struct tmc_sg_table *tmc_alloc_sg_table(struct device *dev,

• 			int node,
  

• 			int nr_tpages,
  

• 			int nr_dpages,
  

• 			void **pages);
  

+void tmc_free_sg_table(struct tmc_sg_table *sg_table); +void tmc_sg_table_sync_table(struct tmc_sg_table *sg_table); +void tmc_sg_table_sync_data_range(struct tmc_sg_table *table,

• 		  u64 offset, u64 size);
  

+ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table,

• 	      u64 offset, size_t len, char **bufpp);
  

+static inline unsigned long +tmc_sg_table_buf_size(struct tmc_sg_table *sg_table) +{

• return sg_table->data_pages.nr_pages << PAGE_SHIFT;

+}

#endif

2.7.4

On 23/05/18 21:25, Mathieu Poirier wrote:

On Fri, May 18, 2018 at 05:39:24PM +0100, Suzuki K Poulose wrote:

...

This patch introduces a generic sg table data structure and associated operations. An SG table can be used to map a set of Data pages where the trace data could be stored by the TMC ETR. The information about the data pages could be stored in different formats, depending on the type of the underlying SG mechanism (e.g, TMC ETR SG vs Coresight CATU). The generic structure provides book keeping of the pages used for the data as well as the table contents. The table should be filled by the user of the infrastructure.

A table can be created by specifying the number of data pages as well as the number of table pages required to hold the pointers, where the latter could be different for different types of tables. The pages are mapped in the appropriate dma data direction mode (i.e, DMA_TO_DEVICE for table pages and DMA_FROM_DEVICE for data pages). The framework can optionally accept a set of allocated data pages (e.g, perf ring buffer) and map them accordingly. The table and data pages are vmap'ed to allow easier access by the drivers. The framework also provides helpers to sync the data written to the pages with appropriate directions.

This will be later used by the TMC ETR SG unit and CATU.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

Changes since v1:

• Address code style issues, more comments

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 290 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 50 ++++ 2 files changed, 340 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 9780798..1e844f8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -17,9 +17,299 @@

...

+static inline dma_addr_t tmc_sg_table_base_paddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return 0;
  

• return sg_table->table_daddr;

+}

+static inline void *tmc_sg_table_base_vaddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return NULL;
  

• return sg_table->table_vaddr;

+}

The above two functions deal with DMA'able and virtual addresses for the table page buffer. Yet the test in the WARN_ON is done on the data page array. Shouldn't this be sg_table->table_pages.pages[0] instead?

The table is as good as empty if there are no data pages associated with the table. Hence the data_pages check.

If not please add a comment justifying your position so that someone else looking at the code does't end up thinking the same in a year from now.

I will add a comment to reflect the above.

...

+static inline void * +tmc_sg_table_data_vaddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.nr_pages))

• return 0;
  

• return sg_table->data_vaddr;

+}

I see that tmc_sg_table_base_vaddr() and tmc_sg_table_data_vaddr() are both returning the virtual address of the contiguous buffer for table and data respectively. Yet there is a discrepency in the naming convention. I would have expected tmc_sg_table_base_vaddr() and tmc_sg_data_base_vaddr() so that there is a little symmetry between them.

Agree. I will fix it.

Suzuki

On 25/05/18 17:43, Mathieu Poirier wrote:

On Fri, May 25, 2018 at 05:07:07PM +0100, Suzuki K Poulose wrote:

...

On 23/05/18 21:25, Mathieu Poirier wrote:

...

On Fri, May 18, 2018 at 05:39:24PM +0100, Suzuki K Poulose wrote:

...

This patch introduces a generic sg table data structure and associated operations. An SG table can be used to map a set of Data pages where the trace data could be stored by the TMC ETR. The information about the data pages could be stored in different formats, depending on the type of the underlying SG mechanism (e.g, TMC ETR SG vs Coresight CATU). The generic structure provides book keeping of the pages used for the data as well as the table contents. The table should be filled by the user of the infrastructure.

A table can be created by specifying the number of data pages as well as the number of table pages required to hold the pointers, where the latter could be different for different types of tables. The pages are mapped in the appropriate dma data direction mode (i.e, DMA_TO_DEVICE for table pages and DMA_FROM_DEVICE for data pages). The framework can optionally accept a set of allocated data pages (e.g, perf ring buffer) and map them accordingly. The table and data pages are vmap'ed to allow easier access by the drivers. The framework also provides helpers to sync the data written to the pages with appropriate directions.

This will be later used by the TMC ETR SG unit and CATU.

Cc: Mathieu Poirier mathieu.poirier@linaro.org Signed-off-by: Suzuki K Poulose suzuki.poulose@arm.com

---

Changes since v1:

• Address code style issues, more comments

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 290 ++++++++++++++++++++++++ drivers/hwtracing/coresight/coresight-tmc.h | 50 ++++ 2 files changed, 340 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 9780798..1e844f8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -17,9 +17,299 @@

...

...

+static inline dma_addr_t tmc_sg_table_base_paddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return 0;
  

• return sg_table->table_daddr;

+}

+static inline void *tmc_sg_table_base_vaddr(struct tmc_sg_table *sg_table) +{

• if (WARN_ON(!sg_table->data_pages.pages[0]))

• return NULL;
  

• return sg_table->table_vaddr;

+}

The above two functions deal with DMA'able and virtual addresses for the table page buffer. Yet the test in the WARN_ON is done on the data page array. Shouldn't this be sg_table->table_pages.pages[0] instead?

The table is as good as empty if there are no data pages associated with the table. Hence the data_pages check.

That is correct. On the flip side you can't have data_pages without table_pages and vice versa, hence my comment.

Agree. On a second thought, those helpers are not used anywhere now. Also, we only use the base addresses just after creation of the table and we have necessary guards to make sure the table is actually created. I suspect this was a left over from my original code. I am tempted to rather remove them.

Suzuki