On 08/04/2025 8:59 pm, Yabin Cui wrote:
When tracing ETM data on multiple CPUs concurrently via the perf interface, the CATU device is shared across different CPU paths. This can lead to race conditions when multiple CPUs attempt to enable or disable the CATU device simultaneously.
To address these race conditions, this patch introduces the following changes:
The enable and disable operations for the CATU device are not reentrant. Therefore, a spinlock is added to ensure that only one CPU can enable or disable a given CATU device at any point in time.
A reference counter is used to manage the enable/disable state of the CATU device. The device is enabled when the first CPU requires it and is only disabled when the last CPU finishes using it. This ensures the device remains active as long as at least one CPU needs it.
Signed-off-by: Yabin Cui yabinc@google.com
drivers/hwtracing/coresight/coresight-catu.c | 25 +++++++++++++------- drivers/hwtracing/coresight/coresight-catu.h | 1 + 2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c index fa170c966bc3..30b78b2f8adb 100644 --- a/drivers/hwtracing/coresight/coresight-catu.c +++ b/drivers/hwtracing/coresight/coresight-catu.c @@ -458,12 +458,17 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, enum cs_mode cs_mode, static int catu_enable(struct coresight_device *csdev, enum cs_mode mode, void *data) {
- int rc;
- int rc = 0; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);
- guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock);
Very minor nit only because you need to resend anyway, but there should be a newline between the variable definitions and the code. Not sure why checkpatch doesn't warn here.
- CS_UNLOCK(catu_drvdata->base);
- rc = catu_enable_hw(catu_drvdata, mode, data);
- CS_LOCK(catu_drvdata->base);
- if (csdev->refcnt == 0) {
CS_UNLOCK(catu_drvdata->base);rc = catu_enable_hw(catu_drvdata, mode, data);CS_LOCK(catu_drvdata->base);- }
- if (!rc)
csdev->refcnt++;@@ -486,12 +491,15 @@ static int catu_disable_hw(struct catu_drvdata *drvdata) static int catu_disable(struct coresight_device *csdev, void *__unused) {
- int rc;
- int rc = 0; struct catu_drvdata *catu_drvdata = csdev_to_catu_drvdata(csdev);
- guard(raw_spinlock_irqsave)(&catu_drvdata->spinlock);
- CS_UNLOCK(catu_drvdata->base);
- rc = catu_disable_hw(catu_drvdata);
- CS_LOCK(catu_drvdata->base);
- if (--csdev->refcnt == 0) {
Hopefully this never underflows if disable is called again after a failed enable. We could add a WARN_ON() but I think this is a general case and not specific to these patches so is probably better to do later as separate change.
Reviewed-by: James Clark james.clark@linaro.org