All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  3:45 ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This series makes ETM TRCCCCTRL based 'cc_threshold' user configurable via
the perf event attribute. But first, this implements an errata work around
affecting ETM TRCIDR3.CCITMIN value on certain cpus, overriding the field.

This series applies on v6.5-rc5.

Cc: Catalin Marinas <catalin.marinas@arm.com> 
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com> 
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Changes in V3:

- Added errata work around affecting TRCIDR3.CCITMIN
- Split the document update into a separate patch

Changes in V2:

https://lore.kernel.org/all/20230808074533.380537-1-anshuman.khandual@arm.com/

- s/treshhold/threshold

Changes in V1:

https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.com/

Anshuman Khandual (3):
  coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  coresight: etm: Make cycle count threshold user configurable
  Documentation: coresight: Add cc_threshold tunable

 Documentation/arch/arm64/silicon-errata.rst   | 10 ++++
 Documentation/trace/coresight/coresight.rst   |  4 ++
 .../hwtracing/coresight/coresight-etm-perf.c  |  2 +
 .../coresight/coresight-etm4x-core.c          | 49 ++++++++++++++++++-
 4 files changed, 63 insertions(+), 2 deletions(-)

-- 
2.25.1


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

* [PATCH V3 0/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  3:45 ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This series makes ETM TRCCCCTRL based 'cc_threshold' user configurable via
the perf event attribute. But first, this implements an errata work around
affecting ETM TRCIDR3.CCITMIN value on certain cpus, overriding the field.

This series applies on v6.5-rc5.

Cc: Catalin Marinas <catalin.marinas@arm.com> 
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com> 
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Changes in V3:

- Added errata work around affecting TRCIDR3.CCITMIN
- Split the document update into a separate patch

Changes in V2:

https://lore.kernel.org/all/20230808074533.380537-1-anshuman.khandual@arm.com/

- s/treshhold/threshold

Changes in V1:

https://lore.kernel.org/all/20230804044720.1478900-1-anshuman.khandual@arm.com/

Anshuman Khandual (3):
  coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  coresight: etm: Make cycle count threshold user configurable
  Documentation: coresight: Add cc_threshold tunable

 Documentation/arch/arm64/silicon-errata.rst   | 10 ++++
 Documentation/trace/coresight/coresight.rst   |  4 ++
 .../hwtracing/coresight/coresight-etm-perf.c  |  2 +
 .../coresight/coresight-etm4x-core.c          | 49 ++++++++++++++++++-
 4 files changed, 63 insertions(+), 2 deletions(-)

-- 
2.25.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] 26+ messages in thread

* [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-11  3:45 ` Anshuman Khandual
@ 2023-08-11  3:45   ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
field in ETM gets an wrong value.

If software uses the value returned by the TRCIDR3.CCITMIN register field,
then it will limit the range which could be used for programming the ETM.
In reality, the ETM could be programmed with a much smaller value than what
is indicated by the TRCIDR3.CCITMIN field and still function correctly.

If software reads the TRCIDR3.CCITMIN register field, corresponding to the
instruction trace counting minimum threshold, observe the value 0x100 or a
minimum cycle count threshold of 256. The correct value should be 0x4 or a
minimum cycle count threshold of 4.

This work arounds the problem via storing 4 in drvdata->ccitmin on affected
systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
threshold granularity.

These errata information has been updated in arch/arm64/silicon-errata.rst,
but without their corresponding configs because these have been implemented
directly in the driver.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
 .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index bedd3a1d7b42..b08f33eda5f1 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -107,6 +107,10 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A76      | #1490853        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A77      | #1491015        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
@@ -125,6 +129,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-X1       | #1502854        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
@@ -133,6 +139,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1349291        | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Neoverse-N1     | #1490853        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
@@ -141,6 +149,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Neoverse-V1     | #1619801        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | MMU-500         | #841119,826419  | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | MMU-600         | #1076982,1209401| N/A                         |
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 703b6fcbb6a5..1f3d29a639ff 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
 	drvdata->trfcr = trfcr;
 }
 
+/*
+ * The following errata on applicable cpu rangess affect the CCITMIN filed
+ * in TCRIDR3 register. Software read for the field returns 0x100 limiting
+ * the cycle threshold granularity, where as the right value should have
+ * been 0x4, which is well supported in the hardware.
+ */
+static struct midr_range etm_wrong_ccitmin_cpus[] = {
+	/* Erratum #1490853 - Cortex-A76 */
+	MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
+	/* Erratum #1490853 - Neoverse-N1 */
+	MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
+	/* Erratum #1491015 - Cortex-A77 */
+	MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
+	/* Erratum #1502854 - Cortex-X1 */
+	MIDR_REV(MIDR_CORTEX_X1, 0, 0),
+	/* Erratum #1619801 - Neoverse-V1 */
+	MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
+	{},
+};
+
+static bool etm4_work_around_wrong_ccitmin(void)
+{
+	return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
+}
+
 static void etm4_init_arch_data(void *info)
 {
 	u32 etmidr0;
@@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
 	etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
 	/* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
 	drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
+	if (etm4_work_around_wrong_ccitmin()) {
+		/*
+		 * Erratum affected cpus will read 256 as the minimum
+		 * instruction trace cycle counting threshold where as
+		 * the correct value should be 4 instead. Override the
+		 * recorded value for 'drvdata->ccitmin' to workaround
+		 * this problem.
+		 */
+		if (drvdata->ccitmin == 256)
+			drvdata->ccitmin = 4;
+	}
+
 	/* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
 	drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
 	drvdata->config.s_ex_level = drvdata->s_ex_level;
-- 
2.25.1


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

* [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-11  3:45   ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
field in ETM gets an wrong value.

If software uses the value returned by the TRCIDR3.CCITMIN register field,
then it will limit the range which could be used for programming the ETM.
In reality, the ETM could be programmed with a much smaller value than what
is indicated by the TRCIDR3.CCITMIN field and still function correctly.

If software reads the TRCIDR3.CCITMIN register field, corresponding to the
instruction trace counting minimum threshold, observe the value 0x100 or a
minimum cycle count threshold of 256. The correct value should be 0x4 or a
minimum cycle count threshold of 4.

This work arounds the problem via storing 4 in drvdata->ccitmin on affected
systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
threshold granularity.

These errata information has been updated in arch/arm64/silicon-errata.rst,
but without their corresponding configs because these have been implemented
directly in the driver.

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
 .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index bedd3a1d7b42..b08f33eda5f1 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -107,6 +107,10 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A76      | #1490853        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-A77      | #1491015        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
@@ -125,6 +129,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Cortex-X1       | #1502854        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
@@ -133,6 +139,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1349291        | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Neoverse-N1     | #1490853        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
@@ -141,6 +149,8 @@ stable kernels.
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
 +----------------+-----------------+-----------------+-----------------------------+
+| ARM            | Neoverse-V1     | #1619801        | N/A                         |
++----------------+-----------------+-----------------+-----------------------------+
 | ARM            | MMU-500         | #841119,826419  | N/A                         |
 +----------------+-----------------+-----------------+-----------------------------+
 | ARM            | MMU-600         | #1076982,1209401| N/A                         |
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 703b6fcbb6a5..1f3d29a639ff 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
 	drvdata->trfcr = trfcr;
 }
 
+/*
+ * The following errata on applicable cpu rangess affect the CCITMIN filed
+ * in TCRIDR3 register. Software read for the field returns 0x100 limiting
+ * the cycle threshold granularity, where as the right value should have
+ * been 0x4, which is well supported in the hardware.
+ */
+static struct midr_range etm_wrong_ccitmin_cpus[] = {
+	/* Erratum #1490853 - Cortex-A76 */
+	MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
+	/* Erratum #1490853 - Neoverse-N1 */
+	MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
+	/* Erratum #1491015 - Cortex-A77 */
+	MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
+	/* Erratum #1502854 - Cortex-X1 */
+	MIDR_REV(MIDR_CORTEX_X1, 0, 0),
+	/* Erratum #1619801 - Neoverse-V1 */
+	MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
+	{},
+};
+
+static bool etm4_work_around_wrong_ccitmin(void)
+{
+	return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
+}
+
 static void etm4_init_arch_data(void *info)
 {
 	u32 etmidr0;
@@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
 	etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
 	/* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
 	drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
+	if (etm4_work_around_wrong_ccitmin()) {
+		/*
+		 * Erratum affected cpus will read 256 as the minimum
+		 * instruction trace cycle counting threshold where as
+		 * the correct value should be 4 instead. Override the
+		 * recorded value for 'drvdata->ccitmin' to workaround
+		 * this problem.
+		 */
+		if (drvdata->ccitmin == 256)
+			drvdata->ccitmin = 4;
+	}
+
 	/* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
 	drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
 	drvdata->config.s_ex_level = drvdata->s_ex_level;
-- 
2.25.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] 26+ messages in thread

* [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
  2023-08-11  3:45 ` Anshuman Khandual
@ 2023-08-11  3:45   ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

Cycle counting is enabled, when requested and supported but with a default
threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
representing the minimum interval between cycle count trace packets.

This makes cycle threshold user configurable, from the user space via perf
event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
in case no explicit request. As expected it creates a sysfs file as well.

/sys/bus/event_source/devices/cs_etm/format/cc_threshold

New 'cc_threshold' uses 'event->attr.config3' as no more space is available
in 'event->attr.config1' or 'event->attr.config2'.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 5ca6278baff4..09f75dffae60 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,		"config:0-3");
 PMU_FORMAT_ATTR(sinkid,		"config2:0-31");
 /* config ID - set if a system configuration is selected */
 PMU_FORMAT_ATTR(configid,	"config2:32-63");
