diff mbox series

[v7,2/7] coresight: tmc-etr: Add support to use reserved trace memory

Message ID 20240307033625.325058-3-lcherian@marvell.com (mailing list archive)
State New, archived
Headers show
Series Coresight for Kernel panic and watchdog reset | expand

Commit Message

Linu Cherian March 7, 2024, 3:36 a.m. UTC
Add support to use reserved memory for coresight ETR trace buffer.

Introduce a new ETR buffer mode called ETR_MODE_RESRV, which
becomes available when ETR device tree node is supplied with a valid
reserved memory region.

ETR_MODE_RESRV can be selected only by explicit user request.

$ echo resrv >/sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred

Signed-off-by: Anil Kumar Reddy <areddy3@marvell.com>
Signed-off-by: Linu Cherian <lcherian@marvell.com>
---
Changelog from v6:
* Removed redundant goto statements
* Setting of etr_buf->size to the reserved memory size is done
  after successful dma map inside the alloc function
* Removed the special casing for ETR_MODE_RESRV 
* Fixed the tab spacing in struct tmc_drvdata 

 .../hwtracing/coresight/coresight-tmc-core.c  | 47 +++++++++++
 .../hwtracing/coresight/coresight-tmc-etr.c   | 82 ++++++++++++++++++-
 drivers/hwtracing/coresight/coresight-tmc.h   | 27 ++++++
 3 files changed, 153 insertions(+), 3 deletions(-)

Comments

James Clark April 12, 2024, 9:57 a.m. UTC | #1
On 07/03/2024 03:36, Linu Cherian wrote:
> Add support to use reserved memory for coresight ETR trace buffer.
> 
> Introduce a new ETR buffer mode called ETR_MODE_RESRV, which
> becomes available when ETR device tree node is supplied with a valid
> reserved memory region.
> 
> ETR_MODE_RESRV can be selected only by explicit user request.
> 
> $ echo resrv >/sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred
> 
> Signed-off-by: Anil Kumar Reddy <areddy3@marvell.com>
> Signed-off-by: Linu Cherian <lcherian@marvell.com>
> ---
> Changelog from v6:
> * Removed redundant goto statements
> * Setting of etr_buf->size to the reserved memory size is done
>   after successful dma map inside the alloc function
> * Removed the special casing for ETR_MODE_RESRV 
> * Fixed the tab spacing in struct tmc_drvdata 
> 
>  .../hwtracing/coresight/coresight-tmc-core.c  | 47 +++++++++++
>  .../hwtracing/coresight/coresight-tmc-etr.c   | 82 ++++++++++++++++++-
>  drivers/hwtracing/coresight/coresight-tmc.h   | 27 ++++++
>  3 files changed, 153 insertions(+), 3 deletions(-)
> 

[...]

>  static bool etr_can_use_flat_mode(struct etr_buf_hw *buf_hw, ssize_t etr_buf_size)
> @@ -874,13 +947,10 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
>  	if (!etr_buf)
>  		return ERR_PTR(-ENOMEM);
>  
> -	etr_buf->size = size;
> -

Hi Linu,

Not sure if this was left in by mistake? It's not mentioned in the
commit message and it doesn't seem to match the description.

Please make sure the current tests pass both with and without a reserved
buffer defined in the DT. I get lots of failures with this patchset
applied on N1SDP. ETF seems to work but ETR doesn't:

  $ sudo perf test -vvv "arm coresight"

  Recording trace (only user mode) with path: CPU1 => tmc_etf0
  CoreSight path testing (CPU1 -> tmc_etf0): PASS

  Recording trace (only user mode) with path: CPU1 => tmc_etr0
  CoreSight path testing (CPU1 -> tmc_etr0): FAIL
  ...

Dmesg:
  [ 1938.622091] coresight tmc_etr0: Unable to allocate ETR buffer

>  	/* If there is user directive for buffer mode, try that first */
>  	if (drvdata->etr_mode != ETR_MODE_AUTO)
>  		rc = tmc_etr_mode_alloc_buf(drvdata->etr_mode, drvdata,
>  					    etr_buf, node, pages);
> -

