From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
Leo Yan <leo.yan@linaro.org>, Mike Leach <mike.leach@linaro.org>
Subject: Re: [PATCH v2] coresight: etm4x: Handle accesses to TRCSTALLCTLR
Date: Wed, 27 Jan 2021 10:43:40 -0700 [thread overview]
Message-ID: <20210127174340.GA1162729@xps15> (raw)
In-Reply-To: <20210127120032.3611851-1-suzuki.poulose@arm.com>
Good day,
On Wed, Jan 27, 2021 at 12:00:32PM +0000, Suzuki K Poulose wrote:
> TRCSTALLCTLR register is only implemented if
>
> TRCIDR3.STALLCTL == 0b1
>
> Make sure the driver touches the register only it is implemented.
>
> Cc: stable@vger.kernel.org
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v1:
> - No change to the patch, fixed the stable email address and
> added usual reviewers.
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 ++++++---
> drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 3 +++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index b40e3c2bf818..814b49dae0c7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -367,7 +367,8 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, 0x0, TRCAUXCTLR);
> etm4x_relaxed_write32(csa, config->eventctrl0, TRCEVENTCTL0R);
> etm4x_relaxed_write32(csa, config->eventctrl1, TRCEVENTCTL1R);
> - etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> etm4x_relaxed_write32(csa, config->ts_ctrl, TRCTSCTLR);
> etm4x_relaxed_write32(csa, config->syncfreq, TRCSYNCPR);
> etm4x_relaxed_write32(csa, config->ccctlr, TRCCCCTLR);
> @@ -1545,7 +1546,8 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
> state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR);
> state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R);
> state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R);
> - state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
> state->trctsctlr = etm4x_read32(csa, TRCTSCTLR);
> state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR);
> state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
> @@ -1657,7 +1659,8 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR);
> etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R);
> etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R);
> - etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
> etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR);
> etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR);
> etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 1c490bcef3ad..cd9249fbf913 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -296,6 +296,9 @@ static ssize_t mode_store(struct device *dev,
> if (kstrtoul(buf, 16, &val))
> return -EINVAL;
>
> + if ((val & ETM_MODE_ISTALL_EN) && !drvdata->stallctl)
> + return -EINVAL;
> +
We have two choices here:
1) Follow what is already done in this function for implementation define
options like ETM_MODE_BB, ETMv4_MODE_CTXID, ETM_MODE_RETURNSTACK and others. In
that case we would have:
/* bit[8], Instruction stall bit */
if ((config->mode & ETM_MODE_ISTALL_EN) && drvdata->stallctl == true))
config->stall_ctrl |= BIT(8);
else
config->stall_ctrl &= ~BIT(8);
2) Return -EINVAL when something is not supported, like you have above. In that
case we'd have to enact the same behavior for all the options, which has the
potential of breaking user space.
I think option 1 is best.
Thanks,
Mathieu
> spin_lock(&drvdata->spinlock);
> config->mode = val & ETMv4_MODE_ALL;
>
> --
> 2.24.1
>
WARNING: multiple messages have this Message-ID (diff)
From: Mathieu Poirier <mathieu.poirier@linaro.org>
To: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: coresight@lists.linaro.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, Leo Yan <leo.yan@linaro.org>,
linux-arm-kernel@lists.infradead.org,
Mike Leach <mike.leach@linaro.org>
Subject: Re: [PATCH v2] coresight: etm4x: Handle accesses to TRCSTALLCTLR
Date: Wed, 27 Jan 2021 10:43:40 -0700 [thread overview]
Message-ID: <20210127174340.GA1162729@xps15> (raw)
In-Reply-To: <20210127120032.3611851-1-suzuki.poulose@arm.com>
Good day,
On Wed, Jan 27, 2021 at 12:00:32PM +0000, Suzuki K Poulose wrote:
> TRCSTALLCTLR register is only implemented if
>
> TRCIDR3.STALLCTL == 0b1
>
> Make sure the driver touches the register only it is implemented.
>
> Cc: stable@vger.kernel.org
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> Changes since v1:
> - No change to the patch, fixed the stable email address and
> added usual reviewers.
> ---
> drivers/hwtracing/coresight/coresight-etm4x-core.c | 9 ++++++---
> drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 3 +++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index b40e3c2bf818..814b49dae0c7 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -367,7 +367,8 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, 0x0, TRCAUXCTLR);
> etm4x_relaxed_write32(csa, config->eventctrl0, TRCEVENTCTL0R);
> etm4x_relaxed_write32(csa, config->eventctrl1, TRCEVENTCTL1R);
> - etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + etm4x_relaxed_write32(csa, config->stall_ctrl, TRCSTALLCTLR);
> etm4x_relaxed_write32(csa, config->ts_ctrl, TRCTSCTLR);
> etm4x_relaxed_write32(csa, config->syncfreq, TRCSYNCPR);
> etm4x_relaxed_write32(csa, config->ccctlr, TRCCCCTLR);
> @@ -1545,7 +1546,8 @@ static int etm4_cpu_save(struct etmv4_drvdata *drvdata)
> state->trcauxctlr = etm4x_read32(csa, TRCAUXCTLR);
> state->trceventctl0r = etm4x_read32(csa, TRCEVENTCTL0R);
> state->trceventctl1r = etm4x_read32(csa, TRCEVENTCTL1R);
> - state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + state->trcstallctlr = etm4x_read32(csa, TRCSTALLCTLR);
> state->trctsctlr = etm4x_read32(csa, TRCTSCTLR);
> state->trcsyncpr = etm4x_read32(csa, TRCSYNCPR);
> state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
> @@ -1657,7 +1659,8 @@ static void etm4_cpu_restore(struct etmv4_drvdata *drvdata)
> etm4x_relaxed_write32(csa, state->trcauxctlr, TRCAUXCTLR);
> etm4x_relaxed_write32(csa, state->trceventctl0r, TRCEVENTCTL0R);
> etm4x_relaxed_write32(csa, state->trceventctl1r, TRCEVENTCTL1R);
> - etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
> + if (drvdata->stallctl)
> + etm4x_relaxed_write32(csa, state->trcstallctlr, TRCSTALLCTLR);
> etm4x_relaxed_write32(csa, state->trctsctlr, TRCTSCTLR);
> etm4x_relaxed_write32(csa, state->trcsyncpr, TRCSYNCPR);
> etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 1c490bcef3ad..cd9249fbf913 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -296,6 +296,9 @@ static ssize_t mode_store(struct device *dev,
> if (kstrtoul(buf, 16, &val))
> return -EINVAL;
>
> + if ((val & ETM_MODE_ISTALL_EN) && !drvdata->stallctl)
> + return -EINVAL;
> +
We have two choices here:
1) Follow what is already done in this function for implementation define
options like ETM_MODE_BB, ETMv4_MODE_CTXID, ETM_MODE_RETURNSTACK and others. In
that case we would have:
/* bit[8], Instruction stall bit */
if ((config->mode & ETM_MODE_ISTALL_EN) && drvdata->stallctl == true))
config->stall_ctrl |= BIT(8);
else
config->stall_ctrl &= ~BIT(8);
2) Return -EINVAL when something is not supported, like you have above. In that
case we'd have to enact the same behavior for all the options, which has the
potential of breaking user space.
I think option 1 is best.
Thanks,
Mathieu
> spin_lock(&drvdata->spinlock);
> config->mode = val & ETMv4_MODE_ALL;
>
> --
> 2.24.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-01-27 17:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-26 14:56 [PATCH] coresight: etm4x: Handle accesses to TRCSTALLCTLR Suzuki K Poulose
2021-01-27 12:00 ` [PATCH v2] " Suzuki K Poulose
2021-01-27 12:00 ` Suzuki K Poulose
2021-01-27 17:43 ` Mathieu Poirier [this message]
2021-01-27 17:43 ` Mathieu Poirier
2021-01-27 18:29 ` Suzuki K Poulose
2021-01-27 18:29 ` Suzuki K Poulose
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210127174340.GA1162729@xps15 \
--to=mathieu.poirier@linaro.org \
--cc=coresight@lists.linaro.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mike.leach@linaro.org \
--cc=stable@vger.kernel.org \
--cc=suzuki.poulose@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.