+PMU_FORMAT_ATTR(cc_threshold,	"config3:0-11");
 
 
 /*
@@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
 	&format_attr_preset.attr,
 	&format_attr_configid.attr,
 	&format_attr_branch_broadcast.attr,
+	&format_attr_cc_threshold.attr,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 1f3d29a639ff..ad28ee044cba 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 	struct etmv4_config *config = &drvdata->config;
 	struct perf_event_attr *attr = &event->attr;
 	unsigned long cfg_hash;
-	int preset;
+	int preset, cc_threshold;
 
 	/* Clear configuration from previous run */
 	memset(config, 0, sizeof(struct etmv4_config));
@@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 	if (attr->config & BIT(ETM_OPT_CYCACC)) {
 		config->cfg |= TRCCONFIGR_CCI;
 		/* TRM: Must program this for cycacc to work */
-		config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+		cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
+		if (cc_threshold) {
+			if (cc_threshold < drvdata->ccitmin)
+				config->ccctlr = drvdata->ccitmin;
+			else
+				config->ccctlr = cc_threshold;
+		} else {
+			config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+		}
 	}
 	if (attr->config & BIT(ETM_OPT_TS)) {
 		/*
-- 
2.25.1


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

* [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  3:45   ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:45 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

Cycle counting is enabled, when requested and supported but with a default
threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
representing the minimum interval between cycle count trace packets.

This makes cycle threshold user configurable, from the user space via perf
event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
in case no explicit request. As expected it creates a sysfs file as well.

/sys/bus/event_source/devices/cs_etm/format/cc_threshold

New 'cc_threshold' uses 'event->attr.config3' as no more space is available
in 'event->attr.config1' or 'event->attr.config2'.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
 drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
index 5ca6278baff4..09f75dffae60 100644
--- a/drivers/hwtracing/coresight/coresight-etm-perf.c
+++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
@@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,		"config:0-3");
 PMU_FORMAT_ATTR(sinkid,		"config2:0-31");
 /* config ID - set if a system configuration is selected */
 PMU_FORMAT_ATTR(configid,	"config2:32-63");
+PMU_FORMAT_ATTR(cc_threshold,	"config3:0-11");
 
 
 /*
@@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
 	&format_attr_preset.attr,
 	&format_attr_configid.attr,
 	&format_attr_branch_broadcast.attr,
+	&format_attr_cc_threshold.attr,
 	NULL,
 };
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
index 1f3d29a639ff..ad28ee044cba 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 	struct etmv4_config *config = &drvdata->config;
 	struct perf_event_attr *attr = &event->attr;
 	unsigned long cfg_hash;
-	int preset;
+	int preset, cc_threshold;
 
 	/* Clear configuration from previous run */
 	memset(config, 0, sizeof(struct etmv4_config));
@@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
 	if (attr->config & BIT(ETM_OPT_CYCACC)) {
 		config->cfg |= TRCCONFIGR_CCI;
 		/* TRM: Must program this for cycacc to work */
-		config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+		cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
+		if (cc_threshold) {
+			if (cc_threshold < drvdata->ccitmin)
+				config->ccctlr = drvdata->ccitmin;
+			else
+				config->ccctlr = cc_threshold;
+		} else {
+			config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
+		}
 	}
 	if (attr->config & BIT(ETM_OPT_TS)) {
 		/*
-- 
2.25.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] 26+ messages in thread

* [PATCH V3 3/3] Documentation: coresight: Add cc_threshold tunable
  2023-08-11  3:45 ` Anshuman Khandual
@ 2023-08-11  3:46   ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:46 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This updates config option to include 'cc_threshold' tunable value.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/trace/coresight/coresight.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
index 4a71ea6cb390..ce55adb80b82 100644
--- a/Documentation/trace/coresight/coresight.rst
+++ b/Documentation/trace/coresight/coresight.rst
@@ -624,6 +624,10 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/
    * - timestamp
      - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP
        <coresight-timestamp>`
+   * - cc_threshold
+     - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the
+       default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
+       value, as indicated via TRCIDR3.CCITMIN, then the minimum value will be used instead.
 
 How to use the STM module
 -------------------------
-- 
2.25.1


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

* [PATCH V3 3/3] Documentation: coresight: Add cc_threshold tunable
@ 2023-08-11  3:46   ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  3:46 UTC (permalink / raw)
  To: linux-arm-kernel, suzuki.poulose
  Cc: Anshuman Khandual, Catalin Marinas, Will Deacon, Mike Leach,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

This updates config option to include 'cc_threshold' tunable value.

Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mike Leach <mike.leach@linaro.org>
Cc: James Clark <james.clark@arm.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: coresight@lists.linaro.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
 Documentation/trace/coresight/coresight.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
index 4a71ea6cb390..ce55adb80b82 100644
--- a/Documentation/trace/coresight/coresight.rst
+++ b/Documentation/trace/coresight/coresight.rst
@@ -624,6 +624,10 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/
    * - timestamp
      - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP
        <coresight-timestamp>`
+   * - cc_threshold
+     - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the
+       default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
+       value, as indicated via TRCIDR3.CCITMIN, then the minimum value will be used instead.
 
 How to use the STM module
 -------------------------
-- 
2.25.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] 26+ messages in thread

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
  2023-08-11  3:45   ` Anshuman Khandual
@ 2023-08-11  8:52     ` Mike Leach
  -1 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  8:52 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Cycle counting is enabled, when requested and supported but with a default
> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
> representing the minimum interval between cycle count trace packets.
>
> This makes cycle threshold user configurable, from the user space via perf
> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
> in case no explicit request. As expected it creates a sysfs file as well.
>
> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>
> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
> in 'event->attr.config1' or 'event->attr.config2'.
>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 5ca6278baff4..09f75dffae60 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
>  /* config ID - set if a system configuration is selected */
>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
>
>
>  /*
> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>         &format_attr_preset.attr,
>         &format_attr_configid.attr,
>         &format_attr_branch_broadcast.attr,
> +       &format_attr_cc_threshold.attr,
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1f3d29a639ff..ad28ee044cba 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>         struct etmv4_config *config = &drvdata->config;
>         struct perf_event_attr *attr = &event->attr;
>         unsigned long cfg_hash;
> -       int preset;
> +       int preset, cc_threshold;
>
>         /* Clear configuration from previous run */
>         memset(config, 0, sizeof(struct etmv4_config));
> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
>                 config->cfg |= TRCCONFIGR_CCI;
>                 /* TRM: Must program this for cycacc to work */
> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> +               if (cc_threshold) {
> +                       if (cc_threshold < drvdata->ccitmin)
> +                               config->ccctlr = drvdata->ccitmin;
> +                       else
> +                               config->ccctlr = cc_threshold;
> +               } else {
> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;

Don't normally have {} round a single statement else clause - did
checkpatch.pl not object here?


Otherwise

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

> +               }
>         }
>         if (attr->config & BIT(ETM_OPT_TS)) {
>                 /*
> --
> 2.25.1
>


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

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

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  8:52     ` Mike Leach
  0 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  8:52 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> Cycle counting is enabled, when requested and supported but with a default
> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
> representing the minimum interval between cycle count trace packets.
>
> This makes cycle threshold user configurable, from the user space via perf
> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
> in case no explicit request. As expected it creates a sysfs file as well.
>
> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>
> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
> in 'event->attr.config1' or 'event->attr.config2'.
>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Leo Yan <leo.yan@linaro.org>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index 5ca6278baff4..09f75dffae60 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
>  /* config ID - set if a system configuration is selected */
>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
>
>
>  /*
> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>         &format_attr_preset.attr,
>         &format_attr_configid.attr,
>         &format_attr_branch_broadcast.attr,
> +       &format_attr_cc_threshold.attr,
>         NULL,
>  };
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 1f3d29a639ff..ad28ee044cba 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>         struct etmv4_config *config = &drvdata->config;
>         struct perf_event_attr *attr = &event->attr;
>         unsigned long cfg_hash;
> -       int preset;
> +       int preset, cc_threshold;
>
>         /* Clear configuration from previous run */
>         memset(config, 0, sizeof(struct etmv4_config));
> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
>                 config->cfg |= TRCCONFIGR_CCI;
>                 /* TRM: Must program this for cycacc to work */
> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> +               if (cc_threshold) {
> +                       if (cc_threshold < drvdata->ccitmin)
> +                               config->ccctlr = drvdata->ccitmin;
> +                       else
> +                               config->ccctlr = cc_threshold;
> +               } else {
> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;

Don't normally have {} round a single statement else clause - did
checkpatch.pl not object here?


Otherwise

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

> +               }
>         }
>         if (attr->config & BIT(ETM_OPT_TS)) {
>                 /*
> --
> 2.25.1
>


-- 
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] 26+ messages in thread

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
  2023-08-11  8:52     ` Mike Leach
@ 2023-08-11  8:57       ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  8:57 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel



On 8/11/23 14:22, Mike Leach wrote:
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> Cycle counting is enabled, when requested and supported but with a default
>> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
>> representing the minimum interval between cycle count trace packets.
>>
>> This makes cycle threshold user configurable, from the user space via perf
>> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
>> in case no explicit request. As expected it creates a sysfs file as well.
>>
>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>>
>> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
>> in 'event->attr.config1' or 'event->attr.config2'.
>>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
>>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index 5ca6278baff4..09f75dffae60 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
>>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
>>  /* config ID - set if a system configuration is selected */
>>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
>> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
>>
>>
>>  /*
>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>>         &format_attr_preset.attr,
>>         &format_attr_configid.attr,
>>         &format_attr_branch_broadcast.attr,
>> +       &format_attr_cc_threshold.attr,
>>         NULL,
>>  };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1f3d29a639ff..ad28ee044cba 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>>         struct etmv4_config *config = &drvdata->config;
>>         struct perf_event_attr *attr = &event->attr;
>>         unsigned long cfg_hash;
>> -       int preset;
>> +       int preset, cc_threshold;
>>
>>         /* Clear configuration from previous run */
>>         memset(config, 0, sizeof(struct etmv4_config));
>> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
>>                 config->cfg |= TRCCONFIGR_CCI;
>>                 /* TRM: Must program this for cycacc to work */
>> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>> +               if (cc_threshold) {
>> +                       if (cc_threshold < drvdata->ccitmin)
>> +                               config->ccctlr = drvdata->ccitmin;
>> +                       else
>> +                               config->ccctlr = cc_threshold;
>> +               } else {
>> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> 
> Don't normally have {} round a single statement else clause - did

I would believe single statement else clause could have { }, only
when the preceding if clause consists of multiple statements just
to be symmetrical ? 

> checkpatch.pl not object here?

No, it does not object.

> 
> 
> Otherwise
> 
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> 
>> +               }
>>         }
>>         if (attr->config & BIT(ETM_OPT_TS)) {
>>                 /*
>> --
>> 2.25.1
>>
> 
> 

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

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  8:57       ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-11  8:57 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel



On 8/11/23 14:22, Mike Leach wrote:
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> Cycle counting is enabled, when requested and supported but with a default
>> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
>> representing the minimum interval between cycle count trace packets.
>>
>> This makes cycle threshold user configurable, from the user space via perf
>> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
>> in case no explicit request. As expected it creates a sysfs file as well.
>>
>> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
>>
>> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
>> in 'event->attr.config1' or 'event->attr.config2'.
>>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Leo Yan <leo.yan@linaro.org>
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
>>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> index 5ca6278baff4..09f75dffae60 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
>>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
>>  /* config ID - set if a system configuration is selected */
>>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
>> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
>>
>>
>>  /*
>> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
>>         &format_attr_preset.attr,
>>         &format_attr_configid.attr,
>>         &format_attr_branch_broadcast.attr,
>> +       &format_attr_cc_threshold.attr,
>>         NULL,
>>  };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1f3d29a639ff..ad28ee044cba 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>>         struct etmv4_config *config = &drvdata->config;
>>         struct perf_event_attr *attr = &event->attr;
>>         unsigned long cfg_hash;
>> -       int preset;
>> +       int preset, cc_threshold;
>>
>>         /* Clear configuration from previous run */
>>         memset(config, 0, sizeof(struct etmv4_config));
>> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
>>                 config->cfg |= TRCCONFIGR_CCI;
>>                 /* TRM: Must program this for cycacc to work */
>> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
>> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
>> +               if (cc_threshold) {
>> +                       if (cc_threshold < drvdata->ccitmin)
>> +                               config->ccctlr = drvdata->ccitmin;
>> +                       else
>> +                               config->ccctlr = cc_threshold;
>> +               } else {
>> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> 
> Don't normally have {} round a single statement else clause - did

I would believe single statement else clause could have { }, only
when the preceding if clause consists of multiple statements just
to be symmetrical ? 

> checkpatch.pl not object here?

No, it does not object.

> 
> 
> Otherwise
> 
> Reviewed-by: Mike Leach <mike.leach@linaro.org>
> 
>> +               }
>>         }
>>         if (attr->config & BIT(ETM_OPT_TS)) {
>>                 /*
>> --
>> 2.25.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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-11  3:45   ` Anshuman Khandual
@ 2023-08-11  9:03     ` Mike Leach
  -1 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:03 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

Hi Anshuman,

A few minor points.

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
> field in ETM gets an wrong value.
>
> If software uses the value returned by the TRCIDR3.CCITMIN register field,
> then it will limit the range which could be used for programming the ETM.
> In reality, the ETM could be programmed with a much smaller value than what
> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>
> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
> instruction trace counting minimum threshold, observe the value 0x100 or a
> minimum cycle count threshold of 256. The correct value should be 0x4 or a
> minimum cycle count threshold of 4.
>
> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
> threshold granularity.
>
> These errata information has been updated in arch/arm64/silicon-errata.rst,
> but without their corresponding configs because these have been implemented
> directly in the driver.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>  .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>  2 files changed, 47 insertions(+)
>
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index bedd3a1d7b42..b08f33eda5f1 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -107,6 +107,10 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
> @@ -125,6 +129,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
> @@ -133,6 +139,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
> @@ -141,6 +149,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | MMU-500         | #841119,826419  | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | MMU-600         | #1076982,1209401| N/A                         |

Could these doc changes not go in patch 3?

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 703b6fcbb6a5..1f3d29a639ff 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>         drvdata->trfcr = trfcr;
>  }
>
> +/*
> + * The following errata on applicable cpu rangess affect the CCITMIN filed

s/rangess/ranges

> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
> + * the cycle threshold granularity, where as the right value should have
> + * been 0x4, which is well supported in the hardware.
> + */
> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
> +       /* Erratum #1490853 - Cortex-A76 */
> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
> +       /* Erratum #1490853 - Neoverse-N1 */
> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
> +       /* Erratum #1491015 - Cortex-A77 */
> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
> +       /* Erratum #1502854 - Cortex-X1 */
> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
> +       /* Erratum #1619801 - Neoverse-V1 */
> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
> +       {},
> +};
> +
> +static bool etm4_work_around_wrong_ccitmin(void)

This is not the actual work around - perhaps this should be
etm4_core_reads_wrong_ccitmin()?

> +{
> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
> +}
> +
>  static void etm4_init_arch_data(void *info)
>  {
>         u32 etmidr0;
> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>         etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>         /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>         drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
> +       if (etm4_work_around_wrong_ccitmin()) {
> +               /*
> +                * Erratum affected cpus will read 256 as the minimum
> +                * instruction trace cycle counting threshold where as
> +                * the correct value should be 4 instead. Override the
> +                * recorded value for 'drvdata->ccitmin' to workaround
> +                * this problem.
> +                */
> +               if (drvdata->ccitmin == 256)

Not sure this check matters - fixed cores will be 4, non fixed cores
as identified by the list need to be 4, we don't care what the read
value is if the core is on the list.

> +                       drvdata->ccitmin = 4;
> +       }
> +
>         /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>         drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>         drvdata->config.s_ex_level = drvdata->s_ex_level;
> --
> 2.25.1
>

Regards

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

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

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-11  9:03     ` Mike Leach
  0 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:03 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

Hi Anshuman,

A few minor points.

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
> field in ETM gets an wrong value.
>
> If software uses the value returned by the TRCIDR3.CCITMIN register field,
> then it will limit the range which could be used for programming the ETM.
> In reality, the ETM could be programmed with a much smaller value than what
> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>
> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
> instruction trace counting minimum threshold, observe the value 0x100 or a
> minimum cycle count threshold of 256. The correct value should be 0x4 or a
> minimum cycle count threshold of 4.
>
> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
> threshold granularity.
>
> These errata information has been updated in arch/arm64/silicon-errata.rst,
> but without their corresponding configs because these have been implemented
> directly in the driver.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: linux-doc@vger.kernel.org
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>  .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>  2 files changed, 47 insertions(+)
>
> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
> index bedd3a1d7b42..b08f33eda5f1 100644
> --- a/Documentation/arch/arm64/silicon-errata.rst
> +++ b/Documentation/arch/arm64/silicon-errata.rst
> @@ -107,6 +107,10 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
> @@ -125,6 +129,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
> @@ -133,6 +139,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
> @@ -141,6 +149,8 @@ stable kernels.
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>  +----------------+-----------------+-----------------+-----------------------------+
> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
> ++----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | MMU-500         | #841119,826419  | N/A                         |
>  +----------------+-----------------+-----------------+-----------------------------+
>  | ARM            | MMU-600         | #1076982,1209401| N/A                         |

Could these doc changes not go in patch 3?

> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> index 703b6fcbb6a5..1f3d29a639ff 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>         drvdata->trfcr = trfcr;
>  }
>
> +/*
> + * The following errata on applicable cpu rangess affect the CCITMIN filed

s/rangess/ranges

> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
> + * the cycle threshold granularity, where as the right value should have
> + * been 0x4, which is well supported in the hardware.
> + */
> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
> +       /* Erratum #1490853 - Cortex-A76 */
> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
> +       /* Erratum #1490853 - Neoverse-N1 */
> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
> +       /* Erratum #1491015 - Cortex-A77 */
> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
> +       /* Erratum #1502854 - Cortex-X1 */
> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
> +       /* Erratum #1619801 - Neoverse-V1 */
> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
> +       {},
> +};
> +
> +static bool etm4_work_around_wrong_ccitmin(void)

This is not the actual work around - perhaps this should be
etm4_core_reads_wrong_ccitmin()?

> +{
> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
> +}
> +
>  static void etm4_init_arch_data(void *info)
>  {
>         u32 etmidr0;
> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>         etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>         /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>         drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
> +       if (etm4_work_around_wrong_ccitmin()) {
> +               /*
> +                * Erratum affected cpus will read 256 as the minimum
> +                * instruction trace cycle counting threshold where as
> +                * the correct value should be 4 instead. Override the
> +                * recorded value for 'drvdata->ccitmin' to workaround
> +                * this problem.
> +                */
> +               if (drvdata->ccitmin == 256)

Not sure this check matters - fixed cores will be 4, non fixed cores
as identified by the list need to be 4, we don't care what the read
value is if the core is on the list.

> +                       drvdata->ccitmin = 4;
> +       }
> +
>         /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>         drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>         drvdata->config.s_ex_level = drvdata->s_ex_level;
> --
> 2.25.1
>

Regards

Mike
-- 
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] 26+ messages in thread

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
  2023-08-11  8:57       ` Anshuman Khandual
@ 2023-08-11  9:04         ` Mike Leach
  -1 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:04 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 09:57, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 8/11/23 14:22, Mike Leach wrote:
> > On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >> Cycle counting is enabled, when requested and supported but with a default
> >> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
> >> representing the minimum interval between cycle count trace packets.
> >>
> >> This makes cycle threshold user configurable, from the user space via perf
> >> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
> >> in case no explicit request. As expected it creates a sysfs file as well.
> >>
> >> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
> >>
> >> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
> >> in 'event->attr.config1' or 'event->attr.config2'.
> >>
> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> Cc: Mike Leach <mike.leach@linaro.org>
> >> Cc: James Clark <james.clark@arm.com>
> >> Cc: Leo Yan <leo.yan@linaro.org>
> >> Cc: coresight@lists.linaro.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
> >>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
> >>  2 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> index 5ca6278baff4..09f75dffae60 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
> >>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
> >>  /* config ID - set if a system configuration is selected */
> >>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
> >> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
> >>
> >>
> >>  /*
> >> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
> >>         &format_attr_preset.attr,
> >>         &format_attr_configid.attr,
> >>         &format_attr_branch_broadcast.attr,
> >> +       &format_attr_cc_threshold.attr,
> >>         NULL,
> >>  };
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> index 1f3d29a639ff..ad28ee044cba 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >>         struct etmv4_config *config = &drvdata->config;
> >>         struct perf_event_attr *attr = &event->attr;
> >>         unsigned long cfg_hash;
> >> -       int preset;
> >> +       int preset, cc_threshold;
> >>
> >>         /* Clear configuration from previous run */
> >>         memset(config, 0, sizeof(struct etmv4_config));
> >> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
> >>                 config->cfg |= TRCCONFIGR_CCI;
> >>                 /* TRM: Must program this for cycacc to work */
> >> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> >> +               if (cc_threshold) {
> >> +                       if (cc_threshold < drvdata->ccitmin)
> >> +                               config->ccctlr = drvdata->ccitmin;
> >> +                       else
> >> +                               config->ccctlr = cc_threshold;
> >> +               } else {
> >> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >
> > Don't normally have {} round a single statement else clause - did
>
> I would believe single statement else clause could have { }, only
> when the preceding if clause consists of multiple statements just
> to be symmetrical ?
>
> > checkpatch.pl not object here?
>
> No, it does not object.
>

Fair enough - good to go then!

> >
> >
> > Otherwise
> >
> > Reviewed-by: Mike Leach <mike.leach@linaro.org>
> >
> >> +               }
> >>         }
> >>         if (attr->config & BIT(ETM_OPT_TS)) {
> >>                 /*
> >> --
> >> 2.25.1
> >>
> >
> >



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

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

* Re: [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable
@ 2023-08-11  9:04         ` Mike Leach
  0 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:04 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 09:57, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
>
>
> On 8/11/23 14:22, Mike Leach wrote:
> > On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> > <anshuman.khandual@arm.com> wrote:
> >>
> >> Cycle counting is enabled, when requested and supported but with a default
> >> threshold value ETM_CYC_THRESHOLD_DEFAULT i.e 0x100 getting into TRCCCCTLR,
> >> representing the minimum interval between cycle count trace packets.
> >>
> >> This makes cycle threshold user configurable, from the user space via perf
> >> event attributes. Although it falls back using ETM_CYC_THRESHOLD_DEFAULT,
> >> in case no explicit request. As expected it creates a sysfs file as well.
> >>
> >> /sys/bus/event_source/devices/cs_etm/format/cc_threshold
> >>
> >> New 'cc_threshold' uses 'event->attr.config3' as no more space is available
> >> in 'event->attr.config1' or 'event->attr.config2'.
> >>
> >> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> Cc: Mike Leach <mike.leach@linaro.org>
> >> Cc: James Clark <james.clark@arm.com>
> >> Cc: Leo Yan <leo.yan@linaro.org>
> >> Cc: coresight@lists.linaro.org
> >> Cc: linux-arm-kernel@lists.infradead.org
> >> Cc: linux-doc@vger.kernel.org
> >> Cc: linux-kernel@vger.kernel.org
> >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> >> ---
> >>  drivers/hwtracing/coresight/coresight-etm-perf.c   |  2 ++
> >>  drivers/hwtracing/coresight/coresight-etm4x-core.c | 12 ++++++++++--
> >>  2 files changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> index 5ca6278baff4..09f75dffae60 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> >> @@ -68,6 +68,7 @@ PMU_FORMAT_ATTR(preset,               "config:0-3");
> >>  PMU_FORMAT_ATTR(sinkid,                "config2:0-31");
> >>  /* config ID - set if a system configuration is selected */
> >>  PMU_FORMAT_ATTR(configid,      "config2:32-63");
> >> +PMU_FORMAT_ATTR(cc_threshold,  "config3:0-11");
> >>
> >>
> >>  /*
> >> @@ -101,6 +102,7 @@ static struct attribute *etm_config_formats_attr[] = {
> >>         &format_attr_preset.attr,
> >>         &format_attr_configid.attr,
> >>         &format_attr_branch_broadcast.attr,
> >> +       &format_attr_cc_threshold.attr,
> >>         NULL,
> >>  };
> >>
> >> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> index 1f3d29a639ff..ad28ee044cba 100644
> >> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
> >> @@ -644,7 +644,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >>         struct etmv4_config *config = &drvdata->config;
> >>         struct perf_event_attr *attr = &event->attr;
> >>         unsigned long cfg_hash;
> >> -       int preset;
> >> +       int preset, cc_threshold;
> >>
> >>         /* Clear configuration from previous run */
> >>         memset(config, 0, sizeof(struct etmv4_config));
> >> @@ -667,7 +667,15 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
> >>         if (attr->config & BIT(ETM_OPT_CYCACC)) {
> >>                 config->cfg |= TRCCONFIGR_CCI;
> >>                 /* TRM: Must program this for cycacc to work */
> >> -               config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >> +               cc_threshold = attr->config3 & ETM_CYC_THRESHOLD_MASK;
> >> +               if (cc_threshold) {
> >> +                       if (cc_threshold < drvdata->ccitmin)
> >> +                               config->ccctlr = drvdata->ccitmin;
> >> +                       else
> >> +                               config->ccctlr = cc_threshold;
> >> +               } else {
> >> +                       config->ccctlr = ETM_CYC_THRESHOLD_DEFAULT;
> >
> > Don't normally have {} round a single statement else clause - did
>
> I would believe single statement else clause could have { }, only
> when the preceding if clause consists of multiple statements just
> to be symmetrical ?
>
> > checkpatch.pl not object here?
>
> No, it does not object.
>

Fair enough - good to go then!

> >
> >
> > Otherwise
> >
> > Reviewed-by: Mike Leach <mike.leach@linaro.org>
> >
> >> +               }
> >>         }
> >>         if (attr->config & BIT(ETM_OPT_TS)) {
> >>                 /*
> >> --
> >> 2.25.1
> >>
> >
> >



-- 
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] 26+ messages in thread

* Re: [PATCH V3 3/3] Documentation: coresight: Add cc_threshold tunable
  2023-08-11  3:46   ` Anshuman Khandual
@ 2023-08-11  9:05     ` Mike Leach
  -1 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:05 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> This updates config option to include 'cc_threshold' tunable value.
>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/trace/coresight/coresight.rst | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
> index 4a71ea6cb390..ce55adb80b82 100644
> --- a/Documentation/trace/coresight/coresight.rst
> +++ b/Documentation/trace/coresight/coresight.rst
> @@ -624,6 +624,10 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/
>     * - timestamp
>       - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP
>         <coresight-timestamp>`
> +   * - cc_threshold
> +     - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the
> +       default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
> +       value, as indicated via TRCIDR3.CCITMIN, then the minimum value will be used instead.
>
>  How to use the STM module
>  -------------------------
> --
> 2.25.1
>
Reviewed by: Mike Leach <mike.leach@linaro.org>
-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

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

* Re: [PATCH V3 3/3] Documentation: coresight: Add cc_threshold tunable
@ 2023-08-11  9:05     ` Mike Leach
  0 siblings, 0 replies; 26+ messages in thread
From: Mike Leach @ 2023-08-11  9:05 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel

On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
>
> This updates config option to include 'cc_threshold' tunable value.
>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mike Leach <mike.leach@linaro.org>
> Cc: James Clark <james.clark@arm.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: coresight@lists.linaro.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
>  Documentation/trace/coresight/coresight.rst | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/trace/coresight/coresight.rst b/Documentation/trace/coresight/coresight.rst
> index 4a71ea6cb390..ce55adb80b82 100644
> --- a/Documentation/trace/coresight/coresight.rst
> +++ b/Documentation/trace/coresight/coresight.rst
> @@ -624,6 +624,10 @@ They are also listed in the folder /sys/bus/event_source/devices/cs_etm/format/
>     * - timestamp
>       - Session local version of the system wide setting: :ref:`ETMv4_MODE_TIMESTAMP
>         <coresight-timestamp>`
> +   * - cc_threshold
> +     - Cycle count threshold value. If nothing is provided here or the provided value is 0, then the
> +       default value i.e 0x100 will be used. If provided value is less than minimum cycles threshold
> +       value, as indicated via TRCIDR3.CCITMIN, then the minimum value will be used instead.
>
>  How to use the STM module
>  -------------------------
> --
> 2.25.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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-11  9:03     ` Mike Leach
@ 2023-08-11  9:31       ` Suzuki K Poulose
  -1 siblings, 0 replies; 26+ messages in thread
From: Suzuki K Poulose @ 2023-08-11  9:31 UTC (permalink / raw)
  To: Mike Leach, Anshuman Khandual
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel

On 11/08/2023 10:03, Mike Leach wrote:
> Hi Anshuman,
> 
> A few minor points.
> 
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>> field in ETM gets an wrong value.
>>
>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>> then it will limit the range which could be used for programming the ETM.
>> In reality, the ETM could be programmed with a much smaller value than what
>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>
>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>> instruction trace counting minimum threshold, observe the value 0x100 or a
>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>> minimum cycle count threshold of 4.
>>
>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>> threshold granularity.
>>
>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>> but without their corresponding configs because these have been implemented
>> directly in the driver.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: linux-doc@vger.kernel.org
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>> index bedd3a1d7b42..b08f33eda5f1 100644
>> --- a/Documentation/arch/arm64/silicon-errata.rst
>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>> @@ -107,6 +107,10 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>> @@ -125,6 +129,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>> @@ -133,6 +139,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>> @@ -141,6 +149,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
> 
> Could these doc changes not go in patch 3?

I believe no, because this patch works around the erratum and it will 
help anybody looking for the workaround using the list above + the patch
3 is not related to the erratum. It is simply documenting the new perf
option.

> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 703b6fcbb6a5..1f3d29a639ff 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>          drvdata->trfcr = trfcr;
>>   }
>>
>> +/*
>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
> 
> s/rangess/ranges
> 
>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>> + * the cycle threshold granularity, where as the right value should have
>> + * been 0x4, which is well supported in the hardware.
>> + */
>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>> +       /* Erratum #1490853 - Cortex-A76 */
>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>> +       /* Erratum #1490853 - Neoverse-N1 */
>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>> +       /* Erratum #1491015 - Cortex-A77 */
>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>> +       /* Erratum #1502854 - Cortex-X1 */
>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>> +       /* Erratum #1619801 - Neoverse-V1 */
>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>> +       {},
>> +};
>> +
>> +static bool etm4_work_around_wrong_ccitmin(void)
> 
> This is not the actual work around - perhaps this should be
> etm4_core_reads_wrong_ccitmin()?
> 
>> +{
>> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
>> +}
>> +
>>   static void etm4_init_arch_data(void *info)
>>   {
>>          u32 etmidr0;
>> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>>          etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>>          /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>>          drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
>> +       if (etm4_work_around_wrong_ccitmin()) {
>> +               /*
>> +                * Erratum affected cpus will read 256 as the minimum
>> +                * instruction trace cycle counting threshold where as
>> +                * the correct value should be 4 instead. Override the
>> +                * recorded value for 'drvdata->ccitmin' to workaround
>> +                * this problem.
>> +                */
>> +               if (drvdata->ccitmin == 256) >
> Not sure this check matters - fixed cores will be 4, non fixed cores
> as identified by the list need to be 4, we don't care what the read
> value is if the core is on the list.

To be on the safer side, we may allow a core to have ccitmin more than 
0x4 if we only fixup what is documented per erratum. So this looks fine
to me.


Suzuki

> 
>> +                       drvdata->ccitmin = 4;
>> +       }
>> +
>>          /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>>          drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>>          drvdata->config.s_ex_level = drvdata->s_ex_level;
>> --
>> 2.25.1
>>
> 
> Regards
> 
> Mike


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

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-11  9:31       ` Suzuki K Poulose
  0 siblings, 0 replies; 26+ messages in thread
From: Suzuki K Poulose @ 2023-08-11  9:31 UTC (permalink / raw)
  To: Mike Leach, Anshuman Khandual
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel

On 11/08/2023 10:03, Mike Leach wrote:
> Hi Anshuman,
> 
> A few minor points.
> 
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>> field in ETM gets an wrong value.
>>
>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>> then it will limit the range which could be used for programming the ETM.
>> In reality, the ETM could be programmed with a much smaller value than what
>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>
>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>> instruction trace counting minimum threshold, observe the value 0x100 or a
>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>> minimum cycle count threshold of 4.
>>
>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>> threshold granularity.
>>
>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>> but without their corresponding configs because these have been implemented
>> directly in the driver.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: linux-doc@vger.kernel.org
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>   2 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>> index bedd3a1d7b42..b08f33eda5f1 100644
>> --- a/Documentation/arch/arm64/silicon-errata.rst
>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>> @@ -107,6 +107,10 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>> @@ -125,6 +129,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>> @@ -133,6 +139,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>> @@ -141,6 +149,8 @@ stable kernels.
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>   +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>   +----------------+-----------------+-----------------+-----------------------------+
>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
> 
> Could these doc changes not go in patch 3?

I believe no, because this patch works around the erratum and it will 
help anybody looking for the workaround using the list above + the patch
3 is not related to the erratum. It is simply documenting the new perf
option.

> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 703b6fcbb6a5..1f3d29a639ff 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>          drvdata->trfcr = trfcr;
>>   }
>>
>> +/*
>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
> 
> s/rangess/ranges
> 
>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>> + * the cycle threshold granularity, where as the right value should have
>> + * been 0x4, which is well supported in the hardware.
>> + */
>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>> +       /* Erratum #1490853 - Cortex-A76 */
>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>> +       /* Erratum #1490853 - Neoverse-N1 */
>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>> +       /* Erratum #1491015 - Cortex-A77 */
>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>> +       /* Erratum #1502854 - Cortex-X1 */
>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>> +       /* Erratum #1619801 - Neoverse-V1 */
>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>> +       {},
>> +};
>> +
>> +static bool etm4_work_around_wrong_ccitmin(void)
> 
> This is not the actual work around - perhaps this should be
> etm4_core_reads_wrong_ccitmin()?
> 
>> +{
>> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
>> +}
>> +
>>   static void etm4_init_arch_data(void *info)
>>   {
>>          u32 etmidr0;
>> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>>          etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>>          /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>>          drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
>> +       if (etm4_work_around_wrong_ccitmin()) {
>> +               /*
>> +                * Erratum affected cpus will read 256 as the minimum
>> +                * instruction trace cycle counting threshold where as
>> +                * the correct value should be 4 instead. Override the
>> +                * recorded value for 'drvdata->ccitmin' to workaround
>> +                * this problem.
>> +                */
>> +               if (drvdata->ccitmin == 256) >
> Not sure this check matters - fixed cores will be 4, non fixed cores
> as identified by the list need to be 4, we don't care what the read
> value is if the core is on the list.

To be on the safer side, we may allow a core to have ccitmin more than 
0x4 if we only fixup what is documented per erratum. So this looks fine
to me.


Suzuki

> 
>> +                       drvdata->ccitmin = 4;
>> +       }
>> +
>>          /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>>          drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>>          drvdata->config.s_ex_level = drvdata->s_ex_level;
>> --
>> 2.25.1
>>
> 
> Regards
> 
> Mike


_______________________________________________
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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-11  9:03     ` Mike Leach
@ 2023-08-18  8:45       ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-18  8:45 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel



On 8/11/23 14:33, Mike Leach wrote:
> Hi Anshuman,
> 
> A few minor points.
> 
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>> field in ETM gets an wrong value.
>>
>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>> then it will limit the range which could be used for programming the ETM.
>> In reality, the ETM could be programmed with a much smaller value than what
>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>
>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>> instruction trace counting minimum threshold, observe the value 0x100 or a
>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>> minimum cycle count threshold of 4.
>>
>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>> threshold granularity.
>>
>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>> but without their corresponding configs because these have been implemented
>> directly in the driver.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: linux-doc@vger.kernel.org
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>  .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>> index bedd3a1d7b42..b08f33eda5f1 100644
>> --- a/Documentation/arch/arm64/silicon-errata.rst
>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>> @@ -107,6 +107,10 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>> @@ -125,6 +129,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>> @@ -133,6 +139,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>> @@ -141,6 +149,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | MMU-600         | #1076982,1209401| N/A                         |
> 
> Could these doc changes not go in patch 3?

As Suzuki had explained earlier, will keep this errata documentation here in this patch.

> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 703b6fcbb6a5..1f3d29a639ff 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>         drvdata->trfcr = trfcr;
>>  }
>>
>> +/*
>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
> 
> s/rangess/ranges

Fixed.

> 
>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>> + * the cycle threshold granularity, where as the right value should have
>> + * been 0x4, which is well supported in the hardware.
>> + */
>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>> +       /* Erratum #1490853 - Cortex-A76 */
>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>> +       /* Erratum #1490853 - Neoverse-N1 */
>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>> +       /* Erratum #1491015 - Cortex-A77 */
>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>> +       /* Erratum #1502854 - Cortex-X1 */
>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>> +       /* Erratum #1619801 - Neoverse-V1 */
>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>> +       {},
>> +};
>> +
>> +static bool etm4_work_around_wrong_ccitmin(void)
> 
> This is not the actual work around - perhaps this should be
> etm4_core_reads_wrong_ccitmin()?

Sounds better, will rename the function.

> 
>> +{
>> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
>> +}
>> +
>>  static void etm4_init_arch_data(void *info)
>>  {
>>         u32 etmidr0;
>> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>>         etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>>         /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>>         drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
>> +       if (etm4_work_around_wrong_ccitmin()) {
>> +               /*
>> +                * Erratum affected cpus will read 256 as the minimum
>> +                * instruction trace cycle counting threshold where as
>> +                * the correct value should be 4 instead. Override the
>> +                * recorded value for 'drvdata->ccitmin' to workaround
>> +                * this problem.
>> +                */
>> +               if (drvdata->ccitmin == 256)
> 
> Not sure this check matters - fixed cores will be 4, non fixed cores
> as identified by the list need to be 4, we don't care what the read
> value is if the core is on the list.

As discussed earlier, will keep this unchanged.

> 
>> +                       drvdata->ccitmin = 4;
>> +       }
>> +
>>         /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>>         drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>>         drvdata->config.s_ex_level = drvdata->s_ex_level;
>> --
>> 2.25.1
>>
> 
> Regards
> 
> Mike

_______________________________________________
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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-18  8:45       ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-18  8:45 UTC (permalink / raw)
  To: Mike Leach
  Cc: linux-arm-kernel, suzuki.poulose, Catalin Marinas, Will Deacon,
	James Clark, Leo Yan, Jonathan Corbet, linux-doc, coresight,
	linux-kernel



On 8/11/23 14:33, Mike Leach wrote:
> Hi Anshuman,
> 
> A few minor points.
> 
> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
> <anshuman.khandual@arm.com> wrote:
>>
>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>> field in ETM gets an wrong value.
>>
>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>> then it will limit the range which could be used for programming the ETM.
>> In reality, the ETM could be programmed with a much smaller value than what
>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>
>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>> instruction trace counting minimum threshold, observe the value 0x100 or a
>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>> minimum cycle count threshold of 4.
>>
>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>> threshold granularity.
>>
>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>> but without their corresponding configs because these have been implemented
>> directly in the driver.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Cc: Mike Leach <mike.leach@linaro.org>
>> Cc: James Clark <james.clark@arm.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: linux-doc@vger.kernel.org
>> Cc: coresight@lists.linaro.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>> ---
>>  Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>  .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>> index bedd3a1d7b42..b08f33eda5f1 100644
>> --- a/Documentation/arch/arm64/silicon-errata.rst
>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>> @@ -107,6 +107,10 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>> @@ -125,6 +129,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>> @@ -133,6 +139,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>> @@ -141,6 +149,8 @@ stable kernels.
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>  +----------------+-----------------+-----------------+-----------------------------+
>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>> ++----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>  +----------------+-----------------+-----------------+-----------------------------+
>>  | ARM            | MMU-600         | #1076982,1209401| N/A                         |
> 
> Could these doc changes not go in patch 3?

As Suzuki had explained earlier, will keep this errata documentation here in this patch.

> 
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 703b6fcbb6a5..1f3d29a639ff 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>         drvdata->trfcr = trfcr;
>>  }
>>
>> +/*
>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
> 
> s/rangess/ranges

Fixed.

> 
>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>> + * the cycle threshold granularity, where as the right value should have
>> + * been 0x4, which is well supported in the hardware.
>> + */
>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>> +       /* Erratum #1490853 - Cortex-A76 */
>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>> +       /* Erratum #1490853 - Neoverse-N1 */
>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>> +       /* Erratum #1491015 - Cortex-A77 */
>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>> +       /* Erratum #1502854 - Cortex-X1 */
>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>> +       /* Erratum #1619801 - Neoverse-V1 */
>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>> +       {},
>> +};
>> +
>> +static bool etm4_work_around_wrong_ccitmin(void)
> 
> This is not the actual work around - perhaps this should be
> etm4_core_reads_wrong_ccitmin()?

