On 20/10/17 13:34, Julien Thierry wrote:

Hi Suzuki,

On 19/10/17 18:15, Suzuki K Poulose wrote:

...

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 | 16 ++++++++++++   drivers/hwtracing/coresight/coresight-tmc-etr.c | 33 ++++++++++++++++++++++++   drivers/hwtracing/coresight/coresight-tmc.c     | 34 ++++++++++++++-----------   drivers/hwtracing/coresight/coresight-tmc.h     |  4 +++   4 files changed, 72 insertions(+), 15 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index e2513b786242..0b6f1eb746de 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -120,6 +120,22 @@ 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) +{ +    /* Adjust the len to available size @pos */ +    if (pos + len > drvdata->len) +        len = drvdata->len - pos; +    if (len > 0)

Do we have some guarantee that "pos <= drvdata->len"? because since len is unsigned this check only covers the case were len is 0.

Maybe it would be better to use a signed variable to store the result of the difference.

...

+        *bufpp = drvdata->buf + pos;

...

• • 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) +{ +    char *bufp = drvdata->buf + pos; +    char *bufend = (char *)(drvdata->vaddr + drvdata->size);

+    /* Adjust the len to available size @pos */ +    if (pos + len > drvdata->len) +        len = drvdata->len - pos;

+    if (len <= 0) +        return len;

Similar issue here.

Thanks for spotting. I will fix it.

Cheers

On Thu, Oct 19, 2017 at 06:15:39PM +0100, Suzuki K Poulose wrote:

Right now we open code filling the trace buffer with synchronization packets when the circular buffer wraps around in different drivers. Move this to a common place.

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

---

drivers/hwtracing/coresight/coresight-etb10.c | 10 +++------ drivers/hwtracing/coresight/coresight-priv.h | 8 ++++++++ drivers/hwtracing/coresight/coresight-tmc-etf.c | 27 ++++++++----------------- drivers/hwtracing/coresight/coresight-tmc-etr.c | 13 +----------- 4 files changed, 20 insertions(+), 38 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 56ecd7aff5eb..d7164ab8e229 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -203,7 +203,6 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) bool lost = false; int i; u8 *buf_ptr;

• const u32 *barrier; u32 read_data, depth; u32 read_ptr, write_ptr; u32 frame_off, frame_endoff;

@@ -234,19 +233,16 @@ static void etb_dump_hw(struct etb_drvdata *drvdata) depth = drvdata->buffer_depth; buf_ptr = drvdata->buf;

• barrier = barrier_pkt; for (i = 0; i < depth; i++) { read_data = readl_relaxed(drvdata->base + ETB_RAM_READ_DATA_REG);

• if (lost && *barrier) {
  

• 	read_data = *barrier;
  

• 	barrier++;
  

• }
  

• *(u32 *)buf_ptr = read_data; buf_ptr += 4; }

• if (lost)

• coresight_insert_barrier_packet(drvdata->buf);
  

• if (frame_off) { buf_ptr -= (frame_endoff * 4); for (i = 0; i < frame_endoff; i++) {

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h index f1d0e21d8cab..d12f64928c00 100644 --- a/drivers/hwtracing/coresight/coresight-priv.h +++ b/drivers/hwtracing/coresight/coresight-priv.h @@ -65,6 +65,7 @@ static DEVICE_ATTR_RO(name) __coresight_simple_func(type, NULL, name, lo_off, hi_off) extern const u32 barrier_pkt[5]; +#define CORESIGHT_BARRIER_PKT_SIZE (sizeof(barrier_pkt) - sizeof(u32))

When using a memcpy() there is no need to have a 0x0 at the end of the barrier_pkt array. A such I suggest you remove that and simply use sizeof() in coresight_insert_barrier_packet().

I'll review the rest of your patches tomorrow.

enum etm_addr_type { ETM_ADDR_TYPE_NONE, @@ -98,6 +99,13 @@ struct cs_buffers { void **data_pages; }; +static inline void coresight_insert_barrier_packet(void *buf) +{

• if (buf)

• memcpy(buf, barrier_pkt, CORESIGHT_BARRIER_PKT_SIZE);
  

+}

static inline void CS_LOCK(void __iomem *addr) { do { diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index 0b6f1eb746de..d89bfb3042a2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -43,39 +43,28 @@ static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata) static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata) {

• bool lost = false; char *bufp;
• const u32 *barrier;
• u32 read_data, status;

• u32 read_data, lost; int i;

• /*

• * Get a hold of the status register and see if a wrap around
  

• * has occurred.
  

• */
  

• status = readl_relaxed(drvdata->base + TMC_STS);
• if (status & TMC_STS_FULL)

• lost = true;
  

• /* Check if the buffer was wrapped around. */
• lost = readl_relaxed(drvdata->base + TMC_STS) & TMC_STS_FULL; bufp = drvdata->buf; drvdata->len = 0;

• barrier = barrier_pkt; while (1) { for (i = 0; i < drvdata->memwidth; i++) { read_data = readl_relaxed(drvdata->base + TMC_RRD); if (read_data == 0xFFFFFFFF)

• 		return;
  

• 	if (lost && *barrier) {
  

• 		read_data = *barrier;
  

• 		barrier++;
  

• 	}
  

• 		goto done;
  memcpy(bufp, &read_data, 4);
  bufp += 4;
  drvdata->len += 4;
  

  } }

+done:

• if (lost)

• coresight_insert_barrier_packet(drvdata->buf);
  

• return;

} static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata) diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 063f253f1c99..41535fa6b6cf 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -104,9 +104,7 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) {

• const u32 *barrier; u32 val;
• u32 *temp; u64 rwp;

rwp = tmc_read_rwp(drvdata); @@ -119,16 +117,7 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata) if (val & TMC_STS_FULL) { drvdata->buf = drvdata->vaddr + rwp - drvdata->paddr; drvdata->len = drvdata->size;

• barrier = barrier_pkt;
  

• temp = (u32 *)drvdata->buf;
  

• while (*barrier) {
  

• 	*temp = *barrier;
  

• 	temp++;
  

• 	barrier++;
  

• }
  

• coresight_insert_barrier_packet(drvdata->buf);
  

  } else { drvdata->buf = drvdata->vaddr; drvdata->len = rwp - drvdata->paddr;

-- 2.13.6

On 31/10/17 22:13, Mathieu Poirier wrote:

On Thu, Oct 19, 2017 at 06:15:40PM +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.

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

---

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 289 +++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tmc.h | 44 ++++ 2 files changed, 332 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 41535fa6b6cf..4b9e2b276122 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -16,10 +16,297 @@ */ #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 @phys_addr to an offset within the buffer.

Did you mean "... map @addr"? It might also be worth it to explicitly mention that it maps a physical address to an offset in the contiguous range.

Yes, definitely. I will fix it.

...

...

+/*

• • 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 nod).
• */

+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;
• rc = tmc_pages_alloc(&sg_table->data_pages,

• 	     sg_table->dev, sg_table->node,
  

Am I missing something very subtle here or sg_table->node should be the same as dev_to_node(sg_table->dev)? If the same both tmc_alloc_table_pages() and tmc_alloc_data_pages() should be using the same construct. Otherwise please add a comment to justify the difference.

Yes, it was a last minute change to switch the table to use dev_to_node(), while the data pages are allocated as requested by the user. Eventually the user would consume the data pages (even though the device produces it). However, the table pages are solely for the consumption of the device, hence the dev_to_node().

I will add a comment to make that explicit.

...

u32 axictl, sts; diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 6deb3afe9db8..5e49c035a1ac 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -19,6 +19,7 @@ #define _CORESIGHT_TMC_H #include <linux/miscdevice.h> +#include <linux/dma-mapping.h> #define TMC_RSZ 0x004 #define TMC_STS 0x00c @@ -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.
• • @daddr: DMA'able page address returned by dma_map_page().
• • @vaddr: Virtual address returned by page_address().

This isn't accurate.

Yes, I will clean that up. It kind of shows the number of revisions this series has changed before reaching here ;-)

...

• */

+struct tmc_pages {

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

+};

+/*

• • struct tmc_sg_table : Generic SG table for TMC

Use a '-' as above or fix the above to be ':'. I don't mind which is used as long as they are the same.

Ok.

...

• • @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,15 @@ 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);
  

  #endif

I like this implementation, much cleaner than what I previously had.

Thanks for the review !

Cheers Suzuki

Hi Suzuki,

On 19/10/17 18:15, Suzuki K Poulose wrote:

This patch adds support for setting up an SG table used by the TMC ETR inbuilt SG unit. The TMC ETR uses 4K page sized tables to hold pointers to the 4K data pages with the last entry in a table pointing to the next table with the entries, by kind of chaining. The 2 LSBs determine the type of the table entry, to one of :

Normal - Points to a 4KB data page. Last - Points to a 4KB data page, but is the last entry in the page table. Link - Points to another 4KB table page with pointers to data.

The code takes care of handling the system page size which could be different than 4K. So we could end up putting multiple ETR SG tables in a single system page, vice versa for the data pages.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 256 ++++++++++++++++++++++++ 1 file changed, 256 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 4b9e2b276122..4424eb67a54c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -21,6 +21,89 @@ #include "coresight-tmc.h" /*

• • The TMC ETR SG has a page size of 4K. The SG table contains pointers
• • to 4KB buffers. However, the OS may be use PAGE_SIZE different from

nit: "the OS may use a PAGE_SIZE different from".

• • 4K (i.e, 16KB or 64KB). This implies that a single OS page could
• • contain more than one SG buffer and tables.
• • A table entry has the following format:
• • ---Bit31------------Bit4-------Bit1-----Bit0--
• • | Address[39:12] | SBZ | Entry Type |

• • ---

• • Address: Bits [39:12] of a physical page address. Bits [11:0] are

• •    always zero.
    

• • Entry type:
• • b00 - Reserved.
• • b01 - Last entry in the tables, points to 4K page buffer.
• • b10 - Normal entry, points to 4K page buffer.
• • b11 - Link. The address points to the base of next table.
• */

+typedef u32 sgte_t;

+#define ETR_SG_PAGE_SHIFT 12 +#define ETR_SG_PAGE_SIZE (1UL << ETR_SG_PAGE_SHIFT) +#define ETR_SG_PAGES_PER_SYSPAGE (1UL << \

• 			 (PAGE_SHIFT - ETR_SG_PAGE_SHIFT))
  

I think this would be slightly easier to understand if defined as: "(PAGE_SIZE / ETR_SG_PAGE_SIZE)".

+#define ETR_SG_PTRS_PER_PAGE (ETR_SG_PAGE_SIZE / sizeof(sgte_t)) +#define ETR_SG_PTRS_PER_SYSPAGE (PAGE_SIZE / sizeof(sgte_t))

+#define ETR_SG_ET_MASK 0x3 +#define ETR_SG_ET_LAST 0x1 +#define ETR_SG_ET_NORMAL 0x2 +#define ETR_SG_ET_LINK 0x3

+#define ETR_SG_ADDR_SHIFT 4

+#define ETR_SG_ENTRY(addr, type) \

• (sgte_t)((((addr) >> ETR_SG_PAGE_SHIFT) << ETR_SG_ADDR_SHIFT) | \

•  (type & ETR_SG_ET_MASK))
  

+#define ETR_SG_ADDR(entry) \

• (((dma_addr_t)(entry) >> ETR_SG_ADDR_SHIFT) << ETR_SG_PAGE_SHIFT)

+#define ETR_SG_ET(entry) ((entry) & ETR_SG_ET_MASK)

+/*

• • struct etr_sg_table : ETR SG Table
• • @sg_table: Generic SG Table holding the data/table pages.
• • @hwaddr: hwaddress used by the TMC, which is the base

• • 	address of the table.
    

• */

+struct etr_sg_table {

• struct tmc_sg_table *sg_table;
• dma_addr_t hwaddr;

+};

+/*

• • tmc_etr_sg_table_entries: Total number of table entries required to map
• • @nr_pages system pages.
• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages.
• • Each TMC page can map (ETR_SG_PTRS_PER_PAGE - 1) buffer pointers,
• • with the last entry pointing to the page containing the table
• • entries. If we spill over to a new page for mapping 1 entry,
• • we could as well replace the link entry of the previous page
• • with the last entry.
• */

+static inline unsigned long __attribute_const__ +tmc_etr_sg_table_entries(int nr_pages) +{

• unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE;
• unsigned long nr_sglinks = nr_sgpages / (ETR_SG_PTRS_PER_PAGE - 1);
• /*

• * If we spill over to a new page for 1 entry, we could as well
  

• * make it the LAST entry in the previous page, skipping the Link
  

• * address.
  

• */
  

• if (nr_sglinks && (nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) < 2))

• nr_sglinks--;
  

• return nr_sgpages + nr_sglinks;

+}

