Re: [PATCH rsa/hwcap2_v3] Implement AT_HWCAP2 version 3

[Ryan S Arnold <rsa at linux dot vnet dot ibm dot com>]

On Tue, 2013-06-11 at 15:16 -0700, Roland McGrath wrote:
> The details of the change seem OK except for some formatting trivia below.
> 
> My main concern with this is that it doesn't seem to make any sense
> for configurations where auxv entry values are 64 bits already.
> Please explain what it's all supposed to mean in that case.

The AT_HWCAP[2] entries are always read from the env as (ElfW(auxv_t) *)
so this would only be a concern on 64-bit platforms.

In this case the kernel may continue to use bits in AT_HWCAP (and use
64-bit masks) and then ignore AT_HWCAP2. Or the kernel can use 32-bit
masks and use AT_HWCAP2. (As of now none of these systems actually need
to use the higher 32-bits)

It'd be nice if such systems had 64-bits for both AT_HWCAP and
AT_HWCAP2, but ultimately our dl_hwcap is only 64-bits anyway so we
couldn't store all of the bits without adding another _dl_hwcap entry

> 
> > 	* elf/elf.h [AT_HWCAP2]: Add a new a_type entry.
> 
> Parens, not brackets.  Brackets are for #if conditions.

OK on this an all the other formatting trivia.

> > diff --git a/ports/sysdeps/powerpc/dl-procinfo.h b/ports/sysdeps/powerpc/dl-procinfo.h
> > index b45465c..a9d98d3 100644
> > --- a/ports/sysdeps/powerpc/dl-procinfo.h
> > +++ b/ports/sysdeps/powerpc/dl-procinfo.h
> > @@ -19,6 +19,7 @@
> >  #ifndef _DL_PROCINFO_H
> >  #define _DL_PROCINFO_H 1
> >  
> > +#include <stdint.h>
> >  #include <ldsodefs.h>
> 
> You aren't adding any use of uint64_t or whatnot here, so don't add
> the header.  Even if you were, this isn't needed since ldsodefs.h
> already defines things of type uint64_t.
> 
> > -_dl_procinfo (int word)
> > +_dl_procinfo (unsigned int type, int word)
> 
> Shouldn't the type of WORD be uint64_t, or at least an unsigned type?

I just left the 'int word' as I found it.  As far as I can tell there's
no reason for it to be signed.  So can change this.

The reason WORD can't be uint64_t (which I was going to do originally)
is because this code is called out of _dl_show_auxv which parses the
preserved dl_auxv directly and processes each entry as it is found,
which means that one part or the other of the 64-bit hwcap might not yet
have been encountered thus far.

Ryan