| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3a731a9e-0621-42b6-b7fc-4b0fd9b7da6e@linaro.org>
Date: Wed, 1 Oct 2025 13:40:37 +0100
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jonathan Corbet <corbet@....net>, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 5/6] coresight: Add format attribute for setting the
timestamp interval
On 30/09/2025 4:14 pm, Leo Yan wrote:
> On Thu, Aug 14, 2025 at 11:49:56AM +0100, James Clark wrote:
>
> [...]
>
>> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
>> @@ -25,6 +25,11 @@
>> #include "coresight-syscfg.h"
>> #include "coresight-trace-id.h"
>>
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> +#include <linux/perf/arm_pmu.h>
>> +#include "coresight-etm4x.h"
>> +#endif
>> +
>> static struct pmu etm_pmu;
>> static bool etm_perf_up;
>>
>> @@ -69,7 +74,10 @@ 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");
>> -
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> +/* Interval = (2 ^ ts_level) */
>> +GEN_PMU_FORMAT_ATTR(ts_level);
>> +#endif
>>
>> /*
>> * contextid always traces the "PID". The PID is in CONTEXTIDR_EL1
>> @@ -103,6 +111,9 @@ static struct attribute *etm_config_formats_attr[] = {
>> &format_attr_configid.attr,
>> &format_attr_branch_broadcast.attr,
>> &format_attr_cc_threshold.attr,
>> +#if IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X)
>> + &format_attr_ts_level.attr,
>> +#endif
>
> By using .visible() callback for attrs, we can improve a bit code
> without spreading "#ifdef IS_ENABLED()" in this file. E.g.,
>
> static umode_t format_attr_is_visible(struct kobject *kobj,
> struct attribute *attr, int n)
> {
> struct device *dev = kobj_to_dev(kobj);
>
> if (attr == &format_attr_ts_level.attr &&
> !IS_ENABLED(CONFIG_CORESIGHT_SOURCE_ETM4X))
> return 0;
>
> return attr->mode;
> }
>
> Otherwise, LGTM:
>
> Reviewed-by: Leo Yan <leo.yan@....com>
>
Unfortunately that won't work because you'd have to always include
coresight-etm4x.h. This file is compiled for both arm32 and arm64 so it
would break the arm32 build.
I could define the TTR_CFG_FLD_ts_level_* stuff somewhere else but then
it becomes messier than just doing the #ifdefs here.
>> NULL,
>> };
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-core.c b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> index 1a2d02bdcb88..42277c201d4f 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c
>> @@ -28,6 +28,7 @@
>> #include <linux/amba/bus.h>
>> #include <linux/seq_file.h>
>> #include <linux/uaccess.h>
>> +#include <linux/perf/arm_pmu.h>
>> #include <linux/perf_event.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> @@ -615,7 +616,7 @@ static void etm4_enable_hw_smp_call(void *info)
>> * +--------------+
>> * |
>> * +------v-------+
>> - * | Counter x | (reload to 1 on underflow)
>> + * | Counter x | (reload to 2 ^ ts_level on underflow)
>> * +--------------+
>> * |
>> * +------v--------------+
>> @@ -626,11 +627,17 @@ static void etm4_enable_hw_smp_call(void *info)
>> * | Timestamp Generator | (timestamp on resource y)
>> * +----------------------+
>> */
>> -static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
>> +static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata,
>> + struct perf_event_attr *attr)
>> {
>> int ctridx;
>> int rselector;
>> struct etmv4_config *config = &drvdata->config;
>> + u8 ts_level = ATTR_CFG_GET_FLD(attr, ts_level);
>> +
>> + /* Disable when ts_level == MAX */
>> + if (ts_level == FIELD_GET(ATTR_CFG_FLD_ts_level_MASK, UINT_MAX))
>> + return 0;
>>
>> /* No point in trying if we don't have at least one counter */
>> if (!drvdata->nr_cntr)
>> @@ -666,12 +673,8 @@ static int etm4_config_timestamp_event(struct etmv4_drvdata *drvdata)
>> return -ENOSPC;
>> }
>>
>> - /*
>> - * Initialise original and reload counter value to the smallest
>> - * possible value in order to get as much precision as we can.
>> - */
>> - config->cntr_val[ctridx] = 1;
>> - config->cntrldvr[ctridx] = 1;
>> + /* Initialise original and reload counter value. */
>> + config->cntr_val[ctridx] = config->cntrldvr[ctridx] = 1 << ts_level;
>>
>> /*
>> * Trace Counter Control Register TRCCNTCTLRn
>> @@ -761,7 +764,7 @@ static int etm4_parse_event_config(struct coresight_device *csdev,
>> * order to correlate instructions executed on different CPUs
>> * (CPU-wide trace scenarios).
>> */
>> - ret = etm4_config_timestamp_event(drvdata);
>> + ret = etm4_config_timestamp_event(drvdata, attr);
>>
>> /*
>> * No need to go further if timestamp intervals can't
>> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h
>> index aaa6633b2d67..54558de158fa 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm4x.h
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
>> @@ -598,6 +598,12 @@
>> #define ETM_CNTR_MAX_VAL 0xFFFF
>> #define ETM_TRACEID_MASK 0x3f
>>
>> +#define ATTR_CFG_FLD_ts_level_CFG config3
>> +#define ATTR_CFG_FLD_ts_level_LO 12
>> +#define ATTR_CFG_FLD_ts_level_HI 15
>> +#define ATTR_CFG_FLD_ts_level_MASK GENMASK(ATTR_CFG_FLD_ts_level_HI, \
>> + ATTR_CFG_FLD_ts_level_LO)
>> +
>> /* ETMv4 programming modes */
>> #define ETM_MODE_EXCLUDE BIT(0)
>> #define ETM_MODE_LOAD BIT(1)
>>
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists