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

[Thomas Gleixner <tglx@linutronix.de>]

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?

> This has been observed to result in boot failures when booting a 32-bit
> kernel on an FVP using the boot-wrapper, e.g.
>
> | Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> | Modules linked in:
> | CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.11.0-rc3-00002-g102b1595b998 #6
> | Hardware name: ARM-Versatile Express
> | PC is at gic_init_bases+0x378/0x76c
> | LR is at gic_init_bases+0x30c/0x76c
> | pc : [<c1a34804>]    lr : [<c1a34798>]    psr: 600000d3
> | sp : c1c01e18  ip : 00000000  fp : 00000001
> | r10: 2f000000  r9 : c1ebcc68  r8 : 00000000
> | r7 : c1c097c0  r6 : c17adae0  r5 : eeff7edc  r4 : c1c05af8
> | r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : 0000001e
> | Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment none
> | Control: 10c0383d  Table: 8020406a  DAC: 00000051
> | Register r0 information: non-paged memory
> | Register r1 information: NULL pointer
> | Register r2 information: NULL pointer
> | Register r3 information: NULL pointer
> | Register r4 information: non-slab/vmalloc memory
> | Register r5 information: non-slab/vmalloc memory
> | Register r6 information: non-slab/vmalloc memory
> | Register r7 information: non-slab/vmalloc memory
> | Register r8 information: NULL pointer
> | Register r9 information: non-slab/vmalloc memory
> | Register r10 information: non-paged memory
> | Register r11 information: non-paged memory
> | Register r12 information: NULL pointer
> | Process swapper/0 (pid: 0, stack limit = 0x(ptrval))
> | Stack: (0xc1c01e18 to 0xc1c02000)
> | 1e00:                                                       c0207c28 2f280000
> | 1e20: f0a7ffff ffe00000 fffff000 eeff7edc 00000000 00000000 ffffffff 00000000
> | 1e40: 00000000 c133cd3c c1c05b00 00000000 00000000 00000000 00000000 c2092410
> | 1e60: c17d615c c04b6710 ff800000 00200000 00000000 f0880000 ff8024c8 eeff7f5c
> | 1e80: c17d6280 c0f90b00 c1ee1434 a00000d3 eeff7ed0 c17d6280 00000001 c2092410
> | 1ea0: c17d615c 00000000 c133cd24 eeff7ed0 2f000000 f0820000 c2092400 00000001
> | 1ec0: c2092410 c17d615c 00000001 c1a34db8 00000000 00000000 eeff7edc c17d7e84
> | 1ee0: c1c01efc 00000001 00000000 00000000 00000000 2f100000 2f2fffff eeff7f3c
> | 1f00: 00000200 00000000 00000000 00000000 00000000 c0f90aec c1b55078 00000000
> | 1f20: 00000000 c1b5513c 00000000 00000000 00000000 00000000 c1c01f6c c2092340
> | 1f40: 00000000 c1c01f6c c1c01f74 c1c01f6c 00000122 00000100 c18183d8 c1aa489c
> | 1f60: 00000000 00000007 00000000 c1c01f6c c1c01f6c c1c01f74 c1c01f74 00000000
> | 1f80: 00000000 c1acfa50 c1b5a000 c191b3c8 c1a0100c efffee00 00000000 00000038
> | 1fa0: 00000000 c1a03fd0 c1a0100c c1a1f6cc 00000000 c1e7c000 c19196d8 00000000
> | 1fc0: c1c04e00 c1a0100c ffffffff ffffffff 00000000 c1a006ec 00000000 00000000
> | 1fe0: 00000000 c1acfa60 00000000 ffffffff 00000000 00000000 00000000 00000000

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! ]---
>
> 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 ?

> SRE may not have been initialized:
>
> (1) gic_init_bases(), before the primary CPU pokes GICv3 sysregs in
>     gic_prio_init().
>
> (2) gic_starting_cpu(), before secondary CPUs initialize GICv3 sysregs
>     in gic_cpu_init().
>
> (3) gic_cpu_pm_notifier(), before CPUs re-initialize GICv3 sysregs in
>     gic_cpu_sys_reg_init().

Thanks,

        tglx