coresight: Serialize enabling/disabling a link device.

commit edda32dabedb01f98b9d7b9a4492c13357834bbe upstream.

When tracing etm data of multiple threads on multiple cpus through perf
interface, some link devices are shared between paths of different cpus.
It creates race conditions when different cpus wants to enable/disable
the same link device at the same time.

Example 1:
Two cpus want to enable different ports of a coresight funnel, thus
calling the funnel enable operation at the same time. But the funnel
enable operation isn't reentrantable.

Example 2:
For an enabled coresight dynamic replicator with refcnt=1, one cpu wants
to disable it, while another cpu wants to enable it. Ideally we still have
an enabled replicator with refcnt=1 at the end. But in reality the result
is uncertain.

Since coresight devices claim themselves when enabled for self-hosted
usage, the race conditions above usually make the link devices not usable
after many cycles.

To fix the race conditions, this patch uses spinlocks to serialize
enabling/disabling link devices.

Fixes: a06ae8609b3d ("coresight: add CoreSight core layer framework")
Signed-off-by: Yabin Cui <yabinc@google.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: stable <stable@vger.kernel.org> # 5.3
Link: https://lore.kernel.org/r/20191104181251.26732-14-mathieu.poirier@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index fa97cb9ab4f9dc6ba8e980fbcace0294e3a309f8..f4a33d2cfd7383df2f0432900f54dabb2144ea64 100644 (file)
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -37,12 +37,14 @@ DEFINE_CORESIGHT_DEVLIST(funnel_devs, "funnel");
  * @atclk:     optional clock for the core parts of the funnel.
  * @csdev:     component vitals needed by the framework.
  * @priority:  port selection order.
+ * @spinlock:  serialize enable/disable operations.
  */
 struct funnel_drvdata {
        void __iomem            *base;
        struct clk              *atclk;
        struct coresight_device *csdev;
        unsigned long           priority;
+       spinlock_t              spinlock;
 };
 
 static int dynamic_funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
