Re: [PATCH V7 3/6] arm64/perf: Add branch stack support in struct arm_pmu

[Mark Rutland]

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.