Sounds better, will rename the function.

> 
>> +{
>> +       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
>> +}
>> +
>>  static void etm4_init_arch_data(void *info)
>>  {
>>         u32 etmidr0;
>> @@ -1214,6 +1239,18 @@ static void etm4_init_arch_data(void *info)
>>         etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
>>         /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
>>         drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
>> +       if (etm4_work_around_wrong_ccitmin()) {
>> +               /*
>> +                * Erratum affected cpus will read 256 as the minimum
>> +                * instruction trace cycle counting threshold where as
>> +                * the correct value should be 4 instead. Override the
>> +                * recorded value for 'drvdata->ccitmin' to workaround
>> +                * this problem.
>> +                */
>> +               if (drvdata->ccitmin == 256)
> 
> Not sure this check matters - fixed cores will be 4, non fixed cores
> as identified by the list need to be 4, we don't care what the read
> value is if the core is on the list.

As discussed earlier, will keep this unchanged.

> 
>> +                       drvdata->ccitmin = 4;
>> +       }
>> +
>>         /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
>>         drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);
>>         drvdata->config.s_ex_level = drvdata->s_ex_level;
>> --
>> 2.25.1
>>
> 
> Regards
> 
> Mike

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

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-18  8:45       ` Anshuman Khandual
@ 2023-08-18  8:50         ` Suzuki K Poulose
  -1 siblings, 0 replies; 26+ messages in thread
