linux-kernel - Re: [PATCH v2 4/5] perf: arm_cspmu: Support implementation specific event validation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2a081ab9-f1f3-569e-72e8-1c743e8357a1@arm.com>
Date:   Thu, 1 Jun 2023 16:09:54 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Ilkka Koskinen <ilkka@...amperecomputing.com>,
        Jonathan Corbet <corbet@....net>,
        Will Deacon <will@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Besar Wicaksono <bwicaksono@...dia.com>,
        Suzuki K Poulose <suzuki.poulose@....com>
Cc:     linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 4/5] perf: arm_cspmu: Support implementation specific
 event validation

On 2023-06-01 04:01, Ilkka Koskinen wrote:
> Some platforms may use e.g. different filtering mechanism and, thus,
> may need different way to validate the events.
> 
> Signed-off-by: Ilkka Koskinen <ilkka@...amperecomputing.com>
> ---
>   drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++++
>   drivers/perf/arm_cspmu/arm_cspmu.h | 2 ++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
> index b4c4ef81c719..a26f484e06b1 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> @@ -593,6 +593,10 @@ static int arm_cspmu_get_event_idx(struct arm_cspmu_hw_events *hw_events,
>   	if (idx >= cspmu->num_logical_ctrs)
>   		return -EAGAIN;
>   
> +	if (cspmu->impl.ops.validate_event &&
> +	    !cspmu->impl.ops.validate_event(cspmu, event))
> +		return -EAGAIN;

Seems like this should be -EINVAL, or maybe the callback should return 
int so it can make its own distinction (yes, I know the outer logic 
doesn't actually propagate it, but there's no reason that couldn't 
improve at some point as well).

Another thought is that once we get into imp-def conditions for whether 
an event is valid in itself, we presumably also need to consider imp-def 
conditions for whether a given pair of events are compatible to be grouped?

Thanks,
Robin.

> +
>   	set_bit(idx, hw_events->used_ctrs);
>   
>   	return idx;
> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.h b/drivers/perf/arm_cspmu/arm_cspmu.h
> index 4a29b921f7e8..0e5c316c96f9 100644
> --- a/drivers/perf/arm_cspmu/arm_cspmu.h
> +++ b/drivers/perf/arm_cspmu/arm_cspmu.h
> @@ -106,6 +106,8 @@ struct arm_cspmu_impl_ops {
>   	void (*set_ev_filter)(struct arm_cspmu *cspmu,
>   			      struct hw_perf_event *hwc,
>   			      u32 filter);
> +	/* Implementation specific event validation */
> +	bool (*validate_event)(struct arm_cspmu *cspmu, struct perf_event *new);
>   	/* Hide/show unsupported events */
>   	umode_t (*event_attr_is_visible)(struct kobject *kobj,
>   					 struct attribute *attr, int unused);