2024-01-05 05:59:23

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 0/7]Coresight for Kernel panic and watchdog reset

This patch series is rebased on v6.7-rc8 and is dependent
on the below patch.
- coresight: tmc: Make etr buffer mode user configurable from sysfs[1]

Changelog from v5:
* Fixed issues reported by CONFIG_DEBUG_ATOMIC_SLEEP
* Fixed a memory leak while reading data from /dev/tmc_etrx in
READ_PREVBOOT mode
* Tested reading trace data from crashdump kernel

Changelog from v4:
* Device tree binding
- Description is made more explicit on the usage of reserved memory
region
- Mismatch in memory region names in dts binding and driver fixed
- Removed "mem" suffix from the memory region names
* Rename "struct tmc_register_snapshot" -> "struct tmc_crash_metadata",
since it contains more than register snapshot.
Related variables are named accordingly.
* Rename struct tmc_drvdata members
resrv_buf -> crash_tbuf
metadata -> crash_mdata
* Size field in metadata refers to RSZ register and hence indicates the
size in 32 bit words. ETR metadata follows this convention, the same
has been extended to ETF metadata as well.
* Added crc32 for more robust metadata and tracedata validation.
* Added/modified dev_dbg messages during metadata validation
* Fixed a typo in patch 5 commit description

Changelog from v3:
* Converted the Coresight ETM driver change to a named configuration.
RFC tag has been removed with this change.
* Fixed yaml issues reported by "make dt_binding_check"
* Added names for reserved memory regions 0 and 1
* Added prevalidation checks for metadata processing
* Fixed a regression introduced in RFC v3
- TMC Status register was getting saved wrongly
* Reverted memremap attribute changes from _WB to _WC to match
with the dma map attributes
* Introduced reserved buffer mode specific .sync op.
This fixes a possible crash when reserved buffer mode was used in
normal trace capture, due to unwanted dma maintenance operations.

v4 is posted here:
https://lore.kernel.org/linux-arm-kernel/[email protected]/
Please note, v4 got wrongly tagged as v1 during transitioning to nonRFC
series.

Using Coresight for Kernel panic and Watchdog reset
===================================================
This patch series is about extending Linux coresight driver support to
address kernel panic and watchdog reset scenarios. This would help
coresight users to debug kernel panic and watchdog reset using
coresight trace data.

Coresight trace capture: Kernel panic
-------------------------------------
From the coresight driver point of view, addressing the kernel panic
situation has four main requirements.

a. Support for allocation of trace buffer pages from reserved memory area.
Platform can advertise this using a new device tree property added to
relevant coresight nodes.

b. Support for stopping coresight blocks at the time of panic

c. Saving required metadata in the specified format

d. Support for reading trace data captured at the time of panic

Allocation of trace buffer pages from reserved RAM
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A new optional device tree property "memory-region" is added to the
ETR/ETF device nodes, that would give the base address and size of trace
buffer.

Static allocation of trace buffers would ensure that both IOMMU enabled
and disabled cases are handled. Also, platforms that support persistent
RAM will allow users to read trace data in the subsequent boot without
booting the crashdump kernel.

Note:
For ETR sink devices, this reserved region will be used for both trace
capture and trace data retrieval.
For ETF sink devices, internal SRAM would be used for trace capture,
and they would be synced to reserved region for retrieval.

Note: Patches 1 & 2 adds support for this.

Disabling coresight blocks at the time of panic
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to avoid the situation of losing relevant trace data after a
kernel panic, it would be desirable to stop the coresight blocks at the
time of panic.

This can be achieved by configuring the comparator, CTI and sink
devices as below,

Comparator(triggers on kernel panic) --->External out --->CTI --
|
ETR/ETF stop <------External In <--------------
Note:

* Patch 6 provides the necessary ETR configuration.
* Patch 7 provides the necessary ETM configuration.

Saving metadata at the time of kernel panic
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Coresight metadata involves all additional data that are required for a
successful trace decode in addition to the trace data. This involves
ETR/ETF, ETE register snapshot etc.

A new optional device property "memory-region" is added to
the ETR/ETF/ETE device nodes for this.

Note: Patches 3 & 4 adds support for this.

Reading trace data captured at the time of panic
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Trace data captured at the time of panic, can be read from rebooted kernel
or from crashdump kernel using the below mentioned interface.

Note: Patch 5 adds support for this.

Steps for reading trace data captured in previous boot
++++++++++++++++++++++++++++++++++++++++++++++++++++++
1. cd /sys/bus/coresight/devices/tmc_etrXX/

2. Change to special mode called, read_prevboot.

#echo 1 > read_prevboot

3. Dump trace buffer data to a file,

#dd if=/dev/tmc_etrXX of=~/cstrace.bin

4. Reset back to normal mode

#echo 0 > read_prevboot

General flow of trace capture and decode incase of kernel panic
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1. Enable source and sink on all the cores using the sysfs interface.
ETR sink will have trace buffers allocated from reserved memory,
by selecting "resrv" buffer mode from sysfs.

2. Run relevant tests.

3. On a kernel panic, all coresight blocks are disabled, necessary
metadata is synced by kernel panic handler.

System would eventually reboot or boot a crashdump kernel.

4. For platforms that supports crashdump kernel, raw trace data can be
dumped using the coresight sysfs interface from the crashdump kernel
itself. Persistent RAM is not a requirement in this case.

5. For platforms that supports persistent RAM, trace data can be dumped
using the coresight sysfs interface in the subsequent Linux boot.
Crashdump kernel is not a requirement in this case. Persistent RAM
ensures that trace data is intact across reboot.

Coresight trace capture: Watchdog reset
---------------------------------------
The main difference between addressing the watchdog reset and kernel panic
case are below,

a. Saving coresight metadata need to be taken care by the
SCP(system control processor) firmware in the specified format,
instead of kernel.

b. Reserved memory region given by firmware for trace buffer and metadata
has to be in persistent RAM.
Note: This is a requirement for watchdog reset case but optional
in kernel panic case.

Watchdog reset can be supported only on platforms that meet the above
two requirements.

Testing Kernel panic on Linux 6.7
---------------------------------
1. Enable the preloaded ETM configuration

#mount -t configfs configs /config
#panic_addr=`cat /proc/kallsyms | grep "\<panic\>" | awk '{print $1}'`
#cd /config/cs-syscfg/features/gen_etrig/params
#echo "0x$panic_addr" > address/value
#echo 1 > /config/cs-syscfg/configurations/panicstop/enable

2. Configure CTI using sysfs interface

#./cti_setup.sh

#cat cti_setup.sh

cd /sys/bus/coresight/devices/

ap_cti_config () {
#ETM trig out[0] trigger to Channel 0
echo 0 4 > channels/trigin_attach
}

etf_cti_config () {
#ETF Flush in trigger from Channel 0
echo 0 1 > channels/trigout_attach
echo 1 > channels/trig_filter_enable
}

etr_cti_config () {
#ETR Flush in from Channel 0
echo 0 1 > channels/trigout_attach
echo 1 > channels/trig_filter_enable
}

ctidevs=`find . -name "cti*"`

for i in $ctidevs
do
cd $i

connection=`find . -name "ete*"`
if [ ! -z "$connection" ]
then
echo "AP CTI config for $i"
ap_cti_config
fi

connection=`find . -name "tmc_etf*"`
if [ ! -z "$connection" ]
then
echo "ETF CTI config for $i"
etf_cti_config
fi

connection=`find . -name "tmc_etr*"`
if [ ! -z "$connection" ]
then
echo "ETR CTI config for $i"
etr_cti_config
fi

cd ..
done

Note: CTI connections are SOC specific and hence the above script is
added just for reference.

3. Choose reserved buffer mode for ETR buffer
#echo "resrv" > /sys/bus/coresight/devices/tmc_etr0/buf_mode_preferred

4. Start Coresight tracing on cores 1 and 2 using sysfs interface

5. Run some application on core 1
#taskset -c 1 dd if=/dev/urandom of=/dev/null &

6. Invoke kernel panic on core 2
#echo 1 > /proc/sys/kernel/panic
#taskset -c 2 echo c > /proc/sysrq-trigger

7. From rebooted kernel or crashdump kernel, enable previous boot mode

#echo 1 > /sys/bus/coresight/devices/tmc_etr0/read_prevboot

Note: For crashdump kernel option, please make sure "crash_kexec_post_notifiers" is
added to the kernel bootargs.

8. Read trace data
#dd if=/dev/tmc_etr0 of=/trace/cstrace.bin

9. Run opencsd decoder tools/scripts to generate the instruction trace.

