Re: [Xen-devel] [PATCH v5 2/7] xen/arm: cpufeature: Provide an helper to check if a capability is supported

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Wed, Jul 20, 2016 at 04:25:55PM +0100, Julien Grall wrote:
> The CPU capabilities will be set depending on the value found in the CPU
> registers. This patch provides a generic to go through a set of capabilities
> and find which one should be enabled.
> 
> The parameter "info" is used to display the kind of capability updated (e.g
> workaround, feature...).
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> with just one minor
suggestion:

> 
> ---
>     Changes in v4:
>         - Add Stefano's acked-by
> 
>     Changes in v3:
>          - Patch added. The code was previously part of "Detect
>          silicon...".
> ---
>  xen/arch/arm/cpufeature.c        | 16 ++++++++++++++++
>  xen/include/asm-arm/cpufeature.h |  9 +++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 7a1b56b..088625b 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -24,6 +24,22 @@
>  
>  DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>  
> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> +                             const char *info)
> +{
> +    int i;
> +
> +    for ( i = 0; caps[i].matches; i++ )
> +    {
> +        if ( !caps[i].matches(&caps[i]) )

So what if the 'struct arm_cpu_capabilitues' has '->matches' set to
NULL?

Perhaps:

        if ( !caps[i].matches || !caps[i]... ?
> +            continue;
> +
> +        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
> +            printk(XENLOG_INFO "%s: %s\n", info, caps[i].desc);
> +        cpus_set_cap(caps[i].capability);
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/asm-arm/cpufeature.h 
> b/xen/include/asm-arm/cpufeature.h
> index 2bebad1..be2414c 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -62,6 +62,15 @@ static inline void cpus_set_cap(unsigned int num)
>          __set_bit(num, cpu_hwcaps);
>  }
>  
> +struct arm_cpu_capabilities {
> +    const char *desc;
> +    u16 capability;
> +    bool_t (*matches)(const struct arm_cpu_capabilities *);
> +};
> +
> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> +                             const char *info);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> https://lists.xen.org/xen-devel

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