+/*

• tmc_pages_get_offset: Go through all the pages in the tmc_pages
• and map @phys_addr to an offset within the buffer.

*/ @@ -307,6 +390,179 @@ ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, return len; } +#ifdef ETR_SG_DEBUG +/* Map a dma address to virtual address */ +static unsigned long +tmc_sg_daddr_to_vaddr(struct tmc_sg_table *sg_table,

• 	dma_addr_t addr, bool table)
  

+{

• long offset;
• unsigned long base;
• struct tmc_pages *tmc_pages;
• if (table) {

• tmc_pages = &sg_table->table_pages;
  

• base = (unsigned long)sg_table->table_vaddr;
  

• } else {

• tmc_pages = &sg_table->data_pages;
  

• base = (unsigned long)sg_table->data_vaddr;
  

• }
• offset = tmc_pages_get_offset(tmc_pages, addr);
• if (offset < 0)

• return 0;
  

• return base + offset;

+}

+/* Dump the given sg_table */ +static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) +{

• sgte_t *ptr;
• int i = 0;
• dma_addr_t addr;
• struct tmc_sg_table *sg_table = etr_table->sg_table;
• ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,

• 			      etr_table->hwaddr, true);
  

• while (ptr) {

• addr = ETR_SG_ADDR(*ptr);
  

• switch (ETR_SG_ET(*ptr)) {
  

• case ETR_SG_ET_NORMAL:
  

• 	pr_debug("%05d: %p\t:[N] 0x%llx\n", i, ptr, addr);
  

• 	ptr++;
  

• 	break;
  

• case ETR_SG_ET_LINK:
  

• 	pr_debug("%05d: *** %p\t:{L} 0x%llx ***\n",
  

• 		 i, ptr, addr);
  

• 	ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,
  

• 					      addr, true);
  

• 	break;
  

• case ETR_SG_ET_LAST:
  

• 	pr_debug("%05d: ### %p\t:[L] 0x%llx ###\n",
  

• 		 i, ptr, addr);
  

• 	return;
  

I get this is debug code, but it seems like if ETR_SG_ET(*ptr) is 0 we get stuck in an infinite loop. I guess it is something that supposedly doesn't happen, still I'd prefer having a default case saying the table might be corrupted and either incrementing ptr to try and get more info or breaking out of the loop.

• }
  

• i++;
  

• }
• pr_debug("******* End of Table *****\n");

+} +#endif

+/*

• • Populate the SG Table page table entries from table/data
• • pages allocated. Each Data page has ETR_SG_PAGES_PER_SYSPAGE SG pages.
• • So does a Table page. So we keep track of indices of the tables
• • in each system page and move the pointers accordingly.
• */

+#define INC_IDX_ROUND(idx, size) (idx = (idx + 1) % size)

Needs more parenthesis around idx and size.

+static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) +{

• dma_addr_t paddr;
• int i, type, nr_entries;
• int tpidx = 0; /* index to the current system table_page */
• int sgtidx = 0; /* index to the sg_table within the current syspage */
• int sgtoffset = 0; /* offset to the next entry within the sg_table */

That's misleading, this seems to be the index of an entry within an ETR_SG_PAGE rather than an offset in bytes.

Maybe ptridx or entryidx would be a better name.

• int dpidx = 0; /* index to the current system data_page */
• int spidx = 0; /* index to the SG page within the current data page */
• sgte_t *ptr; /* pointer to the table entry to fill */
• struct tmc_sg_table *sg_table = etr_table->sg_table;
• dma_addr_t *table_daddrs = sg_table->table_pages.daddrs;
• dma_addr_t *data_daddrs = sg_table->data_pages.daddrs;
• nr_entries = tmc_etr_sg_table_entries(sg_table->data_pages.nr_pages);
• /*

• * Use the contiguous virtual address of the table to update entries.
  

• */
  

• ptr = sg_table->table_vaddr;
• /*

• * Fill all the entries, except the last entry to avoid special
  

• * checks within the loop.
  

• */
  

• for (i = 0; i < nr_entries - 1; i++) {

• if (sgtoffset == ETR_SG_PTRS_PER_PAGE - 1) {
  

• 	/*
  

• 	 * Last entry in a sg_table page is a link address to
  

• 	 * the next table page. If this sg_table is the last
  

• 	 * one in the system page, it links to the first
  

• 	 * sg_table in the next system page. Otherwise, it
  

• 	 * links to the next sg_table page within the system
  

• 	 * page.
  

• 	 */
  

• 	if (sgtidx == ETR_SG_PAGES_PER_SYSPAGE - 1) {
  

• 		paddr = table_daddrs[tpidx + 1];
  

• 	} else {
  

• 		paddr = table_daddrs[tpidx] +
  

• 			(ETR_SG_PAGE_SIZE * (sgtidx + 1));
  

• 	}
  

• 	type = ETR_SG_ET_LINK;
  

• } else {
  

• 	/*
  

• 	 * Update the idices to the data_pages to point to the
  

nit: indices

Cheers,

On 19 October 2017 at 11:15, Suzuki K Poulose suzuki.poulose@arm.com wrote:

This patch adds support for setting up an SG table used by the TMC ETR inbuilt SG unit. The TMC ETR uses 4K page sized tables to hold pointers to the 4K data pages with the last entry in a table pointing to the next table with the entries, by kind of chaining. The 2 LSBs determine the type of the table entry, to one of :

Normal - Points to a 4KB data page. Last - Points to a 4KB data page, but is the last entry in the page table. Link - Points to another 4KB table page with pointers to data.

The code takes care of handling the system page size which could be different than 4K. So we could end up putting multiple ETR SG tables in a single system page, vice versa for the data pages.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 256 ++++++++++++++++++++++++ 1 file changed, 256 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 4b9e2b276122..4424eb67a54c 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -21,6 +21,89 @@ #include "coresight-tmc.h"

/*

• • The TMC ETR SG has a page size of 4K. The SG table contains pointers
• • to 4KB buffers. However, the OS may be use PAGE_SIZE different from
• • 4K (i.e, 16KB or 64KB). This implies that a single OS page could
• • contain more than one SG buffer and tables.
• • A table entry has the following format:
• • ---Bit31------------Bit4-------Bit1-----Bit0--
• • | Address[39:12] | SBZ | Entry Type |

• • ---

• • Address: Bits [39:12] of a physical page address. Bits [11:0] are

• •     always zero.
    

• • Entry type:

• • b00 - Reserved.
    

• • b01 - Last entry in the tables, points to 4K page buffer.
    

• • b10 - Normal entry, points to 4K page buffer.
    

• • b11 - Link. The address points to the base of next table.
    

• */

+typedef u32 sgte_t;

+#define ETR_SG_PAGE_SHIFT 12 +#define ETR_SG_PAGE_SIZE (1UL << ETR_SG_PAGE_SHIFT) +#define ETR_SG_PAGES_PER_SYSPAGE (1UL << \

•                                    (PAGE_SHIFT - ETR_SG_PAGE_SHIFT))
  

+#define ETR_SG_PTRS_PER_PAGE (ETR_SG_PAGE_SIZE / sizeof(sgte_t)) +#define ETR_SG_PTRS_PER_SYSPAGE (PAGE_SIZE / sizeof(sgte_t))

+#define ETR_SG_ET_MASK 0x3 +#define ETR_SG_ET_LAST 0x1 +#define ETR_SG_ET_NORMAL 0x2 +#define ETR_SG_ET_LINK 0x3

+#define ETR_SG_ADDR_SHIFT 4

+#define ETR_SG_ENTRY(addr, type) \

•   (sgte_t)((((addr) >> ETR_SG_PAGE_SHIFT) << ETR_SG_ADDR_SHIFT) | \
  

•            (type & ETR_SG_ET_MASK))
  

+#define ETR_SG_ADDR(entry) \

•   (((dma_addr_t)(entry) >> ETR_SG_ADDR_SHIFT) << ETR_SG_PAGE_SHIFT)
  

+#define ETR_SG_ET(entry) ((entry) & ETR_SG_ET_MASK)

+/*

• • struct etr_sg_table : ETR SG Table
• • @sg_table: Generic SG Table holding the data/table pages.
• • @hwaddr: hwaddress used by the TMC, which is the base

• •                 address of the table.
    

• */

+struct etr_sg_table {

•   struct tmc_sg_table     *sg_table;
  

•   dma_addr_t              hwaddr;
  

+};

+/*

• • tmc_etr_sg_table_entries: Total number of table entries required to map
• • @nr_pages system pages.
• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages.
• • Each TMC page can map (ETR_SG_PTRS_PER_PAGE - 1) buffer pointers,
• • with the last entry pointing to the page containing the table

... with the last entry pointing to another page of table entries. If we ...

• • entries. If we spill over to a new page for mapping 1 entry,
• • we could as well replace the link entry of the previous page
• • with the last entry.
• */

+static inline unsigned long __attribute_const__ +tmc_etr_sg_table_entries(int nr_pages) +{

•   unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE;
  

•   unsigned long nr_sglinks = nr_sgpages / (ETR_SG_PTRS_PER_PAGE - 1);
  

•   /*
  

•    * If we spill over to a new page for 1 entry, we could as well
  

•    * make it the LAST entry in the previous page, skipping the Link
  

•    * address.
  

•    */
  

•   if (nr_sglinks && (nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) < 2))
  

•           nr_sglinks--;
  

•   return nr_sgpages + nr_sglinks;
  

+}

+/*

• tmc_pages_get_offset: Go through all the pages in the tmc_pages
• and map @phys_addr to an offset within the buffer.

*/ @@ -307,6 +390,179 @@ ssize_t tmc_sg_table_get_data(struct tmc_sg_table *sg_table, return len; }

+#ifdef ETR_SG_DEBUG +/* Map a dma address to virtual address */ +static unsigned long +tmc_sg_daddr_to_vaddr(struct tmc_sg_table *sg_table,

•                   dma_addr_t addr, bool table)
  

