Re: [PATCH] coresight: Fix possible deadlock in coresight_panic_cb

Hi Sean

On 28/08/2025 19:17, Sean Anderson wrote:

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 will cause a deadlock if a panic occurs while one of the other coresight functions that uses bus_for_each_dev is running.

Maintain a separate list of coresight devices to access during a panic.

Thanks for the patch. I have a minor comment.

Fixes: 46006ceb5d02 ("coresight: core: Add provision for panic callbacks") Signed-off-by: Sean Anderson sean.anderson@linux.dev

---

drivers/hwtracing/coresight/coresight-core.c | 40 ++++++++++---------- include/linux/coresight.h | 1 + 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index fa758cc21827..1f1bf0e2bf92 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1315,6 +1315,9 @@ void coresight_release_platform_data(struct coresight_device *csdev, coresight_remove_conns_sysfs_group(csdev); } +static DEFINE_SPINLOCK(csdev_lock); +static LIST_HEAD(csdev_list);

May be add a comment here to mention why we maintain this list ?

• struct coresight_device *coresight_register(struct coresight_desc *desc) { int ret;

@@ -1374,11 +1377,16 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) goto out_unlock; }

• scoped_guard(spinlock_irq, &csdev_lock)

• list_add(&csdev->csdev_list, &csdev_list);
  

• if (csdev->type == CORESIGHT_DEV_TYPE_SINK || csdev->type == CORESIGHT_DEV_TYPE_LINKSINK) { ret = etm_perf_add_symlink_sink(csdev);

if (ret) {

• 	scoped_guard(spinlock_irq, &csdev_lock)
  

• 		list_del(&csdev->csdev_list);
  

Could this be moved to coresight_device_release(), which will be called when the device gets unregistered ? That way, you don't need it here and in coresight_unregister() too.

Rest looks good to me

Suzuki

	device_unregister(&csdev->dev);
	/*
	 * As with the above, all resources are free'd

@@ -1427,6 +1435,8 @@ void coresight_unregister(struct coresight_device *csdev) coresight_remove_conns(csdev); coresight_clear_default_sink(csdev); coresight_release_platform_data(csdev, csdev->dev.parent, csdev->pdata);

• scoped_guard(spinlock_irq, &csdev_lock)

• list_del(&csdev->csdev_list);
  

  device_unregister(&csdev->dev); } EXPORT_SYMBOL_GPL(coresight_unregister);

@@ -1563,28 +1573,20 @@ 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);
  

• struct coresight_device *csdev;
• guard(spinlock)(&csdev_lock);
• list_for_each_entry(csdev, &csdev_list, csdev_list) {

• /* Run through panic sync handlers for all enabled devices */
  

• int mode = coresight_get_mode(csdev);
  

• if ((mode == CS_MODE_SYSFS || mode == CS_MODE_PERF) &&
  

•     panic_ops(csdev))
  

• 	panic_ops(csdev)->sync(csdev);
  

• }

return 0; } diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 4ac65c68bbf4..a5e62ebd03b5 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; };