All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE
@ 2024-04-12 14:26 Suzuki K Poulose
  2024-04-12 14:26 ` [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore Suzuki K Poulose
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-12 14:26 UTC (permalink / raw
  To: coresight
  Cc: james.clark, mike.leach, anshuman.khandual, yabinc,
	linux-arm-kernel, Suzuki K Poulose

Our CPUidle state save restore has some bugs which are triggered for an ETM with
system instructions (e.g. ETE). This is a series of fixes to address them.

Changes since v1:
 - Fix inverted check for drvdata->csdev for Patch 1 (Yabin)
 - Remove Data trace register access macro cases (Mike Leach)
 - Fix QFILT field definition (Yabin)
 - (New patch) Fix Resource pair register access, reported-by Yabin

Suzuki K Poulose (4):
  coresight: etm4x: Do not hardcode IOMEM access for register restore
  coresight: etm4x: Do not save/restore Data trace control registers
  coresight: etm4x: Safe access for TRCQCLTR
  coresight: etm4x: Fix access to resource selector registers

 .../coresight/coresight-etm4x-core.c          | 26 +++++++++-------
 drivers/hwtracing/coresight/coresight-etm4x.h | 31 ++-----------------
 2 files changed, 17 insertions(+), 40 deletions(-)

-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore
  2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
@ 2024-04-12 14:26 ` Suzuki K Poulose
  2024-04-12 21:27   ` Yabin Cui
  2024-04-12 14:27 ` [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers Suzuki K Poulose
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-12 14:26 UTC (permalink / raw
  To: coresight
  Cc: james.clark, mike.leach, anshuman.khandual, yabinc,
	linux-arm-kernel, Suzuki K Poulose

When we restore the register state for ETM4x, while coming back
from CPU idle, we hardcode IOMEM access. This is wrong and could
blow up for an ETM with system instructions access (and for ETE).

Fixes: f5bd523690d2 ("coresight: etm4x: Convert all register accesses")
Reported-by: Yabin Cui <yabinc@google.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index c2ca4a02dfce..7bd849e28953 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1843,8 +1843,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 {
 	int i;
 	struct etmv4_save_state *state = drvdata->save_state;
-	struct csdev_access tmp_csa = CSDEV_ACCESS_IOMEM(drvdata->base);
-	struct csdev_access *csa = &tmp_csa;
+	struct csdev_access *csa = &drvdata->csdev->access;
+
+	if (WARN_ON(!drvdata->csdev))
+		return;
 
 	etm4_cs_unlock(drvdata, csa);
 	etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers
  2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
  2024-04-12 14:26 ` [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore Suzuki K Poulose
@ 2024-04-12 14:27 ` Suzuki K Poulose
  2024-04-12 21:28   ` Yabin Cui
  2024-04-12 14:27 ` [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR Suzuki K Poulose
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-12 14:27 UTC (permalink / raw
  To: coresight
  Cc: james.clark, mike.leach, anshuman.khandual, yabinc,
	linux-arm-kernel, Suzuki K Poulose

ETM4x doesn't support Data trace on A class CPUs. As such do not access the
Data trace control registers during CPU idle. This could cause problems for
ETE. While at it, remove all references to the Data trace control registers.

Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Reported-by: Yabin Cui <yabinc@google.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../coresight/coresight-etm4x-core.c          |  6 ----
 drivers/hwtracing/coresight/coresight-etm4x.h | 28 -------------------
 2 files changed, 34 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 7bd849e28953..81df753d8c15 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1739,9 +1739,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR);
 	if (drvdata->nr_pe_cmp)
 		state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
-	state->trcvdctlr = etm4x_read32(csa, TRCVDCTLR);
-	state->trcvdsacctlr = etm4x_read32(csa, TRCVDSACCTLR);
-	state->trcvdarcctlr = etm4x_read32(csa, TRCVDARCCTLR);
 
 	for (i = 0; i < drvdata->nrseqstate - 1; i++)
 		state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
@@ -1872,9 +1869,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR);
 	if (drvdata->nr_pe_cmp)
 		etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
-	etm4x_relaxed_write32(csa, state->trcvdctlr, TRCVDCTLR);
-	etm4x_relaxed_write32(csa, state->trcvdsacctlr, TRCVDSACCTLR);
-	etm4x_relaxed_write32(csa, state->trcvdarcctlr, TRCVDARCCTLR);
 
 	for (i = 0; i < drvdata->nrseqstate - 1; i++)
 		etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 9ea678bc2e8e..9e430f72bbd6 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -43,9 +43,6 @@
 #define TRCVIIECTLR			0x084
 #define TRCVISSCTLR			0x088
 #define TRCVIPCSSCTLR			0x08C
-#define TRCVDCTLR			0x0A0
-#define TRCVDSACCTLR			0x0A4
-#define TRCVDARCCTLR			0x0A8
 /* Derived resources registers */
 #define TRCSEQEVRn(n)			(0x100 + (n * 4)) /* n = 0-2 */
 #define TRCSEQRSTEVR			0x118
@@ -90,9 +87,6 @@
 /* Address Comparator registers n = 0-15 */
 #define TRCACVRn(n)			(0x400 + (n * 8))
 #define TRCACATRn(n)			(0x480 + (n * 8))
-/* Data Value Comparator Value registers, n = 0-7 */
-#define TRCDVCVRn(n)			(0x500 + (n * 16))
-#define TRCDVCMRn(n)			(0x580 + (n * 16))
 /* ContextID/Virtual ContextID comparators, n = 0-7 */
 #define TRCCIDCVRn(n)			(0x600 + (n * 8))
 #define TRCVMIDCVRn(n)			(0x640 + (n * 8))
@@ -272,9 +266,6 @@
 /* List of registers accessible via System instructions */
 #define ETM4x_ONLY_SYSREG_LIST(op, val)		\
 	CASE_##op((val), TRCPROCSELR)		\
-	CASE_##op((val), TRCVDCTLR)		\
-	CASE_##op((val), TRCVDSACCTLR)		\
-	CASE_##op((val), TRCVDARCCTLR)		\
 	CASE_##op((val), TRCOSLAR)
 
 #define ETM_COMMON_SYSREG_LIST(op, val)		\
@@ -422,22 +413,6 @@
 	CASE_##op((val), TRCACATRn(13))		\
 	CASE_##op((val), TRCACATRn(14))		\
 	CASE_##op((val), TRCACATRn(15))		\
-	CASE_##op((val), TRCDVCVRn(0))		\
-	CASE_##op((val), TRCDVCVRn(1))		\
-	CASE_##op((val), TRCDVCVRn(2))		\
-	CASE_##op((val), TRCDVCVRn(3))		\
-	CASE_##op((val), TRCDVCVRn(4))		\
-	CASE_##op((val), TRCDVCVRn(5))		\
-	CASE_##op((val), TRCDVCVRn(6))		\
-	CASE_##op((val), TRCDVCVRn(7))		\
-	CASE_##op((val), TRCDVCMRn(0))		\
-	CASE_##op((val), TRCDVCMRn(1))		\
-	CASE_##op((val), TRCDVCMRn(2))		\
-	CASE_##op((val), TRCDVCMRn(3))		\
-	CASE_##op((val), TRCDVCMRn(4))		\
-	CASE_##op((val), TRCDVCMRn(5))		\
-	CASE_##op((val), TRCDVCMRn(6))		\
-	CASE_##op((val), TRCDVCMRn(7))		\
 	CASE_##op((val), TRCCIDCVRn(0))		\
 	CASE_##op((val), TRCCIDCVRn(1))		\
 	CASE_##op((val), TRCCIDCVRn(2))		\
@@ -907,9 +882,6 @@ struct etmv4_save_state {
 	u32	trcviiectlr;
 	u32	trcvissctlr;
 	u32	trcvipcssctlr;
-	u32	trcvdctlr;
-	u32	trcvdsacctlr;
-	u32	trcvdarcctlr;
 
 	u32	trcseqevr[ETM_MAX_SEQ_STATES];
 	u32	trcseqrstevr;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR
  2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
  2024-04-12 14:26 ` [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore Suzuki K Poulose
  2024-04-12 14:27 ` [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers Suzuki K Poulose
@ 2024-04-12 14:27 ` Suzuki K Poulose
  2024-04-12 21:28   ` Yabin Cui
  2024-04-12 14:27 ` [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers Suzuki K Poulose
  2024-04-22 10:25 ` [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
  4 siblings, 1 reply; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-12 14:27 UTC (permalink / raw
  To: coresight
  Cc: james.clark, mike.leach, anshuman.khandual, yabinc,
	linux-arm-kernel, Suzuki K Poulose

ETM4x implements TRCQCLTR only when the Q elements are supported
and the Q element filtering is supported (TRCIDR0.QFILT). Access
to the register otherwise could be fatal. Fix this by tracking the
availability, like the others.

Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Reported-by: Yabin Cui <yabinc@google.com>
Reviewed-by: Mike Leach <mike.leach@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 ++++++--
 drivers/hwtracing/coresight/coresight-etm4x.h      | 3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 81df753d8c15..b4b84f2317cd 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1240,6 +1240,8 @@ static void etm4_init_arch_data(void *info)
 	drvdata->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0);
 	/* QSUPP, bits[16:15] Q element support field */
 	drvdata->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0);
+	if (drvdata->q_support)
+		drvdata->q_filt = !!(etmidr0 & TRCIDR0_QFILT);
 	/* TSSIZE, bits[28:24] Global timestamp size field */
 	drvdata->ts_size = FIELD_GET(TRCIDR0_TSSIZE_MASK, etmidr0);
 
@@ -1732,7 +1734,8 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 	state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
 	state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR);
 	state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR);
-	state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
+	if (drvdata->q_filt)
+		state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
 
 	state->trcvictlr = etm4x_read32(csa, TRCVICTLR);
 	state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR);
@@ -1862,7 +1865,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 	etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
 	etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR);
 	etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR);