Whitespace change.
Linu Cherian April 14, 2024, 10:09 a.m. UTC | #2
Hi James,

> -----Original Message-----
> From: James Clark <james.clark@arm.com>
> Sent: Friday, April 12, 2024 3:27 PM
> To: Linu Cherian <lcherian@marvell.com>; suzuki.poulose@arm.com
> Cc: linux-arm-kernel@lists.infradead.org; coresight@lists.linaro.org; linux-
> kernel@vger.kernel.org; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> devicetree@vger.kernel.org; Sunil Kovvuri Goutham
> <sgoutham@marvell.com>; George Cherian <gcherian@marvell.com>; Anil
> Kumar Reddy H <areddy3@marvell.com>; mike.leach@linaro.org;
> leo.yan@linaro.org
> Subject: [EXTERNAL] Re: [PATCH v7 2/7] coresight: tmc-etr: Add support to
> use reserved trace memory
> 
> Prioritize security for external emails: Confirm sender and content safety
> before clicking links or opening attachments
> 
> ----------------------------------------------------------------------
> 
> 
> On 07/03/2024 03:36, Linu Cherian wrote:
> > Add support to use reserved memory for coresight ETR trace buffer.
> >
> > Introduce a new ETR buffer mode called ETR_MODE_RESRV, which becomes
> > available when ETR device tree node is supplied with a valid reserved
> > memory region.
> >
> > ETR_MODE_RESRV can be selected only by explicit user request.
> >
> > $ echo resrv
> >/sys/bus/coresight/devices/tmc_etr<N>/buf_mode_preferred
> >
> > Signed-off-by: Anil Kumar Reddy <areddy3@marvell.com>
> > Signed-off-by: Linu Cherian <lcherian@marvell.com>
> > ---
> > Changelog from v6:
> > * Removed redundant goto statements
> > * Setting of etr_buf->size to the reserved memory size is done
> >   after successful dma map inside the alloc function
> > * Removed the special casing for ETR_MODE_RESRV
> > * Fixed the tab spacing in struct tmc_drvdata
> >
> >  .../hwtracing/coresight/coresight-tmc-core.c  | 47 +++++++++++
> >  .../hwtracing/coresight/coresight-tmc-etr.c   | 82 ++++++++++++++++++-
> >  drivers/hwtracing/coresight/coresight-tmc.h   | 27 ++++++
> >  3 files changed, 153 insertions(+), 3 deletions(-)
> >
> 
> [...]
> 
> >  static bool etr_can_use_flat_mode(struct etr_buf_hw *buf_hw, ssize_t
> > etr_buf_size) @@ -874,13 +947,10 @@ static struct etr_buf
> *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> >  	if (!etr_buf)
> >  		return ERR_PTR(-ENOMEM);
> >
> > -	etr_buf->size = size;
> > -
> 
> Hi Linu,
> 
> Not sure if this was left in by mistake? It's not mentioned in the commit
> message and it doesn't seem to match the description.
> 

Yeah, that change was by mistake. Sorry about that.

> Please make sure the current tests pass both with and without a reserved
> buffer defined in the DT. I get lots of failures with this patchset applied on
> N1SDP. ETF seems to work but ETR doesn't:
> 

Ack.

>   $ sudo perf test -vvv "arm coresight"
> 
>   Recording trace (only user mode) with path: CPU1 => tmc_etf0
>   CoreSight path testing (CPU1 -> tmc_etf0): PASS
> 
>   Recording trace (only user mode) with path: CPU1 => tmc_etr0
>   CoreSight path testing (CPU1 -> tmc_etr0): FAIL
>   ...
> 
> Dmesg:
>   [ 1938.622091] coresight tmc_etr0: Unable to allocate ETR buffer
> 
> >  	/* If there is user directive for buffer mode, try that first */
> >  	if (drvdata->etr_mode != ETR_MODE_AUTO)
> >  		rc = tmc_etr_mode_alloc_buf(drvdata->etr_mode, drvdata,
> >  					    etr_buf, node, pages);
> > -
> 
> Whitespace change.
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 72005b0c633e..1325387d6257 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -22,6 +22,7 @@ 
 #include <linux/spinlock.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
 