From: Suzuki K Poulose @ 2023-08-18  8:50 UTC (permalink / raw)
  To: Anshuman Khandual, Mike Leach
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel

On 18/08/2023 09:45, Anshuman Khandual wrote:
> 
> 
> On 8/11/23 14:33, Mike Leach wrote:
>> Hi Anshuman,
>>
>> A few minor points.
>>
>> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
>> <anshuman.khandual@arm.com> wrote:
>>>
>>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>>> field in ETM gets an wrong value.
>>>
>>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>>> then it will limit the range which could be used for programming the ETM.
>>> In reality, the ETM could be programmed with a much smaller value than what
>>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>>
>>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>>> instruction trace counting minimum threshold, observe the value 0x100 or a
>>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>>> minimum cycle count threshold of 4.
>>>
>>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>>> threshold granularity.
>>>
>>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>>> but without their corresponding configs because these have been implemented
>>> directly in the driver.
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: James Clark <james.clark@arm.com>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>>   2 files changed, 47 insertions(+)
>>>
>>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>>> index bedd3a1d7b42..b08f33eda5f1 100644
>>> --- a/Documentation/arch/arm64/silicon-errata.rst
>>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>>> @@ -107,6 +107,10 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>>> @@ -125,6 +129,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>>> @@ -133,6 +139,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>>> @@ -141,6 +149,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
>>
>> Could these doc changes not go in patch 3?
> 
> As Suzuki had explained earlier, will keep this errata documentation here in this patch.
> 
>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 703b6fcbb6a5..1f3d29a639ff 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>>          drvdata->trfcr = trfcr;
>>>   }
>>>
>>> +/*
>>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
>>
>> s/rangess/ranges
> 
> Fixed.
> 
>>
>>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>>> + * the cycle threshold granularity, where as the right value should have
>>> + * been 0x4, which is well supported in the hardware.
>>> + */
>>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>>> +       /* Erratum #1490853 - Cortex-A76 */
>>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>>> +       /* Erratum #1490853 - Neoverse-N1 */
>>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>>> +       /* Erratum #1491015 - Cortex-A77 */
>>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>>> +       /* Erratum #1502854 - Cortex-X1 */
>>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>>> +       /* Erratum #1619801 - Neoverse-V1 */
>>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>>> +       {},
>>> +};
>>> +
>>> +static bool etm4_work_around_wrong_ccitmin(void)
>>
>> This is not the actual work around - perhaps this should be
>> etm4_core_reads_wrong_ccitmin()?
> 
> Sounds better, will rename the function.

