Skip to contentSkip to conversation

arm64: add missing gic_init_secure_percpu for non multi-entry

PATCH New Masahiro Yamada 2 participants 3 messages Apr 15, 2016 Arm (32-bit)

lore
Share & tools
lore

Masahiro Yamada

author
#0 lore raw

PATCH 1/1 arm64: add missing gic_init_secure_percpu for non multi-entry

Per-CPU initialization of GIC is necessary even when CONFIG_ARMV8_MULTIENTRY is undefined.

Signed-off-by Masahiro Yamada <[email protected]>

---

 arch/arm/cpu/armv8/start.S | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/cpu/armv8/start.S +2 -0
index dceedd7..124a274 100644
@@ -208,6 +208,8 @@ WEAK(lowlevel_init)
 	 */
 	ldr	x0, =GICD_BASE
 	bl	gic_init_secure
+	ldr	x0, =GICR_BASE
+	bl	gic_init_secure_percpu
 #endif
 #else /* CONFIG_ARMV8_MULTIENTRY is set */
 #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)
Signature · 1 line
1.9.1
#1 lore raw

PATCH 1/1 arm64: add missing gic_init_secure_percpu for non multi-entry

Quoted 6 lines · Masahiro Yamada

Per-CPU initialization of GIC is necessary even when CONFIG_ARMV8_MULTIENTRY is undefined.

arch/arm/cpu/armv8/start.S WEAK(lowlevel_init) · line 215

Quoted 8 lines
arch/arm/cpu/armv8/start.S +2 -0
@@ -208,6 +208,8 @@ WEAK(lowlevel_init)
  	 */
  	ldr	x0, =GICD_BASE
  	bl	gic_init_secure
+	ldr	x0, =GICR_BASE
+	bl	gic_init_secure_percpu
  #endif
  #else /* CONFIG_ARMV8_MULTIENTRY is set */
  #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)

The #else branch currently there does something quite different for GICV2 and GICV3. The code you added above seems to be identical to that code's GIVC3 case and ignore GICV2. I assume that's a bug?

If so, and the fix is to make the non-multientry code do the exact same call to gic_init_secure_percpu that the multientry code does, I would suggest unifying the two branches, replacing that entire top-level #if/#else block with:

#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) #ifdef CONFIG_ARMV8_MULTIENTRY
branch_if_slave x0, 1f

#endif
	ldr	x0, =GICD_BASE
	bl	gic_init_secure
1:
#if defined(CONFIG_GICV3)
	ldr	x0, =GICR_BASE
	bl	gic_init_secure_percpu
#elif defined(CONFIG_GICV2)
	ldr	x0, =GICD_BASE
	ldr	x1, =GICC_BASE
	bl	gic_init_secure_percpu

#endif
#endif

At least, that's what I think looking at the code with basically zero understanding of what initialization the GIC actually needs...

author
#2 lore raw

PATCH 1/1 arm64: add missing gic_init_secure_percpu for non multi-entry

Hi Stephen,

2016-04-16 0:55 GMT+09:00 Stephen Warren <[email protected]>:

Quoted 9 lines · Masahiro Yamada
Quoted 6 lines

Per-CPU initialization of GIC is necessary even when CONFIG_ARMV8_MULTIENTRY is undefined.

arch/arm/cpu/armv8/start.S +0 -0
Quoted 14 lines

arch/arm/cpu/armv8/start.S WEAK(lowlevel_init) · line 215

Quoted 9 lines
arch/arm/cpu/armv8/start.S +2 -0
@@ -208,6 +208,8 @@ WEAK(lowlevel_init)
         */
        ldr     x0, =GICD_BASE
        bl      gic_init_secure
+       ldr     x0, =GICR_BASE
+       bl      gic_init_secure_percpu
  #endif
  #else /* CONFIG_ARMV8_MULTIENTRY is set */
  #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)

The #else branch currently there does something quite different for GICV2 and GICV3. The code you added above seems to be identical to that code's GIVC3 case and ignore GICV2. I assume that's a bug?

You are right.

My new SoC is equipped with GICv3 and I missed the GICv2 case.

Quoted 16 lines

If so, and the fix is to make the non-multientry code do the exact same call to gic_init_secure_percpu that the multientry code does, I would suggest unifying the two branches, replacing that entire top-level #if/#else block with:

#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) #ifdef CONFIG_ARMV8_MULTIENTRY
branch_if_slave x0, 1f
#endif
ldr x0, =GICD_BASE
bl gic_init_secure
1:
#if defined(CONFIG_GICV3)
ldr x0, =GICR_BASE
bl gic_init_secure_percpu
#elif defined(CONFIG_GICV2)

Quoted 9 lines
        ldr     x0, =GICD_BASE
        ldr     x1, =GICC_BASE
        bl      gic_init_secure_percpu

#endif
#endif

At least, that's what I think looking at the code with basically zero understanding of what initialization the GIC actually needs...

Good idea!
It fixes the problem I noticed
and also makes the code much cleaner.

Could you post it as a patch, please?

Signature · 2 lines
Best Regards
Masahiro Yamada