[PATCH 1/2] coresight: tmc: Fix byte-address alignment for RRP

[PATCH 1/2] coresight: tmc: Fix byte-address alignment for RRP

[Leo Yan]

>From the comment in the code, it claims the requirement for byte-address
alignment for RRP register: 'for 32-bit, 64-bit and 128-bit wide trace
memory, the four LSBs must be 0s. For 256-bit wide trace memory, the
five LSBs must be 0s'.  This isn't consistent with the program, the
program sets five LSBs as zeros for 32/64/128-bit wide trace memory and
set six LSBs zeros for 256-bit wide trace memory.

After checking with the CoreSight Trace Memory Controller technical
reference manual (ARM DDI 0461B, section 3.3.4 RAM Read Pointer
Register), it proves the comment is right and the program does wrong
setting.

This patch fixes byte-address alignment for RRP by following correct
definition in the technical reference manual.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 0549249..e310613 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -438,10 +438,10 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 		case TMC_MEM_INTF_WIDTH_32BITS:
 		case TMC_MEM_INTF_WIDTH_64BITS:
 		case TMC_MEM_INTF_WIDTH_128BITS:
-			mask = GENMASK(31, 5);
+			mask = GENMASK(31, 4);
 			break;
 		case TMC_MEM_INTF_WIDTH_256BITS:
-			mask = GENMASK(31, 6);
+			mask = GENMASK(31, 5);
 			break;
 		}
 
-- 
2.7.4

[PATCH 2/2] coresight: tmc: Fix writing barrier packets for ring buffer

[Leo Yan]

When tmc driver updates ring buffer and detects the ETB RAM has a wrap
around, in the head of ring buffer the first four words is filled trace
data by reading out RRD register, but the driver simply discards the
first four words and leave the memory to store the barrier packets with
value 0x7fff,ffff.  As result we get below data content in the buffer:

  +--------+--------+--------+--------+--------+--------+--------+
  | trace0 | trace1 | trace2 | trace3 | trace4 | trace5 |  ...   |
  |   v    |   v    |   v    |   v    |        |        |        |
  | barrier| barrier| barrier| barrier|        |        |        |
  +--------+--------+--------+--------+--------+--------+--------+

On the other hand, the ring buffer actually has enough space for saving
both barrier packets and trace data from ETB RAM.  Ideally we should
save both barrier packets and complete trace data in the ETB RAM for the
wrap around case.  So below is the ideal data content we can get: the
first four words are for barrier packets and then save trace data from
reading the first word from RRD register.

  +--------+--------+--------+--------+--------+--------+--------+
  | barrier| barrier| barrier| barrier| trace0 | trace1 |  ...   |
  +--------+--------+--------+--------+--------+--------+--------+

To achieve this purpose, this patch introduces a variable 'barrier_sz'
to present the barrier packets length is to be written into the ring
buffer.  There have two cases that we will lose trace data so need to
insert the barrier packets between two discontinuous trace data block;
the first one case is the ETB RAM is in full state, another case is the
ring buffer has no enough space for trace data at this time.

We firstly need to reserve memory space in the ring buffer for barrier
packets and the left memory space can be used to save trace data. This
patch caps the value 'barrier_sz' so ensure barrier length is not longer
than ring buffer and calculate the rest memory space for 'to_read' for
trace data recording.

At the data saving stage, the patch firstly saves barrier packets
without reading RRD register so that will not lose trace data; after
that it will save trace data by accessing RRD register.  At the end, the
read data length is the sum value of barrier length 'barrier_sz' plus
trace data length 'to_read'.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 34 +++++++++++++++++--------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index e310613..9c599c9 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -387,7 +387,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 	const u32 *barrier;
 	u32 *buf_ptr;
 	u64 read_ptr, write_ptr;
-	u32 status, to_read;
+	u32 status, to_read, barrier_sz = 0;
 	unsigned long offset;
 	struct cs_buffers *buf = sink_config;
 	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
@@ -419,11 +419,18 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 	}
 
 	/*
-	 * The TMC RAM buffer may be bigger than the space available in the
-	 * perf ring buffer (handle->size).  If so advance the RRP so that we
-	 * get the latest trace data.
+	 * Drop some trace data when the TMC RAM buffer is full or bigger than
+	 * perf ring buffer.  For this case the trace data is discontinuous
+	 * thus need to inser barrier packets.
 	 */