+{

•   long offset;
  

•   unsigned long base;
  

•   struct tmc_pages *tmc_pages;
  

•   if (table) {
  

•           tmc_pages = &sg_table->table_pages;
  

•           base = (unsigned long)sg_table->table_vaddr;
  

•   } else {
  

•           tmc_pages = &sg_table->data_pages;
  

•           base = (unsigned long)sg_table->data_vaddr;
  

•   }
  

•   offset = tmc_pages_get_offset(tmc_pages, addr);
  

•   if (offset < 0)
  

•           return 0;
  

•   return base + offset;
  

+}

+/* Dump the given sg_table */ +static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) +{

•   sgte_t *ptr;
  

•   int i = 0;
  

•   dma_addr_t addr;
  

•   struct tmc_sg_table *sg_table = etr_table->sg_table;
  

•   ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,
  

•                                         etr_table->hwaddr, true);
  

•   while (ptr) {
  

•           addr = ETR_SG_ADDR(*ptr);
  

•           switch (ETR_SG_ET(*ptr)) {
  

•           case ETR_SG_ET_NORMAL:
  

•                   pr_debug("%05d: %p\t:[N] 0x%llx\n", i, ptr, addr);
  

•                   ptr++;
  

•                   break;
  

•           case ETR_SG_ET_LINK:
  

•                   pr_debug("%05d: *** %p\t:{L} 0x%llx ***\n",
  

•                            i, ptr, addr);
  

•                   ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,
  

•                                                         addr, true);
  

•                   break;
  

•           case ETR_SG_ET_LAST:
  

•                   pr_debug("%05d: ### %p\t:[L] 0x%llx ###\n",
  

•                            i, ptr, addr);
  

•                   return;
  

•           }
  

•           i++;
  

•   }
  

•   pr_debug("******* End of Table *****\n");
  

+} +#endif

+/*

• • Populate the SG Table page table entries from table/data
• • pages allocated. Each Data page has ETR_SG_PAGES_PER_SYSPAGE SG pages.
• • So does a Table page. So we keep track of indices of the tables
• • in each system page and move the pointers accordingly.
• */

+#define INC_IDX_ROUND(idx, size) (idx = (idx + 1) % size) +static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) +{

•   dma_addr_t paddr;
  

•   int i, type, nr_entries;
  

•   int tpidx = 0; /* index to the current system table_page */
  

•   int sgtidx = 0; /* index to the sg_table within the current syspage */
  

•   int sgtoffset = 0; /* offset to the next entry within the sg_table */
  

•   int dpidx = 0; /* index to the current system data_page */
  

•   int spidx = 0; /* index to the SG page within the current data page */
  

•   sgte_t *ptr; /* pointer to the table entry to fill */
  

•   struct tmc_sg_table *sg_table = etr_table->sg_table;
  

•   dma_addr_t *table_daddrs = sg_table->table_pages.daddrs;
  

•   dma_addr_t *data_daddrs = sg_table->data_pages.daddrs;
  

•   nr_entries = tmc_etr_sg_table_entries(sg_table->data_pages.nr_pages);
  

•   /*
  

•    * Use the contiguous virtual address of the table to update entries.
  

•    */
  

•   ptr = sg_table->table_vaddr;
  

•   /*
  

•    * Fill all the entries, except the last entry to avoid special
  

•    * checks within the loop.
  

•    */
  

•   for (i = 0; i < nr_entries - 1; i++) {
  

•           if (sgtoffset == ETR_SG_PTRS_PER_PAGE - 1) {
  

•                   /*
  

•                    * Last entry in a sg_table page is a link address to
  

•                    * the next table page. If this sg_table is the last
  

•                    * one in the system page, it links to the first
  

•                    * sg_table in the next system page. Otherwise, it
  

•                    * links to the next sg_table page within the system
  

•                    * page.
  

•                    */
  

•                   if (sgtidx == ETR_SG_PAGES_PER_SYSPAGE - 1) {
  

•                           paddr = table_daddrs[tpidx + 1];
  

•                   } else {
  

•                           paddr = table_daddrs[tpidx] +
  

•                                   (ETR_SG_PAGE_SIZE * (sgtidx + 1));
  

•                   }
  

•                   type = ETR_SG_ET_LINK;
  

•           } else {
  

•                   /*
  

•                    * Update the idices to the data_pages to point to the
  

•                    * next sg_page in the data buffer.
  

•                    */
  

•                   type = ETR_SG_ET_NORMAL;
  

•                   paddr = data_daddrs[dpidx] + spidx * ETR_SG_PAGE_SIZE;
  

•                   if (!INC_IDX_ROUND(spidx, ETR_SG_PAGES_PER_SYSPAGE))
  

•                           dpidx++;
  

•           }
  

•           *ptr++ = ETR_SG_ENTRY(paddr, type);
  

•           /*
  

•            * Move to the next table pointer, moving the table page index
  

•            * if necessary
  

•            */
  

•           if (!INC_IDX_ROUND(sgtoffset, ETR_SG_PTRS_PER_PAGE)) {
  

•                   if (!INC_IDX_ROUND(sgtidx, ETR_SG_PAGES_PER_SYSPAGE))
  

•                           tpidx++;
  

•           }
  

•   }
  

•   /* Set up the last entry, which is always a data pointer */
  

•   paddr = data_daddrs[dpidx] + spidx * ETR_SG_PAGE_SIZE;
  

•   *ptr++ = ETR_SG_ENTRY(paddr, ETR_SG_ET_LAST);
  

+}

+/*

• • tmc_init_etr_sg_table: Allocate a TMC ETR SG table, data buffer of @size and
• • populate the table.
• • @dev - Device pointer for the TMC
• • @node - NUMA node where the memory should be allocated
• • @size - Total size of the data buffer
• • @pages - Optional list of page virtual address
• */

+static struct etr_sg_table __maybe_unused * +tmc_init_etr_sg_table(struct device *dev, int node,

•             unsigned long size, void **pages)
  

+{

•   int nr_entries, nr_tpages;
  

•   int nr_dpages = size >> PAGE_SHIFT;
  

•   struct tmc_sg_table *sg_table;
  

•   struct etr_sg_table *etr_table;
  

•   etr_table = kzalloc(sizeof(*etr_table), GFP_KERNEL);
  

•   if (!etr_table)
  

•           return ERR_PTR(-ENOMEM);
  

•   nr_entries = tmc_etr_sg_table_entries(nr_dpages);
  

•   nr_tpages = DIV_ROUND_UP(nr_entries, ETR_SG_PTRS_PER_SYSPAGE);
  

•   sg_table = tmc_alloc_sg_table(dev, node, nr_tpages, nr_dpages, pages);
  

•   if (IS_ERR(sg_table)) {
  

•           kfree(etr_table);
  

•           return ERR_PTR(PTR_ERR(sg_table));
  

•   }
  

•   etr_table->sg_table = sg_table;
  

•   /* TMC should use table base address for DBA */
  

•   etr_table->hwaddr = sg_table->table_daddr;
  

•   tmc_etr_sg_table_populate(etr_table);
  

•   /* Sync the table pages for the HW */
  

•   tmc_sg_table_sync_table(sg_table);
  

+#ifdef ETR_SG_DEBUG

•   tmc_etr_sg_table_dump(etr_table);
  

+#endif

•   return etr_table;
  

+}

static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) { u32 axictl, sts; -- 2.13.6

Hi Suzuki,

On 19/10/17 18:15, Suzuki K Poulose wrote:

Make the ETR SG table Circular buffer so that we could start at any of the SG pages and use the entire buffer for tracing. This can be achieved by :

1. Keeping an additional LINK pointer at the very end of the

SG table, i.e, after the LAST buffer entry, to point back to the beginning of the first table. This will allow us to use the buffer normally when we start the trace at offset 0 of the buffer, as the LAST buffer entry hints the TMC-ETR and it automatically wraps to the offset 0.

2. If we want to start at any other ETR SG page aligned offset,

we could : a) Make the preceding page entry as LAST entry. b) Make the original LAST entry a normal entry. c) Use the table pointer to the "new" start offset as the base of the table address. This works as the TMC doesn't mandate that the page table base address should be 4K page aligned.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 159 +++++++++++++++++++++--- 1 file changed, 139 insertions(+), 20 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 4424eb67a54c..c171b244e12a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c

[...]

@@ -519,6 +534,107 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) /* Set up the last entry, which is always a data pointer */ paddr = data_daddrs[dpidx] + spidx * ETR_SG_PAGE_SIZE; *ptr++ = ETR_SG_ENTRY(paddr, ETR_SG_ET_LAST);

• /* followed by a circular link, back to the start of the table */
• *ptr++ = ETR_SG_ENTRY(sg_table->table_daddr, ETR_SG_ET_LINK);

+}

+/*

• • tmc_etr_sg_offset_to_table_index : Translate a given data @offset
• • to the index of the page table "entry". Data pointers always have
• • a fixed location, with ETR_SG_PTRS_PER_PAGE - 1 entries in an
• • ETR_SG_PAGE and 1 link entry per (ETR_SG_PTRS_PER_PAGE -1) entries.
• */

+static inline u32 +tmc_etr_sg_offset_to_table_index(u64 offset) +{

• u64 sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
• return sgpage_idx + sgpage_idx / (ETR_SG_PTRS_PER_PAGE - 1);

+}

+/*

• • tmc_etr_sg_update_type: Update the type of a given entry in the
• • table to the requested entry. This is only used for data buffers
• • to toggle the "NORMAL" vs "LAST" buffer entries.
• */