@@ -75,11 +77,21 @@ static int funnel_enable(struct coresight_device *csdev, int inport,
 {
        int rc = 0;
        struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-       if (drvdata->base)
-               rc = dynamic_funnel_enable_hw(drvdata, inport);
-
+       unsigned long flags;
+       bool first_enable = false;
+
+       spin_lock_irqsave(&drvdata->spinlock, flags);
+       if (atomic_read(&csdev->refcnt[inport]) == 0) {
+               if (drvdata->base)
+                       rc = dynamic_funnel_enable_hw(drvdata, inport);
+               if (!rc)
+                       first_enable = true;
+       }
        if (!rc)
+               atomic_inc(&csdev->refcnt[inport]);
+       spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+       if (first_enable)
                dev_dbg(&csdev->dev, "FUNNEL inport %d enabled\n", inport);
        return rc;
 }
@@ -106,11 +118,19 @@ static void funnel_disable(struct coresight_device *csdev, int inport,
                           int outport)
 {
        struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+       unsigned long flags;
+       bool last_disable = false;
+
+       spin_lock_irqsave(&drvdata->spinlock, flags);
+       if (atomic_dec_return(&csdev->refcnt[inport]) == 0) {
+               if (drvdata->base)
+                       dynamic_funnel_disable_hw(drvdata, inport);
+               last_disable = true;
+       }
+       spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-       if (drvdata->base)
-               dynamic_funnel_disable_hw(drvdata, inport);
-
-       dev_dbg(&csdev->dev, "FUNNEL inport %d disabled\n", inport);
+       if (last_disable)
+               dev_dbg(&csdev->dev, "FUNNEL inport %d disabled\n", inport);
 }
 
 static const struct coresight_ops_link funnel_link_ops = {

diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
index b7d6d59d56db2a59e9b25ec4b9bdf8643edfcf0f..596c4297b03b584fbd315884f760dc1516e3e9fc 100644 (file)
--- a/drivers/hwtracing/coresight/coresight-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-replicator.c
@@ -31,11 +31,13 @@ DEFINE_CORESIGHT_DEVLIST(replicator_devs, "replicator");
  *             whether this one is programmable or not.
  * @atclk:     optional clock for the core parts of the replicator.
  * @csdev:     component vitals needed by the framework
+ * @spinlock:  serialize enable/disable operations.
  */
 struct replicator_drvdata {
        void __iomem            *base;
        struct clk              *atclk;
        struct coresight_device *csdev;
+       spinlock_t              spinlock;
 };
 
 static void dynamic_replicator_reset(struct replicator_drvdata *drvdata)
@@ -97,10 +99,22 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
 {
        int rc = 0;
        struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-
-       if (drvdata->base)
-               rc = dynamic_replicator_enable(drvdata, inport, outport);
+       unsigned long flags;
+       bool first_enable = false;
+
+       spin_lock_irqsave(&drvdata->spinlock, flags);
+       if (atomic_read(&csdev->refcnt[outport]) == 0) {
+               if (drvdata->base)
+                       rc = dynamic_replicator_enable(drvdata, inport,
+                                                      outport);
+               if (!rc)
+                       first_enable = true;
+       }
        if (!rc)
+               atomic_inc(&csdev->refcnt[outport]);
+       spin_unlock_irqrestore(&drvdata->spinlock, flags);
+
+       if (first_enable)
                dev_dbg(&csdev->dev, "REPLICATOR enabled\n");
        return rc;
 }
@@ -137,10 +151,19 @@ static void replicator_disable(struct coresight_device *csdev, int inport,
                               int outport)
 {
        struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+       unsigned long flags;
+       bool last_disable = false;
+
+       spin_lock_irqsave(&drvdata->spinlock, flags);
+       if (atomic_dec_return(&csdev->refcnt[outport]) == 0) {
+               if (drvdata->base)
+                       dynamic_replicator_disable(drvdata, inport, outport);
+               last_disable = true;
+       }
+       spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-       if (drvdata->base)
-               dynamic_replicator_disable(drvdata, inport, outport);
-       dev_dbg(&csdev->dev, "REPLICATOR disabled\n");
+       if (last_disable)
+               dev_dbg(&csdev->dev, "REPLICATOR disabled\n");
 }
 
 static const struct coresight_ops_link replicator_link_ops = {

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 23b7ff00af5ca4bf32eadc7cf3ea640c4a1e5f9c..485311a4bf5d7834700190373dda5741b3a34b24 100644 (file)
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -334,9 +334,10 @@ static int tmc_disable_etf_sink(struct coresight_device *csdev)
 static int tmc_enable_etf_link(struct coresight_device *csdev,
                               int inport, int outport)
 {
-       int ret;
+       int ret = 0;
        unsigned long flags;
        struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+       bool first_enable = false;
 
        spin_lock_irqsave(&drvdata->spinlock, flags);
        if (drvdata->reading) {
@@ -344,12 +345,18 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
                return -EBUSY;
        }
 
-       ret = tmc_etf_enable_hw(drvdata);
+       if (atomic_read(&csdev->refcnt[0]) == 0) {
+               ret = tmc_etf_enable_hw(drvdata);
+               if (!ret) {
+                       drvdata->mode = CS_MODE_SYSFS;
+                       first_enable = true;
+               }
+       }
        if (!ret)
-               drvdata->mode = CS_MODE_SYSFS;
+               atomic_inc(&csdev->refcnt[0]);
        spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-       if (!ret)
+       if (first_enable)
                dev_dbg(&csdev->dev, "TMC-ETF enabled\n");
        return ret;
 }
@@ -359,6 +366,7 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
 {
        unsigned long flags;
        struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+       bool last_disable = false;
 
        spin_lock_irqsave(&drvdata->spinlock, flags);
        if (drvdata->reading) {
@@ -366,11 +374,15 @@ static void tmc_disable_etf_link(struct coresight_device *csdev,
                return;
        }
 
-       tmc_etf_disable_hw(drvdata);
-       drvdata->mode = CS_MODE_DISABLED;
+       if (atomic_dec_return(&csdev->refcnt[0]) == 0) {
+               tmc_etf_disable_hw(drvdata);
+               drvdata->mode = CS_MODE_DISABLED;
+               last_disable = true;
+       }
        spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-       dev_dbg(&csdev->dev, "TMC-ETF disabled\n");
+       if (last_disable)
+               dev_dbg(&csdev->dev, "TMC-ETF disabled\n");
 }
 
 static void *tmc_alloc_etf_buffer(struct coresight_device *csdev,

diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index 55db77f6410b04c2af25e822a9eed3b3f3f53b59..17416419cb7169eef9bc29c21bdabc3a4c91708b 100644 (file)
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -253,9 +253,9 @@ static int coresight_enable_link(struct coresight_device *csdev,
                                 struct coresight_device *parent,
                                 struct coresight_device *child)
 {
-       int ret;
+       int ret = 0;
        int link_subtype;
-       int refport, inport, outport;
+       int inport, outport;
 
        if (!parent || !child)
                return -EINVAL;
@@ -264,29 +264,17 @@ static int coresight_enable_link(struct coresight_device *csdev,
        outport = coresight_find_link_outport(csdev, child);
        link_subtype = csdev->subtype.link_subtype;
 
-       if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG)
-               refport = inport;
-       else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT)
-               refport = outport;
-       else
-               refport = 0;
-
-       if (refport < 0)
-               return refport;
+       if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && inport < 0)
+               return inport;
+       if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT && outport < 0)
+               return outport;
 
-       if (atomic_inc_return(&csdev->refcnt[refport]) == 1) {
-               if (link_ops(csdev)->enable) {
-                       ret = link_ops(csdev)->enable(csdev, inport, outport);
-                       if (ret) {
-                               atomic_dec(&csdev->refcnt[refport]);
-                               return ret;
-                       }
-               }
-       }
-
-       csdev->enable = true;
+       if (link_ops(csdev)->enable)
+               ret = link_ops(csdev)->enable(csdev, inport, outport);
+       if (!ret)
+               csdev->enable = true;
 
-       return 0;
+       return ret;
 }
 
 static void coresight_disable_link(struct coresight_device *csdev,
@@ -295,7 +283,7 @@ static void coresight_disable_link(struct coresight_device *csdev,
 {
        int i, nr_conns;
        int link_subtype;
-       int refport, inport, outport;
+       int inport, outport;
 
        if (!parent || !child)
                return;
@@ -305,20 +293,15 @@ static void coresight_disable_link(struct coresight_device *csdev,
        link_subtype = csdev->subtype.link_subtype;
 
        if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG) {
-               refport = inport;
                nr_conns = csdev->pdata->nr_inport;
        } else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT) {
-               refport = outport;
                nr_conns = csdev->pdata->nr_outport;
        } else {
-               refport = 0;
                nr_conns = 1;
        }
 
-       if (atomic_dec_return(&csdev->refcnt[refport]) == 0) {
-               if (link_ops(csdev)->disable)
-                       link_ops(csdev)->disable(csdev, inport, outport);
-       }
+       if (link_ops(csdev)->disable)
+               link_ops(csdev)->disable(csdev, inport, outport);
 
        for (i = 0; i < nr_conns; i++)
                if (atomic_read(&csdev->refcnt[i]) != 0)