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

[Julien Grall <julien.grall@arm.com>]

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@lists.xen.org
http://lists.xen.org/xen-devel