Or even move the drvdata->ccitmin == 256 check to this function,
pass in the drvdata as an argument. That way, the function
completes its meaning. i.e., it reads wrong value only if the value
is 256.

Suzuki


_______________________________________________
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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-18  8:50         ` Suzuki K Poulose
  0 siblings, 0 replies; 26+ messages in thread
From: Suzuki K Poulose @ 2023-08-18  8:50 UTC (permalink / raw)
  To: Anshuman Khandual, Mike Leach
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel

On 18/08/2023 09:45, Anshuman Khandual wrote:
> 
> 
> On 8/11/23 14:33, Mike Leach wrote:
>> Hi Anshuman,
>>
>> A few minor points.
>>
>> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
>> <anshuman.khandual@arm.com> wrote:
>>>
>>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>>> field in ETM gets an wrong value.
>>>
>>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>>> then it will limit the range which could be used for programming the ETM.
>>> In reality, the ETM could be programmed with a much smaller value than what
>>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>>
>>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>>> instruction trace counting minimum threshold, observe the value 0x100 or a
>>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>>> minimum cycle count threshold of 4.
>>>
>>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>>> threshold granularity.
>>>
>>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>>> but without their corresponding configs because these have been implemented
>>> directly in the driver.
>>>
>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>> Cc: Will Deacon <will@kernel.org>
>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>> Cc: Mike Leach <mike.leach@linaro.org>
>>> Cc: James Clark <james.clark@arm.com>
>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>> Cc: linux-doc@vger.kernel.org
>>> Cc: coresight@lists.linaro.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>> ---
>>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>>   2 files changed, 47 insertions(+)
>>>
>>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>>> index bedd3a1d7b42..b08f33eda5f1 100644
>>> --- a/Documentation/arch/arm64/silicon-errata.rst
>>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>>> @@ -107,6 +107,10 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>>> @@ -125,6 +129,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>>> @@ -133,6 +139,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>>> @@ -141,6 +149,8 @@ stable kernels.
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
>>
>> Could these doc changes not go in patch 3?
> 
> As Suzuki had explained earlier, will keep this errata documentation here in this patch.
> 
>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> index 703b6fcbb6a5..1f3d29a639ff 100644
>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>>          drvdata->trfcr = trfcr;
>>>   }
>>>
>>> +/*
>>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
>>
>> s/rangess/ranges
> 
> Fixed.
> 
>>
>>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>>> + * the cycle threshold granularity, where as the right value should have
>>> + * been 0x4, which is well supported in the hardware.
>>> + */
>>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>>> +       /* Erratum #1490853 - Cortex-A76 */
>>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>>> +       /* Erratum #1490853 - Neoverse-N1 */
>>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>>> +       /* Erratum #1491015 - Cortex-A77 */
>>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>>> +       /* Erratum #1502854 - Cortex-X1 */
>>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>>> +       /* Erratum #1619801 - Neoverse-V1 */
>>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>>> +       {},
>>> +};
>>> +
>>> +static bool etm4_work_around_wrong_ccitmin(void)
>>
>> This is not the actual work around - perhaps this should be
>> etm4_core_reads_wrong_ccitmin()?
> 
> Sounds better, will rename the function.

Or even move the drvdata->ccitmin == 256 check to this function,
pass in the drvdata as an argument. That way, the function
completes its meaning. i.e., it reads wrong value only if the value
is 256.

Suzuki


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

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
  2023-08-18  8:50         ` Suzuki K Poulose