@@ -370,6 +371,50 @@  static inline bool tmc_etr_has_non_secure_access(struct tmc_drvdata *drvdata)
 	return (auth & TMC_AUTH_NSID_MASK) == 0x3;
 }
 
+static struct device_node *tmc_get_region_byname(struct device_node *node,
+						 char *name)
+{
+	int index;
+
+	index = of_property_match_string(node, "memory-region-names", name);
+	if (index < 0)
+		return ERR_PTR(-ENODEV);
+
+	return of_parse_phandle(node, "memory-region", index);
+}
+
+static void tmc_get_reserved_region(struct device *parent)
+{
+	struct tmc_drvdata *drvdata = dev_get_drvdata(parent);
+	struct device_node *node;
+	struct resource res;
+	int rc;
+
+	node = tmc_get_region_byname(parent->of_node, "tracedata");
+	if (IS_ERR_OR_NULL(node)) {
+		dev_dbg(parent, "No reserved trace buffer specified\n");
+		return;
+	}
+
+	rc = of_address_to_resource(node, 0, &res);
+	of_node_put(node);
+	if (rc || res.start == 0 || resource_size(&res) == 0) {
+		dev_err(parent, "Reserved trace buffer memory is invalid\n");
+		return;
+	}
+
+	drvdata->crash_tbuf.vaddr = memremap(res.start,
+						resource_size(&res),
+						MEMREMAP_WC);
+	if (IS_ERR_OR_NULL(drvdata->crash_tbuf.vaddr)) {
+		dev_err(parent, "Reserved trace buffer mapping failed\n");
+		return;
+	}
+
+	drvdata->crash_tbuf.paddr = res.start;
+	drvdata->crash_tbuf.size  = resource_size(&res);
+}
+
 /* Detect and initialise the capabilities of a TMC ETR */
 static int tmc_etr_setup_caps(struct device *parent, u32 devid, void *dev_caps)
 {
@@ -482,6 +527,8 @@  static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 		drvdata->size = readl_relaxed(drvdata->base + TMC_RSZ) * 4;
 	}
 
