[U-Boot] [PATCH v2 4/7] ARM: switch to non-secure state during bootm execution

Masahiro Yamada
2013-06-28T03:18:58Z

Hi Andre.

> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -60,6 +60,8 @@ COBJS-y += reset.o
> COBJS-y += cache.o
> COBJS-y += cache-cp15.o
>
> +COBJS-$(CONFIG_ARMV7_VIRT) += virt-v7.o
> +

Judging from the file name virt-v7.c,
you are thinkig this file is specific to ARMv7, aren't you?

If so, why don't you move this file
to arch/arm/cpu/armv7/ ?

> +static void set_generic_timer_frequency(void)
> +{
> +#ifdef CONFIG_SYS_CLK_FREQ
> + unsigned int reg;
> +
> + reg = read_id_pfr1();
> + if ((reg & CPUID_ARM_TIMER_MASK) == 1U << CPUID_ARM_TIMER_SHIFT)
> + asm("mcr p15, 0, %0, c14, c0, 0\n"
> + : : "r"(CONFIG_SYS_CLK_FREQ));
> +#endif

CPUID_ARM_TIMER_MASK
CPUID_ARM_TIMER_SHIFT

I think these macro names are vague.
There are Generic Timer, Global Timer, Private Timer etc.

Unlike other parts, the care for Cortex-A9 is missing here.
To be more generic, I'd like to suggest to allow Non-secure access
to Global/Private timers before switching to non-secure state.

How about like this for armv7_switch_nonsec function?

/* check whether the CPU supports the security extensions */
reg = read_id_pfr1();
if ((reg & 0xF0) == 0)
return HYP_ERR_NO_SEC_EXT;

if ((reg & CPUID_ARM_TIMER_MASK) == 1U << CPUID_ARM_TIMER_SHIFT)
set_generic_timer_frequency();
else
/* Allow Non-secure access to Global/Private timers */
writel(0xfff, periph_base + SCU_SNSAC);

For more info about SCU Non-secure Access Control (SNSAC) Register,
please refer Cortex A9 mpcore TRM. page 2-11
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0407g/DDI0407G_cortex_a9_mpcore_r3p0_trm.pdf

> + /* enable the GIC distributor */
> + writel(readl(&gicdptr[GICD_CTLR]) | 0x03, &gicdptr[GICD_CTLR]);

I am not sure this code is really necessary.

Because your are setting all available interrupts to Group1 just below,
I think we don't need to do this in secure state.

Best Regards
Masahiro Yamada