@ 2023-08-18 10:45           ` Anshuman Khandual
  -1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-18 10:45 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel



On 8/18/23 14:20, Suzuki K Poulose wrote:
> On 18/08/2023 09:45, Anshuman Khandual wrote:
>>
>>
>> On 8/11/23 14:33, Mike Leach wrote:
>>> Hi Anshuman,
>>>
>>> A few minor points.
>>>
>>> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
>>> <anshuman.khandual@arm.com> wrote:
>>>>
>>>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>>>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>>>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>>>> field in ETM gets an wrong value.
>>>>
>>>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>>>> then it will limit the range which could be used for programming the ETM.
>>>> In reality, the ETM could be programmed with a much smaller value than what
>>>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>>>
>>>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>>>> instruction trace counting minimum threshold, observe the value 0x100 or a
>>>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>>>> minimum cycle count threshold of 4.
>>>>
>>>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>>>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>>>> threshold granularity.
>>>>
>>>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>>>> but without their corresponding configs because these have been implemented
>>>> directly in the driver.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Cc: Mike Leach <mike.leach@linaro.org>
>>>> Cc: James Clark <james.clark@arm.com>
>>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>>> Cc: linux-doc@vger.kernel.org
>>>> Cc: coresight@lists.linaro.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>>>   2 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>>>> index bedd3a1d7b42..b08f33eda5f1 100644
>>>> --- a/Documentation/arch/arm64/silicon-errata.rst
>>>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>>>> @@ -107,6 +107,10 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>>>> @@ -125,6 +129,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>>>> @@ -133,6 +139,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>>>> @@ -141,6 +149,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
>>>
>>> Could these doc changes not go in patch 3?
>>
>> As Suzuki had explained earlier, will keep this errata documentation here in this patch.
>>
>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> index 703b6fcbb6a5..1f3d29a639ff 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>>>          drvdata->trfcr = trfcr;
>>>>   }
>>>>
>>>> +/*
>>>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
>>>
>>> s/rangess/ranges
>>
>> Fixed.
>>
>>>
>>>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>>>> + * the cycle threshold granularity, where as the right value should have
>>>> + * been 0x4, which is well supported in the hardware.
>>>> + */
>>>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>>>> +       /* Erratum #1490853 - Cortex-A76 */
>>>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>>>> +       /* Erratum #1490853 - Neoverse-N1 */
>>>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>>>> +       /* Erratum #1491015 - Cortex-A77 */
>>>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>>>> +       /* Erratum #1502854 - Cortex-X1 */
>>>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>>>> +       /* Erratum #1619801 - Neoverse-V1 */
>>>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>>>> +       {},
>>>> +};
>>>> +
>>>> +static bool etm4_work_around_wrong_ccitmin(void)
>>>
>>> This is not the actual work around - perhaps this should be
>>> etm4_core_reads_wrong_ccitmin()?
>>
>> Sounds better, will rename the function.
> 
> Or even move the drvdata->ccitmin == 256 check to this function,
> pass in the drvdata as an argument. That way, the function
> completes its meaning. i.e., it reads wrong value only if the value
> is 256.

Something like the following ?

--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1159,9 +1159,20 @@ static struct midr_range etm_wrong_ccitmin_cpus[] = {
        {},
 };
 
-static bool etm4_core_reads_wrong_ccitmin(void)
+static bool etm4_core_reads_wrong_ccitmin(struct etmv4_drvdata *drvdata)
 {
-       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
+       if (is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus)) {
+               /*
+                * Erratum affected cpus will read 256 as the minimum
+                * instruction trace cycle counting threshold where as
+                * the correct value should be 4 instead. Override the
+                * recorded value for 'drvdata->ccitmin' to workaround
+                * this problem.
+                */
+               if (drvdata->ccitmin == 256)
+                       return true;
+       }
+       return false;
 }
 
 static void etm4_init_arch_data(void *info)
@@ -1228,17 +1239,8 @@ static void etm4_init_arch_data(void *info)
        etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
        /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
        drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
-       if (etm4_core_reads_wrong_ccitmin()) {
-               /*
-                * Erratum affected cpus will read 256 as the minimum
-                * instruction trace cycle counting threshold where as
-                * the correct value should be 4 instead. Override the
-                * recorded value for 'drvdata->ccitmin' to workaround
-                * this problem.
-                */
-               if (drvdata->ccitmin == 256)
-                       drvdata->ccitmin = 4;
-       }
+       if (etm4_core_reads_wrong_ccitmin(drvdata))
+               drvdata->ccitmin = 4;
 
        /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
        drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);


_______________________________________________
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] 26+ messages in thread

* Re: [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus
@ 2023-08-18 10:45           ` Anshuman Khandual
  0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2023-08-18 10:45 UTC (permalink / raw)
  To: Suzuki K Poulose, Mike Leach
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, James Clark,
	Leo Yan, Jonathan Corbet, linux-doc, coresight, linux-kernel