Sample Core 1 instruction trace dump:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A etm4_enable_hw: ffff800008ae1dd4
CONTEXT EL2 etm4_enable_hw: ffff800008ae1dd4
I etm4_enable_hw: ffff800008ae1dd4:
d503201f nop
I etm4_enable_hw: ffff800008ae1dd8:
d503201f nop
I etm4_enable_hw: ffff800008ae1ddc:
d503201f nop
I etm4_enable_hw: ffff800008ae1de0:
d503201f nop
I etm4_enable_hw: ffff800008ae1de4:
d503201f nop
I etm4_enable_hw: ffff800008ae1de8:
d503233f paciasp
I etm4_enable_hw: ffff800008ae1dec:
a9be7bfd stp x29, x30, [sp, #-32]!
I etm4_enable_hw: ffff800008ae1df0:
910003fd mov x29, sp
I etm4_enable_hw: ffff800008ae1df4:
a90153f3 stp x19, x20, [sp, #16]
I etm4_enable_hw: ffff800008ae1df8:
2a0003f4 mov w20, w0
I etm4_enable_hw: ffff800008ae1dfc:
900085b3 adrp x19, ffff800009b95000 <reserved_mem+0xc48>
I etm4_enable_hw: ffff800008ae1e00:
910f4273 add x19, x19, #0x3d0
I etm4_enable_hw: ffff800008ae1e04:
f8747a60 ldr x0, [x19, x20, lsl #3]
E etm4_enable_hw: ffff800008ae1e08:
b4000140 cbz x0, ffff800008ae1e30 <etm4_starting_cpu+0x50>
I 149.039572921 etm4_enable_hw: ffff800008ae1e30:
a94153f3 ldp x19, x20, [sp, #16]
I 149.039572921 etm4_enable_hw: ffff800008ae1e34:
52800000 mov w0, #0x0 // #0
I 149.039572921 etm4_enable_hw: ffff800008ae1e38:
a8c27bfd ldp x29, x30, [sp], #32

..snip

149.052324811 chacha_block_generic: ffff800008642d80:
9100a3e0 add x0,
I 149.052324811 chacha_block_generic: ffff800008642d84:
b86178a2 ldr w2, [x5, x1, lsl #2]
I 149.052324811 chacha_block_generic: ffff800008642d88:
8b010803 add x3, x0, x1, lsl #2
I 149.052324811 chacha_block_generic: ffff800008642d8c:
b85fc063 ldur w3, [x3, #-4]
I 149.052324811 chacha_block_generic: ffff800008642d90:
0b030042 add w2, w2, w3
I 149.052324811 chacha_block_generic: ffff800008642d94:
b8217882 str w2, [x4, x1, lsl #2]
I 149.052324811 chacha_block_generic: ffff800008642d98:
91000421 add x1, x1, #0x1
I 149.052324811 chacha_block_generic: ffff800008642d9c:
f100443f cmp x1, #0x11


Sample Core 2 instruction trace dump(kernel panic triggered core):
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
A etm4_enable_hw: ffff800008ae1dd4
CONTEXT EL2 etm4_enable_hw: ffff800008ae1dd4
I etm4_enable_hw: ffff800008ae1dd4:
d503201f nop
I etm4_enable_hw: ffff800008ae1dd8:
d503201f nop
I etm4_enable_hw: ffff800008ae1ddc:
d503201f nop
I etm4_enable_hw: ffff800008ae1de0:
d503201f nop
I etm4_enable_hw: ffff800008ae1de4:
d503201f nop
I etm4_enable_hw: ffff800008ae1de8:
d503233f paciasp
I etm4_enable_hw: ffff800008ae1dec:
a9be7bfd stp x29, x30, [sp, #-32]!
I etm4_enable_hw: ffff800008ae1df0:
910003fd mov x29, sp
I etm4_enable_hw: ffff800008ae1df4:
a90153f3 stp x19, x20, [sp, #16]
I etm4_enable_hw: ffff800008ae1df8:
2a0003f4 mov w20, w0
I etm4_enable_hw: ffff800008ae1dfc:
900085b3 adrp x19, ffff800009b95000 <reserved_mem+0xc48>
I etm4_enable_hw: ffff800008ae1e00:
910f4273 add x19, x19, #0x3d0
I etm4_enable_hw: ffff800008ae1e04:
f8747a60 ldr x0, [x19, x20, lsl #3]
E etm4_enable_hw: ffff800008ae1e08:
b4000140 cbz x0, ffff800008ae1e30 <etm4_starting_cpu+0x50>
I 149.046243445 etm4_enable_hw: ffff800008ae1e30:
a94153f3 ldp x19, x20, [sp, #16]
I 149.046243445 etm4_enable_hw: ffff800008ae1e34:
52800000 mov w0, #0x0 // #0
I 149.046243445 etm4_enable_hw: ffff800008ae1e38:
a8c27bfd ldp x29, x30, [sp], #32
I 149.046243445 etm4_enable_hw: ffff800008ae1e3c:
d50323bf autiasp
E 149.046243445 etm4_enable_hw: ffff800008ae1e40:
d65f03c0 ret
A ete_sysreg_write: ffff800008adfa18

..snip

I 149.05422547 panic: ffff800008096300:
a90363f7 stp x23, x24, [sp, #48]
I 149.05422547 panic: ffff800008096304:
6b00003f cmp w1, w0
I 149.05422547 panic: ffff800008096308:
3a411804 ccmn w0, #0x1, #0x4, ne // ne = any
N 149.05422547 panic: ffff80000809630c:
540001e0 b.eq ffff800008096348 <panic+0xe0> // b.none
I 149.05422547 panic: ffff800008096310:
f90023f9 str x25, [sp, #64]
E 149.05422547 panic: ffff800008096314:
97fe44ef bl ffff8000080276d0 <panic_smp_self_stop>
A panic: ffff80000809634c
I 149.05422547 panic: ffff80000809634c:
910102d5 add x21, x22, #0x40
I 149.05422547 panic: ffff800008096350:
52800020 mov w0, #0x1 // #1
E 149.05422547 panic: ffff800008096354:
94166b8b bl ffff800008631180 <bust_spinlocks>
N 149.054225518 bust_spinlocks: ffff800008631180:
340000c0 cbz w0, ffff800008631198 <bust_spinlocks+0x18>
I 149.054225518 bust_spinlocks: ffff800008631184:
f000a321 adrp x1, ffff800009a98000 <pbufs.0+0xbb8>
I 149.054225518 bust_spinlocks: ffff800008631188:
b9405c20 ldr w0, [x1, #92]
I 149.054225518 bust_spinlocks: ffff80000863118c:
11000400 add w0, w0, #0x1
I 149.054225518 bust_spinlocks: ffff800008631190:
b9005c20 str w0, [x1, #92]
E 149.054225518 bust_spinlocks: ffff800008631194:
d65f03c0 ret
A panic: ffff800008096358


TODO
----
* Explore changing CTI sysfs script to system configuration manager profile
* Explore Perf based trace capture and decode

Links:
1. https://lore.kernel.org/linux-arm-kernel/[email protected]/



Linu Cherian (7):
dt-bindings: arm: coresight-tmc: Add "memory-region" property
coresight: tmc-etr: Add support to use reserved trace memory
coresight: core: Add provision for panic callbacks
coresight: tmc: Enable panic sync handling
coresight: tmc: Add support for reading tracedata from previous boot
coresight: tmc: Stop trace capture on FlIn
coresight: config: Add preloaded configuration

.../bindings/arm/arm,coresight-tmc.yaml | 25 ++
drivers/hwtracing/coresight/Makefile | 2 +-
.../coresight/coresight-cfg-preload.c | 2 +
.../coresight/coresight-cfg-preload.h | 2 +
.../hwtracing/coresight/coresight-cfg-pstop.c | 83 +++++
drivers/hwtracing/coresight/coresight-core.c | 32 ++
.../coresight/coresight-etm4x-core.c | 1 +
.../hwtracing/coresight/coresight-tmc-core.c | 173 +++++++++-
.../hwtracing/coresight/coresight-tmc-etf.c | 146 ++++++++-
.../hwtracing/coresight/coresight-tmc-etr.c | 309 +++++++++++++++++-
drivers/hwtracing/coresight/coresight-tmc.h | 76 +++++
include/linux/coresight.h | 25 ++
12 files changed, 870 insertions(+), 6 deletions(-)
create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c

--
2.34.1



2024-01-05 05:59:41

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 2/7] coresight: tmc-etr: Add support to use reserved trace memory

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 <[email protected]>
Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* No changes

.../hwtracing/coresight/coresight-tmc-core.c | 51 +++++++++++
.../hwtracing/coresight/coresight-tmc-etr.c | 87 ++++++++++++++++++-
drivers/hwtracing/coresight/coresight-tmc.h | 27 ++++++
3 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 7ec5365e2b64..224b969d7d90 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,54 @@ 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");
+ goto out;
+ }
+
+ 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");
+ goto out;
+ }
+
+ 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");
+ rc = PTR_ERR(drvdata->crash_tbuf.vaddr);
+ goto out;
+ }
+
+ drvdata->crash_tbuf.paddr = res.start;
+ drvdata->crash_tbuf.size = resource_size(&res);
+
+out:
+ return;
+}
+
/* 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 +531,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 af02ba5d5f15..7fd8c4afef74 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,74 @@ 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,
+ etr_buf->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;
+ 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 +869,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 +895,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 +914,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 +946,19 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
if (!etr_buf)
return ERR_PTR(-ENOMEM);

- etr_buf->size = size;
+ /* Overiride the buffer size here for reserved mode */
+ etr_buf->size = (drvdata->etr_mode == ETR_MODE_RESRV) ?
+ drvdata->crash_tbuf.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);

+ /* Fallback mechanism is not valid for reserved mode */
+ if (rc && (drvdata->etr_mode == ETR_MODE_RESRV))
+ goto done;
+
/*
* 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,
@@ -902,6 +980,7 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
if (rc && buf_hw.has_catu)
rc = tmc_etr_mode_alloc_buf(ETR_MODE_CATU, drvdata,
etr_buf, node, pages);
+done:
if (rc) {
kfree(etr_buf);
return ERR_PTR(rc);
@@ -1829,6 +1908,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",
};

@@ -1847,6 +1927,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;
}
@@ -1874,6 +1957,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 8dcb426ac3e7..10dba9f7d76a 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.
@@ -188,6 +200,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;
@@ -213,6 +229,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 {
@@ -330,6 +347,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);
--
2.34.1


2024-01-05 06:00:04

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 3/7] coresight: core: Add provision for panic callbacks

Panic callback handlers allows coresight device drivers to sync
relevant trace data and trace metadata to reserved memory
regions so that they can be retrieved later in the subsequent
boot or in the crashdump kernel.

Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* No changes


drivers/hwtracing/coresight/coresight-core.c | 32 ++++++++++++++++++++
include/linux/coresight.h | 12 ++++++++
2 files changed, 44 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index 9fabe00a40d6..dfa695c103de 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -20,6 +20,7 @@
#include <linux/property.h>
#include <linux/delay.h>
#include <linux/pm_runtime.h>
+#include <linux/panic_notifier.h>

#include "coresight-etm-perf.h"
#include "coresight-priv.h"
@@ -1800,6 +1801,31 @@ struct bus_type coresight_bustype = {
.name = "coresight",
};

+static int coresight_panic_sync(struct device *dev, void *data)
+{
+
+ struct coresight_device *csdev = container_of(dev, struct coresight_device, dev);
+
+ /* Run through panic sync handlers for all enabled devices */
+ if (csdev->enable && panic_ops(csdev))
+ panic_ops(csdev)->sync(csdev);
+
+ return 0;
+}
+
+static int coresight_panic_cb(struct notifier_block *self,
+ unsigned long v, void *p)
+{
+ bus_for_each_dev(&coresight_bustype, NULL, NULL,
+ coresight_panic_sync);
+
+ return 0;
+}
+
+static struct notifier_block coresight_notifier = {
+ .notifier_call = coresight_panic_cb,
+};
+
static int __init coresight_init(void)
{
int ret;
@@ -1812,6 +1838,10 @@ static int __init coresight_init(void)
if (ret)
goto exit_bus_unregister;

+ /* Register function to be called for panic */
+ ret = atomic_notifier_chain_register(&panic_notifier_list,
+ &coresight_notifier);
+
/* initialise the coresight syscfg API */
ret = cscfg_init();
if (!ret)
@@ -1826,6 +1856,8 @@ static int __init coresight_init(void)
static void __exit coresight_exit(void)
{
cscfg_exit();
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &coresight_notifier);
etm_perf_exit();
bus_unregister(&coresight_bustype);
}
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index a269fffaf991..4fd518738958 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -301,6 +301,7 @@ enum cs_mode {
#define link_ops(csdev) csdev->ops->link_ops
#define helper_ops(csdev) csdev->ops->helper_ops
#define ect_ops(csdev) csdev->ops->ect_ops
+#define panic_ops(csdev) csdev->ops->panic_ops

/**
* struct coresight_ops_sink - basic operations for a sink
@@ -370,11 +371,22 @@ struct coresight_ops_helper {
int (*disable)(struct coresight_device *csdev, void *data);
};

+
+/**
+ * struct coresight_ops_panic - Generic device ops for panic handing
+ *
+ * @sync : Sync the device register state/trace data
+ */
+struct coresight_ops_panic {
+ int (*sync)(struct coresight_device *csdev);
+};
+
struct coresight_ops {
const struct coresight_ops_sink *sink_ops;
const struct coresight_ops_link *link_ops;
const struct coresight_ops_source *source_ops;
const struct coresight_ops_helper *helper_ops;
+ const struct coresight_ops_panic *panic_ops;
};

#if IS_ENABLED(CONFIG_CORESIGHT)
--
2.34.1


2024-01-05 06:00:25

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

memory-region 0: Reserved trace buffer memory

TMC ETR: When available, use this reserved memory region for
trace data capture. Same region is used for trace data
retention after a panic or watchdog reset.

TMC ETF: When available, use this reserved memory region for
trace data retention synced from internal SRAM after a panic or
watchdog reset.

memory-region 1: Reserved meta data memory

TMC ETR, ETF: When available, use this memory for register
snapshot retention synced from hardware registers after a panic
or watchdog reset.

Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* Added Reviewed-by

.../bindings/arm/arm,coresight-tmc.yaml | 25 +++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
index cb8dceaca70e..6d266ac0016b 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
@@ -101,6 +101,28 @@ properties:
and ETF configurations.
$ref: /schemas/graph.yaml#/properties/port

+ memory-region:
+ items:
+ - description: Reserved trace buffer memory for ETR and ETF sinks.
+ For ETR, this reserved memory region is used for trace data capture.
+ Same region is used for trace data retention as well after a panic
+ or watchdog reset.
+ This reserved memory region is used as trace buffer or used for trace
+ data retention only if specifically selected by the user in sysfs
+ interface.
+ The default memory usage models for ETR in sysfs/perf modes are
+ otherwise unaltered.
+
+ For ETF, this reserved memory region is used by default for
+ retention of trace data synced from internal SRAM after a panic
+ or watchdog reset.
+ - description: Reserved meta data memory. Used for ETR and ETF sinks
+ for storing metadata.
+ memory-region-names:
+ items:
+ - const: tracedata
+ - const: metadata
+
required:
- compatible
- reg
@@ -115,6 +137,9 @@ examples:
etr@20070000 {
compatible = "arm,coresight-tmc", "arm,primecell";
reg = <0x20070000 0x1000>;
+ memory-region = <&etr_trace_mem_reserved>,
+ <&etr_mdata_mem_reserved>;
+ memory-region-names = "tracedata", "metadata";

clocks = <&oscclk6a>;
clock-names = "apb_pclk";
--
2.34.1


2024-01-05 06:00:30

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 4/7] coresight: tmc: Enable panic sync handling

- Get reserved region from device tree node for metadata
- Define metadata format for TMC
- Add TMC ETR panic sync handler that syncs register snapshot
to metadata region
- Add TMC ETF panic sync handler that syncs register snapshot
to metadata region and internal SRAM to reserved trace buffer
region.

Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* No changes

.../hwtracing/coresight/coresight-tmc-core.c | 26 ++++++++
.../hwtracing/coresight/coresight-tmc-etf.c | 65 +++++++++++++++++++
.../hwtracing/coresight/coresight-tmc-etr.c | 60 +++++++++++++++++
drivers/hwtracing/coresight/coresight-tmc.h | 42 +++++++++++-
4 files changed, 192 insertions(+), 1 deletion(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 224b969d7d90..4d7530d6ac91 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -415,6 +415,32 @@ static void tmc_get_reserved_region(struct device *parent)
drvdata->crash_tbuf.paddr = res.start;
drvdata->crash_tbuf.size = resource_size(&res);

+ /* Metadata region */
+ node = tmc_get_region_byname(parent->of_node, "metadata");
+ if (IS_ERR_OR_NULL(node)) {
+ dev_dbg(parent, "No metadata memory-region specified\n");
+ goto out;
+ }
+
+ rc = of_address_to_resource(node, 0, &res);
+ of_node_put(node);
+ if (rc || res.start == 0 || resource_size(&res) == 0) {
+ dev_err(parent, "Metadata memory is invalid\n");
+ goto out;
+ }
+
+ drvdata->crash_mdata.vaddr = memremap(res.start,
+ resource_size(&res),
+ MEMREMAP_WC);
+ if (IS_ERR_OR_NULL(drvdata->crash_mdata.vaddr)) {
+ dev_err(parent, "Metadata memory mapping failed\n");
+ rc = PTR_ERR(drvdata->crash_mdata.vaddr);
+ goto out;
+ }
+
+ drvdata->crash_mdata.paddr = res.start;
+ drvdata->crash_mdata.size = resource_size(&res);
+
out:
return;
}
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 7406b65e2cdd..b53cef7628ff 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -588,6 +588,66 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
return to_read;
}

+static int tmc_panic_sync_etf(struct coresight_device *csdev)
+{
+ u32 val;
+ struct csdev_access *csa;
+ struct tmc_crash_metadata *mdata;
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ csa = &drvdata->csdev->access;
+
+ /* Make sure we have valid reserved memory */
+ if (!is_tmc_reserved_region_valid(csdev->dev.parent))
+ return 0;
+
+ mdata = (struct tmc_crash_metadata *)drvdata->crash_mdata.vaddr;
+ mdata->valid = false;
+
+ CS_UNLOCK(drvdata->base);
+
+ /* Proceed only if ETF is enabled or configured as sink */
+ val = readl(drvdata->base + TMC_CTL);
+ if (!(val & TMC_CTL_CAPT_EN))
+ goto out;
+
+ val = readl(drvdata->base + TMC_MODE);
+ if (val != TMC_MODE_CIRCULAR_BUFFER)
+ goto out;
+
+ tmc_flush_and_stop(drvdata);
+
+ /* Sync registers from hardware to metadata region */
+ mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS);
+ mdata->trc_paddr = drvdata->crash_tbuf.paddr;
+
+ /* Sync Internal SRAM to reserved trace buffer region */
+ tmc_etb_dump_hw(drvdata);
+ memcpy(drvdata->crash_tbuf.vaddr, drvdata->buf, drvdata->len);
+ /* Store as per RSZ register convention */
+ mdata->size = drvdata->len >> 2;
+
+ /*
+ * Make sure all previous writes are completed,
+ * before we mark valid
+ */
+ dsb(sy);
+ mdata->valid = true;
+ /*
+ * Below order need to maintained, since crc of metadata
+ * is dependent on first
+ */
+ mdata->crc32_tdata = find_crash_tracedata_crc(drvdata, mdata);
+ mdata->crc32_mdata = find_crash_metadata_crc(mdata);
+
+ tmc_disable_hw(drvdata);
+
+ dev_info(&csdev->dev, "%s: success\n", __func__);
+out:
+ CS_UNLOCK(drvdata->base);
+ return 0;
+}
+
static const struct coresight_ops_sink tmc_etf_sink_ops = {
.enable = tmc_enable_etf_sink,
.disable = tmc_disable_etf_sink,
@@ -601,6 +661,10 @@ static const struct coresight_ops_link tmc_etf_link_ops = {
.disable = tmc_disable_etf_link,
};

+static const struct coresight_ops_panic tmc_etf_sync_ops = {
+ .sync = tmc_panic_sync_etf,
+};
+
const struct coresight_ops tmc_etb_cs_ops = {
.sink_ops = &tmc_etf_sink_ops,
};
@@ -608,6 +672,7 @@ const struct coresight_ops tmc_etb_cs_ops = {
const struct coresight_ops tmc_etf_cs_ops = {
.sink_ops = &tmc_etf_sink_ops,
.link_ops = &tmc_etf_link_ops,
+ .panic_ops = &tmc_etf_sync_ops,
};

int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 7fd8c4afef74..23eb35f50571 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1818,6 +1818,61 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
return 0;
}

+static int tmc_panic_sync_etr(struct coresight_device *csdev)
+{
+ u32 val;
+ struct csdev_access *csa;
+ struct tmc_crash_metadata *mdata;
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ csa = &drvdata->csdev->access;
+
+ /* Being in RESRV mode implies valid reserved memory as well */
+ if (drvdata->etr_mode != ETR_MODE_RESRV)
+ return 0;
+
+ mdata = (struct tmc_crash_metadata *)drvdata->crash_mdata.vaddr;
+ mdata->valid = false;
+
+ CS_UNLOCK(drvdata->base);
+
+ /* Proceed only if ETR is enabled */
+ val = readl(drvdata->base + TMC_CTL);
+ if (!(val & TMC_CTL_CAPT_EN))
+ goto out;
+
+ tmc_flush_and_stop(drvdata);
+
+ /* Sync registers from hardware to metadata region */
+ mdata->size = csdev_access_relaxed_read32(csa, TMC_RSZ);
+ mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS);
+ mdata->rrp = tmc_read_rrp(drvdata);
+ mdata->rwp = tmc_read_rwp(drvdata);
+ mdata->dba = tmc_read_dba(drvdata);
+ mdata->trc_paddr = drvdata->crash_tbuf.paddr;
+
+ /*
+ * Make sure all previous writes are completed,
+ * before we mark valid
+ */
+ dsb(sy);
+ mdata->valid = true;
+ /*
+ * Below order need to maintained, since crc of metadata
+ * is dependent on first
+ */
+ mdata->crc32_tdata = find_crash_tracedata_crc(drvdata, mdata);
+ mdata->crc32_mdata = find_crash_metadata_crc(mdata);
+
+ tmc_disable_hw(drvdata);
+
+ dev_info(&csdev->dev, "%s: success\n", __func__);
+out:
+ CS_UNLOCK(drvdata->base);
+
+ return 0;
+}
+
static const struct coresight_ops_sink tmc_etr_sink_ops = {
.enable = tmc_enable_etr_sink,
.disable = tmc_disable_etr_sink,
@@ -1826,8 +1881,13 @@ static const struct coresight_ops_sink tmc_etr_sink_ops = {
.free_buffer = tmc_free_etr_buffer,
};

+static const struct coresight_ops_panic tmc_etr_sync_ops = {
+ .sync = tmc_panic_sync_etr,
+};
+
const struct coresight_ops tmc_etr_cs_ops = {
.sink_ops = &tmc_etr_sink_ops,
+ .panic_ops = &tmc_etr_sync_ops,
};

int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 10dba9f7d76a..6f13caae6495 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -12,6 +12,7 @@
#include <linux/miscdevice.h>
#include <linux/mutex.h>
#include <linux/refcount.h>
+#include <linux/crc32.h>

#define TMC_RSZ 0x004
#define TMC_STS 0x00c
@@ -131,6 +132,21 @@ enum tmc_mem_intf_width {
#define CORESIGHT_SOC_600_ETR_CAPS \
(TMC_ETR_SAVE_RESTORE | TMC_ETR_AXI_ARCACHE)

+/* TMC metadata region for ETR and ETF configurations */
+struct tmc_crash_metadata {
+ uint32_t crc32_mdata; /* crc of metadata */
+ uint32_t crc32_tdata; /* crc of tracedata */
+ uint32_t valid; /* Indicate if this ETF/ETR was enabled */
+ uint32_t size; /* Ram Size register */
+ uint32_t sts; /* Status register */
+ uint32_t reserved32[3];
+ uint64_t rrp; /* Ram Read pointer register */
+ uint64_t rwp; /* Ram Write pointer register */
+ uint64_t dba; /* Data buffer address register */
+ uint64_t trc_paddr; /* Phys address of trace buffer */
+ uint64_t reserved64[3];
+};
+
enum etr_mode {
ETR_MODE_FLAT, /* Uses contiguous flat buffer */
ETR_MODE_ETR_SG, /* Uses in-built TMC ETR SG mechanism */
@@ -204,6 +220,8 @@ struct tmc_resrv_buf {
* retention (after crash) only when ETR_MODE_RESRV buffer
* mode is enabled. Used by ETF for trace data retention
* (after crash) by default.
+ * @crash_mdata: Reserved memory for storing tmc crash metadata.
+ * Used by ETR/ETF.
*/
struct tmc_drvdata {
void __iomem *base;
@@ -230,6 +248,7 @@ struct tmc_drvdata {
struct etr_buf *sysfs_buf;
struct etr_buf *perf_buf;
struct tmc_resrv_buf crash_tbuf;
+ struct tmc_resrv_buf crash_mdata;
};

struct etr_buf_operations {
@@ -352,11 +371,32 @@ 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)
+ drvdata->crash_tbuf.size &&
+ drvdata->crash_mdata.paddr &&
+ drvdata->crash_mdata.size)
return true;
return false;
}

+static inline uint32_t find_crash_metadata_crc(struct tmc_crash_metadata *md)
+{
+ unsigned long crc_size;
+
+ crc_size = sizeof(struct tmc_crash_metadata) -
+ offsetof(struct tmc_crash_metadata, crc32_tdata);
+ return crc32_le(0, (void *)&md->crc32_tdata, crc_size);
+}
+
+static inline uint32_t find_crash_tracedata_crc(struct tmc_drvdata *drvdata,
+ struct tmc_crash_metadata *md)
+{
+ unsigned long crc_size;
+
+ /* Take CRC of configured buffer size to keep it simple */
+ crc_size = md->size << 2;
+ return crc32_le(0, (void *)drvdata->crash_tbuf.vaddr, crc_size);
+}
+
struct coresight_device *tmc_etr_get_catu_device(struct tmc_drvdata *drvdata);

void tmc_etr_set_catu_ops(const struct etr_buf_operations *catu);
--
2.34.1


2024-01-05 06:00:48

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 5/7] coresight: tmc: Add support for reading tracedata from previous boot

* Introduce a new mode CS_MODE_READ_PREVBOOT for reading tracedata
captured in previous boot.

* Add special handlers for preparing ETR/ETF for this special mode

* User can read the trace data as below

For example, for reading trace data from tmc_etf sink

1. cd /sys/bus/coresight/devices/tmc_etfXX/

2. Change mode to READ_PREVBOOT

#echo 1 > read_prevboot

3. Dump trace buffer data to a file,

#dd if=/dev/tmc_etfXX of=~/cstrace.bin

4. Reset back to normal mode

#echo 0 > read_prevboot

Signed-off-by: Anil Kumar Reddy <[email protected]>
Signed-off-by: Tanmay Jagdale <[email protected]>
Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* Fixed issues reported by CONFIG_DEBUG_ATOMIC_SLEEP in tmc_etr_setup_prevboot_buf
and tmc_etb_setup_prevboot_buf
- Kmalloc invoked with spinlock taken is now done with GFP_ATOMIC
- Removed the requirement for memremap with spinlock taken. This is by
making a assumption that the trace buffer address used for capturing
trace data during kernel crash/hang would be same as that used
for trace data retrieval in the subsequent boot. Added additional
verification to ensure this.

* Fixed a memory leak in tmc_read_unprepare_etr in READ_PREVBOOT mode


.../coresight/coresight-etm4x-core.c | 1 +
.../hwtracing/coresight/coresight-tmc-core.c | 96 ++++++++++-
.../hwtracing/coresight/coresight-tmc-etf.c | 73 +++++++++
.../hwtracing/coresight/coresight-tmc-etr.c | 154 +++++++++++++++++-
drivers/hwtracing/coresight/coresight-tmc.h | 6 +
include/linux/coresight.h | 13 ++
6 files changed, 341 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 34aee59dd147..c26b5f72a494 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1010,6 +1010,7 @@ static void etm4_disable(struct coresight_device *csdev,

switch (mode) {
case CS_MODE_DISABLED:
+ case CS_MODE_READ_PREVBOOT:
break;
case CS_MODE_SYSFS:
etm4_disable_sysfs(csdev);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 4d7530d6ac91..e6525c3dcf66 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -103,6 +103,60 @@ u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata)
return mask;
}

+int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata)
+{
+ int ret = 0;
+ struct tmc_crash_metadata *mdata;
+ struct coresight_device *csdev = drvdata->csdev;
+
+ if (!drvdata->crash_mdata.vaddr) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ mdata = drvdata->crash_mdata.vaddr;
+ /* Check data integrity of metadata */
+ if (mdata->crc32_mdata != find_crash_metadata_crc(mdata)) {
+ dev_dbg(&drvdata->csdev->dev,
+ "CRC mismatch in tmc crash metadata\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ /* Check data integrity of tracedata */
+ if (mdata->crc32_tdata != find_crash_tracedata_crc(drvdata, mdata)) {
+ dev_dbg(&drvdata->csdev->dev,
+ "CRC mismatch in tmc crash tracedata\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ /* Check for valid metadata */
+ if (!mdata->valid) {
+ dev_dbg(&drvdata->csdev->dev,
+ "Data invalid in tmc crash metadata\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Sink specific prevboot mode preparation */
+ ret = prevboot_ops(csdev)->prepare(csdev);
+ if (ret)
+ goto out;
+
+ if (mdata->sts & 0x1)
+ coresight_insert_barrier_packet(drvdata->buf);
+
+out:
+ return ret;
+}
+
+int tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata)
+{
+ struct coresight_device *csdev = drvdata->csdev;
+
+ /* Sink specific prevboot mode preparation */
+ return prevboot_ops(csdev)->unprepare(csdev);
+}
+
static int tmc_read_prepare(struct tmc_drvdata *drvdata)
{
int ret = 0;
@@ -153,6 +207,10 @@ static int tmc_open(struct inode *inode, struct file *file)
struct tmc_drvdata *drvdata = container_of(file->private_data,
struct tmc_drvdata, miscdev);

+ /* Advertise if we are opening with a special mode */
+ if (drvdata->mode == CS_MODE_READ_PREVBOOT)
+ dev_dbg(&drvdata->csdev->dev, "TMC read mode for previous boot\n");
+
ret = tmc_read_prepare(drvdata);
if (ret)
return ret;
@@ -331,9 +389,44 @@ static ssize_t buffer_size_store(struct device *dev,

static DEVICE_ATTR_RW(buffer_size);

+static ssize_t read_prevboot_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->mode == CS_MODE_READ_PREVBOOT));
+}
+
+static ssize_t read_prevboot_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ int ret;
+ unsigned long val, flags;
+ struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ ret = kstrtoul(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ spin_lock_irqsave(&drvdata->spinlock, flags);
+
+ if (val && (drvdata->mode == CS_MODE_DISABLED))
+ drvdata->mode = CS_MODE_READ_PREVBOOT;
+ else if (!val && (drvdata->mode == CS_MODE_READ_PREVBOOT))
+ drvdata->mode = CS_MODE_DISABLED;
+
+ spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+ return size;
+}
+
+static DEVICE_ATTR_RW(read_prevboot);
+
static struct attribute *coresight_tmc_attrs[] = {
&dev_attr_trigger_cntr.attr,
&dev_attr_buffer_size.attr,
+ &dev_attr_read_prevboot.attr,
NULL,
};

@@ -635,7 +728,8 @@ static void tmc_shutdown(struct amba_device *adev)

spin_lock_irqsave(&drvdata->spinlock, flags);

- if (drvdata->mode == CS_MODE_DISABLED)
+ if (drvdata->mode == CS_MODE_DISABLED ||
+ drvdata->mode == CS_MODE_READ_PREVBOOT)
goto out;

if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index b53cef7628ff..72c2315f4e23 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -648,6 +648,56 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev)
return 0;
}

+static int tmc_etb_setup_prevboot_buf(struct tmc_drvdata *drvdata)
+{
+ unsigned long size;
+ struct tmc_crash_metadata *mdata;
+ struct device *dev = &drvdata->csdev->dev;
+
+ mdata = drvdata->crash_mdata.vaddr;
+ size = mdata->size << 2;
+
+ /*
+ * Buffer address given by metadata for retrieval of trace data
+ * from previous boot is expected to be same as the reserved
+ * trace buffer memory region provided through DTS
+ */
+ if (is_tmc_reserved_region_valid(dev->parent)
+ && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
+ drvdata->buf = drvdata->crash_tbuf.vaddr;
+ else {
+ dev_dbg(dev, "Trace buffer address of previous boot invalid\n");
+ return -EINVAL;
+ }
+
+ drvdata->len = size;
+ return 0;
+}
+
+static void tmc_etb_free_prevboot_buf(struct tmc_drvdata *drvdata)
+{
+ void *buf = drvdata->buf;
+
+ if (!buf)
+ return;
+ drvdata->buf = NULL;
+}
+
+static int tmc_etb_prepare_prevboot(struct coresight_device *csdev)
+{
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ return tmc_etb_setup_prevboot_buf(drvdata);
+}
+
+static int tmc_etb_unprepare_prevboot(struct coresight_device *csdev)
+{
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ tmc_etb_free_prevboot_buf(drvdata);
+ return 0;
+}
+
static const struct coresight_ops_sink tmc_etf_sink_ops = {
.enable = tmc_enable_etf_sink,
.disable = tmc_disable_etf_sink,
@@ -665,6 +715,11 @@ static const struct coresight_ops_panic tmc_etf_sync_ops = {
.sync = tmc_panic_sync_etf,
};

+static const struct coresight_ops_prevboot tmc_etf_prevboot_ops = {
+ .prepare = tmc_etb_prepare_prevboot,
+ .unprepare = tmc_etb_unprepare_prevboot,
+};
+
const struct coresight_ops tmc_etb_cs_ops = {
.sink_ops = &tmc_etf_sink_ops,
};
@@ -673,6 +728,7 @@ const struct coresight_ops tmc_etf_cs_ops = {
.sink_ops = &tmc_etf_sink_ops,
.link_ops = &tmc_etf_link_ops,
.panic_ops = &tmc_etf_sync_ops,
+ .prevboot_ops = &tmc_etf_prevboot_ops,
};

int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
@@ -693,6 +749,14 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
goto out;
}

+ if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
+ ret = tmc_read_prepare_prevboot(drvdata);
+ if (ret)
+ goto out;
+ else
+ goto mode_valid;
+ }
+
/* Don't interfere if operated from Perf */
if (drvdata->mode == CS_MODE_PERF) {
ret = -EINVAL;
@@ -716,6 +780,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
__tmc_etb_disable_hw(drvdata);
}

+mode_valid:
drvdata->reading = true;
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -735,8 +800,16 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
drvdata->config_type != TMC_CONFIG_TYPE_ETF))
return -EINVAL;

+
spin_lock_irqsave(&drvdata->spinlock, flags);

+ if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
+ tmc_read_unprepare_prevboot(drvdata);
+ drvdata->reading = false;
+ spin_unlock_irqrestore(&drvdata->spinlock, flags);
+ return 0;
+ }
+
/* Re-enable the TMC if need be */
if (drvdata->mode == CS_MODE_SYSFS) {
/* There is no point in reading a TMC in HW FIFO mode */
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 23eb35f50571..ab7521bbb2f5 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1166,7 +1166,12 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
{
s64 offset;
ssize_t actual = len;
- struct etr_buf *etr_buf = drvdata->sysfs_buf;
+ struct etr_buf *etr_buf;
+
+ if (drvdata->mode == CS_MODE_READ_PREVBOOT)
+ etr_buf = drvdata->prevboot_buf;
+ else
+ etr_buf = drvdata->sysfs_buf;

if (pos + actual > etr_buf->len)
actual = etr_buf->len - pos;
@@ -1873,6 +1878,130 @@ static int tmc_panic_sync_etr(struct coresight_device *csdev)
return 0;
}

+static int tmc_etr_setup_prevboot_buf(struct tmc_drvdata *drvdata)
+{
+ int rc = 0;
+ u64 trace_addr;
+ struct etr_buf *etr_buf;
+ struct etr_flat_buf *resrv_buf;
+ struct tmc_crash_metadata *mdata;
+ struct device *dev = &drvdata->csdev->dev;
+
+ mdata = drvdata->crash_mdata.vaddr;
+
+ etr_buf = kzalloc(sizeof(*etr_buf), GFP_ATOMIC);
+ if (!etr_buf) {
+ rc = -ENOMEM;
+ goto out;
+ }
+ etr_buf->size = drvdata->crash_tbuf.size;
+
+ resrv_buf = kzalloc(sizeof(*resrv_buf), GFP_ATOMIC);
+ if (!resrv_buf) {
+ rc = -ENOMEM;
+ goto rmem_err;
+ }
+
+ /*
+ * Buffer address given by metadata for retrieval of trace data
+ * from previous boot is expected to be same as the reserved
+ * trace buffer memory region provided through DTS
+ */
+ if (is_tmc_reserved_region_valid(dev->parent)
+ && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
+ resrv_buf->vaddr = drvdata->crash_tbuf.vaddr;
+ else {
+ dev_dbg(dev, "Trace buffer address of previous boot invalid\n");
+ rc = -EINVAL;
+ goto map_err;
+ }
+
+ resrv_buf->size = etr_buf->size;
+ resrv_buf->dev = &drvdata->csdev->dev;
+ etr_buf->hwaddr = trace_addr;
+ etr_buf->mode = ETR_MODE_RESRV;
+ etr_buf->private = resrv_buf;
+ etr_buf->ops = etr_buf_ops[ETR_MODE_RESRV];
+
+ drvdata->prevboot_buf = etr_buf;
+
+ return 0;
+
+map_err:
+ kfree(resrv_buf);
+
+rmem_err:
+ kfree(etr_buf);
+
+out:
+ return rc;
+}
+
+static int tmc_etr_sync_prevboot_buf(struct tmc_drvdata *drvdata)
+{
+ u32 status;
+ u64 rrp, rwp, dba;
+ struct tmc_crash_metadata *mdata;
+ struct etr_buf *etr_buf = drvdata->prevboot_buf;
+
+ mdata = drvdata->crash_mdata.vaddr;
+
+ rrp = mdata->rrp;
+ rwp = mdata->rwp;
+ dba = mdata->dba;
+ status = mdata->sts;
+
+ etr_buf->full = !!(status & TMC_STS_FULL);
+
+ /* Sync the buffer pointers */
+ etr_buf->offset = rrp - dba;
+ if (etr_buf->full)
+ etr_buf->len = etr_buf->size;
+ else
+ etr_buf->len = rwp - rrp;
+
+ /* Additional sanity checks for validating metadata */
+ if ((etr_buf->offset > etr_buf->size) ||
+ (etr_buf->len > etr_buf->size)) {
+ dev_dbg(&drvdata->csdev->dev,
+ "Offset and length invalid in tmc crash metadata\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void tmc_etr_free_prevboot_buf(struct tmc_drvdata *drvdata)
+{
+ struct etr_buf *etr_buf = drvdata->prevboot_buf;
+
+ if (!etr_buf)
+ return;
+ drvdata->prevboot_buf = NULL;
+}
+
+static int tmc_etr_prepare_prevboot(struct coresight_device *csdev)
+{
+ int ret = 0;
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ ret = tmc_etr_setup_prevboot_buf(drvdata);
+ if (ret)
+ goto out;
+ ret = tmc_etr_sync_prevboot_buf(drvdata);
+
+out:
+ return ret;
+}
+
+static int tmc_etr_unprepare_prevboot(struct coresight_device *csdev)
+{
+ struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ tmc_etr_free_prevboot_buf(drvdata);
+ return 0;
+}
+
static const struct coresight_ops_sink tmc_etr_sink_ops = {
.enable = tmc_enable_etr_sink,
.disable = tmc_disable_etr_sink,
@@ -1885,9 +2014,15 @@ static const struct coresight_ops_panic tmc_etr_sync_ops = {
.sync = tmc_panic_sync_etr,
};

+static const struct coresight_ops_prevboot tmc_etr_prevboot_ops = {
+ .prepare = tmc_etr_prepare_prevboot,
+ .unprepare = tmc_etr_unprepare_prevboot,
+};
+
const struct coresight_ops tmc_etr_cs_ops = {
.sink_ops = &tmc_etr_sink_ops,
.panic_ops = &tmc_etr_sync_ops,
+ .prevboot_ops = &tmc_etr_prevboot_ops,
};

int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
@@ -1899,12 +2034,21 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
return -EINVAL;

+
spin_lock_irqsave(&drvdata->spinlock, flags);
if (drvdata->reading) {
ret = -EBUSY;
goto out;
}

+ if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
+ ret = tmc_read_prepare_prevboot(drvdata);
+ if (ret)
+ goto out;
+ else
+ goto mode_valid;
+ }
+
/*
* We can safely allow reads even if the ETR is operating in PERF mode,
* since the sysfs session is captured in mode specific data.
@@ -1919,6 +2063,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
if (drvdata->mode == CS_MODE_SYSFS)
__tmc_etr_disable_hw(drvdata);

+mode_valid:
drvdata->reading = true;
out:
spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -1937,6 +2082,12 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)

spin_lock_irqsave(&drvdata->spinlock, flags);

+ if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
+ sysfs_buf = drvdata->prevboot_buf;
+ tmc_read_unprepare_prevboot(drvdata);
+ goto out;
+ }
+
/* RE-enable the TMC if need be */
if (drvdata->mode == CS_MODE_SYSFS) {
/*
@@ -1954,6 +2105,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
drvdata->sysfs_buf = NULL;
}

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

diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 6f13caae6495..6e1e910d5ea4 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -216,6 +216,9 @@ struct tmc_resrv_buf {
* @idr_mutex: Access serialisation for idr.
* @sysfs_buf: SYSFS buffer for ETR.
* @perf_buf: PERF buffer for ETR.
+ * @prevboot_buf: Previous boot buffer for ETR. This is a special purpose
+ * buffer that is used only for mapping the trace buffer from
+ * previous boot and not for capturing trace.
* @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
@@ -247,6 +250,7 @@ struct tmc_drvdata {
struct mutex idr_mutex;
struct etr_buf *sysfs_buf;
struct etr_buf *perf_buf;
+ struct etr_buf *prevboot_buf;
struct tmc_resrv_buf crash_tbuf;
struct tmc_resrv_buf crash_mdata;
};
@@ -298,6 +302,8 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
void tmc_enable_hw(struct tmc_drvdata *drvdata);
void tmc_disable_hw(struct tmc_drvdata *drvdata);
u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
+int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata);
+int tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata);

/* ETB/ETF functions */
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4fd518738958..9f84decf1d7a 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -294,6 +294,7 @@ enum cs_mode {
CS_MODE_DISABLED,
CS_MODE_SYSFS,
CS_MODE_PERF,
+ CS_MODE_READ_PREVBOOT, /* Trace data from previous boot */
};

#define source_ops(csdev) csdev->ops->source_ops
@@ -302,6 +303,7 @@ enum cs_mode {
#define helper_ops(csdev) csdev->ops->helper_ops
#define ect_ops(csdev) csdev->ops->ect_ops
#define panic_ops(csdev) csdev->ops->panic_ops
+#define prevboot_ops(csdev) csdev->ops->prevboot_ops

/**
* struct coresight_ops_sink - basic operations for a sink
@@ -381,12 +383,23 @@ struct coresight_ops_panic {
int (*sync)(struct coresight_device *csdev);
};

+/**
+ * struct coresight_ops_prevboot - Generic device ops for prevboot mode
+ *
+ * @prepare : Preparation for prevboot mode
+ */
+struct coresight_ops_prevboot {
+ int (*prepare)(struct coresight_device *csdev);
+ int (*unprepare)(struct coresight_device *csdev);
+};
+
struct coresight_ops {
const struct coresight_ops_sink *sink_ops;
const struct coresight_ops_link *link_ops;
const struct coresight_ops_source *source_ops;
const struct coresight_ops_helper *helper_ops;
const struct coresight_ops_panic *panic_ops;
+ const struct coresight_ops_prevboot *prevboot_ops;
};

#if IS_ENABLED(CONFIG_CORESIGHT)
--
2.34.1


2024-01-05 06:01:02

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

Configure TMC ETR and ETF to flush and stop trace capture
on FlIn event. As a side effect, do manual flush only if
auto flush fails.

Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* No changes

drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 ++++++++--
drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++--
drivers/hwtracing/coresight/coresight-tmc.h | 3 +++
3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 72c2315f4e23..57a9a9300d36 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -34,7 +34,7 @@ static int __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);
writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
- TMC_FFCR_TRIGON_TRIGIN,
+ TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
drvdata->base + TMC_FFCR);

writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
@@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev)
if (val != TMC_MODE_CIRCULAR_BUFFER)
goto out;

- tmc_flush_and_stop(drvdata);
+ val = readl(drvdata->base + TMC_FFSR);
+ /* Do manual flush and stop only if its not auto-stopped */
+ if (!(val & TMC_FFSR_FT_STOPPED)) {
+ dev_info(&csdev->dev,
+ "%s: Triggering manual flush\n", __func__);
+ tmc_flush_and_stop(drvdata);
+ }

/* Sync registers from hardware to metadata region */
mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index ab7521bbb2f5..4b3c7ec7f62b 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1113,7 +1113,7 @@ static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)

writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
- TMC_FFCR_TRIGON_TRIGIN,
+ TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
drvdata->base + TMC_FFCR);
writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
tmc_enable_hw(drvdata);
@@ -1846,7 +1846,13 @@ static int tmc_panic_sync_etr(struct coresight_device *csdev)
if (!(val & TMC_CTL_CAPT_EN))
goto out;

- tmc_flush_and_stop(drvdata);
+ val = readl(drvdata->base + TMC_FFSR);
+ /* Do manual flush and stop only if its not auto-stopped */
+ if (!(val & TMC_FFSR_FT_STOPPED)) {
+ dev_info(&csdev->dev,
+ "%s: Triggering manual flush\n", __func__);
+ tmc_flush_and_stop(drvdata);
+ }

/* Sync registers from hardware to metadata region */
mdata->size = csdev_access_relaxed_read32(csa, TMC_RSZ);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 6e1e910d5ea4..cf9313b302c7 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -77,6 +77,9 @@
#define TMC_AXICTL_AXCACHE_OS (0xf << 2)
#define TMC_AXICTL_ARCACHE_OS (0xf << 16)

+/* TMC_FFSR - 0x300 */
+#define TMC_FFSR_FT_STOPPED BIT(1)
+
/* TMC_FFCR - 0x304 */
#define TMC_FFCR_FLUSHMAN_BIT 6
#define TMC_FFCR_EN_FMT BIT(0)
--
2.34.1


2024-01-05 06:01:24

by Linu Cherian

[permalink] [raw]
Subject: [PATCH v6 7/7] coresight: config: Add preloaded configuration

Add a preloaded configuration for generating
external trigger on address match. This can be
used by CTI and ETR blocks to stop trace capture
on kernel panic.

Kernel address for panic function to be
programmed as below.

$cd /config/cs-syscfg/features/gen_etrig/params
$echo <panic_address> > address/value

Signed-off-by: Linu Cherian <[email protected]>
---
Changelog from v5:
* No changes

drivers/hwtracing/coresight/Makefile | 2 +-
.../coresight/coresight-cfg-preload.c | 2 +
.../coresight/coresight-cfg-preload.h | 2 +
.../hwtracing/coresight/coresight-cfg-pstop.c | 83 +++++++++++++++++++
4 files changed, 88 insertions(+), 1 deletion(-)
create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c

diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 995d3b2c76df..68b15c8d9462 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_CORESIGHT) += coresight.o
coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \
coresight-sysfs.o coresight-syscfg.o coresight-config.o \
- coresight-cfg-preload.o coresight-cfg-afdo.o \
+ coresight-cfg-preload.o coresight-cfg-afdo.o coresight-cfg-pstop.o \
coresight-syscfg-configfs.o coresight-trace-id.o
obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c b/drivers/hwtracing/coresight/coresight-cfg-preload.c
index e237a4edfa09..4980e68483c5 100644
--- a/drivers/hwtracing/coresight/coresight-cfg-preload.c
+++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c
@@ -13,6 +13,7 @@
static struct cscfg_feature_desc *preload_feats[] = {
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
&strobe_etm4x,
+ &gen_etrig_etm4x,
#endif
NULL
};
@@ -20,6 +21,7 @@ static struct cscfg_feature_desc *preload_feats[] = {
static struct cscfg_config_desc *preload_cfgs[] = {
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
&afdo_etm4x,
+ &pstop_etm4x,
#endif
NULL
};
diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.h b/drivers/hwtracing/coresight/coresight-cfg-preload.h
index 21299e175477..291ba530a6a5 100644
--- a/drivers/hwtracing/coresight/coresight-cfg-preload.h
+++ b/drivers/hwtracing/coresight/coresight-cfg-preload.h
@@ -10,4 +10,6 @@
#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
extern struct cscfg_feature_desc strobe_etm4x;
extern struct cscfg_config_desc afdo_etm4x;
+extern struct cscfg_feature_desc gen_etrig_etm4x;
+extern struct cscfg_config_desc pstop_etm4x;
#endif
diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
new file mode 100644
index 000000000000..037d6773fab8
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(C) 2023 Marvell.
+ * Based on coresight-cfg-afdo.c
+ */
+
+#include "coresight-config.h"
+
+/* ETMv4 includes and features */
+#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
+#include "coresight-etm4x-cfg.h"
+
+/* preload configurations and features */
+
+/* preload in features for ETMv4 */
+
+/* panic_stop feature */
+static struct cscfg_parameter_desc gen_etrig_params[] = {
+ {
+ .name = "address",
+ .value = 0x0,
+ },
+};
+
+static struct cscfg_regval_desc gen_etrig_regs[] = {
+ /* resource selector */
+ {
+ .type = CS_CFG_REG_TYPE_RESOURCE,
+ .offset = TRCRSCTLRn(2),
+ .hw_info = ETM4_CFG_RES_SEL,
+ .val32 = 0x40001,
+ },
+ /* single address comparator */
+ {
+ .type = CS_CFG_REG_TYPE_RESOURCE | CS_CFG_REG_TYPE_VAL_64BIT |
+ CS_CFG_REG_TYPE_VAL_PARAM,
+ .offset = TRCACVRn(0),
+ .val32 = 0x0,
+ },
+ {
+ .type = CS_CFG_REG_TYPE_RESOURCE,
+ .offset = TRCACATRn(0),
+ .val64 = 0xf00,
+ },
+ /* Driver external output[0] with comparator out */
+ {
+ .type = CS_CFG_REG_TYPE_RESOURCE,
+ .offset = TRCEVENTCTL0R,
+ .val32 = 0x2,
+ },
+ /* end of regs */
+};
+
+struct cscfg_feature_desc gen_etrig_etm4x = {
+ .name = "gen_etrig",
+ .description = "Generate external trigger on address match\n"
+ "parameter \'address\': address of kernel address\n",
+ .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4,
+ .nr_params = ARRAY_SIZE(gen_etrig_params),
+ .params_desc = gen_etrig_params,
+ .nr_regs = ARRAY_SIZE(gen_etrig_regs),
+ .regs_desc = gen_etrig_regs,
+};
+
+/* create a panic stop configuration */
+
+/* the total number of parameters in used features */
+#define PSTOP_NR_PARAMS ARRAY_SIZE(gen_etrig_params)
+
+static const char *pstop_ref_names[] = {
+ "gen_etrig",
+};
+
+struct cscfg_config_desc pstop_etm4x = {
+ .name = "panicstop",
+ .description = "Stop ETM on kernel panic\n",
+ .nr_feat_refs = ARRAY_SIZE(pstop_ref_names),
+ .feat_ref_names = pstop_ref_names,
+ .nr_total_params = PSTOP_NR_PARAMS,
+};
+
+/* end of ETM4x configurations */
+#endif /* IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) */
--
2.34.1


2024-01-05 09:54:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v6 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

On 05/01/2024 06:58, Linu Cherian wrote:
> memory-region 0: Reserved trace buffer memory
>
> TMC ETR: When available, use this reserved memory region for
> trace data capture. Same region is used for trace data
> retention after a panic or watchdog reset.
>
> TMC ETF: When available, use this reserved memory region for
> trace data retention synced from internal SRAM after a panic or
> watchdog reset.
>
> memory-region 1: Reserved meta data memory
>
> TMC ETR, ETF: When available, use this memory for register
> snapshot retention synced from hardware registers after a panic
> or watchdog reset.
>
> Reviewed-by: Rob Herring <[email protected]>

...

> + memory-region:
> + items:
> + - description: Reserved trace buffer memory for ETR and ETF sinks.
> + For ETR, this reserved memory region is used for trace data capture.
> + Same region is used for trace data retention as well after a panic
> + or watchdog reset.
> + This reserved memory region is used as trace buffer or used for trace
> + data retention only if specifically selected by the user in sysfs
> + interface.
> + The default memory usage models for ETR in sysfs/perf modes are
> + otherwise unaltered.
> +
> + For ETF, this reserved memory region is used by default for
> + retention of trace data synced from internal SRAM after a panic
> + or watchdog reset.
> + - description: Reserved meta data memory. Used for ETR and ETF sinks
> + for storing metadata.

If there is going to be new version/resend, add a blank line here. Keep
the Rb tag.


Best regards,
Krzysztof


2024-01-05 14:17:40

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v6 2/7] coresight: tmc-etr: Add support to use reserved trace memory

On 05/01/2024 05:58, 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 <[email protected]>
> Signed-off-by: Linu Cherian <[email protected]>
> ---
> Changelog from v5:
> * No changes
>
> .../hwtracing/coresight/coresight-tmc-core.c | 51 +++++++++++
> .../hwtracing/coresight/coresight-tmc-etr.c | 87 ++++++++++++++++++-
> drivers/hwtracing/coresight/coresight-tmc.h | 27 ++++++
> 3 files changed, 164 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 7ec5365e2b64..224b969d7d90 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,54 @@ 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");
> + goto out;

Given we don't return anything , the goto is pointless, you could return
from here.

> + }
> +
> + 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");
> + goto out;

Same here.

> + }
> +
> + 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");
> + rc = PTR_ERR(drvdata->crash_tbuf.vaddr);

This is pointless, as we never use this.

> + goto out;

return

> + }
> +
> + drvdata->crash_tbuf.paddr = res.start;
> + drvdata->crash_tbuf.size = resource_size(&res);
> +
> +out:
> + return;
> +}
> +
> /* 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 +531,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 af02ba5d5f15..7fd8c4afef74 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,74 @@ 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;

Shouldn't we also ensure the etr_buf->size < crash_tbuf.size ? Or we
could simply allocate the full reserve buf, as we can't use it for
anything else. In any case, the buffer should be bounded by the reserved
space.

> +
> + resrv_buf->daddr = dma_map_resource(real_dev, drvdata->crash_tbuf.paddr,
> + etr_buf->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;
> + 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;

Don't we need to sync the buffer for CPU via dma_sync_single_for_cpu() ?
Otherwise, we could be reading stale data ?

> +}
> +
> +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 +869,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 +895,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 +914,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 +946,19 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> if (!etr_buf)
> return ERR_PTR(-ENOMEM);
>
> - etr_buf->size = size;
> + /* Overiride the buffer size here for reserved mode */

minor nit: "Override"

> + etr_buf->size = (drvdata->etr_mode == ETR_MODE_RESRV) ?
> + drvdata->crash_tbuf.size : size;

Isn't it better to do this from the actual code where we are successful
in allocating the buffer ? That way, it works for the AUTO mode too ?

>
> /* 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);
>
> + /* Fallback mechanism is not valid for reserved mode */
> + if (rc && (drvdata->etr_mode == ETR_MODE_RESRV))
> + goto done;
> +
> /*
> * 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,
> @@ -902,6 +980,7 @@ static struct etr_buf *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> if (rc && buf_hw.has_catu)
> rc = tmc_etr_mode_alloc_buf(ETR_MODE_CATU, drvdata,
> etr_buf, node, pages);
> +done:
> if (rc) {
> kfree(etr_buf);
> return ERR_PTR(rc);
> @@ -1829,6 +1908,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",
> };
>
> @@ -1847,6 +1927,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;
> }
> @@ -1874,6 +1957,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 8dcb426ac3e7..10dba9f7d76a 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.
> @@ -188,6 +200,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.

minor nit: Please align the comment like the other fields.

Suzuki

> */
> struct tmc_drvdata {
> void __iomem *base;
> @@ -213,6 +229,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 {
> @@ -330,6 +347,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);


2024-01-05 16:15:08

by Mike Leach

[permalink] [raw]
Subject: Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

Hi,

On Fri, 5 Jan 2024 at 05:59, Linu Cherian <[email protected]> wrote:
>
> Configure TMC ETR and ETF to flush and stop trace capture
> on FlIn event. As a side effect, do manual flush only if
> auto flush fails.
>
> Signed-off-by: Linu Cherian <[email protected]>
> ---
> Changelog from v5:
> * No changes
>
> drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 ++++++++--
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++--
> drivers/hwtracing/coresight/coresight-tmc.h | 3 +++
> 3 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 72c2315f4e23..57a9a9300d36 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -34,7 +34,7 @@ static int __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
> writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);
> writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
> - TMC_FFCR_TRIGON_TRIGIN,
> + TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
> drvdata->base + TMC_FFCR);
>

This is a problem. Setting TMC_FFCR_STOP_ON_FLUSH changes the
fundamentals of trigger event handling. Without this bit ETM can
generate multiple event + triggers which are then embedded into the
formatted trace stream for later search.
With this new bit the capture will stop on the first event. Setting
this bit should be dependent on the mode being set to ETR_MODE_RESRV



> writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
> @@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev)
> if (val != TMC_MODE_CIRCULAR_BUFFER)
> goto out;
>
> - tmc_flush_and_stop(drvdata);
> + val = readl(drvdata->base + TMC_FFSR);
> + /* Do manual flush and stop only if its not auto-stopped */
> + if (!(val & TMC_FFSR_FT_STOPPED)) {
> + dev_info(&csdev->dev,
> + "%s: Triggering manual flush\n", __func__);
> + tmc_flush_and_stop(drvdata);
> + }
>
Is there some reason to believe that the stop on flush will not work?

Using this conditional skips the tmc_wait_for_tmcready() called by
tmc_flush_and_stop() if the formatter is stopped - which is a
different condition test on a different register.

Why is this block of code not in the patch that introduces the
tmc_panic_sync_etf()

> /* Sync registers from hardware to metadata region */
> mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index ab7521bbb2f5..4b3c7ec7f62b 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1113,7 +1113,7 @@ static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>
> writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
> - TMC_FFCR_TRIGON_TRIGIN,
> + TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
> drvdata->base + TMC_FFCR);
> writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
> tmc_enable_hw(drvdata);
> @@ -1846,7 +1846,13 @@ static int tmc_panic_sync_etr(struct coresight_device *csdev)
> if (!(val & TMC_CTL_CAPT_EN))
> goto out;
>
> - tmc_flush_and_stop(drvdata);
> + val = readl(drvdata->base + TMC_FFSR);
> + /* Do manual flush and stop only if its not auto-stopped */
> + if (!(val & TMC_FFSR_FT_STOPPED)) {
> + dev_info(&csdev->dev,
> + "%s: Triggering manual flush\n", __func__);
> + tmc_flush_and_stop(drvdata);
> + }
>

Above comments for etf apply equally to etr.

Regards

Mike

> /* Sync registers from hardware to metadata region */
> mdata->size = csdev_access_relaxed_read32(csa, TMC_RSZ);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 6e1e910d5ea4..cf9313b302c7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -77,6 +77,9 @@
> #define TMC_AXICTL_AXCACHE_OS (0xf << 2)
> #define TMC_AXICTL_ARCACHE_OS (0xf << 16)
>
> +/* TMC_FFSR - 0x300 */
> +#define TMC_FFSR_FT_STOPPED BIT(1)
> +
> /* TMC_FFCR - 0x304 */
> #define TMC_FFCR_FLUSHMAN_BIT 6
> #define TMC_FFCR_EN_FMT BIT(0)
> --
> 2.34.1
>


--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

2024-01-09 10:53:58

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v6 5/7] coresight: tmc: Add support for reading tracedata from previous boot

