| Message ID | 20171206145137.15602-1-julien.grall@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [Xen-devel] xen/arm: gic-v3: Bail out if gicv3_cpu_init fail | expand |
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 >
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,
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
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;