On 8/18/23 14:20, Suzuki K Poulose wrote:
> On 18/08/2023 09:45, Anshuman Khandual wrote:
>>
>>
>> On 8/11/23 14:33, Mike Leach wrote:
>>> Hi Anshuman,
>>>
>>> A few minor points.
>>>
>>> On Fri, 11 Aug 2023 at 04:46, Anshuman Khandual
>>> <anshuman.khandual@arm.com> wrote:
>>>>
>>>> This work arounds errata 1490853 on Cortex-A76, and Neoverse-N1, errata
>>>> 1491015 on Cortex-A77, errata 1502854 on Cortex-X1, and errata 1619801 on
>>>> Neoverse-V1, based affected cpus, where software read for TRCIDR3.CCITMIN
>>>> field in ETM gets an wrong value.
>>>>
>>>> If software uses the value returned by the TRCIDR3.CCITMIN register field,
>>>> then it will limit the range which could be used for programming the ETM.
>>>> In reality, the ETM could be programmed with a much smaller value than what
>>>> is indicated by the TRCIDR3.CCITMIN field and still function correctly.
>>>>
>>>> If software reads the TRCIDR3.CCITMIN register field, corresponding to the
>>>> instruction trace counting minimum threshold, observe the value 0x100 or a
>>>> minimum cycle count threshold of 256. The correct value should be 0x4 or a
>>>> minimum cycle count threshold of 4.
>>>>
>>>> This work arounds the problem via storing 4 in drvdata->ccitmin on affected
>>>> systems where the TRCIDR3.CCITMIN has been 256, thus preserving cycle count
>>>> threshold granularity.
>>>>
>>>> These errata information has been updated in arch/arm64/silicon-errata.rst,
>>>> but without their corresponding configs because these have been implemented
>>>> directly in the driver.
>>>>
>>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>>>> Cc: Will Deacon <will@kernel.org>
>>>> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Cc: Mike Leach <mike.leach@linaro.org>
>>>> Cc: James Clark <james.clark@arm.com>
>>>> Cc: Jonathan Corbet <corbet@lwn.net>
>>>> Cc: linux-doc@vger.kernel.org
>>>> Cc: coresight@lists.linaro.org
>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>>>> ---
>>>>   Documentation/arch/arm64/silicon-errata.rst   | 10 +++++
>>>>   .../coresight/coresight-etm4x-core.c          | 37 +++++++++++++++++++
>>>>   2 files changed, 47 insertions(+)
>>>>
>>>> diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
>>>> index bedd3a1d7b42..b08f33eda5f1 100644
>>>> --- a/Documentation/arch/arm64/silicon-errata.rst
>>>> +++ b/Documentation/arch/arm64/silicon-errata.rst
>>>> @@ -107,6 +107,10 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A76      | #1463225        | ARM64_ERRATUM_1463225       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A76      | #1490853        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-A77      | #1491015        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A77      | #1508412        | ARM64_ERRATUM_1508412       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A510     | #2051678        | ARM64_ERRATUM_2051678       |
>>>> @@ -125,6 +129,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-A715     | #2645198        | ARM64_ERRATUM_2645198       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Cortex-X1       | #1502854        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-X2       | #2119858        | ARM64_ERRATUM_2119858       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Cortex-X2       | #2224489        | ARM64_ERRATUM_2224489       |
>>>> @@ -133,6 +139,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N1     | #1349291        | N/A                         |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Neoverse-N1     | #1490853        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N1     | #1542419        | ARM64_ERRATUM_1542419       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N2     | #2139208        | ARM64_ERRATUM_2139208       |
>>>> @@ -141,6 +149,8 @@ stable kernels.
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | Neoverse-N2     | #2253138        | ARM64_ERRATUM_2253138       |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>> +| ARM            | Neoverse-V1     | #1619801        | N/A                         |
>>>> ++----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | MMU-500         | #841119,826419  | N/A                         |
>>>>   +----------------+-----------------+-----------------+-----------------------------+
>>>>   | ARM            | MMU-600         | #1076982,1209401| N/A                         |
>>>
>>> Could these doc changes not go in patch 3?
>>
>> As Suzuki had explained earlier, will keep this errata documentation here in this patch.
>>
>>>
>>>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> index 703b6fcbb6a5..1f3d29a639ff 100644
>>>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>>>> @@ -1150,6 +1150,31 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata)
>>>>          drvdata->trfcr = trfcr;
>>>>   }
>>>>
>>>> +/*
>>>> + * The following errata on applicable cpu rangess affect the CCITMIN filed
>>>
>>> s/rangess/ranges
>>
>> Fixed.
>>
>>>
>>>> + * in TCRIDR3 register. Software read for the field returns 0x100 limiting
>>>> + * the cycle threshold granularity, where as the right value should have
>>>> + * been 0x4, which is well supported in the hardware.
>>>> + */
>>>> +static struct midr_range etm_wrong_ccitmin_cpus[] = {
>>>> +       /* Erratum #1490853 - Cortex-A76 */
>>>> +       MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 4, 0),
>>>> +       /* Erratum #1490853 - Neoverse-N1 */
>>>> +       MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 4, 0),
>>>> +       /* Erratum #1491015 - Cortex-A77 */
>>>> +       MIDR_RANGE(MIDR_CORTEX_A77, 0, 0, 1, 0),
>>>> +       /* Erratum #1502854 - Cortex-X1 */
>>>> +       MIDR_REV(MIDR_CORTEX_X1, 0, 0),
>>>> +       /* Erratum #1619801 - Neoverse-V1 */
>>>> +       MIDR_REV(MIDR_NEOVERSE_V1, 0, 0),
>>>> +       {},
>>>> +};
>>>> +
>>>> +static bool etm4_work_around_wrong_ccitmin(void)
>>>
>>> This is not the actual work around - perhaps this should be
>>> etm4_core_reads_wrong_ccitmin()?
>>
>> Sounds better, will rename the function.
> 
> Or even move the drvdata->ccitmin == 256 check to this function,
> pass in the drvdata as an argument. That way, the function
> completes its meaning. i.e., it reads wrong value only if the value
> is 256.