Hi

On 05/01/2024 05:58, Linu Cherian wrote:
> * Introduce a new mode CS_MODE_READ_PREVBOOT for reading tracedata
> captured in previous boot.
>
> * Add special handlers for preparing ETR/ETF for this special mode
>
> * User can read the trace data as below
>
> For example, for reading trace data from tmc_etf sink
>
> 1. cd /sys/bus/coresight/devices/tmc_etfXX/
>
> 2. Change mode to READ_PREVBOOT
>
> #echo 1 > read_prevboot

I do not like this "new mode" just for reading the crash trace.
We are in the crash kernel when we have trace metadata available. Could
we not simply expose this via /dev/tmc_xxx. We can certainly figure this
out if we are in a crashkernel or not and the trace metadata is valid.
Also, we should wipe out the trace data/metadata on a normal boot or
after the crashkernel captures the data. We don't expect the user to do
tracing in the crashkernel, even if the user does it, it is upto the
user to overwrite the trace. We could even expose a different file
for the crash trace/metadata (say /dev/tmc_xxx_crash).

But please do not bloat the existing sysfs interface.

Suzuki

>
> 3. Dump trace buffer data to a file,
>
> #dd if=/dev/tmc_etfXX of=~/cstrace.bin
>
> 4. Reset back to normal mode
>
> #echo 0 > read_prevboot
>
> Signed-off-by: Anil Kumar Reddy <[email protected]>
> Signed-off-by: Tanmay Jagdale <[email protected]>
> Signed-off-by: Linu Cherian <[email protected]>
> ---
> Changelog from v5:
> * Fixed issues reported by CONFIG_DEBUG_ATOMIC_SLEEP in tmc_etr_setup_prevboot_buf
> and tmc_etb_setup_prevboot_buf
> - Kmalloc invoked with spinlock taken is now done with GFP_ATOMIC
> - Removed the requirement for memremap with spinlock taken. This is by
> making a assumption that the trace buffer address used for capturing
> trace data during kernel crash/hang would be same as that used
> for trace data retrieval in the subsequent boot. Added additional
> verification to ensure this.
>
> * Fixed a memory leak in tmc_read_unprepare_etr in READ_PREVBOOT mode
>
>
> .../coresight/coresight-etm4x-core.c | 1 +
> .../hwtracing/coresight/coresight-tmc-core.c | 96 ++++++++++-
> .../hwtracing/coresight/coresight-tmc-etf.c | 73 +++++++++
> .../hwtracing/coresight/coresight-tmc-etr.c | 154 +++++++++++++++++-
> drivers/hwtracing/coresight/coresight-tmc.h | 6 +
> include/linux/coresight.h | 13 ++
> 6 files changed, 341 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 34aee59dd147..c26b5f72a494 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1010,6 +1010,7 @@ static void etm4_disable(struct coresight_device *csdev,
>
> switch (mode) {
> case CS_MODE_DISABLED:
> + case CS_MODE_READ_PREVBOOT:
> break;
> case CS_MODE_SYSFS:
> etm4_disable_sysfs(csdev);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 4d7530d6ac91..e6525c3dcf66 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -103,6 +103,60 @@ u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata)
> return mask;
> }
>
> +int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata)
> +{
> + int ret = 0;
> + struct tmc_crash_metadata *mdata;
> + struct coresight_device *csdev = drvdata->csdev;
> +
> + if (!drvdata->crash_mdata.vaddr) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + mdata = drvdata->crash_mdata.vaddr;
> + /* Check data integrity of metadata */
> + if (mdata->crc32_mdata != find_crash_metadata_crc(mdata)) {
> + dev_dbg(&drvdata->csdev->dev,
> + "CRC mismatch in tmc crash metadata\n");
> + ret = -EINVAL;
> + goto out;
> + }
> + /* Check data integrity of tracedata */
> + if (mdata->crc32_tdata != find_crash_tracedata_crc(drvdata, mdata)) {
> + dev_dbg(&drvdata->csdev->dev,
> + "CRC mismatch in tmc crash tracedata\n");
> + ret = -EINVAL;
> + goto out;
> + }
> + /* Check for valid metadata */
> + if (!mdata->valid) {
> + dev_dbg(&drvdata->csdev->dev,
> + "Data invalid in tmc crash metadata\n");
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Sink specific prevboot mode preparation */
> + ret = prevboot_ops(csdev)->prepare(csdev);
> + if (ret)
> + goto out;
> +
> + if (mdata->sts & 0x1)
> + coresight_insert_barrier_packet(drvdata->buf);
> +
> +out:
> + return ret;
> +}
> +
> +int tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata)
> +{
> + struct coresight_device *csdev = drvdata->csdev;
> +
> + /* Sink specific prevboot mode preparation */
> + return prevboot_ops(csdev)->unprepare(csdev);
> +}
> +
> static int tmc_read_prepare(struct tmc_drvdata *drvdata)
> {
> int ret = 0;
> @@ -153,6 +207,10 @@ static int tmc_open(struct inode *inode, struct file *file)
> struct tmc_drvdata *drvdata = container_of(file->private_data,
> struct tmc_drvdata, miscdev);
>
> + /* Advertise if we are opening with a special mode */
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT)
> + dev_dbg(&drvdata->csdev->dev, "TMC read mode for previous boot\n");
> +
> ret = tmc_read_prepare(drvdata);
> if (ret)
> return ret;
> @@ -331,9 +389,44 @@ static ssize_t buffer_size_store(struct device *dev,
>
> static DEVICE_ATTR_RW(buffer_size);
>
> +static ssize_t read_prevboot_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->mode == CS_MODE_READ_PREVBOOT));
> +}
> +
> +static ssize_t read_prevboot_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t size)
> +{
> + int ret;
> + unsigned long val, flags;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> +
> + ret = kstrtoul(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + spin_lock_irqsave(&drvdata->spinlock, flags);
> +
> + if (val && (drvdata->mode == CS_MODE_DISABLED))
> + drvdata->mode = CS_MODE_READ_PREVBOOT;
> + else if (!val && (drvdata->mode == CS_MODE_READ_PREVBOOT))
> + drvdata->mode = CS_MODE_DISABLED;
> +
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> +
> + return size;
> +}
> +
> +static DEVICE_ATTR_RW(read_prevboot);
> +
> static struct attribute *coresight_tmc_attrs[] = {
> &dev_attr_trigger_cntr.attr,
> &dev_attr_buffer_size.attr,
> + &dev_attr_read_prevboot.attr,
> NULL,
> };
>
> @@ -635,7 +728,8 @@ static void tmc_shutdown(struct amba_device *adev)
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> - if (drvdata->mode == CS_MODE_DISABLED)
> + if (drvdata->mode == CS_MODE_DISABLED ||
> + drvdata->mode == CS_MODE_READ_PREVBOOT)
> goto out;
>
> if (drvdata->config_type == TMC_CONFIG_TYPE_ETR)
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index b53cef7628ff..72c2315f4e23 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -648,6 +648,56 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev)
> return 0;
> }
>
> +static int tmc_etb_setup_prevboot_buf(struct tmc_drvdata *drvdata)
> +{
> + unsigned long size;
> + struct tmc_crash_metadata *mdata;
> + struct device *dev = &drvdata->csdev->dev;
> +
> + mdata = drvdata->crash_mdata.vaddr;
> + size = mdata->size << 2;
> +
> + /*
> + * Buffer address given by metadata for retrieval of trace data
> + * from previous boot is expected to be same as the reserved
> + * trace buffer memory region provided through DTS
> + */
> + if (is_tmc_reserved_region_valid(dev->parent)
> + && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
> + drvdata->buf = drvdata->crash_tbuf.vaddr;
> + else {
> + dev_dbg(dev, "Trace buffer address of previous boot invalid\n");
> + return -EINVAL;
> + }
> +
> + drvdata->len = size;
> + return 0;
> +}
> +
> +static void tmc_etb_free_prevboot_buf(struct tmc_drvdata *drvdata)
> +{
> + void *buf = drvdata->buf;
> +
> + if (!buf)
> + return;
> + drvdata->buf = NULL;
> +}
> +
> +static int tmc_etb_prepare_prevboot(struct coresight_device *csdev)
> +{
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + return tmc_etb_setup_prevboot_buf(drvdata);
> +}
> +
> +static int tmc_etb_unprepare_prevboot(struct coresight_device *csdev)
> +{
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + tmc_etb_free_prevboot_buf(drvdata);
> + return 0;
> +}
> +
> static const struct coresight_ops_sink tmc_etf_sink_ops = {
> .enable = tmc_enable_etf_sink,
> .disable = tmc_disable_etf_sink,
> @@ -665,6 +715,11 @@ static const struct coresight_ops_panic tmc_etf_sync_ops = {
> .sync = tmc_panic_sync_etf,
> };
>
> +static const struct coresight_ops_prevboot tmc_etf_prevboot_ops = {
> + .prepare = tmc_etb_prepare_prevboot,
> + .unprepare = tmc_etb_unprepare_prevboot,
> +};
> +
> const struct coresight_ops tmc_etb_cs_ops = {
> .sink_ops = &tmc_etf_sink_ops,
> };
> @@ -673,6 +728,7 @@ const struct coresight_ops tmc_etf_cs_ops = {
> .sink_ops = &tmc_etf_sink_ops,
> .link_ops = &tmc_etf_link_ops,
> .panic_ops = &tmc_etf_sync_ops,
> + .prevboot_ops = &tmc_etf_prevboot_ops,
> };
>
> int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> @@ -693,6 +749,14 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> goto out;
> }
>
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> + ret = tmc_read_prepare_prevboot(drvdata);
> + if (ret)
> + goto out;
> + else
> + goto mode_valid;
> + }
> +
> /* Don't interfere if operated from Perf */
> if (drvdata->mode == CS_MODE_PERF) {
> ret = -EINVAL;
> @@ -716,6 +780,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> __tmc_etb_disable_hw(drvdata);
> }
>
> +mode_valid:
> drvdata->reading = true;
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -735,8 +800,16 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
> drvdata->config_type != TMC_CONFIG_TYPE_ETF))
> return -EINVAL;
>
> +
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> + tmc_read_unprepare_prevboot(drvdata);
> + drvdata->reading = false;
> + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + return 0;
> + }
> +
> /* Re-enable the TMC if need be */
> if (drvdata->mode == CS_MODE_SYSFS) {
> /* There is no point in reading a TMC in HW FIFO mode */
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 23eb35f50571..ab7521bbb2f5 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1166,7 +1166,12 @@ ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
> {
> s64 offset;
> ssize_t actual = len;
> - struct etr_buf *etr_buf = drvdata->sysfs_buf;
> + struct etr_buf *etr_buf;
> +
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT)
> + etr_buf = drvdata->prevboot_buf;
> + else
> + etr_buf = drvdata->sysfs_buf;
>
> if (pos + actual > etr_buf->len)
> actual = etr_buf->len - pos;
> @@ -1873,6 +1878,130 @@ static int tmc_panic_sync_etr(struct coresight_device *csdev)
> return 0;
> }
>
> +static int tmc_etr_setup_prevboot_buf(struct tmc_drvdata *drvdata)
> +{
> + int rc = 0;
> + u64 trace_addr;
> + struct etr_buf *etr_buf;
> + struct etr_flat_buf *resrv_buf;
> + struct tmc_crash_metadata *mdata;
> + struct device *dev = &drvdata->csdev->dev;
> +
> + mdata = drvdata->crash_mdata.vaddr;
> +
> + etr_buf = kzalloc(sizeof(*etr_buf), GFP_ATOMIC);
> + if (!etr_buf) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + etr_buf->size = drvdata->crash_tbuf.size;
> +
> + resrv_buf = kzalloc(sizeof(*resrv_buf), GFP_ATOMIC);
> + if (!resrv_buf) {
> + rc = -ENOMEM;
> + goto rmem_err;
> + }
> +
> + /*
> + * Buffer address given by metadata for retrieval of trace data
> + * from previous boot is expected to be same as the reserved
> + * trace buffer memory region provided through DTS
> + */
> + if (is_tmc_reserved_region_valid(dev->parent)
> + && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
> + resrv_buf->vaddr = drvdata->crash_tbuf.vaddr;
> + else {
> + dev_dbg(dev, "Trace buffer address of previous boot invalid\n");
> + rc = -EINVAL;
> + goto map_err;
> + }
> +
> + resrv_buf->size = etr_buf->size;
> + resrv_buf->dev = &drvdata->csdev->dev;
> + etr_buf->hwaddr = trace_addr;
> + etr_buf->mode = ETR_MODE_RESRV;
> + etr_buf->private = resrv_buf;
> + etr_buf->ops = etr_buf_ops[ETR_MODE_RESRV];
> +
> + drvdata->prevboot_buf = etr_buf;
> +
> + return 0;
> +
> +map_err:
> + kfree(resrv_buf);
> +
> +rmem_err:
> + kfree(etr_buf);
> +
> +out:
> + return rc;
> +}
> +
> +static int tmc_etr_sync_prevboot_buf(struct tmc_drvdata *drvdata)
> +{
> + u32 status;
> + u64 rrp, rwp, dba;
> + struct tmc_crash_metadata *mdata;
> + struct etr_buf *etr_buf = drvdata->prevboot_buf;
> +
> + mdata = drvdata->crash_mdata.vaddr;
> +
> + rrp = mdata->rrp;
> + rwp = mdata->rwp;
> + dba = mdata->dba;
> + status = mdata->sts;
> +
> + etr_buf->full = !!(status & TMC_STS_FULL);
> +
> + /* Sync the buffer pointers */
> + etr_buf->offset = rrp - dba;
> + if (etr_buf->full)
> + etr_buf->len = etr_buf->size;
> + else
> + etr_buf->len = rwp - rrp;
> +
> + /* Additional sanity checks for validating metadata */
> + if ((etr_buf->offset > etr_buf->size) ||
> + (etr_buf->len > etr_buf->size)) {
> + dev_dbg(&drvdata->csdev->dev,
> + "Offset and length invalid in tmc crash metadata\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void tmc_etr_free_prevboot_buf(struct tmc_drvdata *drvdata)
> +{
> + struct etr_buf *etr_buf = drvdata->prevboot_buf;
> +
> + if (!etr_buf)
> + return;
> + drvdata->prevboot_buf = NULL;
> +}
> +
> +static int tmc_etr_prepare_prevboot(struct coresight_device *csdev)
> +{
> + int ret = 0;
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + ret = tmc_etr_setup_prevboot_buf(drvdata);
> + if (ret)
> + goto out;
> + ret = tmc_etr_sync_prevboot_buf(drvdata);
> +
> +out:
> + return ret;
> +}
> +
> +static int tmc_etr_unprepare_prevboot(struct coresight_device *csdev)
> +{
> + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + tmc_etr_free_prevboot_buf(drvdata);
> + return 0;
> +}
> +
> static const struct coresight_ops_sink tmc_etr_sink_ops = {
> .enable = tmc_enable_etr_sink,
> .disable = tmc_disable_etr_sink,
> @@ -1885,9 +2014,15 @@ static const struct coresight_ops_panic tmc_etr_sync_ops = {
> .sync = tmc_panic_sync_etr,
> };
>
> +static const struct coresight_ops_prevboot tmc_etr_prevboot_ops = {
> + .prepare = tmc_etr_prepare_prevboot,
> + .unprepare = tmc_etr_unprepare_prevboot,
> +};
> +
> const struct coresight_ops tmc_etr_cs_ops = {
> .sink_ops = &tmc_etr_sink_ops,
> .panic_ops = &tmc_etr_sync_ops,
> + .prevboot_ops = &tmc_etr_prevboot_ops,
> };
>
> int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> @@ -1899,12 +2034,21 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
> return -EINVAL;
>
> +
> spin_lock_irqsave(&drvdata->spinlock, flags);
> if (drvdata->reading) {
> ret = -EBUSY;
> goto out;
> }
>
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> + ret = tmc_read_prepare_prevboot(drvdata);
> + if (ret)
> + goto out;
> + else
> + goto mode_valid;
> + }
> +
> /*
> * We can safely allow reads even if the ETR is operating in PERF mode,
> * since the sysfs session is captured in mode specific data.
> @@ -1919,6 +2063,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> if (drvdata->mode == CS_MODE_SYSFS)
> __tmc_etr_disable_hw(drvdata);
>
> +mode_valid:
> drvdata->reading = true;
> out:
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> @@ -1937,6 +2082,12 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
>
> + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> + sysfs_buf = drvdata->prevboot_buf;
> + tmc_read_unprepare_prevboot(drvdata);
> + goto out;
> + }
> +
> /* RE-enable the TMC if need be */
> if (drvdata->mode == CS_MODE_SYSFS) {
> /*
> @@ -1954,6 +2105,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
> drvdata->sysfs_buf = NULL;
> }
>
> +out:
> drvdata->reading = false;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 6f13caae6495..6e1e910d5ea4 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -216,6 +216,9 @@ struct tmc_resrv_buf {
> * @idr_mutex: Access serialisation for idr.
> * @sysfs_buf: SYSFS buffer for ETR.
> * @perf_buf: PERF buffer for ETR.
> + * @prevboot_buf: Previous boot buffer for ETR. This is a special purpose
> + * buffer that is used only for mapping the trace buffer from
> + * previous boot and not for capturing trace.
> * @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
> @@ -247,6 +250,7 @@ struct tmc_drvdata {
> struct mutex idr_mutex;
> struct etr_buf *sysfs_buf;
> struct etr_buf *perf_buf;
> + struct etr_buf *prevboot_buf;
> struct tmc_resrv_buf crash_tbuf;
> struct tmc_resrv_buf crash_mdata;
> };
> @@ -298,6 +302,8 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
> void tmc_enable_hw(struct tmc_drvdata *drvdata);
> void tmc_disable_hw(struct tmc_drvdata *drvdata);
> u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
> +int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata);
> +int tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata);
>
> /* ETB/ETF functions */
> int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 4fd518738958..9f84decf1d7a 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -294,6 +294,7 @@ enum cs_mode {
> CS_MODE_DISABLED,
> CS_MODE_SYSFS,
> CS_MODE_PERF,
> + CS_MODE_READ_PREVBOOT, /* Trace data from previous boot */
> };
>
> #define source_ops(csdev) csdev->ops->source_ops
> @@ -302,6 +303,7 @@ enum cs_mode {
> #define helper_ops(csdev) csdev->ops->helper_ops
> #define ect_ops(csdev) csdev->ops->ect_ops
> #define panic_ops(csdev) csdev->ops->panic_ops
> +#define prevboot_ops(csdev) csdev->ops->prevboot_ops
>
> /**
> * struct coresight_ops_sink - basic operations for a sink
> @@ -381,12 +383,23 @@ struct coresight_ops_panic {
> int (*sync)(struct coresight_device *csdev);
> };
>
> +/**
> + * struct coresight_ops_prevboot - Generic device ops for prevboot mode
> + *
> + * @prepare : Preparation for prevboot mode
> + */
> +struct coresight_ops_prevboot {
> + int (*prepare)(struct coresight_device *csdev);
> + int (*unprepare)(struct coresight_device *csdev);
> +};
> +
> struct coresight_ops {
> const struct coresight_ops_sink *sink_ops;
> const struct coresight_ops_link *link_ops;
> const struct coresight_ops_source *source_ops;
> const struct coresight_ops_helper *helper_ops;
> const struct coresight_ops_panic *panic_ops;
> + const struct coresight_ops_prevboot *prevboot_ops;
> };
>
> #if IS_ENABLED(CONFIG_CORESIGHT)


2024-01-09 10:59:14

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

On 05/01/2024 16:11, Mike Leach wrote:
> Hi,
>
> On Fri, 5 Jan 2024 at 05:59, Linu Cherian <[email protected]> wrote:
>>
>> Configure TMC ETR and ETF to flush and stop trace capture
>> on FlIn event. As a side effect, do manual flush only if
>> auto flush fails.
>>
>> Signed-off-by: Linu Cherian <[email protected]>
>> ---
>> Changelog from v5:
>> * No changes
>>
>> drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 ++++++++--
>> drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++--
>> drivers/hwtracing/coresight/coresight-tmc.h | 3 +++
>> 3 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index 72c2315f4e23..57a9a9300d36 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> @@ -34,7 +34,7 @@ static int __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
>> writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);
>> writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
>> TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
>> - TMC_FFCR_TRIGON_TRIGIN,
>> + TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
>> drvdata->base + TMC_FFCR);
>>
>
> This is a problem. Setting TMC_FFCR_STOP_ON_FLUSH changes the
> fundamentals of trigger event handling. Without this bit ETM can
> generate multiple event + triggers which are then embedded into the
> formatted trace stream for later search.
> With this new bit the capture will stop on the first event. Setting
> this bit should be dependent on the mode being set to ETR_MODE_RESRV
>
>
>
>> writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
>> @@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct coresight_device *csdev)
>> if (val != TMC_MODE_CIRCULAR_BUFFER)
>> goto out;
>>
>> - tmc_flush_and_stop(drvdata);
>> + val = readl(drvdata->base + TMC_FFSR);
>> + /* Do manual flush and stop only if its not auto-stopped */
>> + if (!(val & TMC_FFSR_FT_STOPPED)) {
>> + dev_info(&csdev->dev,
>> + "%s: Triggering manual flush\n", __func__);
>> + tmc_flush_and_stop(drvdata);
>> + }
>>
> Is there some reason to believe that the stop on flush will not work?
>
> Using this conditional skips the tmc_wait_for_tmcready() called by
> tmc_flush_and_stop() if the formatter is stopped - which is a
> different condition test on a different register.

Or even a new sysfs handle to turn this on ? e.g.,

/sys/bus/coresight/devices/tmc_etfN/stop_on_flush

That can be honoured only in the sysfs mode and set accordingly by the
user ?

e.g.,

for f in $(find /sys/bus/coresight/devices/ -maxdepth 1 -name tmc_\*)
do
echo 1 > $f/stop_on_flush
done


Suzuki


>
> Why is this block of code not in the patch that introduces the
> tmc_panic_sync_etf()
>
>> /* Sync registers from hardware to metadata region */
>> mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> index ab7521bbb2f5..4b3c7ec7f62b 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
>> @@ -1113,7 +1113,7 @@ static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>>
>> writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
>> TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
>> - TMC_FFCR_TRIGON_TRIGIN,
>> + TMC_FFCR_TRIGON_TRIGIN | TMC_FFCR_STOP_ON_FLUSH,
>> drvdata->base + TMC_FFCR);
>> writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
>> tmc_enable_hw(drvdata);
>> @@ -1846,7 +1846,13 @@ static int tmc_panic_sync_etr(struct coresight_device *csdev)
>> if (!(val & TMC_CTL_CAPT_EN))
>> goto out;
>>
>> - tmc_flush_and_stop(drvdata);
>> + val = readl(drvdata->base + TMC_FFSR);
>> + /* Do manual flush and stop only if its not auto-stopped */
>> + if (!(val & TMC_FFSR_FT_STOPPED)) {
>> + dev_info(&csdev->dev,
>> + "%s: Triggering manual flush\n", __func__);
>> + tmc_flush_and_stop(drvdata);
>> + }
>>
>
> Above comments for etf apply equally to etr.
>
> Regards
>
> Mike
>
>> /* Sync registers from hardware to metadata region */
>> mdata->size = csdev_access_relaxed_read32(csa, TMC_RSZ);
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
>> index 6e1e910d5ea4..cf9313b302c7 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc.h
>> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
>> @@ -77,6 +77,9 @@
>> #define TMC_AXICTL_AXCACHE_OS (0xf << 2)
>> #define TMC_AXICTL_ARCACHE_OS (0xf << 16)
>>
>> +/* TMC_FFSR - 0x300 */
>> +#define TMC_FFSR_FT_STOPPED BIT(1)
>> +
>> /* TMC_FFCR - 0x304 */
>> #define TMC_FFCR_FLUSHMAN_BIT 6
>> #define TMC_FFCR_EN_FMT BIT(0)
>> --
>> 2.34.1
>>
>
>
> --
> Mike Leach
> Principal Engineer, ARM Ltd.
> Manchester Design Centre. UK


2024-01-12 14:04:42

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v6 7/7] coresight: config: Add preloaded configuration

On 05/01/2024 05:58, Linu Cherian wrote:
> Add a preloaded configuration for generating
> external trigger on address match. This can be
> used by CTI and ETR blocks to stop trace capture
> on kernel panic.
>
> Kernel address for panic function to be
> programmed as below.
>
> $cd /config/cs-syscfg/features/gen_etrig/params
> $echo <panic_address> > address/value
>
> Signed-off-by: Linu Cherian <[email protected]>
> ---
> Changelog from v5:
> * No changes
>
> drivers/hwtracing/coresight/Makefile | 2 +-
> .../coresight/coresight-cfg-preload.c | 2 +
> .../coresight/coresight-cfg-preload.h | 2 +
> .../hwtracing/coresight/coresight-cfg-pstop.c | 83 +++++++++++++++++++
> 4 files changed, 88 insertions(+), 1 deletion(-)
> create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c
>
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 995d3b2c76df..68b15c8d9462 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -5,7 +5,7 @@
> obj-$(CONFIG_CORESIGHT) += coresight.o
> coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \
> coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> - coresight-cfg-preload.o coresight-cfg-afdo.o \
> + coresight-cfg-preload.o coresight-cfg-afdo.o coresight-cfg-pstop.o \
> coresight-syscfg-configfs.o coresight-trace-id.o
> obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
> coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \
> diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> index e237a4edfa09..4980e68483c5 100644
> --- a/drivers/hwtracing/coresight/coresight-cfg-preload.c
> +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> @@ -13,6 +13,7 @@
> static struct cscfg_feature_desc *preload_feats[] = {
> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> &strobe_etm4x,
> + &gen_etrig_etm4x,
> #endif
> NULL
> };
> @@ -20,6 +21,7 @@ static struct cscfg_feature_desc *preload_feats[] = {
> static struct cscfg_config_desc *preload_cfgs[] = {
> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> &afdo_etm4x,
> + &pstop_etm4x,
> #endif
> NULL
> };
> diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.h b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> index 21299e175477..291ba530a6a5 100644
> --- a/drivers/hwtracing/coresight/coresight-cfg-preload.h
> +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> @@ -10,4 +10,6 @@
> #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> extern struct cscfg_feature_desc strobe_etm4x;
> extern struct cscfg_config_desc afdo_etm4x;
> +extern struct cscfg_feature_desc gen_etrig_etm4x;
> +extern struct cscfg_config_desc pstop_etm4x;
> #endif
> diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> new file mode 100644
> index 000000000000..037d6773fab8
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(C) 2023 Marvell.
> + * Based on coresight-cfg-afdo.c
> + */
> +
> +#include "coresight-config.h"
> +
> +/* ETMv4 includes and features */
> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> +#include "coresight-etm4x-cfg.h"
> +
> +/* preload configurations and features */
> +
> +/* preload in features for ETMv4 */
> +
> +/* panic_stop feature */
> +static struct cscfg_parameter_desc gen_etrig_params[] = {
> + {
> + .name = "address",
> + .value = 0x0,
> + },
> +};
> +
> +static struct cscfg_regval_desc gen_etrig_regs[] = {
> + /* resource selector */
> + {
> + .type = CS_CFG_REG_TYPE_RESOURCE,
> + .offset = TRCRSCTLRn(2),
> + .hw_info = ETM4_CFG_RES_SEL,
> + .val32 = 0x40001,
> + },
> + /* single address comparator */
> + {
> + .type = CS_CFG_REG_TYPE_RESOURCE | CS_CFG_REG_TYPE_VAL_64BIT |
> + CS_CFG_REG_TYPE_VAL_PARAM,
> + .offset = TRCACVRn(0),
> + .val32 = 0x0,
> + },
> + {
> + .type = CS_CFG_REG_TYPE_RESOURCE,
> + .offset = TRCACATRn(0),
> + .val64 = 0xf00,
> + },
> + /* Driver external output[0] with comparator out */
> + {
> + .type = CS_CFG_REG_TYPE_RESOURCE,
> + .offset = TRCEVENTCTL0R,
> + .val32 = 0x2,
> + },
> + /* end of regs */
> +};
> +
> +struct cscfg_feature_desc gen_etrig_etm4x = {
> + .name = "gen_etrig",
> + .description = "Generate external trigger on address match\n"
> + "parameter \'address\': address of kernel address\n",
> + .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4,
> + .nr_params = ARRAY_SIZE(gen_etrig_params),
> + .params_desc = gen_etrig_params,
> + .nr_regs = ARRAY_SIZE(gen_etrig_regs),
> + .regs_desc = gen_etrig_regs,
> +};
> +
> +/* create a panic stop configuration */
> +
> +/* the total number of parameters in used features */
> +#define PSTOP_NR_PARAMS ARRAY_SIZE(gen_etrig_params)
> +
> +static const char *pstop_ref_names[] = {
> + "gen_etrig",
> +};
> +
> +struct cscfg_config_desc pstop_etm4x = {
> + .name = "panicstop",
> + .description = "Stop ETM on kernel panic\n",

Since this is actually generic, i.e., Stop trace on a Kernel address, we
could rename this ? Or why don't we pre-populate the address of "panic"
at load time. That way the user doesn't have to figure out the kernel
address (e.g., if KASLR is enabled)

Suzuki


> + .nr_feat_refs = ARRAY_SIZE(pstop_ref_names),
> + .feat_ref_names = pstop_ref_names,
> + .nr_total_params = PSTOP_NR_PARAMS,
> +};
> +
> +/* end of ETM4x configurations */
> +#endif /* IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X) */


2024-01-12 17:05:35

by Linu Cherian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v6 5/7] coresight: tmc: Add support for reading tracedata from previous boot

Hi Suzuki,

> -----Original Message-----
> From: Suzuki K Poulose <[email protected]>
> Sent: Tuesday, January 9, 2024 4:22 PM
> To: Linu Cherian <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; George Cherian <[email protected]>; Anil
> Kumar Reddy H <[email protected]>; Tanmay Jagdale
> <[email protected]>
> Subject: [EXT] Re: [PATCH v6 5/7] coresight: tmc: Add support for reading
> tracedata from previous boot
>
> External Email
>
> ----------------------------------------------------------------------
> Hi
>
> On 05/01/2024 05:58, Linu Cherian wrote:
> > * Introduce a new mode CS_MODE_READ_PREVBOOT for reading
> tracedata
> > captured in previous boot.
> >
> > * Add special handlers for preparing ETR/ETF for this special mode
> >
> > * User can read the trace data as below
> >
> > For example, for reading trace data from tmc_etf sink
> >
> > 1. cd /sys/bus/coresight/devices/tmc_etfXX/
> >
> > 2. Change mode to READ_PREVBOOT
> >
> > #echo 1 > read_prevboot
>
> I do not like this "new mode" just for reading the crash trace.
> We are in the crash kernel when we have trace metadata available. Could we
> not simply expose this via /dev/tmc_xxx. We can certainly figure this out if
> we are in a crashkernel or not and the trace metadata is valid.
> Also, we should wipe out the trace data/metadata on a normal boot or after
> the crashkernel captures the data.

Somehow couldn’t get the intention behind wiping the tracedata/metadata after retrieval OR its just enough to add a provision to the user for wiping it. Can you please clarify.

We don't expect the user to do tracing in
> the crashkernel, even if the user does it, it is upto the user to overwrite the
> trace. We could even expose a different file for the crash trace/metadata
> (say /dev/tmc_xxx_crash).
>
> But please do not bloat the existing sysfs interface.
>
Since we intend to support the watchdog case as well, we cannot restrict reading the tracedata only from the crashdump kernel. In that regard, using a separate file /dev/tmc_xxx_crash looks better and cleaner option to me.



> Suzuki
>
> >
> > 3. Dump trace buffer data to a file,
> >
> > #dd if=/dev/tmc_etfXX of=~/cstrace.bin
> >
> > 4. Reset back to normal mode
> >
> > #echo 0 > read_prevboot
> >
> > Signed-off-by: Anil Kumar Reddy <[email protected]>
> > Signed-off-by: Tanmay Jagdale <[email protected]>
> > Signed-off-by: Linu Cherian <[email protected]>
> > ---
> > Changelog from v5:
> > * Fixed issues reported by CONFIG_DEBUG_ATOMIC_SLEEP in
> tmc_etr_setup_prevboot_buf
> > and tmc_etb_setup_prevboot_buf
> > - Kmalloc invoked with spinlock taken is now done with GFP_ATOMIC
> > - Removed the requirement for memremap with spinlock taken. This is by
> > making a assumption that the trace buffer address used for capturing
> > trace data during kernel crash/hang would be same as that used
> > for trace data retrieval in the subsequent boot. Added additional
> > verification to ensure this.
> >
> > * Fixed a memory leak in tmc_read_unprepare_etr in READ_PREVBOOT
> mode
> >
> >
> > .../coresight/coresight-etm4x-core.c | 1 +
> > .../hwtracing/coresight/coresight-tmc-core.c | 96 ++++++++++-
> > .../hwtracing/coresight/coresight-tmc-etf.c | 73 +++++++++
> > .../hwtracing/coresight/coresight-tmc-etr.c | 154 +++++++++++++++++-
> > drivers/hwtracing/coresight/coresight-tmc.h | 6 +
> > include/linux/coresight.h | 13 ++
> > 6 files changed, 341 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > index 34aee59dd147..c26b5f72a494 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> > @@ -1010,6 +1010,7 @@ static void etm4_disable(struct coresight_device
> > *csdev,
> >
> > switch (mode) {
> > case CS_MODE_DISABLED:
> > + case CS_MODE_READ_PREVBOOT:
> > break;
> > case CS_MODE_SYSFS:
> > etm4_disable_sysfs(csdev);
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > index 4d7530d6ac91..e6525c3dcf66 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > @@ -103,6 +103,60 @@ u32 tmc_get_memwidth_mask(struct
> tmc_drvdata *drvdata)
> > return mask;
> > }
> >
> > +int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata) {
> > + int ret = 0;
> > + struct tmc_crash_metadata *mdata;
> > + struct coresight_device *csdev = drvdata->csdev;
> > +
> > + if (!drvdata->crash_mdata.vaddr) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + mdata = drvdata->crash_mdata.vaddr;
> > + /* Check data integrity of metadata */
> > + if (mdata->crc32_mdata != find_crash_metadata_crc(mdata)) {
> > + dev_dbg(&drvdata->csdev->dev,
> > + "CRC mismatch in tmc crash metadata\n");
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + /* Check data integrity of tracedata */
> > + if (mdata->crc32_tdata != find_crash_tracedata_crc(drvdata, mdata))
> {
> > + dev_dbg(&drvdata->csdev->dev,
> > + "CRC mismatch in tmc crash tracedata\n");
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > + /* Check for valid metadata */
> > + if (!mdata->valid) {
> > + dev_dbg(&drvdata->csdev->dev,
> > + "Data invalid in tmc crash metadata\n");
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + /* Sink specific prevboot mode preparation */
> > + ret = prevboot_ops(csdev)->prepare(csdev);
> > + if (ret)
> > + goto out;
> > +
> > + if (mdata->sts & 0x1)
> > + coresight_insert_barrier_packet(drvdata->buf);
> > +
> > +out:
> > + return ret;
> > +}
> > +
> > +int tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata) {
> > + struct coresight_device *csdev = drvdata->csdev;
> > +
> > + /* Sink specific prevboot mode preparation */
> > + return prevboot_ops(csdev)->unprepare(csdev);
> > +}
> > +
> > static int tmc_read_prepare(struct tmc_drvdata *drvdata)
> > {
> > int ret = 0;
> > @@ -153,6 +207,10 @@ static int tmc_open(struct inode *inode, struct file
> *file)
> > struct tmc_drvdata *drvdata = container_of(file->private_data,
> > struct tmc_drvdata,
> miscdev);
> >
> > + /* Advertise if we are opening with a special mode */
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT)
> > + dev_dbg(&drvdata->csdev->dev, "TMC read mode for
> previous boot\n");
> > +
> > ret = tmc_read_prepare(drvdata);
> > if (ret)
> > return ret;
> > @@ -331,9 +389,44 @@ static ssize_t buffer_size_store(struct device
> > *dev,
> >
> > static DEVICE_ATTR_RW(buffer_size);
> >
> > +static ssize_t read_prevboot_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->mode ==
> > +CS_MODE_READ_PREVBOOT)); }
> > +
> > +static ssize_t read_prevboot_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t size)
> > +{
> > + int ret;
> > + unsigned long val, flags;
> > + struct tmc_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +
> > + ret = kstrtoul(buf, 0, &val);
> > + if (ret)
> > + return ret;
> > +
> > + spin_lock_irqsave(&drvdata->spinlock, flags);
> > +
> > + if (val && (drvdata->mode == CS_MODE_DISABLED))
> > + drvdata->mode = CS_MODE_READ_PREVBOOT;
> > + else if (!val && (drvdata->mode == CS_MODE_READ_PREVBOOT))
> > + drvdata->mode = CS_MODE_DISABLED;
> > +
> > + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> > +
> > + return size;
> > +}
> > +
> > +static DEVICE_ATTR_RW(read_prevboot);
> > +
> > static struct attribute *coresight_tmc_attrs[] = {
> > &dev_attr_trigger_cntr.attr,
> > &dev_attr_buffer_size.attr,
> > + &dev_attr_read_prevboot.attr,
> > NULL,
> > };
> >
> > @@ -635,7 +728,8 @@ static void tmc_shutdown(struct amba_device
> *adev)
> >
> > spin_lock_irqsave(&drvdata->spinlock, flags);
> >
> > - if (drvdata->mode == CS_MODE_DISABLED)
> > + if (drvdata->mode == CS_MODE_DISABLED ||
> > + drvdata->mode == CS_MODE_READ_PREVBOOT)
> > goto out;
> >
> > if (drvdata->config_type == TMC_CONFIG_TYPE_ETR) diff --git
> > a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > index b53cef7628ff..72c2315f4e23 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > @@ -648,6 +648,56 @@ static int tmc_panic_sync_etf(struct
> coresight_device *csdev)
> > return 0;
> > }
> >
> > +static int tmc_etb_setup_prevboot_buf(struct tmc_drvdata *drvdata) {
> > + unsigned long size;
> > + struct tmc_crash_metadata *mdata;
> > + struct device *dev = &drvdata->csdev->dev;
> > +
> > + mdata = drvdata->crash_mdata.vaddr;
> > + size = mdata->size << 2;
> > +
> > + /*
> > + * Buffer address given by metadata for retrieval of trace data
> > + * from previous boot is expected to be same as the reserved
> > + * trace buffer memory region provided through DTS
> > + */
> > + if (is_tmc_reserved_region_valid(dev->parent)
> > + && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
> > + drvdata->buf = drvdata->crash_tbuf.vaddr;
> > + else {
> > + dev_dbg(dev, "Trace buffer address of previous boot
> invalid\n");
> > + return -EINVAL;
> > + }
> > +
> > + drvdata->len = size;
> > + return 0;
> > +}
> > +
> > +static void tmc_etb_free_prevboot_buf(struct tmc_drvdata *drvdata) {
> > + void *buf = drvdata->buf;
> > +
> > + if (!buf)
> > + return;
> > + drvdata->buf = NULL;
> > +}
> > +
> > +static int tmc_etb_prepare_prevboot(struct coresight_device *csdev) {
> > + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev-
> >dev.parent);
> > +
> > + return tmc_etb_setup_prevboot_buf(drvdata);
> > +}
> > +
> > +static int tmc_etb_unprepare_prevboot(struct coresight_device *csdev)
> > +{
> > + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev-
> >dev.parent);
> > +
> > + tmc_etb_free_prevboot_buf(drvdata);
> > + return 0;
> > +}
> > +
> > static const struct coresight_ops_sink tmc_etf_sink_ops = {
> > .enable = tmc_enable_etf_sink,
> > .disable = tmc_disable_etf_sink,
> > @@ -665,6 +715,11 @@ static const struct coresight_ops_panic
> tmc_etf_sync_ops = {
> > .sync = tmc_panic_sync_etf,
> > };
> >
> > +static const struct coresight_ops_prevboot tmc_etf_prevboot_ops = {
> > + .prepare = tmc_etb_prepare_prevboot,
> > + .unprepare = tmc_etb_unprepare_prevboot,
> > +};
> > +
> > const struct coresight_ops tmc_etb_cs_ops = {
> > .sink_ops = &tmc_etf_sink_ops,
> > };
> > @@ -673,6 +728,7 @@ const struct coresight_ops tmc_etf_cs_ops = {
> > .sink_ops = &tmc_etf_sink_ops,
> > .link_ops = &tmc_etf_link_ops,
> > .panic_ops = &tmc_etf_sync_ops,
> > + .prevboot_ops = &tmc_etf_prevboot_ops,
> > };
> >
> > int tmc_read_prepare_etb(struct tmc_drvdata *drvdata) @@ -693,6
> > +749,14 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
> > goto out;
> > }
> >
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> > + ret = tmc_read_prepare_prevboot(drvdata);
> > + if (ret)
> > + goto out;
> > + else
> > + goto mode_valid;
> > + }
> > +
> > /* Don't interfere if operated from Perf */
> > if (drvdata->mode == CS_MODE_PERF) {
> > ret = -EINVAL;
> > @@ -716,6 +780,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata
> *drvdata)
> > __tmc_etb_disable_hw(drvdata);
> > }
> >
> > +mode_valid:
> > drvdata->reading = true;
> > out:
> > spin_unlock_irqrestore(&drvdata->spinlock, flags); @@ -735,8
> > +800,16 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
> > drvdata->config_type != TMC_CONFIG_TYPE_ETF))
> > return -EINVAL;
> >
> > +
> > spin_lock_irqsave(&drvdata->spinlock, flags);
> >
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> > + tmc_read_unprepare_prevboot(drvdata);
> > + drvdata->reading = false;
> > + spin_unlock_irqrestore(&drvdata->spinlock, flags);
> > + return 0;
> > + }
> > +
> > /* Re-enable the TMC if need be */
> > if (drvdata->mode == CS_MODE_SYSFS) {
> > /* There is no point in reading a TMC in HW FIFO mode */ diff
> > --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > index 23eb35f50571..ab7521bbb2f5 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > @@ -1166,7 +1166,12 @@ ssize_t tmc_etr_get_sysfs_trace(struct
> tmc_drvdata *drvdata,
> > {
> > s64 offset;
> > ssize_t actual = len;
> > - struct etr_buf *etr_buf = drvdata->sysfs_buf;
> > + struct etr_buf *etr_buf;
> > +
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT)
> > + etr_buf = drvdata->prevboot_buf;
> > + else
> > + etr_buf = drvdata->sysfs_buf;
> >
> > if (pos + actual > etr_buf->len)
> > actual = etr_buf->len - pos;
> > @@ -1873,6 +1878,130 @@ static int tmc_panic_sync_etr(struct
> coresight_device *csdev)
> > return 0;
> > }
> >
> > +static int tmc_etr_setup_prevboot_buf(struct tmc_drvdata *drvdata) {
> > + int rc = 0;
> > + u64 trace_addr;
> > + struct etr_buf *etr_buf;
> > + struct etr_flat_buf *resrv_buf;
> > + struct tmc_crash_metadata *mdata;
> > + struct device *dev = &drvdata->csdev->dev;
> > +
> > + mdata = drvdata->crash_mdata.vaddr;
> > +
> > + etr_buf = kzalloc(sizeof(*etr_buf), GFP_ATOMIC);
> > + if (!etr_buf) {
> > + rc = -ENOMEM;
> > + goto out;
> > + }
> > + etr_buf->size = drvdata->crash_tbuf.size;
> > +
> > + resrv_buf = kzalloc(sizeof(*resrv_buf), GFP_ATOMIC);
> > + if (!resrv_buf) {
> > + rc = -ENOMEM;
> > + goto rmem_err;
> > + }
> > +
> > + /*
> > + * Buffer address given by metadata for retrieval of trace data
> > + * from previous boot is expected to be same as the reserved
> > + * trace buffer memory region provided through DTS
> > + */
> > + if (is_tmc_reserved_region_valid(dev->parent)
> > + && (drvdata->crash_tbuf.paddr == mdata->trc_paddr))
> > + resrv_buf->vaddr = drvdata->crash_tbuf.vaddr;
> > + else {
> > + dev_dbg(dev, "Trace buffer address of previous boot
> invalid\n");
> > + rc = -EINVAL;
> > + goto map_err;
> > + }
> > +
> > + resrv_buf->size = etr_buf->size;
> > + resrv_buf->dev = &drvdata->csdev->dev;
> > + etr_buf->hwaddr = trace_addr;
> > + etr_buf->mode = ETR_MODE_RESRV;
> > + etr_buf->private = resrv_buf;
> > + etr_buf->ops = etr_buf_ops[ETR_MODE_RESRV];
> > +
> > + drvdata->prevboot_buf = etr_buf;
> > +
> > + return 0;
> > +
> > +map_err:
> > + kfree(resrv_buf);
> > +
> > +rmem_err:
> > + kfree(etr_buf);
> > +
> > +out:
> > + return rc;
> > +}
> > +
> > +static int tmc_etr_sync_prevboot_buf(struct tmc_drvdata *drvdata) {
> > + u32 status;
> > + u64 rrp, rwp, dba;
> > + struct tmc_crash_metadata *mdata;
> > + struct etr_buf *etr_buf = drvdata->prevboot_buf;
> > +
> > + mdata = drvdata->crash_mdata.vaddr;
> > +
> > + rrp = mdata->rrp;
> > + rwp = mdata->rwp;
> > + dba = mdata->dba;
> > + status = mdata->sts;
> > +
> > + etr_buf->full = !!(status & TMC_STS_FULL);
> > +
> > + /* Sync the buffer pointers */
> > + etr_buf->offset = rrp - dba;
> > + if (etr_buf->full)
> > + etr_buf->len = etr_buf->size;
> > + else
> > + etr_buf->len = rwp - rrp;
> > +
> > + /* Additional sanity checks for validating metadata */
> > + if ((etr_buf->offset > etr_buf->size) ||
> > + (etr_buf->len > etr_buf->size)) {
> > + dev_dbg(&drvdata->csdev->dev,
> > + "Offset and length invalid in tmc crash metadata\n");
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void tmc_etr_free_prevboot_buf(struct tmc_drvdata *drvdata) {
> > + struct etr_buf *etr_buf = drvdata->prevboot_buf;
> > +
> > + if (!etr_buf)
> > + return;
> > + drvdata->prevboot_buf = NULL;
> > +}
> > +
> > +static int tmc_etr_prepare_prevboot(struct coresight_device *csdev) {
> > + int ret = 0;
> > + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev-
> >dev.parent);
> > +
> > + ret = tmc_etr_setup_prevboot_buf(drvdata);
> > + if (ret)
> > + goto out;
> > + ret = tmc_etr_sync_prevboot_buf(drvdata);
> > +
> > +out:
> > + return ret;
> > +}
> > +
> > +static int tmc_etr_unprepare_prevboot(struct coresight_device *csdev)
> > +{
> > + struct tmc_drvdata *drvdata = dev_get_drvdata(csdev-
> >dev.parent);
> > +
> > + tmc_etr_free_prevboot_buf(drvdata);
> > + return 0;
> > +}
> > +
> > static const struct coresight_ops_sink tmc_etr_sink_ops = {
> > .enable = tmc_enable_etr_sink,
> > .disable = tmc_disable_etr_sink,
> > @@ -1885,9 +2014,15 @@ static const struct coresight_ops_panic
> tmc_etr_sync_ops = {
> > .sync = tmc_panic_sync_etr,
> > };
> >
> > +static const struct coresight_ops_prevboot tmc_etr_prevboot_ops = {
> > + .prepare = tmc_etr_prepare_prevboot,
> > + .unprepare = tmc_etr_unprepare_prevboot,
> > +};
> > +
> > const struct coresight_ops tmc_etr_cs_ops = {
> > .sink_ops = &tmc_etr_sink_ops,
> > .panic_ops = &tmc_etr_sync_ops,
> > + .prevboot_ops = &tmc_etr_prevboot_ops,
> > };
> >
> > int tmc_read_prepare_etr(struct tmc_drvdata *drvdata) @@ -1899,12
> > +2034,21 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> > if (WARN_ON_ONCE(drvdata->config_type !=
> TMC_CONFIG_TYPE_ETR))
> > return -EINVAL;
> >
> > +
> > spin_lock_irqsave(&drvdata->spinlock, flags);
> > if (drvdata->reading) {
> > ret = -EBUSY;
> > goto out;
> > }
> >
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> > + ret = tmc_read_prepare_prevboot(drvdata);
> > + if (ret)
> > + goto out;
> > + else
> > + goto mode_valid;
> > + }
> > +
> > /*
> > * We can safely allow reads even if the ETR is operating in PERF
> mode,
> > * since the sysfs session is captured in mode specific data.
> > @@ -1919,6 +2063,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata
> *drvdata)
> > if (drvdata->mode == CS_MODE_SYSFS)
> > __tmc_etr_disable_hw(drvdata);
> >
> > +mode_valid:
> > drvdata->reading = true;
> > out:
> > spin_unlock_irqrestore(&drvdata->spinlock, flags); @@ -1937,6
> > +2082,12 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
> >
> > spin_lock_irqsave(&drvdata->spinlock, flags);
> >
> > + if (drvdata->mode == CS_MODE_READ_PREVBOOT) {
> > + sysfs_buf = drvdata->prevboot_buf;
> > + tmc_read_unprepare_prevboot(drvdata);
> > + goto out;
> > + }
> > +
> > /* RE-enable the TMC if need be */
> > if (drvdata->mode == CS_MODE_SYSFS) {
> > /*
> > @@ -1954,6 +2105,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata
> *drvdata)
> > drvdata->sysfs_buf = NULL;
> > }
> >
> > +out:
> > drvdata->reading = false;
> > spin_unlock_irqrestore(&drvdata->spinlock, flags);
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc.h
> > b/drivers/hwtracing/coresight/coresight-tmc.h
> > index 6f13caae6495..6e1e910d5ea4 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc.h
> > +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> > @@ -216,6 +216,9 @@ struct tmc_resrv_buf {
> > * @idr_mutex: Access serialisation for idr.
> > * @sysfs_buf: SYSFS buffer for ETR.
> > * @perf_buf: PERF buffer for ETR.
> > + * @prevboot_buf: Previous boot buffer for ETR. This is a special purpose
> > + * buffer that is used only for mapping the trace buffer from
> > + * previous boot and not for capturing trace.
> > * @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
> > @@ -247,6 +250,7 @@ struct tmc_drvdata {
> > struct mutex idr_mutex;
> > struct etr_buf *sysfs_buf;
> > struct etr_buf *perf_buf;
> > + struct etr_buf *prevboot_buf;
> > struct tmc_resrv_buf crash_tbuf;
> > struct tmc_resrv_buf crash_mdata;
> > };
> > @@ -298,6 +302,8 @@ void tmc_flush_and_stop(struct tmc_drvdata
> *drvdata);
> > void tmc_enable_hw(struct tmc_drvdata *drvdata);
> > void tmc_disable_hw(struct tmc_drvdata *drvdata);
> > u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
> > +int tmc_read_prepare_prevboot(struct tmc_drvdata *drvdata); int
> > +tmc_read_unprepare_prevboot(struct tmc_drvdata *drvdata);
> >
> > /* ETB/ETF functions */
> > int tmc_read_prepare_etb(struct tmc_drvdata *drvdata); diff --git
> > a/include/linux/coresight.h b/include/linux/coresight.h index
> > 4fd518738958..9f84decf1d7a 100644
> > --- a/include/linux/coresight.h
> > +++ b/include/linux/coresight.h
> > @@ -294,6 +294,7 @@ enum cs_mode {
> > CS_MODE_DISABLED,
> > CS_MODE_SYSFS,
> > CS_MODE_PERF,
> > + CS_MODE_READ_PREVBOOT, /* Trace data from previous boot */
> > };
> >
> > #define source_ops(csdev) csdev->ops->source_ops
> > @@ -302,6 +303,7 @@ enum cs_mode {
> > #define helper_ops(csdev) csdev->ops->helper_ops
> > #define ect_ops(csdev) csdev->ops->ect_ops
> > #define panic_ops(csdev) csdev->ops->panic_ops
> > +#define prevboot_ops(csdev) csdev->ops->prevboot_ops
> >
> > /**
> > * struct coresight_ops_sink - basic operations for a sink @@
> > -381,12 +383,23 @@ struct coresight_ops_panic {
> > int (*sync)(struct coresight_device *csdev);
> > };
> >
> > +/**
> > + * struct coresight_ops_prevboot - Generic device ops for prevboot
> > +mode
> > + *
> > + * @prepare : Preparation for prevboot mode
> > + */
> > +struct coresight_ops_prevboot {
> > + int (*prepare)(struct coresight_device *csdev);
> > + int (*unprepare)(struct coresight_device *csdev); };
> > +
> > struct coresight_ops {
> > const struct coresight_ops_sink *sink_ops;
> > const struct coresight_ops_link *link_ops;
> > const struct coresight_ops_source *source_ops;
> > const struct coresight_ops_helper *helper_ops;
> > const struct coresight_ops_panic *panic_ops;
> > + const struct coresight_ops_prevboot *prevboot_ops;
> > };
> >
> > #if IS_ENABLED(CONFIG_CORESIGHT)

2024-01-12 23:08:41

by Linu Cherian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v6 2/7] coresight: tmc-etr: Add support to use reserved trace memory

Hi Suzuki,

> -----Original Message-----
> From: Suzuki K Poulose <[email protected]>
> Sent: Friday, January 5, 2024 7:47 PM
> To: Linu Cherian <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; George Cherian <[email protected]>; Anil
> Kumar Reddy H <[email protected]>
> Subject: [EXT] Re: [PATCH v6 2/7] coresight: tmc-etr: Add support to use
> reserved trace memory
>
> External Email
>
> ----------------------------------------------------------------------
> On 05/01/2024 05:58, 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 <[email protected]>
> > Signed-off-by: Linu Cherian <[email protected]>
> > ---
> > Changelog from v5:
> > * No changes
> >
> > .../hwtracing/coresight/coresight-tmc-core.c | 51 +++++++++++
> > .../hwtracing/coresight/coresight-tmc-etr.c | 87 ++++++++++++++++++-
> > drivers/hwtracing/coresight/coresight-tmc.h | 27 ++++++
> > 3 files changed, 164 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c
> > b/drivers/hwtracing/coresight/coresight-tmc-core.c
> > index 7ec5365e2b64..224b969d7d90 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,54 @@ 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");
> > + goto out;
>
> Given we don't return anything , the goto is pointless, you could return from
> here.
>

Ack.

> > + }
> > +
> > + 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");
> > + goto out;
>
> Same here.


Ack.

>
> > + }
> > +
> > + 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");
> > + rc = PTR_ERR(drvdata->crash_tbuf.vaddr);
>
> This is pointless, as we never use this.
>

Ack.


> > + goto out;
>
> return
>
> > + }
> > +
> > + drvdata->crash_tbuf.paddr = res.start;
> > + drvdata->crash_tbuf.size = resource_size(&res);
> > +
> > +out:
> > + return;
> > +}
> > +
> > /* 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 +531,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 af02ba5d5f15..7fd8c4afef74 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,74 @@ 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;
>
> Shouldn't we also ensure the etr_buf->size < crash_tbuf.size ? Or we could
> simply allocate the full reserve buf, as we can't use it for anything else. In any
> case, the buffer should be bounded by the reserved space.

This is taken care in tmc_alloc_etr_buf.ie. etr_buf_size is updated to crash_tbuf.size with ETR_MODE_RESRV buffer mode. Anyways, will move the size settings to here as suggested below.
>
> > +
> > + resrv_buf->daddr = dma_map_resource(real_dev, drvdata-
> >crash_tbuf.paddr,
> > + etr_buf->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;
> > + 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;
>
> Don't we need to sync the buffer for CPU via dma_sync_single_for_cpu() ?
> Otherwise, we could be reading stale data ?
>

From both CPU and DMA perspective we are mapping them as non cacheable and hence considered coherent always.
Ie. For CPU we use memremap with MEMREMAP_WC and for DMA we use dma_map_resource to map the trace buffer.

> > +}
> > +
> > +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 +869,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 +895,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 +914,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 +946,19 @@ static struct etr_buf
> *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> > if (!etr_buf)
> > return ERR_PTR(-ENOMEM);
> >
> > - etr_buf->size = size;
> > + /* Overiride the buffer size here for reserved mode */
>
> minor nit: "Override"
>

