[U-Boot] [PATCH] arm64: add missing gic_init_secure_percpu for non multi-entry

[U-Boot] [PATCH] arm64: add missing gic_init_secure_percpu for non multi-entry

[Masahiro Yamada]

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(+)

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)
-- 
1.9.1

[U-Boot] [PATCH] arm64: add missing gic_init_secure_percpu for non multi-entry

[Stephen Warren]

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?

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

[U-Boot] [PATCH] arm64: add missing gic_init_secure_percpu for non multi-entry

[Masahiro Yamada]

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