Re: [Xen-devel] [RFC 05/16] xen/arm: Add cpu_hwcap bitmap

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 09/05/2016 10:53, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
> > diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> > index 7b519cd..2bebad1 100644
> > --- a/xen/include/asm-arm/cpufeature.h
> > +++ b/xen/include/asm-arm/cpufeature.h
> > @@ -35,6 +35,35 @@
> >   #endif
> >   #define cpu_has_security  (boot_cpu_feature32(security) > 0)
> >
> > +#define ARM_NCAPS           0
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <xen/types.h>
> > +#include <xen/lib.h>
> > +#include <xen/bitops.h>
> > +
> > +extern DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> > +
> > +static inline bool_t cpus_have_cap(unsigned int num)
> > +{
> > +    if ( num >= ARM_NCAPS )
> > +        return 0;
> > +
> > +    return test_bit(num, cpu_hwcaps);
> > +}
> > +
> > +static inline void cpus_set_cap(unsigned int num)
> > +{
> > +    if (num >= ARM_NCAPS)
> > +        printk(XENLOG_WARNING "Attempt to set an illegal CPU capability (%d >= 
> > %d)\n",
> > +               num, ARM_NCAPS);
> > +    else
> > +        __set_bit(num, cpu_hwcaps);
> > +}
> > +
> > +#endif /* __ASSEMBLY__ */
> Why everything static line? Wouldn't it better to add the functions to
> cpufeature.c? They are not going to be called too many times, right?
This code is based on Linux where the 2 functions were already static 
inline.
I agree that cpus_set_cap won't be called often (mostly CPU bring up), 
however cpus_have_cap could be used at runtime to check whether the 
AArch32 mode is present...
But I think those 2 functions should be kept together.

Note that today, we always use the boot CPU data to know if a feature 
exists (see include/asm-arm/cpufeatures.h).
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel