This series is minor refactoring for trace filter and comes from the
previous patch set [1].
The first patch introduces a new helper has_feat_trf() for detecing if
FEAT_TRF is implemented. This is more readable.
The second patch refactors the ts_source_show() function. It simplifies
to directly return the TS value when FEAT_TRF is implemented.
[1] https://lore.kernel.org/linux-arm-kernel/20250519174945.2245271-1-leo.yan@a…
Changes from v2:
- Added the has_feat_trf() helper for more readable (James Clark)
Leo Yan (2):
coresight: etm4x: Introduce has_feat_trf() helper
coresight: etm4x: Refactor returning TS field
drivers/hwtracing/coresight/coresight-etm4x-core.c | 8 ++++----
drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 18 +++---------------
drivers/hwtracing/coresight/coresight-etm4x.h | 9 +++++++++
3 files changed, 16 insertions(+), 19 deletions(-)
--
2.34.1
On Wed, 14 May 2025 17:19:48 +0100, Yeoreum Yun wrote:
> While enable active config via cscfg_csdev_enable_active_config(),
> active config could be deactivated via configfs' sysfs interface.
> This could make UAF issue in below scenario:
>
> CPU0 CPU1
> (sysfs enable) load module
> cscfg_load_config_sets()
> activate config. // sysfs
> (sys_active_cnt == 1)
> ...
> cscfg_csdev_enable_active_config()
> lock(csdev->cscfg_csdev_lock)
> // here load config activate by CPU1
> unlock(csdev->cscfg_csdev_lock)
>
> [...]
Applied, thanks!
[1/3] coresight/etm4: fix missing disable active config
https://git.kernel.org/coresight/c/895b12b7
[2/3] coresight: holding cscfg_csdev_lock while removing cscfg from csdev
https://git.kernel.org/coresight/c/53b9e265
[3/3] coresight: prevent deactivate active config while enabling the config
https://git.kernel.org/coresight/c/408c97c4
Best regards,
--
Suzuki K Poulose <suzuki.poulose(a)arm.com>
The timestamp appears all zeros with latest kernel, which is caused by
wrong bit field setting. The first patch fixes the regression.
The second patch refactors the ts_source_show() function. It simplifies
the code for directly returning the TS value.
Changes from v1:
- Retained the older condition check (Suzuki).
Leo Yan (2):
coresight: etm4x: Fix timestamp bit field handling
coresight: etm4x: Refactor returning TS field
drivers/hwtracing/coresight/coresight-etm4x-core.c | 2 +-
drivers/hwtracing/coresight/coresight-etm4x-sysfs.c | 16 ++--------------
2 files changed, 3 insertions(+), 15 deletions(-)
--
2.34.1
ETF may fail to re-enable after reading, and driver->reading will
not be set to false, this will cause failure to enable/disable to ETF.
This change set driver->reading to false even if re-enabling fail.
Fixes: 669c4614236a ("coresight: tmc: Don't enable TMC when it's not ready.")
Co-developed-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
Signed-off-by: Yuanfang Zhang <quic_yuanfang(a)quicinc.com>
Signed-off-by: Mao Jinlong <quic_jinlmao(a)quicinc.com>
---
drivers/hwtracing/coresight/coresight-tmc-etf.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index d858740001c2..c9e2d95ae295 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -747,7 +747,6 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
char *buf = NULL;
enum tmc_mode mode;
unsigned long flags;
- int rc = 0;
/* config types are set a boot time and never change */
if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETB &&
@@ -773,11 +772,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
* can't be NULL.
*/
memset(drvdata->buf, 0, drvdata->size);
- rc = __tmc_etb_enable_hw(drvdata);
- if (rc) {
- raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
- return rc;
- }
+ __tmc_etb_enable_hw(drvdata);
} else {
/*
* The ETB/ETF is not tracing and the buffer was just read.
--
2.25.1
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(a)arm.com>
Fixes: c382ee674c8b ("arm64/sysreg/tools: Move TRFCR definitions to sysreg")
Signed-off-by: Leo Yan <leo.yan(a)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);
--
2.34.1
Hi Levi,
On Wed, May 14, 2025 at 12:04:39PM +0100, Yeoreum Yun wrote:
[...]
> > > +static bool cscfg_config_desc_get(struct cscfg_config_desc *config_desc)
> >
> > I would like to change the return type to int, so the error is handled
> > within the function. As a result, the caller _cscfg_activate_config()
> > does not need to explicitly return an error value.
>
> I think it's not good since cscfg_config_desc_get() failed only when
> try_module_get() failed and its return type is "bool".
Understood. I thought it would be easier for later maintenance if we
encapsulate error handling in the function, but I don't have strong
opinion. It is fine for me to return bool type.
Thanks,
Leo