linux-kernel - Re: [PATCH v2 1/3] coresight: catu: Introduce refcount and spinlock for enabling/disabling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALJ9ZPNBGBd9OTBgedoQStXh3d2oGDGi6fUF7PXbU5qTEBRHew@mail.gmail.com>
Date: Tue, 1 Apr 2025 18:21:59 -0700
From: Yabin Cui <yabinc@...gle.com>
To: Suzuki K Poulose <suzuki.poulose@....com>, Mike Leach <mike.leach@...aro.org>, 
	James Clark <james.clark@...aro.org>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, leo.yan@....com
Subject: Re: [PATCH v2 1/3] coresight: catu: Introduce refcount and spinlock
 for enabling/disabling

On Tue, Apr 1, 2025 at 6:19 PM Yabin Cui <yabinc@...gle.com> 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:
>
> 1. 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.
>
> 2. 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@...gle.com>
> ---
>  drivers/hwtracing/coresight/coresight-catu.c | 27 ++++++++++++++------
>  drivers/hwtracing/coresight/coresight-catu.h |  1 +
>  2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
> index fa170c966bc3..b1d490dd7249 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);
>
> -       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++;
>         return rc;
>  }
>
> @@ -486,12 +491,17 @@ 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) {
> +               CS_UNLOCK(catu_drvdata->base);
> +               rc = catu_disable_hw(catu_drvdata);
> +               CS_LOCK(catu_drvdata->base);
> +       } else {
> +               rc = -EBUSY;
> +       }
>         return rc;
>  }
>
> @@ -550,6 +560,7 @@ static int __catu_probe(struct device *dev, struct resource *res)
>         dev->platform_data = pdata;
>
>         drvdata->base = base;
> +       raw_spin_lock_init(&drvdata->spinlock);
>         catu_desc.access = CSDEV_ACCESS_IOMEM(base);
>         catu_desc.pdata = pdata;
>         catu_desc.dev = dev;
> diff --git a/drivers/hwtracing/coresight/coresight-catu.h b/drivers/hwtracing/coresight/coresight-catu.h
> index 141feac1c14b..755776cd19c5 100644
> --- a/drivers/hwtracing/coresight/coresight-catu.h
> +++ b/drivers/hwtracing/coresight/coresight-catu.h
> @@ -65,6 +65,7 @@ struct catu_drvdata {
>         void __iomem *base;
>         struct coresight_device *csdev;
>         int irq;
> +       raw_spinlock_t spinlock;
>  };
>
>  #define CATU_REG32(name, offset)                                       \
> --
> 2.49.0.472.ge94155a9ec-goog
>