-	etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
+	if (drvdata->q_filt)
+		etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
 
 	etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR);
 	etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
index 9e430f72bbd6..9e9165f62e81 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.h
+++ b/drivers/hwtracing/coresight/coresight-etm4x.h
@@ -135,6 +135,7 @@
 #define TRCIDR0_TRCCCI				BIT(7)
 #define TRCIDR0_RETSTACK			BIT(9)
 #define TRCIDR0_NUMEVENT_MASK			GENMASK(11, 10)
+#define TRCIDR0_QFILT				BIT(14)
 #define TRCIDR0_QSUPP_MASK			GENMASK(16, 15)
 #define TRCIDR0_TSSIZE_MASK			GENMASK(28, 24)
 
@@ -954,6 +955,7 @@ struct etmv4_save_state {
  * @os_unlock:  True if access to management registers is allowed.
  * @instrp0:	Tracing of load and store instructions
  *		as P0 elements is supported.
+ * @q_filt:	Q element filtering support, if Q elements are supported.
  * @trcbb:	Indicates if the trace unit supports branch broadcast tracing.
  * @trccond:	If the trace unit supports conditional
  *		instruction tracing.
@@ -1016,6 +1018,7 @@ struct etmv4_drvdata {
 	bool				boot_enable;
 	bool				os_unlock;
 	bool				instrp0;
+	bool				q_filt;
 	bool				trcbb;
 	bool				trccond;
 	bool				retstack;
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers
  2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
                   ` (2 preceding siblings ...)
  2024-04-12 14:27 ` [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR Suzuki K Poulose
@ 2024-04-12 14:27 ` Suzuki K Poulose
  2024-04-12 21:28   ` Yabin Cui
  2024-04-16 11:49   ` Mike Leach
  2024-04-22 10:25 ` [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
  4 siblings, 2 replies; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-12 14:27 UTC (permalink / raw
  To: coresight
  Cc: james.clark, mike.leach, anshuman.khandual, yabinc,
	linux-arm-kernel, Suzuki K Poulose

Resource selector pair 0 is always implemented and reserved. We must not
touch it, even during save/restore for CPU Idle. Rest of the driver is
well behaved. Fix the offending ones.

Reported-by: Yabin Cui <yabinc@google.com>
Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index b4b84f2317cd..577cf485e761 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1758,7 +1758,8 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
 		state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
 	}
 
-	for (i = 0; i < drvdata->nr_resource * 2; i++)
+	/* Resource selector pair 0 is reserved */
+	for (i = 2; i < drvdata->nr_resource * 2; i++)
 		state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
 
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
@@ -1889,7 +1890,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
 		etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
 	}
 
-	for (i = 0; i < drvdata->nr_resource * 2; i++)
+	/* Resource selector pair 0 is reserved */
+	for (i = 2; i < drvdata->nr_resource * 2; i++)
 		etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
 
 	for (i = 0; i < drvdata->nr_ss_cmp; i++) {
-- 
2.34.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore
  2024-04-12 14:26 ` [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore Suzuki K Poulose
@ 2024-04-12 21:27   ` Yabin Cui
  0 siblings, 0 replies; 11+ messages in thread
From: Yabin Cui @ 2024-04-12 21:27 UTC (permalink / raw
  To: Suzuki K Poulose
  Cc: coresight, james.clark, mike.leach, anshuman.khandual,
	linux-arm-kernel

Tested-by: Yabin Cui <yabinc@google.com>

On Fri, Apr 12, 2024 at 7:27 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> When we restore the register state for ETM4x, while coming back
> from CPU idle, we hardcode IOMEM access. This is wrong and could
> blow up for an ETM with system instructions access (and for ETE).
>
> Fixes: f5bd523690d2 ("coresight: etm4x: Convert all register accesses")
> Reported-by: Yabin Cui <yabinc@google.com>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index c2ca4a02dfce..7bd849e28953 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1843,8 +1843,10 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>  {
>         int i;
>         struct etmv4_save_state *state = drvdata->save_state;
> -       struct csdev_access tmp_csa = CSDEV_ACCESS_IOMEM(drvdata->base);
> -       struct csdev_access *csa = &tmp_csa;
> +       struct csdev_access *csa = &drvdata->csdev->access;
> +
> +       if (WARN_ON(!drvdata->csdev))
> +               return;
>
>         etm4_cs_unlock(drvdata, csa);
>         etm4x_relaxed_write32(csa, state->trcclaimset, TRCCLAIMSET);
> --
> 2.34.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers
  2024-04-12 14:27 ` [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers Suzuki K Poulose
@ 2024-04-12 21:28   ` Yabin Cui
  0 siblings, 0 replies; 11+ messages in thread
From: Yabin Cui @ 2024-04-12 21:28 UTC (permalink / raw
  To: Suzuki K Poulose
  Cc: coresight, james.clark, mike.leach, anshuman.khandual,
	linux-arm-kernel

Tested-by: Yabin Cui <yabinc@google.com>

On Fri, Apr 12, 2024 at 7:27 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> ETM4x doesn't support Data trace on A class CPUs. As such do not access the
> Data trace control registers during CPU idle. This could cause problems for
> ETE. While at it, remove all references to the Data trace control registers.
>
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Reported-by: Yabin Cui <yabinc@google.com>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  .../coresight/coresight-etm4x-core.c          |  6 ----
>  drivers/hwtracing/coresight/coresight-etm4x.h | 28 -------------------
>  2 files changed, 34 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 7bd849e28953..81df753d8c15 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1739,9 +1739,6 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>         state->trcvissctlr = etm4x_read32(csa, TRCVISSCTLR);
>         if (drvdata->nr_pe_cmp)
>                 state->trcvipcssctlr = etm4x_read32(csa, TRCVIPCSSCTLR);
> -       state->trcvdctlr = etm4x_read32(csa, TRCVDCTLR);
> -       state->trcvdsacctlr = etm4x_read32(csa, TRCVDSACCTLR);
> -       state->trcvdarcctlr = etm4x_read32(csa, TRCVDARCCTLR);
>
>         for (i = 0; i < drvdata->nrseqstate - 1; i++)
>                 state->trcseqevr[i] = etm4x_read32(csa, TRCSEQEVRn(i));
> @@ -1872,9 +1869,6 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>         etm4x_relaxed_write32(csa, state->trcvissctlr, TRCVISSCTLR);
>         if (drvdata->nr_pe_cmp)
>                 etm4x_relaxed_write32(csa, state->trcvipcssctlr, TRCVIPCSSCTLR);
> -       etm4x_relaxed_write32(csa, state->trcvdctlr, TRCVDCTLR);
> -       etm4x_relaxed_write32(csa, state->trcvdsacctlr, TRCVDSACCTLR);
> -       etm4x_relaxed_write32(csa, state->trcvdarcctlr, TRCVDARCCTLR);
>
>         for (i = 0; i < drvdata->nrseqstate - 1; i++)
>                 etm4x_relaxed_write32(csa, state->trcseqevr[i], TRCSEQEVRn(i));
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 9ea678bc2e8e..9e430f72bbd6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -43,9 +43,6 @@
>  #define TRCVIIECTLR                    0x084
>  #define TRCVISSCTLR                    0x088
>  #define TRCVIPCSSCTLR                  0x08C
> -#define TRCVDCTLR                      0x0A0
> -#define TRCVDSACCTLR                   0x0A4
> -#define TRCVDARCCTLR                   0x0A8
>  /* Derived resources registers */
>  #define TRCSEQEVRn(n)                  (0x100 + (n * 4)) /* n = 0-2 */
>  #define TRCSEQRSTEVR                   0x118
> @@ -90,9 +87,6 @@
>  /* Address Comparator registers n = 0-15 */
>  #define TRCACVRn(n)                    (0x400 + (n * 8))
>  #define TRCACATRn(n)                   (0x480 + (n * 8))
> -/* Data Value Comparator Value registers, n = 0-7 */
> -#define TRCDVCVRn(n)                   (0x500 + (n * 16))
> -#define TRCDVCMRn(n)                   (0x580 + (n * 16))
>  /* ContextID/Virtual ContextID comparators, n = 0-7 */
>  #define TRCCIDCVRn(n)                  (0x600 + (n * 8))
>  #define TRCVMIDCVRn(n)                 (0x640 + (n * 8))
> @@ -272,9 +266,6 @@
>  /* List of registers accessible via System instructions */
>  #define ETM4x_ONLY_SYSREG_LIST(op, val)                \
>         CASE_##op((val), TRCPROCSELR)           \
> -       CASE_##op((val), TRCVDCTLR)             \
> -       CASE_##op((val), TRCVDSACCTLR)          \
> -       CASE_##op((val), TRCVDARCCTLR)          \
>         CASE_##op((val), TRCOSLAR)
>
>  #define ETM_COMMON_SYSREG_LIST(op, val)                \
> @@ -422,22 +413,6 @@
>         CASE_##op((val), TRCACATRn(13))         \
>         CASE_##op((val), TRCACATRn(14))         \
>         CASE_##op((val), TRCACATRn(15))         \
> -       CASE_##op((val), TRCDVCVRn(0))          \
> -       CASE_##op((val), TRCDVCVRn(1))          \
> -       CASE_##op((val), TRCDVCVRn(2))          \
> -       CASE_##op((val), TRCDVCVRn(3))          \
> -       CASE_##op((val), TRCDVCVRn(4))          \
> -       CASE_##op((val), TRCDVCVRn(5))          \
> -       CASE_##op((val), TRCDVCVRn(6))          \
> -       CASE_##op((val), TRCDVCVRn(7))          \
> -       CASE_##op((val), TRCDVCMRn(0))          \
> -       CASE_##op((val), TRCDVCMRn(1))          \
> -       CASE_##op((val), TRCDVCMRn(2))          \
> -       CASE_##op((val), TRCDVCMRn(3))          \
> -       CASE_##op((val), TRCDVCMRn(4))          \
> -       CASE_##op((val), TRCDVCMRn(5))          \
> -       CASE_##op((val), TRCDVCMRn(6))          \
> -       CASE_##op((val), TRCDVCMRn(7))          \
>         CASE_##op((val), TRCCIDCVRn(0))         \
>         CASE_##op((val), TRCCIDCVRn(1))         \
>         CASE_##op((val), TRCCIDCVRn(2))         \
> @@ -907,9 +882,6 @@ struct etmv4_save_state {
>         u32     trcviiectlr;
>         u32     trcvissctlr;
>         u32     trcvipcssctlr;
> -       u32     trcvdctlr;
> -       u32     trcvdsacctlr;
> -       u32     trcvdarcctlr;
>
>         u32     trcseqevr[ETM_MAX_SEQ_STATES];
>         u32     trcseqrstevr;
> --
> 2.34.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR
  2024-04-12 14:27 ` [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR Suzuki K Poulose
@ 2024-04-12 21:28   ` Yabin Cui
  0 siblings, 0 replies; 11+ messages in thread
From: Yabin Cui @ 2024-04-12 21:28 UTC (permalink / raw
  To: Suzuki K Poulose
  Cc: coresight, james.clark, mike.leach, anshuman.khandual,
	linux-arm-kernel

Tested-by: Yabin Cui <yabinc@google.com>

On Fri, Apr 12, 2024 at 7:27 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> ETM4x implements TRCQCLTR only when the Q elements are supported
> and the Q element filtering is supported (TRCIDR0.QFILT). Access
> to the register otherwise could be fatal. Fix this by tracking the
> availability, like the others.
>
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Reported-by: Yabin Cui <yabinc@google.com>
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 ++++++--
>  drivers/hwtracing/coresight/coresight-etm4x.h      | 3 +++
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 81df753d8c15..b4b84f2317cd 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1240,6 +1240,8 @@ static void etm4_init_arch_data(void *info)
>         drvdata->nr_event = FIELD_GET(TRCIDR0_NUMEVENT_MASK, etmidr0);
>         /* QSUPP, bits[16:15] Q element support field */
>         drvdata->q_support = FIELD_GET(TRCIDR0_QSUPP_MASK, etmidr0);
> +       if (drvdata->q_support)
> +               drvdata->q_filt = !!(etmidr0 & TRCIDR0_QFILT);
>         /* TSSIZE, bits[28:24] Global timestamp size field */
>         drvdata->ts_size = FIELD_GET(TRCIDR0_TSSIZE_MASK, etmidr0);
>
> @@ -1732,7 +1734,8 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>         state->trcccctlr = etm4x_read32(csa, TRCCCCTLR);
>         state->trcbbctlr = etm4x_read32(csa, TRCBBCTLR);
>         state->trctraceidr = etm4x_read32(csa, TRCTRACEIDR);
> -       state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
> +       if (drvdata->q_filt)
> +               state->trcqctlr = etm4x_read32(csa, TRCQCTLR);
>
>         state->trcvictlr = etm4x_read32(csa, TRCVICTLR);
>         state->trcviiectlr = etm4x_read32(csa, TRCVIIECTLR);
> @@ -1862,7 +1865,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>         etm4x_relaxed_write32(csa, state->trcccctlr, TRCCCCTLR);
>         etm4x_relaxed_write32(csa, state->trcbbctlr, TRCBBCTLR);
>         etm4x_relaxed_write32(csa, state->trctraceidr, TRCTRACEIDR);
> -       etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
> +       if (drvdata->q_filt)
> +               etm4x_relaxed_write32(csa, state->trcqctlr, TRCQCTLR);
>
>         etm4x_relaxed_write32(csa, state->trcvictlr, TRCVICTLR);
>         etm4x_relaxed_write32(csa, state->trcviiectlr, TRCVIIECTLR);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
> index 9e430f72bbd6..9e9165f62e81 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> @@ -135,6 +135,7 @@
>  #define TRCIDR0_TRCCCI                         BIT(7)
>  #define TRCIDR0_RETSTACK                       BIT(9)
>  #define TRCIDR0_NUMEVENT_MASK                  GENMASK(11, 10)
> +#define TRCIDR0_QFILT                          BIT(14)
>  #define TRCIDR0_QSUPP_MASK                     GENMASK(16, 15)
>  #define TRCIDR0_TSSIZE_MASK                    GENMASK(28, 24)
>
> @@ -954,6 +955,7 @@ struct etmv4_save_state {
>   * @os_unlock:  True if access to management registers is allowed.
>   * @instrp0:   Tracing of load and store instructions
>   *             as P0 elements is supported.
> + * @q_filt:    Q element filtering support, if Q elements are supported.
>   * @trcbb:     Indicates if the trace unit supports branch broadcast tracing.
>   * @trccond:   If the trace unit supports conditional
>   *             instruction tracing.
> @@ -1016,6 +1018,7 @@ struct etmv4_drvdata {
>         bool                            boot_enable;
>         bool                            os_unlock;
>         bool                            instrp0;
> +       bool                            q_filt;
>         bool                            trcbb;
>         bool                            trccond;
>         bool                            retstack;
> --
> 2.34.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers
  2024-04-12 14:27 ` [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers Suzuki K Poulose
@ 2024-04-12 21:28   ` Yabin Cui
  2024-04-16 11:49   ` Mike Leach
  1 sibling, 0 replies; 11+ messages in thread
From: Yabin Cui @ 2024-04-12 21:28 UTC (permalink / raw
  To: Suzuki K Poulose
  Cc: coresight, james.clark, mike.leach, anshuman.khandual,
	linux-arm-kernel

Tested-by: Yabin Cui <yabinc@google.com>

On Fri, Apr 12, 2024 at 7:27 AM Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Resource selector pair 0 is always implemented and reserved. We must not
> touch it, even during save/restore for CPU Idle. Rest of the driver is
> well behaved. Fix the offending ones.
>
> Reported-by: Yabin Cui <yabinc@google.com>
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index b4b84f2317cd..577cf485e761 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1758,7 +1758,8 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>                 state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
>         }
>
> -       for (i = 0; i < drvdata->nr_resource * 2; i++)
> +       /* Resource selector pair 0 is reserved */
> +       for (i = 2; i < drvdata->nr_resource * 2; i++)
>                 state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
>
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> @@ -1889,7 +1890,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>                 etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
>         }
>
> -       for (i = 0; i < drvdata->nr_resource * 2; i++)
> +       /* Resource selector pair 0 is reserved */
> +       for (i = 2; i < drvdata->nr_resource * 2; i++)
>                 etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
>
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> --
> 2.34.1
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers
  2024-04-12 14:27 ` [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers Suzuki K Poulose
  2024-04-12 21:28   ` Yabin Cui
@ 2024-04-16 11:49   ` Mike Leach
  1 sibling, 0 replies; 11+ messages in thread
From: Mike Leach @ 2024-04-16 11:49 UTC (permalink / raw
  To: Suzuki K Poulose
  Cc: coresight, james.clark, anshuman.khandual, yabinc,
	linux-arm-kernel

On Fri, 12 Apr 2024 at 15:27, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Resource selector pair 0 is always implemented and reserved. We must not
> touch it, even during save/restore for CPU Idle. Rest of the driver is
> well behaved. Fix the offending ones.
>
> Reported-by: Yabin Cui <yabinc@google.com>
> Fixes: f188b5e76aae ("coresight: etm4x: Save/restore state across CPU low power states")
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index b4b84f2317cd..577cf485e761 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1758,7 +1758,8 @@ static int __etm4_cpu_save(struct etmv4_drvdata *drvdata)
>                 state->trccntvr[i] = etm4x_read32(csa, TRCCNTVRn(i));
>         }
>
> -       for (i = 0; i < drvdata->nr_resource * 2; i++)
> +       /* Resource selector pair 0 is reserved */
> +       for (i = 2; i < drvdata->nr_resource * 2; i++)
>                 state->trcrsctlr[i] = etm4x_read32(csa, TRCRSCTLRn(i));
>
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> @@ -1889,7 +1890,8 @@ static void __etm4_cpu_restore(struct etmv4_drvdata *drvdata)
>                 etm4x_relaxed_write32(csa, state->trccntvr[i], TRCCNTVRn(i));
>         }
>
> -       for (i = 0; i < drvdata->nr_resource * 2; i++)
> +       /* Resource selector pair 0 is reserved */
> +       for (i = 2; i < drvdata->nr_resource * 2; i++)
>                 etm4x_relaxed_write32(csa, state->trcrsctlr[i], TRCRSCTLRn(i));
>
>         for (i = 0; i < drvdata->nr_ss_cmp; i++) {
> --
> 2.34.1
>

Reviewed-by: Mike Leach <mike.leach@linaro.org>


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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE
  2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
                   ` (3 preceding siblings ...)
  2024-04-12 14:27 ` [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers Suzuki K Poulose
@ 2024-04-22 10:25 ` Suzuki K Poulose
  4 siblings, 0 replies; 11+ messages in thread
From: Suzuki K Poulose @ 2024-04-22 10:25 UTC (permalink / raw
  To: coresight, Suzuki K Poulose
  Cc: mike.leach, yabinc, anshuman.khandual, linux-arm-kernel,
	james.clark

On Fri, 12 Apr 2024 15:26:58 +0100, Suzuki K Poulose wrote:
> Our CPUidle state save restore has some bugs which are triggered for an ETM with
> system instructions (e.g. ETE). This is a series of fixes to address them.
> 
> Changes since v1:
>  - Fix inverted check for drvdata->csdev for Patch 1 (Yabin)
>  - Remove Data trace register access macro cases (Mike Leach)
>  - Fix QFILT field definition (Yabin)
>  - (New patch) Fix Resource pair register access, reported-by Yabin
> 
> [...]

Applied, thanks!

[1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore
      https://git.kernel.org/coresight/c/1e7ba33fa591de1cf60afffcabb45600b3607025
[2/4] coresight: etm4x: Do not save/restore Data trace control registers
      https://git.kernel.org/coresight/c/5eb3a0c2c52368cb9902e9a6ea04888e093c487d
[3/4] coresight: etm4x: Safe access for TRCQCLTR
      https://git.kernel.org/coresight/c/46bf8d7cd8530eca607379033b9bc4ac5590a0cd
[4/4] coresight: etm4x: Fix access to resource selector registers
      https://git.kernel.org/coresight/c/d6fc00d0f640d6010b51054aa8b0fd191177dbc9

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-04-22 10:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-12 14:26 [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose
2024-04-12 14:26 ` [PATCH v2 1/4] coresight: etm4x: Do not hardcode IOMEM access for register restore Suzuki K Poulose
2024-04-12 21:27   ` Yabin Cui
2024-04-12 14:27 ` [PATCH v2 2/4] coresight: etm4x: Do not save/restore Data trace control registers Suzuki K Poulose
2024-04-12 21:28   ` Yabin Cui
2024-04-12 14:27 ` [PATCH v2 3/4] coresight: etm4x: Safe access for TRCQCLTR Suzuki K Poulose
2024-04-12 21:28   ` Yabin Cui
2024-04-12 14:27 ` [PATCH v2 4/4] coresight: etm4x: Fix access to resource selector registers Suzuki K Poulose
2024-04-12 21:28   ` Yabin Cui
2024-04-16 11:49   ` Mike Leach
2024-04-22 10:25 ` [PATCH v2 0/4] coresight: etm4x: Fix CPU idle PM support for ETE Suzuki K Poulose

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.