Something like the following ?

--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
@@ -1159,9 +1159,20 @@ static struct midr_range etm_wrong_ccitmin_cpus[] = {
        {},
 };
 
-static bool etm4_core_reads_wrong_ccitmin(void)
+static bool etm4_core_reads_wrong_ccitmin(struct etmv4_drvdata *drvdata)
 {
-       return is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus);
+       if (is_midr_in_range_list(read_cpuid_id(), etm_wrong_ccitmin_cpus)) {
+               /*
+                * Erratum affected cpus will read 256 as the minimum
+                * instruction trace cycle counting threshold where as
+                * the correct value should be 4 instead. Override the
+                * recorded value for 'drvdata->ccitmin' to workaround
+                * this problem.
+                */
+               if (drvdata->ccitmin == 256)
+                       return true;
+       }
+       return false;
 }
 
 static void etm4_init_arch_data(void *info)
@@ -1228,17 +1239,8 @@ static void etm4_init_arch_data(void *info)
        etmidr3 = etm4x_relaxed_read32(csa, TRCIDR3);
        /* CCITMIN, bits[11:0] minimum threshold value that can be programmed */
        drvdata->ccitmin = FIELD_GET(TRCIDR3_CCITMIN_MASK, etmidr3);
-       if (etm4_core_reads_wrong_ccitmin()) {
-               /*
-                * Erratum affected cpus will read 256 as the minimum
-                * instruction trace cycle counting threshold where as
-                * the correct value should be 4 instead. Override the
-                * recorded value for 'drvdata->ccitmin' to workaround
-                * this problem.
-                */
-               if (drvdata->ccitmin == 256)
-                       drvdata->ccitmin = 4;
-       }
+       if (etm4_core_reads_wrong_ccitmin(drvdata))
+               drvdata->ccitmin = 4;
 
        /* EXLEVEL_S, bits[19:16] Secure state instruction tracing */
        drvdata->s_ex_level = FIELD_GET(TRCIDR3_EXLEVEL_S_MASK, etmidr3);


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

end of thread, other threads:[~2023-08-18 10:46 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-11  3:45 [PATCH V3 0/3] coresight: etm: Make cycle count threshold user configurable Anshuman Khandual
2023-08-11  3:45 ` Anshuman Khandual
2023-08-11  3:45 ` [PATCH V3 1/3] coresight: etm: Override TRCIDR3.CCITMIN on errata affected cpus Anshuman Khandual
2023-08-11  3:45   ` Anshuman Khandual
2023-08-11  9:03   ` Mike Leach
2023-08-11  9:03     ` Mike Leach
2023-08-11  9:31     ` Suzuki K Poulose
2023-08-11  9:31       ` Suzuki K Poulose
2023-08-18  8:45     ` Anshuman Khandual
2023-08-18  8:45       ` Anshuman Khandual
2023-08-18  8:50       ` Suzuki K Poulose
2023-08-18  8:50         ` Suzuki K Poulose
2023-08-18 10:45         ` Anshuman Khandual
2023-08-18 10:45           ` Anshuman Khandual
2023-08-11  3:45 ` [PATCH V3 2/3] coresight: etm: Make cycle count threshold user configurable Anshuman Khandual
2023-08-11  3:45   ` Anshuman Khandual
2023-08-11  8:52   ` Mike Leach
2023-08-11  8:52     ` Mike Leach
2023-08-11  8:57     ` Anshuman Khandual
2023-08-11  8:57       ` Anshuman Khandual
2023-08-11  9:04       ` Mike Leach
2023-08-11  9:04         ` Mike Leach
2023-08-11  3:46 ` [PATCH V3 3/3] Documentation: coresight: Add cc_threshold tunable Anshuman Khandual
2023-08-11  3:46   ` Anshuman Khandual
2023-08-11  9:05   ` Mike Leach
2023-08-11  9:05     ` Mike Leach

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.