+	tmc_get_reserved_region(dev);
+
 	desc.dev = dev;
 
 	switch (drvdata->config_type) {
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index e75428fa1592..2bbf53480c66 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -30,6 +30,7 @@  struct etr_buf_hw {
 	bool	has_iommu;
 	bool	has_etr_sg;
 	bool	has_catu;
+	bool	has_resrv;
 };
 
 /*
@@ -694,6 +695,75 @@  static const struct etr_buf_operations etr_flat_buf_ops = {
 	.get_data = tmc_etr_get_data_flat_buf,
 };
 
+/*
+ * tmc_etr_alloc_resrv_buf: Allocate a contiguous DMA buffer from reserved region.
+ */
+static int tmc_etr_alloc_resrv_buf(struct tmc_drvdata *drvdata,
+				  struct etr_buf *etr_buf, int node,
+				  void **pages)
+{
+	struct etr_flat_buf *resrv_buf;
+	struct device *real_dev = drvdata->csdev->dev.parent;
+
+	/* We cannot reuse existing pages for resrv buf */
+	if (pages)
+		return -EINVAL;
+
+	resrv_buf = kzalloc(sizeof(*resrv_buf), GFP_KERNEL);
+	if (!resrv_buf)
+		return -ENOMEM;
+
+	resrv_buf->daddr = dma_map_resource(real_dev, drvdata->crash_tbuf.paddr,
+					   drvdata->crash_tbuf.size,
+					   DMA_FROM_DEVICE, 0);
+	if (dma_mapping_error(real_dev, resrv_buf->daddr)) {
+		dev_err(real_dev, "failed to map source buffer address\n");
+		kfree(resrv_buf);
+		return -ENOMEM;
+	}
+
+	resrv_buf->vaddr = drvdata->crash_tbuf.vaddr;
+	resrv_buf->size = etr_buf->size = drvdata->crash_tbuf.size;
+	resrv_buf->dev = &drvdata->csdev->dev;
+	etr_buf->hwaddr = resrv_buf->daddr;
+	etr_buf->mode = ETR_MODE_RESRV;
+	etr_buf->private = resrv_buf;
+	return 0;
+}
+
+static void tmc_etr_free_resrv_buf(struct etr_buf *etr_buf)
+{
+	struct etr_flat_buf *resrv_buf = etr_buf->private;
+
+	if (resrv_buf && resrv_buf->daddr) {
+		struct device *real_dev = resrv_buf->dev->parent;
+
+		dma_unmap_resource(real_dev, resrv_buf->daddr,
+				resrv_buf->size, DMA_FROM_DEVICE, 0);
+	}
+	kfree(resrv_buf);
+}
+
+static void tmc_etr_sync_resrv_buf(struct etr_buf *etr_buf, u64 rrp, u64 rwp)
+{
+	/*
+	 * Adjust the buffer to point to the beginning of the trace data
+	 * and update the available trace data.
+	 */
+	etr_buf->offset = rrp - etr_buf->hwaddr;
+	if (etr_buf->full)
+		etr_buf->len = etr_buf->size;
+	else
+		etr_buf->len = rwp - rrp;
+}
+
+static const struct etr_buf_operations etr_resrv_buf_ops = {
+	.alloc = tmc_etr_alloc_resrv_buf,
+	.free = tmc_etr_free_resrv_buf,
+	.sync = tmc_etr_sync_resrv_buf,
+	.get_data = tmc_etr_get_data_flat_buf,
+};
+
 /*
  * tmc_etr_alloc_sg_buf: Allocate an SG buf @etr_buf. Setup the parameters
  * appropriately.
@@ -800,6 +870,7 @@  static const struct etr_buf_operations *etr_buf_ops[] = {
 	[ETR_MODE_FLAT] = &etr_flat_buf_ops,
 	[ETR_MODE_ETR_SG] = &etr_sg_buf_ops,
 	[ETR_MODE_CATU] = NULL,
+	[ETR_MODE_RESRV] = &etr_resrv_buf_ops
 };
 
 void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu)
@@ -825,6 +896,7 @@  static inline int tmc_etr_mode_alloc_buf(int mode,
 	case ETR_MODE_FLAT:
 	case ETR_MODE_ETR_SG:
 	case ETR_MODE_CATU:
+	case ETR_MODE_RESRV:
 		if (etr_buf_ops[mode] && etr_buf_ops[mode]->alloc)
 			rc = etr_buf_ops[mode]->alloc(drvdata, etr_buf,
 						      node, pages);
@@ -843,6 +915,7 @@  static void get_etr_buf_hw(struct device *dev, struct etr_buf_hw *buf_hw)
 	buf_hw->has_iommu = iommu_get_domain_for_dev(dev->parent);
 	buf_hw->has_etr_sg = tmc_etr_has_cap(drvdata, TMC_ETR_SG);
 	buf_hw->has_catu = !!tmc_etr_get_catu_device(drvdata);
+	buf_hw->has_resrv = is_tmc_reserved_region_valid(dev->parent);
 }
 
 static bool etr_can_use_flat_mode(struct etr_buf_hw *buf_hw, ssize_t etr_buf_size)
@@ -874,13 +947,10 @@  static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
 	if (!etr_buf)
 		return ERR_PTR(-ENOMEM);
 
-	etr_buf->size = size;
-
 	/* If there is user directive for buffer mode, try that first */
 	if (drvdata->etr_mode != ETR_MODE_AUTO)
 		rc = tmc_etr_mode_alloc_buf(drvdata->etr_mode, drvdata,
 					    etr_buf, node, pages);
-
 	/*
 	 * 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,
@@ -1830,6 +1900,7 @@  static const char *const buf_modes_str[] = {
 	[ETR_MODE_FLAT]		= "flat",
 	[ETR_MODE_ETR_SG]	= "tmc-sg",
 	[ETR_MODE_CATU]		= "catu",
+	[ETR_MODE_RESRV]	= "resrv",
 	[ETR_MODE_AUTO]		= "auto",
 };
 
@@ -1848,6 +1919,9 @@  static ssize_t buf_modes_available_show(struct device *dev,
 	if (buf_hw.has_catu)
 		size += sysfs_emit_at(buf, size, "%s ", buf_modes_str[ETR_MODE_CATU]);
 
+	if (buf_hw.has_resrv)
+		size += sysfs_emit_at(buf, size, "%s ", buf_modes_str[ETR_MODE_RESRV]);
+
 	size += sysfs_emit_at(buf, size, "\n");
 	return size;
 }
@@ -1875,6 +1949,8 @@  static ssize_t buf_mode_preferred_store(struct device *dev,
 		drvdata->etr_mode = ETR_MODE_ETR_SG;
 	else if (sysfs_streq(buf, buf_modes_str[ETR_MODE_CATU]) && buf_hw.has_catu)
 		drvdata->etr_mode = ETR_MODE_CATU;
+	else if (sysfs_streq(buf, buf_modes_str[ETR_MODE_RESRV]) && buf_hw.has_resrv)
+		drvdata->etr_mode = ETR_MODE_RESRV;
 	else if (sysfs_streq(buf, buf_modes_str[ETR_MODE_AUTO]))
 		drvdata->etr_mode = ETR_MODE_AUTO;
 	else
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index cef979c897e6..2abc5387cdf7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -135,6 +135,7 @@  enum etr_mode {
 	ETR_MODE_FLAT,		/* Uses contiguous flat buffer */
 	ETR_MODE_ETR_SG,	/* Uses in-built TMC ETR SG mechanism */
 	ETR_MODE_CATU,		/* Use SG mechanism in CATU */
+	ETR_MODE_RESRV,		/* Use reserved region contiguous buffer */
 	ETR_MODE_AUTO,		/* Use the default mechanism */
 };
 