Ack.


> > + etr_buf->size = (drvdata->etr_mode == ETR_MODE_RESRV) ?
> > + drvdata->crash_tbuf.size : size;
>
> Isn't it better to do this from the actual code where we are successful in
> allocating the buffer ? That way, it works for the AUTO mode too ?
>

Ack. Will do that.


> >
> > /* 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);
> >
> > + /* Fallback mechanism is not valid for reserved mode */
> > + if (rc && (drvdata->etr_mode == ETR_MODE_RESRV))
> > + goto done;
> > +
> > /*
> > * 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, @@ -902,6 +980,7 @@ static struct etr_buf
> *tmc_alloc_etr_buf(struct tmc_drvdata *drvdata,
> > if (rc && buf_hw.has_catu)
> > rc = tmc_etr_mode_alloc_buf(ETR_MODE_CATU, drvdata,
> > etr_buf, node, pages);
> > +done:
> > if (rc) {
> > kfree(etr_buf);
> > return ERR_PTR(rc);
> > @@ -1829,6 +1908,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",
> > };
> >
> > @@ -1847,6 +1927,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;
> > }
> > @@ -1874,6 +1957,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 8dcb426ac3e7..10dba9f7d76a 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.
> > @@ -188,6 +200,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.
>
> minor nit: Please align the comment like the other fields.
>

Ack.

Linu Cherian.

2024-01-12 23:10:34

by Linu Cherian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v6 1/7] dt-bindings: arm: coresight-tmc: Add "memory-region" property

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Friday, January 5, 2024 3:24 PM
> To: Linu Cherian <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; George Cherian <[email protected]>; Rob
> Herring <[email protected]>
> Subject: [EXT] Re: [PATCH v6 1/7] dt-bindings: arm: coresight-tmc: Add
> "memory-region" property
>
> External Email
>
> ----------------------------------------------------------------------
> On 05/01/2024 06:58, Linu Cherian wrote:
> > memory-region 0: Reserved trace buffer memory
> >
> > TMC ETR: When available, use this reserved memory region for
> > trace data capture. Same region is used for trace data
> > retention after a panic or watchdog reset.
> >
> > TMC ETF: When available, use this reserved memory region for
> > trace data retention synced from internal SRAM after a panic or
> > watchdog reset.
> >
> > memory-region 1: Reserved meta data memory
> >
> > TMC ETR, ETF: When available, use this memory for register
> > snapshot retention synced from hardware registers after a panic
> > or watchdog reset.
> >
> > Reviewed-by: Rob Herring <[email protected]>
>
> ...
>
> > + memory-region:
> > + items:
> > + - description: Reserved trace buffer memory for ETR and ETF sinks.
> > + For ETR, this reserved memory region is used for trace data capture.
> > + Same region is used for trace data retention as well after a panic
> > + or watchdog reset.
> > + This reserved memory region is used as trace buffer or used for
> trace
> > + data retention only if specifically selected by the user in sysfs
> > + interface.
> > + The default memory usage models for ETR in sysfs/perf modes are
> > + otherwise unaltered.
> > +
> > + For ETF, this reserved memory region is used by default for
> > + retention of trace data synced from internal SRAM after a panic
> > + or watchdog reset.
> > + - description: Reserved meta data memory. Used for ETR and ETF sinks
> > + for storing metadata.
>
> If there is going to be new version/resend, add a blank line here. Keep the Rb
> tag.
>

Okay. Will take care in next version.

Linu Cherian.

2024-01-16 01:40:01

by Linu Cherian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn

Hi Mike,

> -----Original Message-----
> From: Mike Leach <[email protected]>
> Sent: Friday, January 5, 2024 9:41 PM
> To: Linu Cherian <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; George Cherian <[email protected]>
> Subject: [EXT] Re: [PATCH v6 6/7] coresight: tmc: Stop trace capture on FlIn
>
> External Email
>
> ----------------------------------------------------------------------
> Hi,
>
> On Fri, 5 Jan 2024 at 05:59, Linu Cherian <[email protected]> wrote:
> >
> > Configure TMC ETR and ETF to flush and stop trace capture on FlIn
> > event. As a side effect, do manual flush only if auto flush fails.
> >
> > Signed-off-by: Linu Cherian <[email protected]>
> > ---
> > Changelog from v5:
> > * No changes
> >
> > drivers/hwtracing/coresight/coresight-tmc-etf.c | 10 ++++++++--
> > drivers/hwtracing/coresight/coresight-tmc-etr.c | 10 ++++++++--
> > drivers/hwtracing/coresight/coresight-tmc.h | 3 +++
> > 3 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > index 72c2315f4e23..57a9a9300d36 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> > @@ -34,7 +34,7 @@ static int __tmc_etb_enable_hw(struct tmc_drvdata
> *drvdata)
> > writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base +
> TMC_MODE);
> > writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> > TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
> > - TMC_FFCR_TRIGON_TRIGIN,
> > + TMC_FFCR_TRIGON_TRIGIN |
> > + TMC_FFCR_STOP_ON_FLUSH,
> > drvdata->base + TMC_FFCR);
> >
>
> This is a problem. Setting TMC_FFCR_STOP_ON_FLUSH changes the
> fundamentals of trigger event handling. Without this bit ETM can generate
> multiple event + triggers which are then embedded into the formatted trace
> stream for later search.
> With this new bit the capture will stop on the first event. Setting this bit should
> be dependent on the mode being set to ETR_MODE_RESRV
>
>
>

Okay, will add a configuration dependency here.


> > writel_relaxed(drvdata->trigger_cntr, drvdata->base +
> > TMC_TRG); @@ -615,7 +615,13 @@ static int tmc_panic_sync_etf(struct
> coresight_device *csdev)
> > if (val != TMC_MODE_CIRCULAR_BUFFER)
> > goto out;
> >
> > - tmc_flush_and_stop(drvdata);
> > + val = readl(drvdata->base + TMC_FFSR);
> > + /* Do manual flush and stop only if its not auto-stopped */
> > + if (!(val & TMC_FFSR_FT_STOPPED)) {
> > + dev_info(&csdev->dev,
> > + "%s: Triggering manual flush\n", __func__);
> > + tmc_flush_and_stop(drvdata);
> > + }
> >
> Is there some reason to believe that the stop on flush will not work?
>