+static inline void tmc_etr_sg_update_type(sgte_t *entry, u32 type) +{

• WARN_ON(ETR_SG_ET(*entry) == ETR_SG_ET_LINK);
• WARN_ON(!ETR_SG_ET(*entry));
• *entry &= ~ETR_SG_ET_MASK;
• *entry |= type;

+}

+/*

• • tmc_etr_sg_table_index_to_daddr: Return the hardware address to the table
• • entry @index. Use this address to let the table begin @index.
• */

+static inline dma_addr_t +tmc_etr_sg_table_index_to_daddr(struct tmc_sg_table *sg_table, u32 index) +{

• u32 sys_page_idx = index / ETR_SG_PTRS_PER_SYSPAGE;
• u32 sys_page_offset = index % ETR_SG_PTRS_PER_SYSPAGE;
• sgte_t *ptr;
• ptr = (sgte_t *)sg_table->table_pages.daddrs[sys_page_idx];
• return (dma_addr_t)&ptr[sys_page_offset];

+}

+/*

• • tmc_etr_sg_table_rotate : Rotate the SG circular buffer, moving
• • the "base" to a requested offset. We do so by :
• • 1. Reset the current LAST buffer.
• • 2. Mark the "previous" buffer in the table to the "base" as LAST.
• • 3. Update the hwaddr to point to the table pointer for the buffer
• • which starts at "base".
• */

+static int __maybe_unused +tmc_etr_sg_table_rotate(struct etr_sg_table *etr_table, u64 base_offset) +{

• u32 last_entry, first_entry;
• u64 last_offset;
• struct tmc_sg_table *sg_table = etr_table->sg_table;
• sgte_t *table_ptr = sg_table->table_vaddr;
• ssize_t buf_size = tmc_sg_table_buf_size(sg_table);
• /* Offset should always be SG PAGE_SIZE aligned */
• if (base_offset & (ETR_SG_PAGE_SIZE - 1)) {

• pr_debug("unaligned base offset %llx\n", base_offset);
  

• return -EINVAL;
  

• }
• /* Make sure the offset is within the range */
• if (base_offset < 0 || base_offset > buf_size) {

base_offset is unsigned, so the left operand of the '||' is useless (would've expected the compiler to emit a warning for this).

• base_offset = (base_offset + buf_size) % buf_size;
  

• pr_debug("Resetting offset to %llx\n", base_offset);
  

• }
• first_entry = tmc_etr_sg_offset_to_table_index(base_offset);
• if (first_entry == etr_table->first_entry) {

• pr_debug("Head is already at %llx, skipping\n", base_offset);
  

• return 0;
  

• }
• /* Last entry should be the previous one to the new "base" */
• last_offset = ((base_offset - ETR_SG_PAGE_SIZE) + buf_size) % buf_size;
• last_entry = tmc_etr_sg_offset_to_table_index(last_offset);
• /* Reset the current Last page to Normal and new Last page to NORMAL */

Current Last page to NORMAL and new Last page to LAST?

Cheers,

On Thu, Oct 19, 2017 at 06:15:42PM +0100, Suzuki K Poulose wrote:

Make the ETR SG table Circular buffer so that we could start at any of the SG pages and use the entire buffer for tracing. This can be achieved by :

1. Keeping an additional LINK pointer at the very end of the

SG table, i.e, after the LAST buffer entry, to point back to the beginning of the first table. This will allow us to use the buffer normally when we start the trace at offset 0 of the buffer, as the LAST buffer entry hints the TMC-ETR and it automatically wraps to the offset 0.

2. If we want to start at any other ETR SG page aligned offset,

we could : a) Make the preceding page entry as LAST entry. b) Make the original LAST entry a normal entry. c) Use the table pointer to the "new" start offset as the base of the table address. This works as the TMC doesn't mandate that the page table base address should be 4K page aligned.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 159 +++++++++++++++++++++--- 1 file changed, 139 insertions(+), 20 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 4424eb67a54c..c171b244e12a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -71,36 +71,41 @@ typedef u32 sgte_t;

• @sg_table: Generic SG Table holding the data/table pages.
• @hwaddr: hwaddress used by the TMC, which is the base

• 	address of the table.
  

• • @nr_entries: Total number of pointers in the table.
• • @first_entry: Index to the current "start" of the buffer.
• • @last_entry: Index to the last entry of the buffer.
  */

struct etr_sg_table { struct tmc_sg_table *sg_table; dma_addr_t hwaddr;

• u32 nr_entries;
• u32 first_entry;
• u32 last_entry;

}; /*

• tmc_etr_sg_table_entries: Total number of table entries required to map
• @nr_pages system pages.

• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages.

• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages and
• • an additional Link pointer for making it a Circular buffer.
  • Each TMC page can map (ETR_SG_PTRS_PER_PAGE - 1) buffer pointers,
  • with the last entry pointing to the page containing the table

• • entries. If we spill over to a new page for mapping 1 entry,
• • we could as well replace the link entry of the previous page
• • with the last entry.

• • entries. If we fill the last table in full with the pointers, (i.e,
• • nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) == 0, we don't have to allocate
• • another table and hence skip the Link pointer. Also we could use the
• • link entry of the last page to make it circular.
  */

static inline unsigned long __attribute_const__ tmc_etr_sg_table_entries(int nr_pages) { unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE; unsigned long nr_sglinks = nr_sgpages / (ETR_SG_PTRS_PER_PAGE - 1);

• /*

• * If we spill over to a new page for 1 entry, we could as well
  

• * make it the LAST entry in the previous page, skipping the Link
  

• * address.
  

• */
  

• if (nr_sglinks && (nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) < 2))

• if (nr_sglinks && !(nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1))) nr_sglinks--;

• return nr_sgpages + nr_sglinks;

• /* Add an entry for the circular link */
• return nr_sgpages + nr_sglinks + 1;

} /* @@ -417,14 +422,21 @@ tmc_sg_daddr_to_vaddr(struct tmc_sg_table *sg_table, /* Dump the given sg_table */ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {

• sgte_t *ptr;

• sgte_t *ptr, *start; int i = 0; dma_addr_t addr; struct tmc_sg_table *sg_table = etr_table->sg_table;

• ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,

• start = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table, etr_table->hwaddr, true);

• while (ptr) {

• if (!start) {

• pr_debug("ERROR: Failed to translate table base: 0x%llx\n",
  

• 			 etr_table->hwaddr);
  

• return;
  

• }
• ptr = start;
• do { addr = ETR_SG_ADDR(*ptr); switch (ETR_SG_ET(*ptr)) { case ETR_SG_ET_NORMAL:

@@ -436,14 +448,17 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) i, ptr, addr); ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table, addr, true);

• 	if (!ptr)
  

• 		pr_debug("ERROR: Bad Link 0x%llx\n", addr);
  break;
  

  case ETR_SG_ET_LAST: pr_debug("%05d: ### %p\t:[L] 0x%llx ###\n", i, ptr, addr);

• 	return;
  

• 	ptr++;
  

• 	break;
  

  } i++;

• }

• } while (ptr && ptr != start); pr_debug("******* End of Table *****\n");

} #endif @@ -458,7 +473,7 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) { dma_addr_t paddr;

• int i, type, nr_entries;

• int i, type; int tpidx = 0; /* index to the current system table_page */ int sgtidx = 0; /* index to the sg_table within the current syspage */ int sgtoffset = 0; /* offset to the next entry within the sg_table */

@@ -469,16 +484,16 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) dma_addr_t *table_daddrs = sg_table->table_pages.daddrs; dma_addr_t *data_daddrs = sg_table->data_pages.daddrs;

• nr_entries = tmc_etr_sg_table_entries(sg_table->data_pages.nr_pages); /*
  • Use the contiguous virtual address of the table to update entries.
  */ ptr = sg_table->table_vaddr; /*

• * Fill all the entries, except the last entry to avoid special
  

• * Fill all the entries, except the last two entries (i.e, the last
  

• * buffer and the circular link back to the base) to avoid special
  

  • checks within the loop.
  */

