From 321577251c5e5edd2d734100bb25f8d428ef6ccf Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Thu, 17 Oct 2019 17:29:16 -0700 Subject: [PATCH] BACKPORT:FROMGIT: coresight: Serialize enabling/disabling a link device. Backport: coresight-dynamic-replicator.c and coresight-replicator.c has been merged upstream into coresight-replicator.c, in commit 455328b1772a190e27fe7ef3a2416dfee6234317. So backport the change to both of the files. 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 Signed-off-by: Mathieu Poirier Cc: stable # 5.3 Link: https://lore.kernel.org/r/20191104181251.26732-14-mathieu.poirier@linaro.org Signed-off-by: Greg Kroah-Hartman (cherry picked from commit edda32dabedb01f98b9d7b9a4492c13357834bbe git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-next) Bug: 140266694 Change-Id: I59a1a36c3e4771a57910f6f437fd2ce5f3cfab6e Signed-off-by: Yabin Cui --- .../coresight/coresight-dynamic-replicator.c | 58 +++++++++++++++---- .../hwtracing/coresight/coresight-funnel.c | 28 +++++++-- .../coresight/coresight-replicator.c | 6 +- .../hwtracing/coresight/coresight-tmc-etf.c | 26 ++++++--- drivers/hwtracing/coresight/coresight.c | 45 +++++--------- 5 files changed, 108 insertions(+), 55 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c index 299667b887fc..a48215b9fc19 100644 --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c @@ -26,12 +26,14 @@ * @dev: the device entity associated with this component * @atclk: optional clock for the core parts of the replicator. * @csdev: component vitals needed by the framework + * @spinlock: serialize enable/disable operations. */ struct replicator_state { void __iomem *base; struct device *dev; struct clk *atclk; struct coresight_device *csdev; + spinlock_t spinlock; }; /* @@ -50,12 +52,11 @@ static void replicator_reset(struct replicator_state *drvdata) CS_LOCK(drvdata->base); } -static int replicator_enable(struct coresight_device *csdev, int inport, - int outport) +static int dynamic_replicator_enable(struct replicator_state *drvdata, + int inport, int outport) { int rc = 0; u32 reg; - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); switch (outport) { case 0: @@ -76,21 +77,40 @@ static int replicator_enable(struct coresight_device *csdev, int inport, rc = coresight_claim_device_unlocked(drvdata->base); /* Ensure that the outport is enabled. */ - if (!rc) { + if (!rc) writel_relaxed(0x00, drvdata->base + reg); - dev_dbg(drvdata->dev, "REPLICATOR enabled\n"); - } - CS_LOCK(drvdata->base); return rc; } -static void replicator_disable(struct coresight_device *csdev, int inport, - int outport) +static int replicator_enable(struct coresight_device *csdev, int inport, + int outport) +{ + int rc = 0; + struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); + unsigned long flags; + bool first_enable = false; + + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_read(&csdev->refcnt[outport]) == 0) { + 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; +} + +static void dynamic_replicator_disable(struct replicator_state *drvdata, + int inport, int outport) { u32 reg; - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent); switch (outport) { case 0: @@ -113,8 +133,24 @@ static void replicator_disable(struct coresight_device *csdev, int inport, (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff)) coresight_disclaim_device_unlocked(drvdata->base); CS_LOCK(drvdata->base); +} - dev_dbg(drvdata->dev, "REPLICATOR disabled\n"); +static void replicator_disable(struct coresight_device *csdev, int inport, + int outport) +{ + struct replicator_state *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) { + dynamic_replicator_disable(drvdata, inport, outport); + last_disable = true; + } + spin_unlock_irqrestore(&drvdata->spinlock, flags); + + if (last_disable) + dev_dbg(drvdata->dev, "REPLICATOR disabled\n"); } static const struct coresight_ops_link replicator_link_ops = { diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c index 927925151509..f5275e19a99c 100644 --- a/drivers/hwtracing/coresight/coresight-funnel.c +++ b/drivers/hwtracing/coresight/coresight-funnel.c @@ -34,6 +34,7 @@ * @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; @@ -41,6 +42,7 @@ struct funnel_drvdata { struct clk *atclk; struct coresight_device *csdev; unsigned long priority; + spinlock_t spinlock; }; static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port) @@ -73,10 +75,20 @@ static int funnel_enable(struct coresight_device *csdev, int inport, { int rc; struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); + unsigned long flags; + bool first_enable = false; - rc = funnel_enable_hw(drvdata, inport); - + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_read(&csdev->refcnt[inport]) == 0) { + rc = 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(drvdata->dev, "FUNNEL inport %d enabled\n", inport); return rc; } @@ -102,10 +114,18 @@ 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; - funnel_disable_hw(drvdata, inport); + spin_lock_irqsave(&drvdata->spinlock, flags); + if (atomic_dec_return(&csdev->refcnt[inport]) == 0) { + funnel_disable_hw(drvdata, inport); + last_disable = true; + } + spin_unlock_irqrestore(&drvdata->spinlock, flags); - dev_dbg(drvdata->dev, "FUNNEL inport %d disabled\n", inport); + if (last_disable) + dev_dbg(drvdata->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 feac98315471..344e73e36e14 100644 --- a/drivers/hwtracing/coresight/coresight-replicator.c +++ b/drivers/hwtracing/coresight/coresight-replicator.c @@ -35,7 +35,8 @@ static int replicator_enable(struct coresight_device *csdev, int inport, { struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - dev_dbg(drvdata->dev, "REPLICATOR enabled\n"); + if (atomic_inc_return(&csdev->refcnt[outport]) == 1) + dev_dbg(drvdata->dev, "REPLICATOR enabled\n"); return 0; } @@ -44,7 +45,8 @@ static void replicator_disable(struct coresight_device *csdev, int inport, { struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - dev_dbg(drvdata->dev, "REPLICATOR disabled\n"); + if (atomic_dec_return(&csdev->refcnt[outport]) == 0) + dev_dbg(drvdata->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 0064e8211d9f..1dc796f4f2a2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -314,9 +314,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) { @@ -324,12 +325,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(drvdata->dev, "TMC-ETF enabled\n"); return ret; } @@ -339,6 +346,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) { @@ -346,11 +354,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(drvdata->dev, "TMC-ETF disabled\n"); + if (last_disable) + dev_dbg(drvdata->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 bfad28dfc26d..1e32efd14b33 100644 --- 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 (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_MERG && inport < 0) + return inport; + if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT && outport < 0) + return outport; - if (refport < 0) - return refport; + if (link_ops(csdev)->enable) + ret = link_ops(csdev)->enable(csdev, inport, outport); + if (!ret) + csdev->enable = true; - 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; - - 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->nr_inport; } else if (link_subtype == CORESIGHT_DEV_SUBTYPE_LINK_SPLIT) { - refport = outport; nr_conns = csdev->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)