Timestamps in the trace data appear as all zeros on recent kernels, although the feature works correctly on old kernels (e.g., v6.12).
Since commit c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg"), the TRFCR_ELx_TS_{VIRTUAL|GUEST_PHYSICAL|PHYSICAL} macros were updated to remove the bit shift. As a result, the driver no longer shifts bits when operates the timestamp field.
Fix this by using the FIELD_PREP() and FIELD_GET() helpers. Simplify the ts_source_show() function: return -1 when the value is zero, as this indciates an invalid value; otherwise return the decoded TS value directly.
Reported-by: Tamas Zsoldos tamas.zsoldos@arm.com Fixes: c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg") Signed-off-by: Leo Yan leo.yan@arm.com --- .../hwtracing/coresight/coresight-etm4x-core.c | 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 6a5898355a83..acb4a58e4bb9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1237,7 +1237,7 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata) * tracing at the kernel EL and EL0, forcing to use the * virtual time as the timestamp. */ - trfcr = (TRFCR_EL1_TS_VIRTUAL | + trfcr = (FIELD_PREP(TRFCR_EL1_TS_MASK, TRFCR_EL1_TS_VIRTUAL) | TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 49d5fb87a74b..8a2749eeb9a5 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2315,23 +2315,10 @@ static ssize_t ts_source_show(struct device *dev, int val; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
- if (!drvdata->trfcr) { + val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr); + if (!val) val = -1; - goto out; - } - - switch (drvdata->trfcr & TRFCR_EL1_TS_MASK) { - case TRFCR_EL1_TS_VIRTUAL: - case TRFCR_EL1_TS_GUEST_PHYSICAL: - case TRFCR_EL1_TS_PHYSICAL: - val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr); - break; - default: - val = -1; - break; - }
-out: return sysfs_emit(buf, "%d\n", val); } static DEVICE_ATTR_RO(ts_source);
On 19/05/2025 2:50 pm, Leo Yan wrote:
Timestamps in the trace data appear as all zeros on recent kernels, although the feature works correctly on old kernels (e.g., v6.12).
Since commit c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg"), the TRFCR_ELx_TS_{VIRTUAL|GUEST_PHYSICAL|PHYSICAL} macros were updated to remove the bit shift. As a result, the driver no longer shifts bits when operates the timestamp field.
Fix this by using the FIELD_PREP() and FIELD_GET() helpers. Simplify the ts_source_show() function: return -1 when the value is zero, as this indciates an invalid value; otherwise return the decoded TS value directly.
Reported-by: Tamas Zsoldos tamas.zsoldos@arm.com Fixes: c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg") Signed-off-by: Leo Yan leo.yan@arm.com
.../hwtracing/coresight/coresight-etm4x-core.c | 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c index 6a5898355a83..acb4a58e4bb9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1237,7 +1237,7 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata) * tracing at the kernel EL and EL0, forcing to use the * virtual time as the timestamp. */
- trfcr = (TRFCR_EL1_TS_VIRTUAL |
- trfcr = (FIELD_PREP(TRFCR_EL1_TS_MASK, TRFCR_EL1_TS_VIRTUAL) | TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 49d5fb87a74b..8a2749eeb9a5 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2315,23 +2315,10 @@ static ssize_t ts_source_show(struct device *dev, int val; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
- if (!drvdata->trfcr) {
- val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr);
- if (!val) val = -1;
goto out;- }
- switch (drvdata->trfcr & TRFCR_EL1_TS_MASK) {
- case TRFCR_EL1_TS_VIRTUAL:
- case TRFCR_EL1_TS_GUEST_PHYSICAL:
- case TRFCR_EL1_TS_PHYSICAL:
val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr);break;- default:
val = -1;break;- }
-out: return sysfs_emit(buf, "%d\n", val); } static DEVICE_ATTR_RO(ts_source);
Reviewed-by: James Clark james.clark@linaro.org
On 19/05/2025 15:11, James Clark wrote:
On 19/05/2025 2:50 pm, Leo Yan wrote:
Timestamps in the trace data appear as all zeros on recent kernels, although the feature works correctly on old kernels (e.g., v6.12).
Since commit c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg"), the TRFCR_ELx_TS_{VIRTUAL|GUEST_PHYSICAL|PHYSICAL} macros were updated to remove the bit shift. As a result, the driver no longer shifts bits when operates the timestamp field.
Fix this by using the FIELD_PREP() and FIELD_GET() helpers. Simplify the ts_source_show() function: return -1 when the value is zero, as this indciates an invalid value; otherwise return the decoded TS value directly.
Reported-by: Tamas Zsoldos tamas.zsoldos@arm.com Fixes: c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg") Signed-off-by: Leo Yan leo.yan@arm.com
.../hwtracing/coresight/coresight-etm4x-core.c | 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/ drivers/hwtracing/coresight/coresight-etm4x-core.c index 6a5898355a83..acb4a58e4bb9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1237,7 +1237,7 @@ static void cpu_detect_trace_filtering(struct etmv4_drvdata *drvdata) * tracing at the kernel EL and EL0, forcing to use the * virtual time as the timestamp. */ - trfcr = (TRFCR_EL1_TS_VIRTUAL | + trfcr = (FIELD_PREP(TRFCR_EL1_TS_MASK, TRFCR_EL1_TS_VIRTUAL) | TRFCR_EL1_ExTRE | TRFCR_EL1_E0TRE); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/ drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 49d5fb87a74b..8a2749eeb9a5 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2315,23 +2315,10 @@ static ssize_t ts_source_show(struct device *dev, int val; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); - if (!drvdata->trfcr) { + val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr); + if (!val)
I think this might be problematic. TS==0 is a reserved value for software, and doesn't imply a TS is not in effect. Thus I think we should retain the older check as before to ensure TRFCR is effective.
The rest looks good to me.
Suzuki
val = -1; - goto out; - }
- switch (drvdata->trfcr & TRFCR_EL1_TS_MASK) { - case TRFCR_EL1_TS_VIRTUAL: - case TRFCR_EL1_TS_GUEST_PHYSICAL: - case TRFCR_EL1_TS_PHYSICAL: - val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr); - break; - default: - val = -1; - break; - } -out: return sysfs_emit(buf, "%d\n", val); } static DEVICE_ATTR_RO(ts_source);
Reviewed-by: James Clark james.clark@linaro.org
On Mon, May 19, 2025 at 04:12:32PM +0100, Suzuki Kuruppassery Poulose wrote:
[...]
--- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2315,23 +2315,10 @@ static ssize_t ts_source_show(struct device *dev, int val; struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent); - if (!drvdata->trfcr) { + val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr); + if (!val)
I think this might be problematic. TS==0 is a reserved value for software, and doesn't imply a TS is not in effect.
Good point.
Thus I think we should retain the older check as before to ensure TRFCR is effective.
If we intend to return TS values directly (include '0' case), then it makes sense to make change like:
if (!drvdata->trfcr) val = -1; else val = FIELD_GET(TRFCR_EL1_TS_MASK, drvdata->trfcr);
return sysfs_emit(buf, "%d\n", val);
I checked again in Linux perf tool, it would be fine if we return 0 for TS==0 case, as the perf tool now only use virtual time counter (see cs_etm__has_virtual_ts()).
@James, could you confirm if this fine?
Thanks, Leo
-
James Clark -
Leo Yan -
Suzuki K Poulose