• for (i = 0; i < nr_entries - 1; i++) {

• for (i = 0; i < etr_table->nr_entries - 2; i++) { if (sgtoffset == ETR_SG_PTRS_PER_PAGE - 1) { /* * Last entry in a sg_table page is a link address to

@@ -519,6 +534,107 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) /* Set up the last entry, which is always a data pointer */ paddr = data_daddrs[dpidx] + spidx * ETR_SG_PAGE_SIZE; *ptr++ = ETR_SG_ENTRY(paddr, ETR_SG_ET_LAST);

• /* followed by a circular link, back to the start of the table */
• *ptr++ = ETR_SG_ENTRY(sg_table->table_daddr, ETR_SG_ET_LINK);

+}

+/*

• • tmc_etr_sg_offset_to_table_index : Translate a given data @offset
• • to the index of the page table "entry". Data pointers always have
• • a fixed location, with ETR_SG_PTRS_PER_PAGE - 1 entries in an
• • ETR_SG_PAGE and 1 link entry per (ETR_SG_PTRS_PER_PAGE -1) entries.
• */

+static inline u32 +tmc_etr_sg_offset_to_table_index(u64 offset) +{

• u64 sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
• return sgpage_idx + sgpage_idx / (ETR_SG_PTRS_PER_PAGE - 1);

+}

+/*

• • tmc_etr_sg_update_type: Update the type of a given entry in the
• • table to the requested entry. This is only used for data buffers
• • to toggle the "NORMAL" vs "LAST" buffer entries.
• */

+static inline void tmc_etr_sg_update_type(sgte_t *entry, u32 type) +{

• WARN_ON(ETR_SG_ET(*entry) == ETR_SG_ET_LINK);
• WARN_ON(!ETR_SG_ET(*entry));
• *entry &= ~ETR_SG_ET_MASK;
• *entry |= type;

+}

+/*

• • tmc_etr_sg_table_index_to_daddr: Return the hardware address to the table
• • entry @index. Use this address to let the table begin @index.
• */

+static inline dma_addr_t +tmc_etr_sg_table_index_to_daddr(struct tmc_sg_table *sg_table, u32 index) +{

• u32 sys_page_idx = index / ETR_SG_PTRS_PER_SYSPAGE;
• u32 sys_page_offset = index % ETR_SG_PTRS_PER_SYSPAGE;
• sgte_t *ptr;
• ptr = (sgte_t *)sg_table->table_pages.daddrs[sys_page_idx];
• return (dma_addr_t)&ptr[sys_page_offset];

+}

+/*

• • tmc_etr_sg_table_rotate : Rotate the SG circular buffer, moving
• • the "base" to a requested offset. We do so by :
• • 1. Reset the current LAST buffer.
• • 2. Mark the "previous" buffer in the table to the "base" as LAST.
• • 3. Update the hwaddr to point to the table pointer for the buffer
• • which starts at "base".
• */

+static int __maybe_unused +tmc_etr_sg_table_rotate(struct etr_sg_table *etr_table, u64 base_offset) +{

• u32 last_entry, first_entry;
• u64 last_offset;
• struct tmc_sg_table *sg_table = etr_table->sg_table;
• sgte_t *table_ptr = sg_table->table_vaddr;
• ssize_t buf_size = tmc_sg_table_buf_size(sg_table);
• /* Offset should always be SG PAGE_SIZE aligned */
• if (base_offset & (ETR_SG_PAGE_SIZE - 1)) {

• pr_debug("unaligned base offset %llx\n", base_offset);
  

• return -EINVAL;
  

• }
• /* Make sure the offset is within the range */
• if (base_offset < 0 || base_offset > buf_size) {

• base_offset = (base_offset + buf_size) % buf_size;
  

• pr_debug("Resetting offset to %llx\n", base_offset);
  

• }
• first_entry = tmc_etr_sg_offset_to_table_index(base_offset);
• if (first_entry == etr_table->first_entry) {

• pr_debug("Head is already at %llx, skipping\n", base_offset);
  

• return 0;
  

• }
• /* Last entry should be the previous one to the new "base" */
• last_offset = ((base_offset - ETR_SG_PAGE_SIZE) + buf_size) % buf_size;
• last_entry = tmc_etr_sg_offset_to_table_index(last_offset);
• /* Reset the current Last page to Normal and new Last page to NORMAL */
• tmc_etr_sg_update_type(&table_ptr[etr_table->last_entry],

• 		 ETR_SG_ET_NORMAL);
  

• tmc_etr_sg_update_type(&table_ptr[last_entry], ETR_SG_ET_LAST);
• etr_table->hwaddr = tmc_etr_sg_table_index_to_daddr(sg_table,

• 					    first_entry);
  

• etr_table->first_entry = first_entry;
• etr_table->last_entry = last_entry;
• pr_debug("table rotated to offset %llx-%llx, entries (%d - %d), dba: %llx\n",

• 	base_offset, last_offset, first_entry, last_entry,
  

• 	etr_table->hwaddr);
  

The above line generates a warning when compiling for ARMv7.

• /* Sync the table for device */
• tmc_sg_table_sync_table(sg_table);

+#ifdef ETR_SG_DEBUG

• tmc_etr_sg_table_dump(etr_table);

+#endif

• return 0;

} /* @@ -552,6 +668,9 @@ tmc_init_etr_sg_table(struct device *dev, int node, } etr_table->sg_table = sg_table;

• etr_table->nr_entries = nr_entries;
• etr_table->first_entry = 0;
• etr_table->last_entry = nr_entries - 2; /* TMC should use table base address for DBA */ etr_table->hwaddr = sg_table->table_daddr; tmc_etr_sg_table_populate(etr_table);

-- 2.13.6

On Thu, Oct 19, 2017 at 06:15:42PM +0100, Suzuki K Poulose wrote:

Make the ETR SG table Circular buffer so that we could start at any of the SG pages and use the entire buffer for tracing. This can be achieved by :

1. Keeping an additional LINK pointer at the very end of the

SG table, i.e, after the LAST buffer entry, to point back to the beginning of the first table. This will allow us to use the buffer normally when we start the trace at offset 0 of the buffer, as the LAST buffer entry hints the TMC-ETR and it automatically wraps to the offset 0.

2. If we want to start at any other ETR SG page aligned offset,

we could : a) Make the preceding page entry as LAST entry. b) Make the original LAST entry a normal entry. c) Use the table pointer to the "new" start offset as the base of the table address. This works as the TMC doesn't mandate that the page table base address should be 4K page aligned.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 159 +++++++++++++++++++++--- 1 file changed, 139 insertions(+), 20 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 4424eb67a54c..c171b244e12a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -71,36 +71,41 @@ typedef u32 sgte_t;

• @sg_table: Generic SG Table holding the data/table pages.
• @hwaddr: hwaddress used by the TMC, which is the base

• 	address of the table.
  

• • @nr_entries: Total number of pointers in the table.
• • @first_entry: Index to the current "start" of the buffer.
• • @last_entry: Index to the last entry of the buffer.
  */

struct etr_sg_table { struct tmc_sg_table *sg_table; dma_addr_t hwaddr;

• u32 nr_entries;
• u32 first_entry;
• u32 last_entry;

}; /*

• tmc_etr_sg_table_entries: Total number of table entries required to map
• @nr_pages system pages.

• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages.

• • We need to map @nr_pages * ETR_SG_PAGES_PER_SYSPAGE data pages and
• • an additional Link pointer for making it a Circular buffer.
  • Each TMC page can map (ETR_SG_PTRS_PER_PAGE - 1) buffer pointers,
  • with the last entry pointing to the page containing the table

• • entries. If we spill over to a new page for mapping 1 entry,
• • we could as well replace the link entry of the previous page
• • with the last entry.

• • entries. If we fill the last table in full with the pointers, (i.e,
• • nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) == 0, we don't have to allocate
• • another table and hence skip the Link pointer. Also we could use the
• • link entry of the last page to make it circular.
  */

static inline unsigned long __attribute_const__ tmc_etr_sg_table_entries(int nr_pages) { unsigned long nr_sgpages = nr_pages * ETR_SG_PAGES_PER_SYSPAGE; unsigned long nr_sglinks = nr_sgpages / (ETR_SG_PTRS_PER_PAGE - 1);

• /*

• * If we spill over to a new page for 1 entry, we could as well
  

• * make it the LAST entry in the previous page, skipping the Link
  

• * address.
  

• */
  

• if (nr_sglinks && (nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1) < 2))

• if (nr_sglinks && !(nr_sgpages % (ETR_SG_PTRS_PER_PAGE - 1))) nr_sglinks--;

• return nr_sgpages + nr_sglinks;

• /* Add an entry for the circular link */
• return nr_sgpages + nr_sglinks + 1;

} /* @@ -417,14 +422,21 @@ tmc_sg_daddr_to_vaddr(struct tmc_sg_table *sg_table, /* Dump the given sg_table */ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) {

• sgte_t *ptr;

• sgte_t *ptr, *start; int i = 0; dma_addr_t addr; struct tmc_sg_table *sg_table = etr_table->sg_table;

• ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table,

• start = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table, etr_table->hwaddr, true);

• while (ptr) {

• if (!start) {

• pr_debug("ERROR: Failed to translate table base: 0x%llx\n",
  

• 			 etr_table->hwaddr);
  

• return;
  

• }
• ptr = start;
• do { addr = ETR_SG_ADDR(*ptr); switch (ETR_SG_ET(*ptr)) { case ETR_SG_ET_NORMAL:

@@ -436,14 +448,17 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) i, ptr, addr); ptr = (sgte_t *)tmc_sg_daddr_to_vaddr(sg_table, addr, true);

• 	if (!ptr)
  

• 		pr_debug("ERROR: Bad Link 0x%llx\n", addr);
  break;
  

  case ETR_SG_ET_LAST: pr_debug("%05d: ### %p\t:[L] 0x%llx ###\n", i, ptr, addr);

• 	return;
  

• 	ptr++;
  

• 	break;
  

  } i++;

• }

• } while (ptr && ptr != start); pr_debug("******* End of Table *****\n");

} #endif @@ -458,7 +473,7 @@ static void tmc_etr_sg_table_dump(struct etr_sg_table *etr_table) static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) { dma_addr_t paddr;

• int i, type, nr_entries;

• int i, type; int tpidx = 0; /* index to the current system table_page */ int sgtidx = 0; /* index to the sg_table within the current syspage */ int sgtoffset = 0; /* offset to the next entry within the sg_table */

@@ -469,16 +484,16 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) dma_addr_t *table_daddrs = sg_table->table_pages.daddrs; dma_addr_t *data_daddrs = sg_table->data_pages.daddrs;

• nr_entries = tmc_etr_sg_table_entries(sg_table->data_pages.nr_pages); /*
  • Use the contiguous virtual address of the table to update entries.
  */ ptr = sg_table->table_vaddr; /*

• * Fill all the entries, except the last entry to avoid special
  

• * Fill all the entries, except the last two entries (i.e, the last
  

• * buffer and the circular link back to the base) to avoid special
  

  • checks within the loop.
  */