CTI misconfiguration or even skipping the CTI configuration could be the reasons.

> Using this conditional skips the tmc_wait_for_tmcready() called by
> tmc_flush_and_stop() if the formatter is stopped - which is a different
> condition test on a different register.
>

Okay got it. While add a tmc_wait_for_tmcready() for the event based flush as well.

> Why is this block of code not in the patch that introduces the
> tmc_panic_sync_etf()

Ack. Will merge the above block of code alone to the patch that introduces tmc_panic_sync_xxx

>
> > /* Sync registers from hardware to metadata region */
> > mdata->sts = csdev_access_relaxed_read32(csa, TMC_STS); diff
> > --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > index ab7521bbb2f5..4b3c7ec7f62b 100644
> > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> > @@ -1113,7 +1113,7 @@ static int __tmc_etr_enable_hw(struct
> > tmc_drvdata *drvdata)
> >
> > writel_relaxed(TMC_FFCR_EN_FMT | TMC_FFCR_EN_TI |
> > TMC_FFCR_FON_FLIN | TMC_FFCR_FON_TRIG_EVT |
> > - TMC_FFCR_TRIGON_TRIGIN,
> > + TMC_FFCR_TRIGON_TRIGIN |
> > + TMC_FFCR_STOP_ON_FLUSH,
> > drvdata->base + TMC_FFCR);
> > writel_relaxed(drvdata->trigger_cntr, drvdata->base + TMC_TRG);
> > tmc_enable_hw(drvdata);
> > @@ -1846,7 +1846,13 @@ static int tmc_panic_sync_etr(struct
> coresight_device *csdev)
> > if (!(val & TMC_CTL_CAPT_EN))
> > goto out;
> >
> > - tmc_flush_and_stop(drvdata);
> > + val = readl(drvdata->base + TMC_FFSR);
> > + /* Do manual flush and stop only if its not auto-stopped */
> > + if (!(val & TMC_FFSR_FT_STOPPED)) {
> > + dev_info(&csdev->dev,
> > + "%s: Triggering manual flush\n", __func__);
> > + tmc_flush_and_stop(drvdata);
> > + }
> >
>
> Above comments for etf apply equally to etr.

