* Re: [PATCH] coresight: trbe: Add ISB after TRBLIMITR write
2025-06-09 10:19 [PATCH] coresight: trbe: Add ISB after TRBLIMITR write James Clark
@ 2025-06-09 10:28 ` Yeoreum Yun
2025-06-09 11:14 ` Leo Yan
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Yeoreum Yun @ 2025-06-09 10:28 UTC (permalink / raw)
To: James Clark
Cc: Suzuki K Poulose, Mike Leach, Alexander Shishkin, Marc Zyngier,
coresight, linux-arm-kernel, linux-kernel
Look good to me.
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
> DEN0154 states that hardware will be allowed to ignore writes to TRB*
> registers while the trace buffer is enabled. Add an ISB to ensure that
> it's disabled before clearing the other registers.
>
> This is purely defensive because it's expected that arm_trbe_disable()
> would be called before teardown which has the required ISB.
>
> Fixes: a2b579c41fe9 ("coresight: trbe: Remove redundant disable call")
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 8267dd1a2130..10f3fb401edf 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -257,6 +257,7 @@ static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
> static void trbe_reset_local(struct trbe_cpudata *cpudata)
> {
> write_sysreg_s(0, SYS_TRBLIMITR_EL1);
> + isb();
> trbe_drain_buffer();
> write_sysreg_s(0, SYS_TRBPTR_EL1);
> write_sysreg_s(0, SYS_TRBBASER_EL1);
>
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250609-james-cs-trblimitr-isb-523f20d874d6
>
> Best regards,
> --
> James Clark <james.clark@linaro.org>
>
>
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] coresight: trbe: Add ISB after TRBLIMITR write
2025-06-09 10:19 [PATCH] coresight: trbe: Add ISB after TRBLIMITR write James Clark
2025-06-09 10:28 ` Yeoreum Yun
@ 2025-06-09 11:14 ` Leo Yan
2025-06-10 9:40 ` Anshuman Khandual
2025-07-09 10:02 ` Suzuki K Poulose
3 siblings, 0 replies; 5+ messages in thread
From: Leo Yan @ 2025-06-09 11:14 UTC (permalink / raw)
To: James Clark
Cc: Suzuki K Poulose, Mike Leach, Alexander Shishkin, Marc Zyngier,
coresight, linux-arm-kernel, linux-kernel
On Mon, Jun 09, 2025 at 11:19:05AM +0100, James Clark wrote:
> DEN0154 states that hardware will be allowed to ignore writes to TRB*
> registers while the trace buffer is enabled. Add an ISB to ensure that
> it's disabled before clearing the other registers.
>
> This is purely defensive because it's expected that arm_trbe_disable()
> would be called before teardown which has the required ISB.
>
> Fixes: a2b579c41fe9 ("coresight: trbe: Remove redundant disable call")
> Signed-off-by: James Clark <james.clark@linaro.org>
Reviewed-by: Leo Yan <leo.yan@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 8267dd1a2130..10f3fb401edf 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -257,6 +257,7 @@ static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
> static void trbe_reset_local(struct trbe_cpudata *cpudata)
> {
> write_sysreg_s(0, SYS_TRBLIMITR_EL1);
> + isb();
> trbe_drain_buffer();
> write_sysreg_s(0, SYS_TRBPTR_EL1);
> write_sysreg_s(0, SYS_TRBBASER_EL1);
>
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250609-james-cs-trblimitr-isb-523f20d874d6
>
> Best regards,
> --
> James Clark <james.clark@linaro.org>
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] coresight: trbe: Add ISB after TRBLIMITR write
2025-06-09 10:19 [PATCH] coresight: trbe: Add ISB after TRBLIMITR write James Clark
2025-06-09 10:28 ` Yeoreum Yun
2025-06-09 11:14 ` Leo Yan
@ 2025-06-10 9:40 ` Anshuman Khandual
2025-07-09 10:02 ` Suzuki K Poulose
3 siblings, 0 replies; 5+ messages in thread
From: Anshuman Khandual @ 2025-06-10 9:40 UTC (permalink / raw)
To: James Clark, Suzuki K Poulose, Mike Leach, Alexander Shishkin,
Marc Zyngier
Cc: coresight, linux-arm-kernel, linux-kernel
On 09/06/25 3:49 PM, James Clark wrote:
> DEN0154 states that hardware will be allowed to ignore writes to TRB*
> registers while the trace buffer is enabled. Add an ISB to ensure that
> it's disabled before clearing the other registers.
>
> This is purely defensive because it's expected that arm_trbe_disable()
> would be called before teardown which has the required ISB.
>
> Fixes: a2b579c41fe9 ("coresight: trbe: Remove redundant disable call")
> Signed-off-by: James Clark <james.clark@linaro.org>
Agreed. An isb() barrier is required to ensure that the TRBE is disabled
even after clearing TRBLIMITR_EL1 and before writing/clearing other TRBE
registers. Besides trbe_reset_local() seems to be the only function where
such a scenario exists.
LGTM.
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index 8267dd1a2130..10f3fb401edf 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -257,6 +257,7 @@ static void trbe_drain_and_disable_local(struct trbe_cpudata *cpudata)
> static void trbe_reset_local(struct trbe_cpudata *cpudata)
> {
> write_sysreg_s(0, SYS_TRBLIMITR_EL1);
> + isb();
> trbe_drain_buffer();
> write_sysreg_s(0, SYS_TRBPTR_EL1);
> write_sysreg_s(0, SYS_TRBBASER_EL1);
>
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250609-james-cs-trblimitr-isb-523f20d874d6
>
> Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] coresight: trbe: Add ISB after TRBLIMITR write
2025-06-09 10:19 [PATCH] coresight: trbe: Add ISB after TRBLIMITR write James Clark
` (2 preceding siblings ...)
2025-06-10 9:40 ` Anshuman Khandual
@ 2025-07-09 10:02 ` Suzuki K Poulose
3 siblings, 0 replies; 5+ messages in thread
From: Suzuki K Poulose @ 2025-07-09 10:02 UTC (permalink / raw)
To: Mike Leach, Alexander Shishkin, Marc Zyngier, James Clark
Cc: Suzuki K Poulose, coresight, linux-arm-kernel, linux-kernel
On Mon, 09 Jun 2025 11:19:05 +0100, James Clark wrote:
> DEN0154 states that hardware will be allowed to ignore writes to TRB*
> registers while the trace buffer is enabled. Add an ISB to ensure that
> it's disabled before clearing the other registers.
>
> This is purely defensive because it's expected that arm_trbe_disable()
> would be called before teardown which has the required ISB.
>
> [...]
Applied, thanks!
[1/1] coresight: trbe: Add ISB after TRBLIMITR write
https://git.kernel.org/coresight/c/ba3264a1
Best regards,
--
Suzuki K Poulose <suzuki.poulose@arm.com>
^ permalink raw reply [flat|nested] 5+ messages in thread