Re: [PATCH v8 7/7] xen/arm: check for GICv3 platform support

[Julien Grall <julien.grall@linaro.org>]

On 07/23/2014 02:41 PM, vijay.kilari@gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> 
> ID_AA64PFR0_EL1 register provides information about GIC support.
> Check for this register in GICv3 driver.
> 
> Also print GICv3 support information in boot log
> 
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> ---
> v8: - Rearranged prf64 structure member variables
> v7: - return -ENODEV on lack GICv3 platform support instead of
>       panic
> ---
>  xen/arch/arm/gic-v3.c            |    7 +++++++
>  xen/arch/arm/setup.c             |    5 +++--
>  xen/include/asm-arm/cpufeature.h |    1 +
>  xen/include/asm-arm/processor.h  |    4 ++--
>  4 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 410d308..9291a7a 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -39,6 +39,7 @@
>  #include <asm/device.h>
>  #include <asm/gic.h>
>  #include <asm/gic_v3_defs.h>
> +#include <asm/cpufeature.h>
>  
>  struct rdist_region {
>      paddr_t base;
> @@ -1143,6 +1144,12 @@ static int __init gicv3_init(struct dt_device_node *node, const void *data)
>      int res, i;
>      uint32_t reg;
>  
> +    if ( !cpu_has_gicv3 )
> +    {
> +        dprintk(XENLOG_ERR, "Platform does not support GICv3\n");

AFAIU, this field indicates if we support GICv3 system registers or not.
It's entirely possible to use a GICv3 without system registers (see
4.4.1 System Register Access to CPU Interface Registers).

So I would be more specific in the error message. Smth like:

"The GICv3 drivers request system registers support.".

> +        return -ENODEV;
> +    }
> +
>      dt_device_set_used_by(node, DOMID_XEN);
>  
>      res = dt_device_get_address(node, 0, &gicv3.dbase, &gicv3.dbase_size);
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 78dc7f5..6a90c81 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -100,9 +100,10 @@ static void __init processor_id(void)
>             cpu_has_el2_32 ? "64+32" : cpu_has_el2_64 ? "64" : "No",
>             cpu_has_el1_32 ? "64+32" : cpu_has_el1_64 ? "64" : "No",
>             cpu_has_el0_32 ? "64+32" : cpu_has_el0_64 ? "64" : "No");
> -    printk("    Extensions:%s%s\n",
> +    printk("    Extensions:%s%s%s\n",
>             cpu_has_fp ? " FloatingPoint" : "",
> -           cpu_has_simd ? " AdvancedSIMD" : "");
> +           cpu_has_simd ? " AdvancedSIMD" : "",
> +           cpu_has_gicv3 ? " GICv3" : "");

I would rename into GICv3SysReg.

Regards,

-- 
Julien Grall