[PATCH v3 18/31] coresight: cti: Fix race condition by using device mode

[Mike Leach]

Hi Leo,

On Mon, 15 Sept 2025 at 11:34, Leo Yan <leo.yan at arm.com> wrote:
>
> A CTI device can be enabled as a helper in a CoreSight path or directly
> via the SysFS knob. Currently, the driver lacks protection when multiple
> flows contend for access to the same CTI device.
>
> Use the device mode to mark the CTI as in use, and reset it to the
> DISABLED state when the device is no longer needed. Any conflict will
> cause the attempt to acquire the mode to fail, ensuring the device can
> only be enabled exclusively.
>
> Fixes: 1b5b1646e63d ("coresight: Fix CTI module refcount leak by making it a helper device")
> Signed-off-by: Leo Yan <leo.yan at arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-cti-core.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 2c8bf5dbe8b8206c92ae5ea64a26c947ef5b9582..ba4635dfc2e30b4e9ae4972f91bdc6647975b719 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -87,20 +87,31 @@ void cti_write_all_hw_regs(struct cti_drvdata *drvdata)
>  }
>
>  /* write regs to hardware and enable */
> -static int cti_enable_hw(struct cti_drvdata *drvdata)
> +static int cti_enable_hw(struct cti_drvdata *drvdata, enum cs_mode mode)
>  {
>         struct cti_config *config = &drvdata->config;
> +       struct coresight_device *csdev = drvdata->csdev;
>         unsigned long flags;
>         int rc = 0;
>
>         raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> +       if (!drvdata->config.enable_req_count) {
> +               coresight_set_mode(csdev, mode);
> +       } else {
> +               /* The device has been configured with an incompatible mode */
> +               if (coresight_get_mode(csdev) != mode) {
> +                       rc = -EBUSY;
> +                       goto cti_err_not_enabled;
> +               }
> +       }
> +

Would this be simpler to check along the lines of

if (curr_mode != DISABLED && curr_mode != mode)
     rc = -EBUSY; goto err_not_enabled.

then put the set_mode() in the code that is run only once on enable,
removing the need to reset mode in error path?

>         /* no need to do anything if enabled or unpowered*/
>         if (config->hw_enabled || !config->hw_powered)
>                 goto cti_state_unchanged;
>
>         /* claim the device */
> -       rc = coresight_claim_device(drvdata->csdev);
> +       rc = coresight_claim_device(csdev);
>         if (rc)
>                 goto cti_err_not_enabled;
>
> @@ -116,6 +127,8 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
>
>         /* cannot enable due to error */
>  cti_err_not_enabled:
> +       if (!drvdata->config.enable_req_count)
> +               coresight_set_mode(csdev, CS_MODE_DISABLED);
>         raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
>         return rc;
>  }
> @@ -165,6 +178,8 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
>         if (--drvdata->config.enable_req_count > 0)
>                 goto cti_not_disabled;
>
> +       coresight_set_mode(csdev, CS_MODE_DISABLED);
> +
>         /* no need to do anything if disabled or cpu unpowered */
>         if (!config->hw_enabled || !config->hw_powered)
>                 goto cti_not_disabled;
> @@ -803,7 +818,7 @@ int cti_enable(struct coresight_device *csdev, enum cs_mode mode, void *data)
>  {
>         struct cti_drvdata *drvdata = csdev_to_cti_drvdata(csdev);
>
> -       return cti_enable_hw(drvdata);
> +       return cti_enable_hw(drvdata, mode);
>  }
>
>  int cti_disable(struct coresight_device *csdev, void *data)
>
> --
> 2.34.1
>

Regards

Mike

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