| 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: <25d64842-79c7-4d70-a5d7-08cd60208681@linaro.org>
Date: Tue, 20 May 2025 14:24:23 +0100
From: James Clark <james.clark@...aro.org>
To: Leo Yan <leo.yan@....com>
Cc: Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
Adrian Hunter <adrian.hunter@...el.com>, Jonathan Corbet <corbet@....net>,
Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>,
Joey Gouly <joey.gouly@....com>, Suzuki K Poulose <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linux-doc@...r.kernel.org, kvmarm@...ts.linux.dev
Subject: Re: [PATCH 07/10] perf: arm_spe: Add support for filtering on data
source
On 20/05/2025 12:43 pm, Leo Yan wrote:
> On Tue, May 06, 2025 at 12:41:39PM +0100, James Clark wrote:
>> SPE_FEAT_FDS adds the ability to filter on the data source of packets.
>> Like the other existing filters, enable filtering with PMSFCR_EL1.FDS
>> when any of the filter bits are set.
>>
>> Each bit maps to data sources 0-63 described by bits[0:5] in the data
>> source packet (although the full range of data source is 16 bits so
>> higher value data sources can't be filtered on). The filter is an OR of
>> all the bits, so for example setting bits 0 and 3 filters packets from
>> data sources 0 OR 3.
>>
>> Signed-off-by: James Clark <james.clark@...aro.org>
>> ---
>> drivers/perf/arm_spe_pmu.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
>> index 9309b846f642..d04318411f77 100644
>> --- a/drivers/perf/arm_spe_pmu.c
>> +++ b/drivers/perf/arm_spe_pmu.c
>> @@ -87,6 +87,7 @@ struct arm_spe_pmu {
>> #define SPE_PMU_FEAT_INV_FILT_EVT (1UL << 6)
>> #define SPE_PMU_FEAT_DISCARD (1UL << 7)
>> #define SPE_PMU_FEAT_EFT (1UL << 8)
>> +#define SPE_PMU_FEAT_FDS (1UL << 9)
>> #define SPE_PMU_FEAT_DEV_PROBED (1UL << 63)
>> u64 features;
>>
>> @@ -232,6 +233,10 @@ static const struct attribute_group arm_spe_pmu_cap_group = {
>> #define ATTR_CFG_FLD_inv_event_filter_LO 0
>> #define ATTR_CFG_FLD_inv_event_filter_HI 63
>>
>> +#define ATTR_CFG_FLD_data_src_filter_CFG config4 /* PMSDSFR_EL1 */
>> +#define ATTR_CFG_FLD_data_src_filter_LO 0
>> +#define ATTR_CFG_FLD_data_src_filter_HI 63
>> +
>> GEN_PMU_FORMAT_ATTR(ts_enable);
>> GEN_PMU_FORMAT_ATTR(pa_enable);
>> GEN_PMU_FORMAT_ATTR(pct_enable);
>> @@ -248,6 +253,7 @@ GEN_PMU_FORMAT_ATTR(float_filter);
>> GEN_PMU_FORMAT_ATTR(float_filter_mask);
>> GEN_PMU_FORMAT_ATTR(event_filter);
>> GEN_PMU_FORMAT_ATTR(inv_event_filter);
>> +GEN_PMU_FORMAT_ATTR(data_src_filter);
>> GEN_PMU_FORMAT_ATTR(min_latency);
>> GEN_PMU_FORMAT_ATTR(discard);
>>
>> @@ -268,6 +274,7 @@ static struct attribute *arm_spe_pmu_formats_attr[] = {
>> &format_attr_float_filter_mask.attr,
>> &format_attr_event_filter.attr,
>> &format_attr_inv_event_filter.attr,
>> + &format_attr_data_src_filter.attr,
>> &format_attr_min_latency.attr,
>> &format_attr_discard.attr,
>> NULL,
>> @@ -286,6 +293,9 @@ static umode_t arm_spe_pmu_format_attr_is_visible(struct kobject *kobj,
>> if (attr == &format_attr_inv_event_filter.attr && !(spe_pmu->features & SPE_PMU_FEAT_INV_FILT_EVT))
>> return 0;
>>
>> + if (attr == &format_attr_data_src_filter.attr && !(spe_pmu->features & SPE_PMU_FEAT_FDS))
>> + return 0;
>> +
>> if ((attr == &format_attr_branch_filter_mask.attr ||
>> attr == &format_attr_load_filter_mask.attr ||
>> attr == &format_attr_store_filter_mask.attr ||
>> @@ -406,6 +416,9 @@ static u64 arm_spe_event_to_pmsfcr(struct perf_event *event)
>> if (ATTR_CFG_GET_FLD(attr, inv_event_filter))
>> reg |= PMSFCR_EL1_FnE;
>>
>> + if (ATTR_CFG_GET_FLD(attr, data_src_filter))
>> + reg |= PMSFCR_EL1_FDS;
>> +
>> if (ATTR_CFG_GET_FLD(attr, min_latency))
>> reg |= PMSFCR_EL1_FL;
>>
>> @@ -430,6 +443,12 @@ static u64 arm_spe_event_to_pmslatfr(struct perf_event *event)
>> return FIELD_PREP(PMSLATFR_EL1_MINLAT, ATTR_CFG_GET_FLD(attr, min_latency));
>> }
>>
>> +static u64 arm_spe_event_to_pmsdsfr(struct perf_event *event)
>> +{
>> + struct perf_event_attr *attr = &event->attr;
>> + return ATTR_CFG_GET_FLD(attr, data_src_filter);
>> +}
>
> Seems to me, arm_spe_event_to_pmsdsfr() is not needed as it does not do
> any conversion from event config to register value. So simply read the
> field value in opened code would be fine.
>
> I am fine to keep it and would leave SPE driver maintainers to decide
> which is preferring. Otherwise, LGTM:
>
> Reviewed-by: Leo Yan <leo.yan@....com>
>
It's purely for consistency with the existing code. See
arm_spe_event_to_pmsevfr() etc.
>> +
>> static void arm_spe_pmu_pad_buf(struct perf_output_handle *handle, int len)
>> {
>> struct arm_spe_pmu_buf *buf = perf_get_aux(handle);
>> @@ -788,6 +807,10 @@ static int arm_spe_pmu_event_init(struct perf_event *event)
>> if (arm_spe_event_to_pmsnevfr(event) & arm_spe_pmsevfr_res0(spe_pmu->pmsver))
>> return -EOPNOTSUPP;
>>
>> + if (arm_spe_event_to_pmsdsfr(event) &&
>> + !(spe_pmu->features & SPE_PMU_FEAT_FDS))
>> + return -EOPNOTSUPP;
>> +
>> if (attr->exclude_idle)
>> return -EOPNOTSUPP;
>>
>> @@ -857,6 +880,11 @@ static void arm_spe_pmu_start(struct perf_event *event, int flags)
>> write_sysreg_s(reg, SYS_PMSNEVFR_EL1);
>> }
>>
>> + if (spe_pmu->features & SPE_PMU_FEAT_FDS) {
>> + reg = arm_spe_event_to_pmsdsfr(event);
>> + write_sysreg_s(reg, SYS_PMSDSFR_EL1);
>> + }
>> +
>> reg = arm_spe_event_to_pmslatfr(event);
>> write_sysreg_s(reg, SYS_PMSLATFR_EL1);
>>
>> @@ -1116,6 +1144,9 @@ static void __arm_spe_pmu_dev_probe(void *info)
>> if (FIELD_GET(PMSIDR_EL1_EFT, reg))
>> spe_pmu->features |= SPE_PMU_FEAT_EFT;
>>
>> + if (FIELD_GET(PMSIDR_EL1_FDS, reg))
>> + spe_pmu->features |= SPE_PMU_FEAT_FDS;
>> +
>> /* This field has a spaced out encoding, so just use a look-up */
>> fld = FIELD_GET(PMSIDR_EL1_INTERVAL, reg);
>> switch (fld) {
>>
>> --
>> 2.34.1
>>
>
Powered by blists - more mailing lists