Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap

[Suzuki K Poulose <suzuki dot poulose at arm dot com>]

Hi,

On 02/04/2019 16:06, Andrew Murray wrote:
> On Tue, Apr 02, 2019 at 03:58:21PM +0100, Dave Martin wrote:
> > On Mon, Apr 01, 2019 at 11:45:11AM +0100, Andrew Murray wrote:
> > > The introduction of AT_HWCAP2 introduced accessors which ensure that
> > > hwcap features are set and tested appropriately.
> > >
> > > Let's now mandate access to elf_hwcap via these accessors by making
> > > elf_hwcap static within cpufeature.c.
> >
> > Looks reasonable except for a couple of minor nits below.
> >
> > I had wondered whether putting these accessors out of line would affect
> > any hot paths, but I can't see these used from anything that looks like
> > a hot path.  So we're probably fine.
> >
> > cpus_have_const_cap() is preferred for places where this matters,
> > anyway.

Btw, thats for cpu_hwcaps, which is completely different from elf_hwcaps.

> > [...]
> >
> > > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > > index 986ceeacd19f..84ca52fa75e5 100644
> > > --- a/arch/arm64/kernel/cpufeature.c
> > > +++ b/arch/arm64/kernel/cpufeature.c
> > > @@ -35,8 +35,7 @@
> > >   #include <asm/traps.h>
> > >   #include <asm/virt.h>
> > >   
> > > -unsigned long elf_hwcap __read_mostly;
> > > -EXPORT_SYMBOL_GPL(elf_hwcap);
> > > +static unsigned long elf_hwcap __read_mostly;
> >
> > Now that this doesn't correspond directly to ELF_HWCAP any more and we
> > hide it, can we rename it to avoid confusion?
> >
> > Maybe "kernel_hwcap"?
>
> Yes this seems reasonable.

nit:

As mentioned above we have "cpu_hwcaps" for the features only internally
by the kernel. Naming it "kernel_hwcap" kind of looses the hint that the
major purpose is for userspace consumption and could easily confuse with
the poorly named "cpu_hwcaps" which should have been called kernel_hwcaps.

How about "user_hwcaps" ? Or preferrably something closer to that.

Cheers
Suzuki