@@ -164,6 +165,17 @@  struct etr_buf {
 	void				*private;
 };
 
+/**
+ * @paddr	: Start address of reserved memory region.
+ * @vaddr	: Corresponding CPU virtual address.
+ * @size	: Size of reserved memory region.
+ */
+struct tmc_resrv_buf {
+	phys_addr_t     paddr;
+	void		*vaddr;
+	size_t		size;
+};
+
 /**
  * struct tmc_drvdata - specifics associated to an TMC component
  * @base:	memory mapped base address for this component.
@@ -187,6 +199,10 @@  struct etr_buf {
  * @idr_mutex:	Access serialisation for idr.
  * @sysfs_buf:	SYSFS buffer for ETR.
  * @perf_buf:	PERF buffer for ETR.
+ * @crash_tbuf:	Used by ETR as hardware trace buffer and for trace data
+ *		retention (after crash) only when ETR_MODE_RESRV buffer
+ *		mode is enabled. Used by ETF for trace data retention
+ *		(after crash) by default.
  */
 struct tmc_drvdata {
 	void __iomem		*base;
@@ -211,6 +227,7 @@  struct tmc_drvdata {
 	struct mutex		idr_mutex;
 	struct etr_buf		*sysfs_buf;
 	struct etr_buf		*perf_buf;
+	struct tmc_resrv_buf	crash_tbuf;
 };
 
 struct etr_buf_operations {
@@ -328,6 +345,16 @@  tmc_sg_table_buf_size(struct tmc_sg_table *sg_table)
 	return (unsigned long)sg_table->data_pages.nr_pages << PAGE_SHIFT;
 }
 
+static inline bool is_tmc_reserved_region_valid(struct device *dev)
+{
+	struct tmc_drvdata *drvdata = dev_get_drvdata(dev);
+
+	if (drvdata->crash_tbuf.paddr &&
+		drvdata->crash_tbuf.size)
+		return true;
+	return false;
+}
+
 struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);
 
 void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu);