[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20250912151314.3761026-1-sean.anderson@linux.dev>
Date: Fri, 12 Sep 2025 11:13:14 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Suzuki K Poulose <suzuki.poulose@....com>,
coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org
Cc: Yeoreum Yun <yeoreum.yun@....com>,
Mike Leach <mike.leach@...aro.org>,
Leo Yan <leo.yan@....com>,
Linu Cherian <lcherian@...vell.com>,
linux-kernel@...r.kernel.org,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
James Clark <james.clark@...aro.org>,
Sean Anderson <sean.anderson@...ux.dev>
Subject: [PATCH v3] coresight: Fix possible deadlock in coresight_panic_cb
coresight_panic_cb is called with interrupts disabled during panics.
However, bus_for_each_dev calls bus_to_subsys which takes
bus_kset->list_lock without disabling IRQs. This may cause a deadlock.
Instead of adding a panic API for coresight devices, just have coresight
drivers that need a panic callback register directly with the panic
notifier.
Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks")
Fixes: 6dbcbcfc4496 ("coresight: tmc: Enable panic sync handling")
Signed-off-by: Sean Anderson <sean.anderson@...ux.dev>
---
Changes in v3:
- Rewrite patch to remove the panic sync API entirely
Changes in v2:
- Add a comment describing csdev_lock/list
- Consolidate list removal in coresight_device_release
drivers/hwtracing/coresight/coresight-core.c | 42 -------------------
.../hwtracing/coresight/coresight-tmc-core.c | 20 ++++++++-
.../hwtracing/coresight/coresight-tmc-etf.c | 12 +++---
.../hwtracing/coresight/coresight-tmc-etr.c | 12 +++---
drivers/hwtracing/coresight/coresight-tmc.h | 10 ++++-
include/linux/coresight.h | 11 +----
6 files changed, 38 insertions(+), 69 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index fa758cc21827..297af270bf3d 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -19,7 +19,6 @@
#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"
@@ -1563,36 +1562,6 @@ const struct bus_type coresight_bustype = {
.name = "coresight",
};
-static int coresight_panic_sync(struct device *dev, void *data)
-{
- int mode;
- struct coresight_device *csdev;
-
- /* Run through panic sync handlers for all enabled devices */
- csdev = container_of(dev, struct coresight_device, dev);
- mode = coresight_get_mode(csdev);
-
- if ((mode == CS_MODE_SYSFS) || (mode == CS_MODE_PERF)) {
- if (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;
@@ -1605,20 +1574,11 @@ 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);
- if (ret)
- goto exit_perf;
-
/* initialise the coresight syscfg API */
ret = cscfg_init();
if (!ret)
return 0;
- atomic_notifier_chain_unregister(&panic_notifier_list,
- &coresight_notifier);
-exit_perf:
etm_perf_exit();
exit_bus_unregister:
bus_unregister(&coresight_bustype);
@@ -1628,8 +1588,6 @@ 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/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index 88afb16bb6be..012f76dbf7d3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -16,6 +16,7 @@
#include <linux/fs.h>
#include <linux/miscdevice.h>
#include <linux/mutex.h>
+#include <linux/panic_notifier.h>
#include <linux/property.h>
#include <linux/uaccess.h>
#include <linux/slab.h>
@@ -834,6 +835,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
desc.type = CORESIGHT_DEV_TYPE_SINK;
desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_SYSMEM;
desc.ops = &tmc_etr_cs_ops;
+ drvdata->panic_notifier.notifier_call = tmc_panic_sync_etr;
ret = tmc_etr_setup_caps(dev, devid, &desc.access);
if (ret)
goto out;
@@ -847,6 +849,7 @@ static int __tmc_probe(struct device *dev, struct resource *res)
desc.subtype.sink_subtype = CORESIGHT_DEV_SUBTYPE_SINK_BUFFER;
desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_FIFO;
desc.ops = &tmc_etf_cs_ops;
+ drvdata->panic_notifier.notifier_call = tmc_panic_sync_etf;
dev_list = &etf_devs;
break;
default:
@@ -869,11 +872,18 @@ static int __tmc_probe(struct device *dev, struct resource *res)
dev->platform_data = pdata;
desc.pdata = pdata;
+ if (drvdata->panic_notifier.notifier_call) {
+ ret = atomic_notifier_chain_register(&panic_notifier_list,
+ &drvdata->panic_notifier);
+ if (ret)
+ goto out;
+ }
+
coresight_clear_self_claim_tag(&desc.access);
drvdata->csdev = coresight_register(&desc);
if (IS_ERR(drvdata->csdev)) {
ret = PTR_ERR(drvdata->csdev);
- goto out;
+ goto err;
}
drvdata->miscdev.name = desc.name;
@@ -882,7 +892,10 @@ static int __tmc_probe(struct device *dev, struct resource *res)
ret = misc_register(&drvdata->miscdev);
if (ret) {
coresight_unregister(drvdata->csdev);
- goto out;
+err:
+ if (drvdata->panic_notifier.notifier_call)
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &drvdata->panic_notifier);
}
out:
@@ -944,6 +957,9 @@ static void __tmc_remove(struct device *dev)
if (drvdata->crashdev.fops)
misc_deregister(&drvdata->crashdev);
coresight_unregister(drvdata->csdev);
+ if (drvdata->panic_notifier.notifier_call)
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &drvdata->panic_notifier);
}
static void tmc_remove(struct amba_device *adev)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 0f45ab5e5249..9c8dfd5436ab 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -601,11 +601,14 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
return to_read;
}
-static int tmc_panic_sync_etf(struct coresight_device *csdev)
+int tmc_panic_sync_etf(struct notifier_block *nb, unsigned long action,
+ void *data)
{
u32 val;
struct tmc_crash_metadata *mdata;
- struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ struct tmc_drvdata *drvdata = container_of(nb, struct tmc_drvdata,
+ panic_notifier);
+ struct coresight_device *csdev = drvdata->csdev;
mdata = (struct tmc_crash_metadata *)drvdata->crash_mdata.vaddr;
@@ -689,10 +692,6 @@ 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,
};
@@ -700,7 +699,6 @@ 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 b07fcdb3fe1a..91061462cd92 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1824,11 +1824,14 @@ static int tmc_disable_etr_sink(struct coresight_device *csdev)
return 0;
}
-static int tmc_panic_sync_etr(struct coresight_device *csdev)
+int tmc_panic_sync_etr(struct notifier_block *nb, unsigned long action,
+ void *data)
{
u32 val;
struct tmc_crash_metadata *mdata;
- struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+ struct tmc_drvdata *drvdata = container_of(nb, struct tmc_drvdata,
+ panic_notifier);
+ struct coresight_device *csdev = drvdata->csdev;
mdata = (struct tmc_crash_metadata *)drvdata->crash_mdata.vaddr;
@@ -1900,13 +1903,8 @@ 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 6541a27a018e..d8e6bf5526c6 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -242,6 +242,7 @@ struct tmc_resrv_buf {
* (after crash) by default.
* @crash_mdata: Reserved memory for storing tmc crash metadata.
* Used by ETR/ETF.
+ * @panic_notifier: Notifier block used to clean up during a panic
*/
struct tmc_drvdata {
struct clk *pclk;
@@ -271,6 +272,7 @@ struct tmc_drvdata {
struct etr_buf *perf_buf;
struct tmc_resrv_buf resrv_buf;
struct tmc_resrv_buf crash_mdata;
+ struct notifier_block panic_notifier;
};
struct etr_buf_operations {
@@ -322,18 +324,24 @@ void tmc_disable_hw(struct tmc_drvdata *drvdata);
u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
int tmc_read_prepare_crashdata(struct tmc_drvdata *drvdata);
+struct notifier_block;
+
/* ETB/ETF functions */
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata);
+int tmc_panic_sync_etf(struct notifier_block *nb, unsigned long action,
+ void *data);
extern const struct coresight_ops tmc_etb_cs_ops;
extern const struct coresight_ops tmc_etf_cs_ops;
-
ssize_t tmc_etb_get_sysfs_trace(struct tmc_drvdata *drvdata,
loff_t pos, size_t len, char **bufpp);
+
/* ETR functions */
int tmc_read_prepare_etr(struct tmc_drvdata *drvdata);
int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata);
void tmc_etr_disable_hw(struct tmc_drvdata *drvdata);
+int tmc_panic_sync_etr(struct notifier_block *nb, unsigned long action,
+ void *data);
extern const struct coresight_ops tmc_etr_cs_ops;
ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
loff_t pos, size_t len, char **bufpp);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 4ac65c68bbf4..64e12ffd507b 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -302,6 +302,7 @@ struct coresight_device {
/* system configuration and feature lists */
struct list_head feature_csdev_list;
struct list_head config_csdev_list;
+ struct list_head csdev_list;
raw_spinlock_t cscfg_csdev_lock;
void *active_cscfg_ctxt;
};
@@ -427,15 +428,6 @@ struct coresight_ops_helper {
};
-/**
- * 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 {
int (*trace_id)(struct coresight_device *csdev, enum cs_mode mode,
struct coresight_device *sink);
@@ -443,7 +435,6 @@ struct coresight_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;
};
static inline u32 csdev_access_relaxed_read32(struct csdev_access *csa,
--
2.35.1.1320.gc452695387.dirty
Powered by blists - more mailing lists