Subject: Re: [PATCH v2 05/12] arm64: Basic Branch Target Identification support

From: Mark Rutland <[email-masked]>
Date: Fri, 18 Oct 2019 12:10:03 +0100

On Fri, Oct 11, 2019 at 06:20:15PM +0100, Dave Martin wrote:
> On Fri, Oct 11, 2019 at 04:10:29PM +0100, Mark Rutland wrote:
> > On Thu, Oct 10, 2019 at 07:44:33PM +0100, Dave Martin wrote:
> > > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > > +{
> > > +	if (system_supports_bti() && (prot & PROT_BTI))
> > > +		return VM_ARM64_BTI;
> > > +
> > > +	return 0;
> > > +}
> > 
> > Can we call this arch_calc_vm_prot_bits() directly, with all the
> > arguments:
> > 
> > static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot,
> > 						   unsigned long pkey)
> > {
> > 	...
> > }
> > #define arch_calc_vm_prot_bits arch_calc_vm_prot_bits
> > 
> > ... as that makes it a bit easier to match definition with use, and just
> > definign the name makes it a bit clearer that that's probably for the
> > benefit of some ifdeffery.
> > 
> > Likewise for the other functions here.
> > 
> > > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > > +{
> > > +	return (vm_flags & VM_ARM64_BTI) ? __pgprot(PTE_GP) : __pgprot(0);
> > > +}
> > > +
> > > +#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr)
> > > +static inline int arm64_validate_prot(unsigned long prot, unsigned long addr)
> 
> Can do, though it looks like a used sparc as a template, and that has a
> sparc_ prefix.
> 
> powerpc uses the generic name, as does x86 ... in its UAPI headers.
> Odd.
> 
> I can change the names here, though I'm not sure it adds a lot of value.
> 
> If you feel strongly I can do it.

I'd really prefer it because it minimizes surprises, and makes it much
easier to hop around the codebase and find the thing you're looking for.

I'll reply on the other issue in a separate reply.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel