diff mbox

arm64: add missing gic_init_secure_percpu for non multi-entry

Message ID 1460720491-25148-1-git-send-email-yamada.masahiro@socionext.com
State New
Headers show

Commit Message

Masahiro Yamada April 15, 2016, 11:41 a.m. UTC
Per-CPU initialization of GIC is necessary even when
CONFIG_ARMV8_MULTIENTRY is undefined.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 arch/arm/cpu/armv8/start.S | 2 ++
 1 file changed, 2 insertions(+)

-- 
1.9.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Masahiro Yamada April 15, 2016, 6:12 p.m. UTC | #1
Hi Stephen,

2016-04-16 0:55 GMT+09:00 Stephen Warren <swarren@wwwdotorg.org>:
> On 04/15/2016 05:41 AM, Masahiro Yamada wrote:

>>

>> Per-CPU initialization of GIC is necessary even when

>> CONFIG_ARMV8_MULTIENTRY is undefined.

>

>

>> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S

>

>

>> @@ -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.



> 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...

>


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

Could you post it as a patch, please?





-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
index dceedd7..124a274 100644
--- a/arch/arm/cpu/armv8/start.S
+++ b/arch/arm/cpu/armv8/start.S
@@ -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)