[PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail

[PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail

[Julien Grall]

From: Julien Grall <julien.grall@arm.com>

When system registers are not enabled, all the access to them will trap
in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only
on success. As the rest of the code (e.g gicv3_hyp_init) relies on
system register, it is better to bail out directly.

This will save time on debugging early boot issue on GICv3 platform.

Signed-off-by: Julien Grall <julien.grall@linaro.org>

---

This makes easier to debug early boot issue on GICv3 platform. It would
be worth considering to backport it.
---
 xen/arch/arm/gic-v3.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 473e26111f..a0d290b55c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -847,8 +847,12 @@ static int gicv3_secondary_cpu_init(void)
     spin_lock(&gicv3.lock);
 
     res = gicv3_cpu_init();
+    if ( res )
+        goto out;
+
     gicv3_hyp_init();
 
+out:
     spin_unlock(&gicv3.lock);
 
     return res;
@@ -1705,8 +1709,12 @@ static int __init gicv3_init(void)
         panic("GICv3: ITS: initialization failed: %d\n", res);
 
     res = gicv3_cpu_init();
+    if ( res )
+        goto out;
+
     gicv3_hyp_init();
 
+out:
     spin_unlock(&gicv3.lock);
 
     return res;
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail

[Stefano Stabellini]

On Wed, 6 Dec 2017, Julien Grall wrote:
> From: Julien Grall <julien.grall@arm.com>
> 
> When system registers are not enabled, all the access to them will trap
                                             ^ accesses


> in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only
> on success. As the rest of the code (e.g gicv3_hyp_init) relies on
> system register, it is better to bail out directly.
> 
> This will save time on debugging early boot issue on GICv3 platform.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

This is good:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


Do we also want to print a warning or an error message?



> ---
> 
> This makes easier to debug early boot issue on GICv3 platform. It would
> be worth considering to backport it.
> ---
>  xen/arch/arm/gic-v3.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 473e26111f..a0d290b55c 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -847,8 +847,12 @@ static int gicv3_secondary_cpu_init(void)
>      spin_lock(&gicv3.lock);
>  
>      res = gicv3_cpu_init();
> +    if ( res )
> +        goto out;
> +
>      gicv3_hyp_init();
>  
> +out:
>      spin_unlock(&gicv3.lock);
>  
>      return res;
> @@ -1705,8 +1709,12 @@ static int __init gicv3_init(void)
>          panic("GICv3: ITS: initialization failed: %d\n", res);
>  
>      res = gicv3_cpu_init();
> +    if ( res )
> +        goto out;
> +
>      gicv3_hyp_init();
>  
> +out:
>      spin_unlock(&gicv3.lock);
>  
>      return res;
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail

[Julien Grall]

Hi Stefano,

On 07/12/17 23:05, Stefano Stabellini wrote:
> On Wed, 6 Dec 2017, Julien Grall wrote:
>> From: Julien Grall <julien.grall@arm.com>
>>
>> When system registers are not enabled, all the access to them will trap
>                                               ^ accesses
> 
> 
>> in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only
>> on success. As the rest of the code (e.g gicv3_hyp_init) relies on
>> system register, it is better to bail out directly.
>>
>> This will save time on debugging early boot issue on GICv3 platform.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> This is good:
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
> Do we also want to print a warning or an error message?

AFAICT, all the path that return an error in gicv3_cpu_init (and the 
callers) have already a warning/error message. So no need to add an 
extra one here.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail

[Stefano Stabellini]

On Fri, 8 Dec 2017, Julien Grall wrote:
> Hi Stefano,
> 
> On 07/12/17 23:05, Stefano Stabellini wrote:
> > On Wed, 6 Dec 2017, Julien Grall wrote:
> > > From: Julien Grall <julien.grall@arm.com>
> > > 
> > > When system registers are not enabled, all the access to them will trap
> >                                               ^ accesses
> > 
> > 
> > > in EL2. In Xen, system registers will be enabled by gicv3_cpu_init only
> > > on success. As the rest of the code (e.g gicv3_hyp_init) relies on
> > > system register, it is better to bail out directly.
> > > 
> > > This will save time on debugging early boot issue on GICv3 platform.
> > > 
> > > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > 
> > This is good:
> > 
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> > 
> > 
> > Do we also want to print a warning or an error message?
> 
> AFAICT, all the path that return an error in gicv3_cpu_init (and the callers)
> have already a warning/error message. So no need to add an extra one here.

committed

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel