Re: [PATCH] powerpc: Support auxilliary vector components for cache geometries

[Paul Clarke <pc at us dot ibm dot com>]

On 03/30/2017 03:14 PM, Florian Weimer wrote:
> * Paul Clarke:
> > > > +  val = getauxval (AT_L1I_CACHEGEOMETRY);
> > > > +  if (val)
> > > > +    printf("AT_L1I_CACHEGEOMETRY: associativity %ld; line size %ld\n",
> > > > +	   (val & 0xffff0000) >> 16, val & 0x0000ffff);
> > > > +  else
> > > > +    rc = EXIT_UNSUPPORTED;
> > >
> > > honestly, what is the value of this test ?  you basically just
> > > printf the values everywhere, or you exit unsupported.  there is
> > > no actual "test" here that i can see as you don't validate the
> > > results anywhere.
> >
> > I debated this with colleagues before sending (and perhaps should've
> > deferred to their experience).  I was reluctant to add new code
> > without exercising it, at least a successful compile and run.  However
> > it's difficult to determine a true "failure" case without knowing too
> > much about the kernel.  I also like that it provides an example of
> > use.  If those reasons are not sufficient, I can also remove it from
> > the patch.
>
> Can you at least add consistency checks which check that the values
> use the right endianess?  I think that part could be useful.

That seems error-prone.  Given "all possibilities", the values returned in the lowest-order and next-to-lowest-order 16 bits of a long return value from getauxval() are basically arbitrary.  I'm not sure how one could determine correct endianness of an arbitrary value.  What values are guaranteed correct or incorrect for associativity and cache line size?

Perhaps:
cachelinesize > cachesize?
associativity > cachesize?

Those are certainly not catch-all.

I'm fine dropping the test completely (as I did in v2) if it won't have sufficient value.

Regards,
PC