• for (i = 0; i < nr_entries - 1; i++) {

• for (i = 0; i < etr_table->nr_entries - 2; i++) { if (sgtoffset == ETR_SG_PTRS_PER_PAGE - 1) { /* * Last entry in a sg_table page is a link address to

@@ -519,6 +534,107 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) /* Set up the last entry, which is always a data pointer */ paddr = data_daddrs[dpidx] + spidx * ETR_SG_PAGE_SIZE; *ptr++ = ETR_SG_ENTRY(paddr, ETR_SG_ET_LAST);

• /* followed by a circular link, back to the start of the table */
• *ptr++ = ETR_SG_ENTRY(sg_table->table_daddr, ETR_SG_ET_LINK);

+}

+/*

• • tmc_etr_sg_offset_to_table_index : Translate a given data @offset
• • to the index of the page table "entry". Data pointers always have
• • a fixed location, with ETR_SG_PTRS_PER_PAGE - 1 entries in an
• • ETR_SG_PAGE and 1 link entry per (ETR_SG_PTRS_PER_PAGE -1) entries.
• */

+static inline u32 +tmc_etr_sg_offset_to_table_index(u64 offset) +{

• u64 sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
• return sgpage_idx + sgpage_idx / (ETR_SG_PTRS_PER_PAGE - 1);

+}

This function is the source of a bizarre linking error when compiling [14/17] on armv7 as pasted here:

UPD include/generated/compile.h CC init/version.o AR init/built-in.o AR built-in.o LD vmlinux.o MODPOST vmlinux.o drivers/hwtracing/coresight/coresight-tmc-etr.o: In function `tmc_etr_sg_offset_to_table_index': /home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:553: undefined reference to `__aeabi_uldivmod' /home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:551: undefined reference to `__aeabi_uldivmod' /home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:553: undefined reference to `__aeabi_uldivmod' drivers/hwtracing/coresight/coresight-tmc-etr.o: In function `tmc_etr_sg_table_rotate': /home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:609: undefined reference to `__aeabi_uldivmod'

Please see if you can reproduce on your side.

Thanks, Mathieu

+/*

• • tmc_etr_sg_update_type: Update the type of a given entry in the
• • table to the requested entry. This is only used for data buffers
• • to toggle the "NORMAL" vs "LAST" buffer entries.
• */

+static inline void tmc_etr_sg_update_type(sgte_t *entry, u32 type) +{

• WARN_ON(ETR_SG_ET(*entry) == ETR_SG_ET_LINK);
• WARN_ON(!ETR_SG_ET(*entry));
• *entry &= ~ETR_SG_ET_MASK;
• *entry |= type;

+}

+/*

• • tmc_etr_sg_table_index_to_daddr: Return the hardware address to the table
• • entry @index. Use this address to let the table begin @index.
• */

+static inline dma_addr_t +tmc_etr_sg_table_index_to_daddr(struct tmc_sg_table *sg_table, u32 index) +{

• u32 sys_page_idx = index / ETR_SG_PTRS_PER_SYSPAGE;
• u32 sys_page_offset = index % ETR_SG_PTRS_PER_SYSPAGE;
• sgte_t *ptr;
• ptr = (sgte_t *)sg_table->table_pages.daddrs[sys_page_idx];
• return (dma_addr_t)&ptr[sys_page_offset];

+}

+/*

• • tmc_etr_sg_table_rotate : Rotate the SG circular buffer, moving
• • the "base" to a requested offset. We do so by :
• • 1. Reset the current LAST buffer.
• • 2. Mark the "previous" buffer in the table to the "base" as LAST.
• • 3. Update the hwaddr to point to the table pointer for the buffer
• • which starts at "base".
• */

+static int __maybe_unused +tmc_etr_sg_table_rotate(struct etr_sg_table *etr_table, u64 base_offset) +{

• u32 last_entry, first_entry;
• u64 last_offset;
• struct tmc_sg_table *sg_table = etr_table->sg_table;
• sgte_t *table_ptr = sg_table->table_vaddr;
• ssize_t buf_size = tmc_sg_table_buf_size(sg_table);
• /* Offset should always be SG PAGE_SIZE aligned */
• if (base_offset & (ETR_SG_PAGE_SIZE - 1)) {

• pr_debug("unaligned base offset %llx\n", base_offset);
  

• return -EINVAL;
  

• }
• /* Make sure the offset is within the range */
• if (base_offset < 0 || base_offset > buf_size) {

• base_offset = (base_offset + buf_size) % buf_size;
  

• pr_debug("Resetting offset to %llx\n", base_offset);
  

• }
• first_entry = tmc_etr_sg_offset_to_table_index(base_offset);
• if (first_entry == etr_table->first_entry) {

• pr_debug("Head is already at %llx, skipping\n", base_offset);
  

• return 0;
  

• }
• /* Last entry should be the previous one to the new "base" */
• last_offset = ((base_offset - ETR_SG_PAGE_SIZE) + buf_size) % buf_size;
• last_entry = tmc_etr_sg_offset_to_table_index(last_offset);
• /* Reset the current Last page to Normal and new Last page to NORMAL */
• tmc_etr_sg_update_type(&table_ptr[etr_table->last_entry],

• 		 ETR_SG_ET_NORMAL);
  

• tmc_etr_sg_update_type(&table_ptr[last_entry], ETR_SG_ET_LAST);
• etr_table->hwaddr = tmc_etr_sg_table_index_to_daddr(sg_table,

• 					    first_entry);
  

• etr_table->first_entry = first_entry;
• etr_table->last_entry = last_entry;
• pr_debug("table rotated to offset %llx-%llx, entries (%d - %d), dba: %llx\n",

• 	base_offset, last_offset, first_entry, last_entry,
  

• 	etr_table->hwaddr);
  

• /* Sync the table for device */
• tmc_sg_table_sync_table(sg_table);

+#ifdef ETR_SG_DEBUG

• tmc_etr_sg_table_dump(etr_table);

+#endif

• return 0;

} /* @@ -552,6 +668,9 @@ tmc_init_etr_sg_table(struct device *dev, int node, } etr_table->sg_table = sg_table;

• etr_table->nr_entries = nr_entries;
• etr_table->first_entry = 0;
• etr_table->last_entry = nr_entries - 2; /* TMC should use table base address for DBA */ etr_table->hwaddr = sg_table->table_daddr; tmc_etr_sg_table_populate(etr_table);

-- 2.13.6

On 7 November 2017 at 03:36, Suzuki K Poulose Suzuki.Poulose@arm.com wrote:

On 06/11/17 19:07, Mathieu Poirier wrote:

...

On Thu, Oct 19, 2017 at 06:15:42PM +0100, Suzuki K Poulose wrote:

...

...

...

+/*

• • tmc_etr_sg_offset_to_table_index : Translate a given data @offset
• • to the index of the page table "entry". Data pointers always have
• • a fixed location, with ETR_SG_PTRS_PER_PAGE - 1 entries in an
• • ETR_SG_PAGE and 1 link entry per (ETR_SG_PTRS_PER_PAGE -1) entries.
• */

+static inline u32 +tmc_etr_sg_offset_to_table_index(u64 offset) +{

•   u64 sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
  

•   return sgpage_idx + sgpage_idx / (ETR_SG_PTRS_PER_PAGE - 1);
  

+}

This function is the source of a bizarre linking error when compiling [14/17] on armv7 as pasted here:

UPD include/generated/compile.h CC init/version.o AR init/built-in.o AR built-in.o LD vmlinux.o MODPOST vmlinux.o drivers/hwtracing/coresight/coresight-tmc-etr.o: In function `tmc_etr_sg_offset_to_table_index':

/home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:553: undefined reference to `__aeabi_uldivmod'

/home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:551: undefined reference to `__aeabi_uldivmod'

/home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:553: undefined reference to `__aeabi_uldivmod' drivers/hwtracing/coresight/coresight-tmc-etr.o: In function `tmc_etr_sg_table_rotate':

/home/mpoirier/work/linaro/coresight/kernel-maint/drivers/hwtracing/coresight/coresight-tmc-etr.c:609: undefined reference to `__aeabi_uldivmod'

Please see if you can reproduce on your side.

Uh ! I had gcc-7, which didn't complain about it. But if switch to 4.9, it does. It looks like division of 64bit entity on arm32 is triggering it. We don't need this to be u64 above, as it is the page_idx and could simply switch to "unsigned long" rather than explicitly using div64 helpers.

The following change fixes the issue for me. Please could you check if it solves the problem for you ?

Unfortunately it doesn't.

Mathieu

@@ -551,7 +553,7 @@ static void tmc_etr_sg_table_populate(struct etr_sg_table *etr_table) static inline u32 tmc_etr_sg_offset_to_table_index(u64 offset) {

•   u64 sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
  

•   unsigned long sgpage_idx = offset >> ETR_SG_PAGE_SHIFT;
     return sgpage_idx + sgpage_idx / (ETR_SG_PTRS_PER_PAGE - 1);
  

}

Thanks for the testing !

Suzuki

On Thu, Oct 19, 2017 at 06:15:43PM +0100, Suzuki K Poulose wrote:

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 | 433 +++++++++++++++++++----- drivers/hwtracing/coresight/coresight-tmc.h | 60 +++- 2 files changed, 403 insertions(+), 90 deletions(-)

[..]

+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;
• 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);
  

dev_warn(table->dev, "Unable to map RWP %llx to offset\n", rwq);

It looks a little better and we respect indentation rules. Same for r_offset.

• 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 + w_offset - r_offset :
  

• 	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)
  

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 of the type of 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)
  

Please fix indentation. Also @flags isn't used.

+{

• 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 :
  

• *  a) The ETR cannot use Scatter-Gather.
  

• *  b) if not a, we have an IOMMU backup
  

Please rework the above sentence.

• *  c) if none of the above holds, use it for smaller memory (< 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. @etr_buf->vaddr
• • will always point to the beginning of the "trace buffer".
• */

+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;

/* Zero out the memory to help with debug */

• memset(drvdata->vaddr, 0, drvdata->size);

• memset(etr_buf->vaddr, 0, etr_buf->size);

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); @@ -707,16 +987,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); }

@@ -732,62 +1018,52 @@ 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)

Please fix indentation

{

• char *bufp = drvdata->buf + pos;
• char *bufend = (char *)(drvdata->vaddr + drvdata->size);
• /* Adjust the len to available size @pos */
• if (pos + len > drvdata->len)

• len = drvdata->len - pos;
  

• s64 offset;
• struct etr_buf *etr_buf = drvdata->etr_buf;

• if (pos + len > etr_buf->len)

• len = etr_buf->len - pos;
  

  if (len <= 0) return len;

• /*

• * 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 + len) > bufend)

• len = bufend - bufp;
  

• *bufpp = bufp;
• return len;

• /* 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, len, 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);
  

Indentation

+}

• 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); @@ -796,7 +1072,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); @@ -807,34 +1084,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; }

@@ -842,21 +1116,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; @@ -865,8 +1138,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"); @@ -945,8 +1218,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; }

@@ -965,8 +1238,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)) @@ -988,17 +1260,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 5e49c035a1ac..50ebc17c4645 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -55,6 +55,8 @@ #define TMC_STS_TMCREADY_BIT 2 #define TMC_STS_FULL BIT(0) #define TMC_STS_TRIGGERED BIT(1) +#define TMC_STS_MEMERR BIT(5)

/*

• TMC_AXICTL - 0x110

@@ -134,6 +136,37 @@ 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}
• • @vaddr : Virtual address of the buffer used for trace.
• • @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;
• void *vaddr;
• unsigned long offset;
• u64 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 +174,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 +192,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 +205,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.

-- 2.13.6

On Thu, Oct 19, 2017 at 06:15:44PM +0100, Suzuki K Poulose wrote:

Now that we can dynamically switch between contiguous memory and SG table depending on the trace buffer size, provide the support for selecting an appropriate buffer size.

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

---

.../ABI/testing/sysfs-bus-coresight-devices-tmc | 8 ++++++ drivers/hwtracing/coresight/coresight-tmc.c | 32 ++++++++++++++++++++++ 2 files changed, 40 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc index 4fe677ed1305..3675c380caf8 100644 --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tmc @@ -83,3 +83,11 @@ KernelVersion: 4.7 Contact: Mathieu Poirier mathieu.poirier@linaro.org Description: (R) Indicates the capabilities of the Coresight TMC. The value is read directly from the DEVID register, 0xFC8,

+What: /sys/bus/coresight/devices/<memory_map>.tmc/buffer-size +Date: September 2017 +KernelVersion: 4.15

More like 4.16 now.

+Contact: Mathieu Poirier mathieu.poirier@linaro.org +Description: (RW) Size of the trace buffer for TMC-ETR when used in SYSFS

• mode. Writable only for TMC-ETR configurations. The value
  

• should be aligned to the kernel pagesize.
  

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index c7201e40d737..2349b1805694 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -283,8 +283,40 @@ static ssize_t trigger_cntr_store(struct device *dev, } static DEVICE_ATTR_RW(trigger_cntr); +static ssize_t buffer_size_show(struct device *dev,

• 		struct device_attribute *attr, char *buf)
  

+{

• struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
• return sprintf(buf, "%#x\n", drvdata->size);

+}

+static ssize_t buffer_size_store(struct device *dev,

• 	     struct device_attribute *attr,
  

• 	     const char *buf, size_t size)
  

Indentation (I know trigger_cntr_store() is wrong).

+{

• int ret;
• unsigned long val;
• struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
• if (drvdata->config_type != TMC_CONFIG_TYPE_ETR)

• return -EPERM;
  

I think -EINVAL would be more appropriate but definitely not a big deal.

• ret = kstrtoul(buf, 0, &val);
• if (ret)

• return ret;
  

• /* The buffer size should be page aligned */
• if (val & (PAGE_SIZE - 1))

• return -EINVAL;
  

• drvdata->size = val;
• return size;

+}

+static DEVICE_ATTR_RW(buffer_size);

static struct attribute *coresight_tmc_attrs[] = { &dev_attr_trigger_cntr.attr,

• &dev_attr_buffer_size.attr, NULL,

}; -- 2.13.6

On 02/11/17 19:40, Mathieu Poirier wrote:

On Thu, Oct 19, 2017 at 06:15:46PM +0100, Suzuki K Poulose wrote:

...

Track if the ETR is dma-coherent or not. This will be useful in deciding if we should use software buffering for perf.

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

---

drivers/hwtracing/coresight/coresight-tmc.c | 5 ++++- drivers/hwtracing/coresight/coresight-tmc.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c index 4939333cc6c7..5a8c41130f96 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.c +++ b/drivers/hwtracing/coresight/coresight-tmc.c @@ -347,6 +347,9 @@ static int tmc_etr_setup_caps(struct tmc_drvdata *drvdata, if (!(devid & TMC_DEVID_NOSCAT)) tmc_etr_set_cap(drvdata, TMC_ETR_SG);

• if (device_get_dma_attr(drvdata->dev) == DEV_DMA_COHERENT)

• tmc_etr_set_cap(drvdata, TMC_ETR_COHERENT);
  

• /* Check if the AXI address width is available */ if (devid & TMC_DEVID_AXIAW_VALID) dma_mask = ((devid >> TMC_DEVID_AXIAW_SHIFT) &

@@ -397,7 +400,7 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id) if (!drvdata) goto out;

• drvdata->dev = &adev->dev;

• drvdata->dev = dev;

What is that one for?

Oops, that was a minor cleanup and need not be part of this patch. I will leave things as it is. It is not worth a separate patch.

Cheers Suzuki

On Thu, Oct 19, 2017 at 06:15:47PM +0100, Suzuki K Poulose wrote:

Since the ETR could be driven either by SYSFS or by perf, it becomes complicated how we deal with the buffers used for each of these modes. The ETR driver cannot simply free the current attached buffer without knowing the provider (i.e, sysfs vs perf).

To solve this issue, we provide:

1. the driver-mode specific etr buffer to be retained in the drvdata
2. the etr_buf for a session should be passed on when enabling the hardware, which will be stored in drvdata->etr_buf. This will be replaced (not free'd) as soon as the hardware is disabled, after necessary sync operation.

If I get you right the problem you're trying to solve is what to do with a sysFS buffer that hasn't been read (and freed) when a perf session is requested. In my opinion it should simply be freed. Indeed the user probably doesn't care much about that sysFS buffer, if it did the data would have been harvested.

The advantages of this are :

1. The common code path doesn't need to worry about how to dispose an existing buffer, if it is about to start a new session with a different buffer, possibly in a different mode.
2. The driver mode can control its buffers and can get access to the saved session even when the hardware is operating in a different mode. (e.g, we can still access a trace buffer from a sysfs mode even if the etr is now used in perf mode, without disrupting the current session.)

Towards this, we introduce a sysfs specific data which will hold the etr_buf used for sysfs mode of operation, controlled solely by the sysfs mode handling code.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 59 ++++++++++++++++--------- drivers/hwtracing/coresight/coresight-tmc.h | 2 + 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index f12b7c5f68b2..ef7498f05b34 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -961,11 +961,16 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset); } -static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata) +static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata,

• 	      struct etr_buf *etr_buf)
  

{ u32 axictl, sts;

• struct etr_buf *etr_buf = drvdata->etr_buf;

• /* Callers should provide an appropriate buffer for use */
• if (WARN_ON(!etr_buf || drvdata->etr_buf))

• return;
  

• drvdata->etr_buf = etr_buf; /* Zero out the memory to help with debug */ memset(etr_buf->vaddr, 0, etr_buf->size);

@@ -1023,12 +1028,15 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata)

• 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.

• • We are protected here by drvdata->reading != 0, which ensures the
• • sysfs_buf stays alive.
  */

ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata, loff_t pos, size_t len, char **bufpp) { s64 offset;

• struct etr_buf *etr_buf = drvdata->etr_buf;

• struct etr_buf *etr_buf = drvdata->sysfs_buf;

if (pos + len > etr_buf->len) len = etr_buf->len - pos; @@ -1058,7 +1066,14 @@ tmc_etr_free_sysfs_buf(struct etr_buf *buf) static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata) {

• tmc_sync_etr_buf(drvdata);

• struct etr_buf *etr_buf = drvdata->etr_buf;
• if (WARN_ON(drvdata->sysfs_buf != etr_buf)) {

• tmc_etr_free_sysfs_buf(drvdata->sysfs_buf);
  

• drvdata->sysfs_buf = NULL;
  

• } else {

• tmc_sync_etr_buf(drvdata);
  

• }

} static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) @@ -1077,6 +1092,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata) tmc_disable_hw(drvdata); CS_LOCK(drvdata->base);

• /* Reset the ETR buf used by hardware */
• drvdata->etr_buf = NULL;

} static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) @@ -1085,7 +1102,7 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) bool used = false; unsigned long flags; struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);

• struct etr_buf *new_buf = NULL, *free_buf = NULL;

• struct etr_buf *sysfs_buf = NULL, *new_buf = NULL, *free_buf = NULL;

/* @@ -1097,7 +1114,8 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) * with the lock released. */ spin_lock_irqsave(&drvdata->spinlock, flags);

• if (!drvdata->etr_buf || (drvdata->etr_buf->size != drvdata->size)) {

• sysfs_buf = READ_ONCE(drvdata->sysfs_buf);
• if (!sysfs_buf || (sysfs_buf->size != drvdata->size)) { spin_unlock_irqrestore(&drvdata->spinlock, flags); /* Allocate memory with the spinlock released */ free_buf = new_buf = tmc_etr_setup_sysfs_buf(drvdata);

@@ -1125,15 +1143,16 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev) * 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->etr_buf ||

•    (new_buf && drvdata->etr_buf->size != new_buf->size)) {
  

• sysfs_buf = READ_ONCE(drvdata->sysfs_buf);
• if (!sysfs_buf ||

•    (new_buf && sysfs_buf->size != new_buf->size)) {
  

  used = true;

• free_buf = drvdata->etr_buf;
  

• drvdata->etr_buf = new_buf;
  

• free_buf = sysfs_buf;
  

• drvdata->sysfs_buf = new_buf;
  

  }

drvdata->mode = CS_MODE_SYSFS;

• tmc_etr_enable_hw(drvdata);

• tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf);

out: spin_unlock_irqrestore(&drvdata->spinlock, flags); @@ -1218,13 +1237,13 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) goto out; }

• /* If drvdata::etr_buf is NULL the trace data has been read already */
• if (drvdata->etr_buf == NULL) {

• /* If sysfs_buf is NULL the trace data has been read already */
• if (!drvdata->sysfs_buf) { ret = -EINVAL; goto out; }

• /* Disable the TMC if need be */

• /* Disable the TMC if we are trying to read from a running session */ if (drvdata->mode == CS_MODE_SYSFS) tmc_etr_disable_hw(drvdata);

@@ -1238,7 +1257,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) { unsigned long flags;

• struct etr_buf *etr_buf = NULL;

• struct etr_buf *sysfs_buf = NULL;

/* config types are set a boot time and never change */ if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR)) @@ -1254,22 +1273,22 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) * so we don't have to explicitly clear it. Also, since the * tracer is still enabled drvdata::buf can't be NULL. */

• tmc_etr_enable_hw(drvdata);
  

• tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf);
  

  } else { /*
  • The ETR is not tracing and the buffer was just read.
  • As such prepare to free the trace buffer.
  */

• etr_buf =  drvdata->etr_buf;
  

• drvdata->etr_buf = NULL;
  

• sysfs_buf = drvdata->sysfs_buf;
  

• drvdata->sysfs_buf = NULL;
  

  }

drvdata->reading = false; spin_unlock_irqrestore(&drvdata->spinlock, flags); /* Free allocated memory out side of the spinlock */

• if (etr_buf)

• tmc_free_etr_buf(etr_buf);
  

• if (sysfs_buf)

• tmc_etr_free_sysfs_buf(sysfs_buf);
  

return 0; } diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 69da0b584a6b..14a3dec50b0f 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -185,6 +185,7 @@ struct etr_buf {

• @trigger_cntr: amount of words to store after a trigger.
• @etr_caps: Bitmask of capabilities of the TMC ETR, inferred from the

• device configuration register (DEVID)
  

• • @sysfs_data: SYSFS buffer for ETR.
  */

struct tmc_drvdata { void __iomem *base; @@ -204,6 +205,7 @@ struct tmc_drvdata { enum tmc_mem_intf_width memwidth; u32 trigger_cntr; u32 etr_caps;

• struct etr_buf *sysfs_buf;

}; struct etr_buf_operations { -- 2.13.6

On 02/11/17 20:36, Mathieu Poirier wrote:

On Thu, Oct 19, 2017 at 06:15:49PM +0100, Suzuki K Poulose wrote:

...

We zero out the entire trace buffer used for ETR before it is enabled, for helping with debugging. Since we could be restoring a session in perf mode, this could destroy the data.

I'm not sure to follow you with "... restoring a session in perf mode ...". When operating from the perf interface all the memory allocated for a session is cleanup after, there is no re-using of memory as in sysFS.

We could directly use the perf ring buffer for the ETR. In that case, the perf ring buffer could contain trace data collected from the previous "schedule" which the userspace hasn't collected yet. So, doing a memset here would destroy that data.

Cheers Suzuki

...

Get rid of this step, if someone wants to debug, they can always add it as and when needed.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 31353fc34b53..849684f85443 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -971,8 +971,6 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata, return; drvdata->etr_buf = etr_buf;

• /* Zero out the memory to help with debug */
• memset(etr_buf->vaddr, 0, etr_buf->size);

I agree, this can be costly when dealing with large areas of memory.

...

CS_UNLOCK(drvdata->base); @@ -1267,9 +1265,8 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata) if (drvdata->mode == CS_MODE_SYSFS) { /* * The trace run will continue with the same allocated trace

•  * buffer. The trace buffer is cleared in tmc_etr_enable_hw(),
  

•  * so we don't have to explicitly clear it. Also, since the
  

•  * tracer is still enabled drvdata::buf can't be NULL.
  

•  * buffer. Since the tracer is still enabled drvdata::buf can't
  

•  * be NULL.
  

  */ tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf); } else {

-- 2.13.6

On Thu, Oct 19, 2017 at 06:15:50PM +0100, Suzuki K Poulose wrote:

Add support for creating buffers which can be used in save-restore mode (e.g, for use by perf). If the TMC-ETR supports save-restore feature, we could support the mode in all buffer backends. However,

Instead of using the term backend simply write continuous buffer or SG mode. It is a lot less cryptic that way.

if it doesn't, we should fall back to using in built SG mechanism, where we can rotate the SG table by making some adjustments in the page table.

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

---

drivers/hwtracing/coresight/coresight-tmc-etr.c | 132 +++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tmc.h | 15 +++ 2 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 849684f85443..f8e654e1f5b2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -590,7 +590,7 @@ tmc_etr_sg_table_index_to_daddr(struct tmc_sg_table *sg_table, u32 index)

• 3. Update the hwaddr to point to the table pointer for the buffer
• which starts at "base".

*/ -static int __maybe_unused +static int tmc_etr_sg_table_rotate(struct etr_sg_table *etr_table, u64 base_offset) { u32 last_entry, first_entry; @@ -700,6 +700,9 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, return -ENOMEM; etr_buf->vaddr = vaddr; etr_buf->hwaddr = paddr;

• etr_buf->rrp = paddr;
• etr_buf->rwp = paddr;
• etr_buf->status = 0; etr_buf->mode = ETR_MODE_FLAT; etr_buf->private = drvdata; return 0;

@@ -754,13 +757,19 @@ static int tmc_etr_alloc_sg_buf(struct tmc_drvdata *drvdata, void **pages) { struct etr_sg_table *etr_table;

• struct tmc_sg_table *sg_table;

etr_table = tmc_init_etr_sg_table(drvdata->dev, node, etr_buf->size, pages); if (IS_ERR(etr_table)) return -ENOMEM;

• sg_table = etr_table->sg_table;

As far as I can tell this doesn't do anything.

etr_buf->vaddr = tmc_sg_table_data_vaddr(etr_table->sg_table); etr_buf->hwaddr = etr_table->hwaddr;

• /* TMC ETR SG automatically sets the RRP/RWP when enabled */

If TMC ETR SG automatically sets the RRP/RWP, why explicitly setting it?

• etr_buf->rrp = etr_table->hwaddr;
• etr_buf->rwp = etr_table->hwaddr;
• etr_buf->status = 0; etr_buf->mode = ETR_MODE_ETR_SG; etr_buf->private = etr_table; return 0;

@@ -816,11 +825,49 @@ static void tmc_etr_sync_sg_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp) tmc_sg_table_sync_data_range(table, r_offset, etr_buf->len); } +static int tmc_etr_restore_sg_buf(struct etr_buf *etr_buf,

• 		   u64 r_offset, u64 w_offset,
  

• 		   u32 status, bool has_save_restore)
  

Indentation

+{

• int rc;
• struct etr_sg_table *etr_table = etr_buf->private;
• struct device *dev = etr_table->sg_table->dev;
• /*

• * It is highly unlikely that we have an ETR with in-built SG and
  

• * Save-Restore capability and we are not sure if the PTRs will
  

• * be updated.
  

• */
  

• if (has_save_restore) {

• dev_warn_once(dev,
  

• "Unexpected feature combination of SG and save-restore\n");
  

• return -EINVAL;
  

• }
• /*

• * Since we cannot program RRP/RWP different from DBAL, the offsets
  

• * should match.
  

• */
  

• if (r_offset != w_offset) {

• dev_dbg(dev, "Mismatched RRP/RWP offsets\n");
  

• return -EINVAL;
  

• }
• rc = tmc_etr_sg_table_rotate(etr_table, w_offset);
• if (!rc) {

• etr_buf->hwaddr = etr_table->hwaddr;
  

• etr_buf->rrp = etr_table->hwaddr;
  

• etr_buf->rwp = etr_table->hwaddr;
  

• }
• return rc;

+}

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,

• .restore = tmc_etr_restore_sg_buf,

}; static const struct etr_buf_operations *etr_buf_ops[] = { @@ -861,10 +908,42 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata, { int rc = -ENOMEM; bool has_etr_sg, has_iommu;

• bool has_flat, has_save_restore; 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);

• has_save_restore = tmc_etr_has_cap(drvdata, TMC_ETR_SAVE_RESTORE);
• /*

• * We can normally use flat DMA buffer provided that the buffer
  

• * is not used in save restore fashion without hardware support.
  

• */
  

• has_flat = !(flags & ETR_BUF_F_RESTORE_PTRS) || has_save_restore;
• /*

• * To support save-restore on a given ETR we have the following
  

• * conditions:
  

• *  1) If the buffer requires save-restore of a pointers as well
  

• *     as the Status bit, we require ETR support for it and we coul
  

• *     support all the backends.
  

• *  2) If the buffer requires only save-restore of pointers, then
  

• *     we could exploit a circular ETR SG list. None of the other
  

• *     backends can support it without the ETR feature.
  

• *
  

• * If the buffer will be used in a save-restore mode without
  

• * the ETR support for SAVE_RESTORE, we can only support TMC
  

• * ETR in-built SG tables which can be rotated to make it work.
  

• */
  

• if ((flags & ETR_BUF_F_RESTORE_STATUS) && !has_save_restore)

• return ERR_PTR(-EINVAL);
  

• if (!has_flat && !has_etr_sg) {

• dev_dbg(drvdata->dev,
  

• 	"No available backends for ETR buffer with flags %x\n",
  

• 	flags);
  

• return ERR_PTR(-EINVAL);
  

• }

etr_buf = kzalloc(sizeof(*etr_buf), GFP_KERNEL); if (!etr_buf) @@ -883,7 +962,7 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata, * Fallback to available mechanisms. * */

• if (!pages &&

• if (!pages && has_flat && (!has_etr_sg || has_iommu || size < SZ_1M)) rc = tmc_etr_mode_alloc_buf(ETR_MODE_FLAT, drvdata, etr_buf, node, pages);

@@ -961,6 +1040,51 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata) tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset); } +/*

• • tmc_etr_restore_generic: Common helper to restore the buffer
• • status for FLAT buffers, which use linear TMC ETR address range.
• • This is only possible with in-built ETR capability to save-restore
• • the pointers. The DBA will still point to the original start of the
• • buffer.
• */

+static int tmc_etr_buf_generic_restore(struct etr_buf *etr_buf,

• 			u64 r_offset, u64 w_offset,
  

• 			u32 status, bool has_save_restore)
  

Indentation

+{

• u64 size = etr_buf->size;
• if (!has_save_restore)

• return -EINVAL;
  

• etr_buf->rrp = etr_buf->hwaddr + (r_offset % size);
• etr_buf->rwp = etr_buf->hwaddr + (w_offset % size);
• etr_buf->status = status;
• return 0;

+}

+static int __maybe_unused +tmc_restore_etr_buf(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf,

•     u64 r_offset, u64 w_offset, u32 status)
  

+{

• bool has_save_restore = tmc_etr_has_cap(drvdata, TMC_ETR_SAVE_RESTORE);
• if (WARN_ON_ONCE(!has_save_restore && etr_buf->mode != ETR_MODE_ETR_SG))

• return -EINVAL;
  

• /*

• * If we use a circular SG list without ETR support, we can't
  

• * support restoring "Full" bit.
  

• */
  

• if (WARN_ON_ONCE(!has_save_restore && status))

• return -EINVAL;
  

• if (status & ~TMC_STS_FULL)

• return -EINVAL;
  

• if (etr_buf->ops->restore)

• return etr_buf->ops->restore(etr_buf, r_offset, w_offset,
  

• 			      status, has_save_restore);
  

• else

• return tmc_etr_buf_generic_restore(etr_buf, r_offset, w_offset,
  

• 			       status, has_save_restore);
  

+}

static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata, struct etr_buf *etr_buf) { @@ -1004,8 +1128,8 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata, * STS to "not full"). */ if (tmc_etr_has_cap(drvdata, TMC_ETR_SAVE_RESTORE)) {

• tmc_write_rrp(drvdata, etr_buf->hwaddr);
  

• tmc_write_rwp(drvdata, etr_buf->hwaddr);
  

• tmc_write_rrp(drvdata, etr_buf->rrp);
  

• tmc_write_rwp(drvdata, etr_buf->rwp);
  

  sts = readl_relaxed(drvdata->base + TMC_STS) & ~TMC_STS_FULL; writel_relaxed(sts, drvdata->base + TMC_STS); }

diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h index 14a3dec50b0f..2c5b905b6494 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -142,12 +142,22 @@ enum etr_mode { ETR_MODE_ETR_SG, /* Uses in-built TMC ETR SG mechanism */ }; +/* ETR buffer should support save-restore */ +#define ETR_BUF_F_RESTORE_PTRS 0x1 +#define ETR_BUF_F_RESTORE_STATUS 0x2

+#define ETR_BUF_F_RESTORE_MINIMAL ETR_BUF_F_RESTORE_PTRS +#define ETR_BUF_F_RESTORE_FULL (ETR_BUF_F_RESTORE_PTRS |\

• 			 ETR_BUF_F_RESTORE_STATUS)
  

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

• • @status : Value for STATUS if the ETR supports save-restore.
• • @rrp : Value for RRP{LO:HI} if the ETR supports save-restore
• • @rwp : Value for RWP{LO:HI} if the ETR supports save-restore
  • @size : Size of the buffer.
  • @hwaddr : Address to be programmed in the TMC:DBA{LO,HI}
  • @vaddr : Virtual address of the buffer used for trace.

@@ -159,6 +169,9 @@ struct etr_buf_operations; struct etr_buf { enum etr_mode mode; bool full;

• u32 status;
• dma_addr_t rrp;
• dma_addr_t rwp; ssize_t size; dma_addr_t hwaddr; void *vaddr;

@@ -212,6 +225,8 @@ 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);

• int (*restore)(struct etr_buf *etr_buf, u64 r_offset,

•        u64 w_offset, u32 status, bool has_save_restore);
  

  ssize_t (*get_data)(struct etr_buf *etr_buf, u64 offset, size_t len, char **bufpp); void (*free)(struct etr_buf *etr_buf);

-- 2.13.6