[PATCH] irqchip/gic-v3: init SRE before poking sysregs

[Mark Rutland]

On Tue, Aug 20, 2024 at 06:36:10PM +0200, Thomas Gleixner wrote:
> On Tue, Aug 20 2024 at 16:55, Mark Rutland wrote:
> 
> > The GICv3 driver pokes GICv3 system registers in gic_prio_init() before
> > gic_cpu_sys_reg_init() ensures that SRE has been initialized. On arm64
> > the architecture code will have initialized ZRE prior to this, but on
> > 32-bit ARM that is not the case, and consequently in gic_prio_init() the
> > system register accesses may result in an UNDEF.
> >
> > This is a regression introduced by commit:
> >
> >   d447bf09a4013541 ("irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier")
> >
> > ... which added gic_prio_init().
> 
> That's already clear from the fixes tag, no?

Maybe; this is trying to explain *why* I've given that specific fixes
tag -- i.e. it's a true regression, and it's solely down to that commit
and not some interaction with other commits, etc.

I'd prefer to keep that explicit, but if you have strong feelings about
it I'll drop that wording.

[...]

[ OOPS gunk removed ]
 
> In which way is all of this OOPS gunk useful to describe the problem?
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces
> 
> > | Call trace:
> > |  gic_init_bases from gic_of_init+0x1c0/0x29c
> > |  gic_of_init from of_irq_init+0x1d4/0x324
> > |  of_irq_init from init_IRQ+0xa8/0x108
> > |  init_IRQ from start_kernel+0x540/0x6b8
> > |  start_kernel from 0x0
> 
> This is really the only relevant information if at all, no?
> 
> > | Code: e2033040 e3530000 13a01001 03a01000 (ee140f16)
> > | ---[ end trace 0000000000000000 ]---
> > | Kernel panic - not syncing: Attempted to kill the idle task!
> > | ---[ end Kernel panic - not syncing: Attempted to kill the idle task! ]---

Agreed; I'll drop this entirely since the releveant instruction is
inside gic_prio_init(), and that doesn't show up in the trace because it
got inlined.

[...]

> > Fix this by factoring out the SRE initialization into a new
> > gic_sre_init(), and calling this in the early in the three paths where
> 
> calling this in the early ?

Whoops; that first "in the" should be remvoed and this should be:

  Fix this by factoring out the SRE initialization into a new
  gic_sre_init(), and calling this early in the three paths where [...]

Mark.