-	if (to_read > handle->size) {
+	barrier_sz = CORESIGHT_BARRIER_PKT_SIZE;
+
+	/*
+	 * The sum of trace data and inserted barriers may be bigger than the
+	 * space available in the perf ring buffer (handle->size).  If so
+	 * advance the RRP so that we get the latest trace data.
+	 */
+	if (to_read + barrier_sz > handle->size) {
 		u32 mask = 0;
 
 		/*
@@ -445,11 +452,15 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 			break;
 		}
 
+		/* Make sure barrier isn't out of bound */
+		barrier_sz = min_t(u32, barrier_sz, handle->size);
+
 		/*
-		 * Make sure the new size is aligned in accordance with the
+		 * Firstly reserve the memory for inserted barrier packets;
+		 * make sure the new size is aligned in accordance with the
 		 * requirement explained above.
 		 */
-		to_read = handle->size & mask;
+		to_read = (handle->size - barrier_sz) & mask;
 		/* Move the RAM read pointer up */
 		read_ptr = (write_ptr + drvdata->size) - to_read;
 		/* Make sure we are still within our limits */
@@ -468,13 +479,14 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 	barrier = barrier_pkt;
 
 	/* for every byte to read */
-	for (i = 0; i < to_read; i += 4) {
+	for (i = 0; i < to_read + barrier_sz; i += 4) {
 		buf_ptr = buf->data_pages[cur] + offset;
-		*buf_ptr = readl_relaxed(drvdata->base + TMC_RRD);
 
-		if (lost && *barrier) {
+		if (i < barrier_sz) {
 			*buf_ptr = *barrier;
 			barrier++;
+		} else {
+			*buf_ptr = readl_relaxed(drvdata->base + TMC_RRD);
 		}
 
 		offset += 4;
@@ -495,7 +507,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
 	if (buf->snapshot)
 		local_set(&buf->data_size, (cur * PAGE_SIZE) + offset);
 	else
-		local_add(to_read, &buf->data_size);
+		local_add(to_read + barrier_sz, &buf->data_size);
 
 	CS_LOCK(drvdata->base);
 }
-- 
2.7.4

[PATCH 2/2] coresight: tmc: Fix writing barrier packets for ring buffer

[Mathieu Poirier]

On Wed, 5 Sep 2018 at 01:56, Leo Yan <leo.yan@linaro.org> wrote:
>
> When tmc driver updates ring buffer and detects the ETB RAM has a wrap
> around, in the head of ring buffer the first four words is filled trace
> data by reading out RRD register, but the driver simply discards the
> first four words and leave the memory to store the barrier packets with
> value 0x7fff,ffff.  As result we get below data content in the buffer:
>
>   +--------+--------+--------+--------+--------+--------+--------+
>   | trace0 | trace1 | trace2 | trace3 | trace4 | trace5 |  ...   |
>   |   v    |   v    |   v    |   v    |        |        |        |
>   | barrier| barrier| barrier| barrier|        |        |        |
>   +--------+--------+--------+--------+--------+--------+--------+
>
> On the other hand, the ring buffer actually has enough space for saving
> both barrier packets and trace data from ETB RAM.  Ideally we should
> save both barrier packets and complete trace data in the ETB RAM for the
> wrap around case.  So below is the ideal data content we can get: the
> first four words are for barrier packets and then save trace data from
> reading the first word from RRD register.
>
>   +--------+--------+--------+--------+--------+--------+--------+
>   | barrier| barrier| barrier| barrier| trace0 | trace1 |  ...   |
>   +--------+--------+--------+--------+--------+--------+--------+
>
> To achieve this purpose, this patch introduces a variable 'barrier_sz'
> to present the barrier packets length is to be written into the ring
> buffer.  There have two cases that we will lose trace data so need to
> insert the barrier packets between two discontinuous trace data block;
> the first one case is the ETB RAM is in full state, another case is the
> ring buffer has no enough space for trace data at this time.
>
> We firstly need to reserve memory space in the ring buffer for barrier
> packets and the left memory space can be used to save trace data. This
> patch caps the value 'barrier_sz' so ensure barrier length is not longer
> than ring buffer and calculate the rest memory space for 'to_read' for
> trace data recording.
>
> At the data saving stage, the patch firstly saves barrier packets
> without reading RRD register so that will not lose trace data; after
> that it will save trace data by accessing RRD register.  At the end, the
> read data length is the sum value of barrier length 'barrier_sz' plus
> trace data length 'to_read'.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etf.c | 34 +++++++++++++++++--------
>  1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index e310613..9c599c9 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -387,7 +387,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>         const u32 *barrier;
>         u32 *buf_ptr;
>         u64 read_ptr, write_ptr;
> -       u32 status, to_read;
> +       u32 status, to_read, barrier_sz = 0;

This doesn't apply on my next branch.  Please rebase and send both
patches again.

Mathieu

>         unsigned long offset;
>         struct cs_buffers *buf = sink_config;
>         struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> @@ -419,11 +419,18 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>         }
>
>         /*
> -        * The TMC RAM buffer may be bigger than the space available in the
> -        * perf ring buffer (handle->size).  If so advance the RRP so that we
> -        * get the latest trace data.
> +        * Drop some trace data when the TMC RAM buffer is full or bigger than
> +        * perf ring buffer.  For this case the trace data is discontinuous
> +        * thus need to inser barrier packets.
>          */
> -       if (to_read > handle->size) {
> +       barrier_sz = CORESIGHT_BARRIER_PKT_SIZE;
> +
> +       /*
> +        * The sum of trace data and inserted barriers may be bigger than the
> +        * space available in the perf ring buffer (handle->size).  If so
> +        * advance the RRP so that we get the latest trace data.
> +        */
> +       if (to_read + barrier_sz > handle->size) {
>                 u32 mask = 0;
>
>                 /*
> @@ -445,11 +452,15 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>                         break;
>                 }
>
> +               /* Make sure barrier isn't out of bound */
> +               barrier_sz = min_t(u32, barrier_sz, handle->size);
> +
>                 /*
> -                * Make sure the new size is aligned in accordance with the
> +                * Firstly reserve the memory for inserted barrier packets;
> +                * make sure the new size is aligned in accordance with the
>                  * requirement explained above.
>                  */
> -               to_read = handle->size & mask;
> +               to_read = (handle->size - barrier_sz) & mask;
>                 /* Move the RAM read pointer up */
>                 read_ptr = (write_ptr + drvdata->size) - to_read;
>                 /* Make sure we are still within our limits */
> @@ -468,13 +479,14 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>         barrier = barrier_pkt;
>
>         /* for every byte to read */
> -       for (i = 0; i < to_read; i += 4) {
> +       for (i = 0; i < to_read + barrier_sz; i += 4) {
>                 buf_ptr = buf->data_pages[cur] + offset;
> -               *buf_ptr = readl_relaxed(drvdata->base + TMC_RRD);
>
> -               if (lost && *barrier) {
> +               if (i < barrier_sz) {
>                         *buf_ptr = *barrier;
>                         barrier++;
> +               } else {
> +                       *buf_ptr = readl_relaxed(drvdata->base + TMC_RRD);
>                 }
>
>                 offset += 4;
> @@ -495,7 +507,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
>         if (buf->snapshot)
>                 local_set(&buf->data_size, (cur * PAGE_SIZE) + offset);
>         else
> -               local_add(to_read, &buf->data_size);
> +               local_add(to_read + barrier_sz, &buf->data_size);
>
>         CS_LOCK(drvdata->base);
>  }
> --
> 2.7.4
>

[PATCH 2/2] coresight: tmc: Fix writing barrier packets for ring buffer

[leo.yan at linaro.org]

Hi Mathieu,

On Mon, Sep 10, 2018 at 01:42:31PM -0600, Mathieu Poirier wrote:

[...]

> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > index e310613..9c599c9 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > @@ -387,7 +387,7 @@ static void tmc_update_etf_buffer(struct coresight_device *csdev,
> >         const u32 *barrier;
> >         u32 *buf_ptr;
> >         u64 read_ptr, write_ptr;
> > -       u32 status, to_read;
> > +       u32 status, to_read, barrier_sz = 0;
> 
> This doesn't apply on my next branch.  Please rebase and send both
> patches again.

Done.  Rebased these two patches against to CoreSight next branch [1];
please review the patch series v2 which has been sent out.

[...]

Thanks,
Leo Yan

[1] https://git.linaro.org/kernel/coresight.git/log/?h=next