linux-kernel - Re: [PATCH V7 3/6] arm64/perf: Add branch stack support in struct arm

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: <Y+P3g8/85FIe/sUK@FVFF77S0Q05N.cambridge.arm.com>
Date:   Wed, 8 Feb 2023 19:26:59 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     Anshuman Khandual <anshuman.khandual@....com>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH V7 3/6] arm64/perf: Add branch stack support in struct
 arm_pmu

On Fri, Jan 13, 2023 at 09:45:22AM +0530, Anshuman Khandual wrote:
> 
> On 1/12/23 19:24, Mark Rutland wrote:
> > On Thu, Jan 05, 2023 at 08:40:36AM +0530, Anshuman Khandual wrote:
> >>  struct arm_pmu {
> >>  	struct pmu	pmu;
> >>  	cpumask_t	supported_cpus;
> >>  	char		*name;
> >>  	int		pmuver;
> >> +	int		features;
> >>  	irqreturn_t	(*handle_irq)(struct arm_pmu *pmu);
> >>  	void		(*enable)(struct perf_event *event);
> >>  	void		(*disable)(struct perf_event *event);
> > 
> > Hmm, we already have the secure_access field separately. How about we fold that
> > in and go with:
> > 
> > 	unsigned int	secure_access    : 1,
> > 			has_branch_stack : 1;
> 
> Something like this would work, but should we use __u32 instead of unsigned int
> to ensure 32 bit width ?

I don't think that's necessary; the exact size doesn't really matter, and
unsigned int is 32-bits on all targets suppropted by Linux, not just arm and
arm64.

I do agree that if this were a userspace ABI detail, it might be preferable to
use __u32. However, I think using it here gives the misleading impression that
there is an ABI concern when there is not, and as above it's not necessary, so
I'd prefer unsigned int here.

Thanks,
Mark.