Ack.

Linu Cherian

2024-01-17 11:05:10

by Linu Cherian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v6 7/7] coresight: config: Add preloaded configuration

Hi Suzuki,

> -----Original Message-----
> From: Suzuki K Poulose <[email protected]>
> Sent: Friday, January 12, 2024 7:34 PM
> To: Linu Cherian <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; Sunil Kovvuri Goutham
> <[email protected]>; George Cherian <[email protected]>
> Subject: [EXT] Re: [PATCH v6 7/7] coresight: config: Add preloaded
> configuration
>
> External Email
>
> ----------------------------------------------------------------------
> On 05/01/2024 05:58, Linu Cherian wrote:
> > Add a preloaded configuration for generating external trigger on
> > address match. This can be used by CTI and ETR blocks to stop trace
> > capture on kernel panic.
> >
> > Kernel address for panic function to be programmed as below.
> >
> > $cd /config/cs-syscfg/features/gen_etrig/params
> > $echo <panic_address> > address/value
> >
> > Signed-off-by: Linu Cherian <[email protected]>
> > ---
> > Changelog from v5:
> > * No changes
> >
> > drivers/hwtracing/coresight/Makefile | 2 +-
> > .../coresight/coresight-cfg-preload.c | 2 +
> > .../coresight/coresight-cfg-preload.h | 2 +
> > .../hwtracing/coresight/coresight-cfg-pstop.c | 83 +++++++++++++++++++
> > 4 files changed, 88 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/hwtracing/coresight/coresight-cfg-pstop.c
> >
> > diff --git a/drivers/hwtracing/coresight/Makefile
> > b/drivers/hwtracing/coresight/Makefile
> > index 995d3b2c76df..68b15c8d9462 100644
> > --- a/drivers/hwtracing/coresight/Makefile
> > +++ b/drivers/hwtracing/coresight/Makefile
> > @@ -5,7 +5,7 @@
> > obj-$(CONFIG_CORESIGHT) += coresight.o
> > coresight-y := coresight-core.o coresight-etm-perf.o coresight-platform.o \
> > coresight-sysfs.o coresight-syscfg.o coresight-config.o \
> > - coresight-cfg-preload.o coresight-cfg-afdo.o \
> > + coresight-cfg-preload.o coresight-cfg-afdo.o coresight-cfg-
> pstop.o
> > +\
> > coresight-syscfg-configfs.o coresight-trace-id.o
> > obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o
> > coresight-tmc-y := coresight-tmc-core.o coresight-tmc-etf.o \ diff
> > --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > index e237a4edfa09..4980e68483c5 100644
> > --- a/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > @@ -13,6 +13,7 @@
> > static struct cscfg_feature_desc *preload_feats[] = {
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > &strobe_etm4x,
> > + &gen_etrig_etm4x,
> > #endif
> > NULL
> > };
> > @@ -20,6 +21,7 @@ static struct cscfg_feature_desc *preload_feats[] = {
> > static struct cscfg_config_desc *preload_cfgs[] = {
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > &afdo_etm4x,
> > + &pstop_etm4x,
> > #endif
> > NULL
> > };
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > index 21299e175477..291ba530a6a5 100644
> > --- a/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.h
> > @@ -10,4 +10,6 @@
> > #if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > extern struct cscfg_feature_desc strobe_etm4x;
> > extern struct cscfg_config_desc afdo_etm4x;
> > +extern struct cscfg_feature_desc gen_etrig_etm4x; extern struct
> > +cscfg_config_desc pstop_etm4x;
> > #endif
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> > b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> > new file mode 100644
> > index 000000000000..037d6773fab8
> > --- /dev/null
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
> > @@ -0,0 +1,83 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright(C) 2023 Marvell.
> > + * Based on coresight-cfg-afdo.c
> > + */
> > +
> > +#include "coresight-config.h"
> > +
> > +/* ETMv4 includes and features */
> > +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
> > +#include "coresight-etm4x-cfg.h"
> > +
> > +/* preload configurations and features */
> > +
> > +/* preload in features for ETMv4 */
> > +
> > +/* panic_stop feature */
> > +static struct cscfg_parameter_desc gen_etrig_params[] = {
> > + {
> > + .name = "address",
> > + .value = 0x0,
> > + },
> > +};
> > +
> > +static struct cscfg_regval_desc gen_etrig_regs[] = {
> > + /* resource selector */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCRSCTLRn(2),
> > + .hw_info = ETM4_CFG_RES_SEL,
> > + .val32 = 0x40001,
> > + },
> > + /* single address comparator */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE |
> CS_CFG_REG_TYPE_VAL_64BIT |
> > + CS_CFG_REG_TYPE_VAL_PARAM,
> > + .offset = TRCACVRn(0),
> > + .val32 = 0x0,
> > + },
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCACATRn(0),
> > + .val64 = 0xf00,
> > + },
> > + /* Driver external output[0] with comparator out */
> > + {
> > + .type = CS_CFG_REG_TYPE_RESOURCE,
> > + .offset = TRCEVENTCTL0R,
> > + .val32 = 0x2,
> > + },
> > + /* end of regs */
> > +};
> > +
> > +struct cscfg_feature_desc gen_etrig_etm4x = {
> > + .name = "gen_etrig",
> > + .description = "Generate external trigger on address match\n"
> > + "parameter \'address\': address of kernel address\n",
> > + .match_flags = CS_CFG_MATCH_CLASS_SRC_ETM4,
> > + .nr_params = ARRAY_SIZE(gen_etrig_params),
> > + .params_desc = gen_etrig_params,
> > + .nr_regs = ARRAY_SIZE(gen_etrig_regs),
> > + .regs_desc = gen_etrig_regs,
> > +};
> > +
> > +/* create a panic stop configuration */
> > +
> > +/* the total number of parameters in used features */
> > +#define PSTOP_NR_PARAMS ARRAY_SIZE(gen_etrig_params)
> > +
> > +static const char *pstop_ref_names[] = {
> > + "gen_etrig",
> > +};
> > +
> > +struct cscfg_config_desc pstop_etm4x = {
> > + .name = "panicstop",
> > + .description = "Stop ETM on kernel panic\n",
>
> Since this is actually generic, i.e., Stop trace on a Kernel address, we could
> rename this ? Or why don't we pre-populate the address of "panic"
> at load time. That way the user doesn't have to figure out the kernel address
> (e.g., if KASLR is enabled)
>

Yeah, will hardcode that parameter value to panic symbol. That will avoid the hassle of user figuring out the kernel address for panic.

diff --git a/drivers/hwtracing/coresight/coresight-cfg-pstop.c b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
index 037d6773fab8..c2bfbd07bfaf 100644
--- a/drivers/hwtracing/coresight/coresight-cfg-pstop.c
+++ b/drivers/hwtracing/coresight/coresight-cfg-pstop.c
@@ -18,7 +18,7 @@
static struct cscfg_parameter_desc gen_etrig_params[] = {
{
.name = "address",
- .value = 0x0,
+ .value = (u64)panic,
},
};


Linu Cherian.