linux-kernel - Re: [PATCH] drivers: perf: arm_pmuv3: Update 'pmc

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: <66125bc2-ff2a-a209-a753-c2e3d8310a45@arm.com>
Date:   Mon, 9 Oct 2023 10:43:34 +0100
From:   James Clark <james.clark@....com>
To:     Anshuman Khandual <anshuman.khandual@....com>,
        linux-arm-kernel@...ts.infradead.org,
        Rob Herring <robh@...nel.org>, raphael.gault@....com
Cc:     Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers: perf: arm_pmuv3: Update 'pmc_width' based on
 actual HW event width



On 09/10/2023 05:37, Anshuman Khandual wrote:
> This updates 'perf_event_mmap_page->pmc_width' based on actual HW event's
> width that are currently missing i.e ARMPMU_EVT_63BIT and ARMPMU_EVT_47BIT.
> 

Might be worth adding why this is needed or what the actual effect is.

> Cc: Will Deacon <will@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: linux-arm-kernel@...ts.infradead.org
> Cc: linux-kernel@...r.kernel.org
> Signed-off-by: Anshuman Khandual <anshuman.khandual@....com>
> ---
> This applies on v6.6-rc5.
> 
>  drivers/perf/arm_pmuv3.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index fe4db1831662..94723d00548e 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -1375,6 +1375,10 @@ void arch_perf_update_userpage(struct perf_event *event,
>  	if (userpg->cap_user_rdpmc) {
>  		if (event->hw.flags & ARMPMU_EVT_64BIT)
>  			userpg->pmc_width = 64;
> +		else if (event->hw.flags & ARMPMU_EVT_63BIT)
> +			userpg->pmc_width = 63;
> +		else if (event->hw.flags & ARMPMU_EVT_47BIT)
> +			userpg->pmc_width = 47;

Although it doesn't explicitly say it, the bit of the docs about
pmc_width in Documentation/arch/arm64/perf.rst loosely implies that this
is always either 64 or 32. Now that this isn't the case it could mislead
someone in userspace that they don't have to handle the now arbitrary
bit widths rather than just whole bytes/ints.

I think the fix is as simple as adding something like "the width may not
match the requested value or necessarily be a multiple of 8". Unless we
think this is already widely known and I suppose we could leave it as
is. (The existing bit in perf that uses it already handles it correctly).

>  		else
>  			userpg->pmc_width = 32;
>  	}