[U-Boot] [PATCH 1/3] ARM: rmobile: Switch CPU to non-secure HYP mode for r8a7790 based boards
Oleksandr
olekstysh at gmail.com
Thu Feb 7 15:28:18 UTC 2019
On 05.02.19 20:48, Marek Vasut wrote:
Hi Marek
> On 1/31/19 6:38 PM, Oleksandr Tyshchenko wrote:
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko at epam.com>
>>
>> Both Lager and Stout boards are based on r8a7790 SoC.
>>
>> Leave platform specific functions for bringing seconadary CPUs up empty,
>> since our target is to use PSCI for that.
>>
>> Also take care of updating arch timer while we are in secure mode.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko at epam.com>
>> ---
>> arch/arm/mach-rmobile/Kconfig.32 | 4 ++++
>> board/renesas/lager/lager.c | 51 ++++++++++++++++++++++++++++++++++++++++
>> board/renesas/stout/stout.c | 51 ++++++++++++++++++++++++++++++++++++++++
>> include/configs/lager.h | 3 +++
>> include/configs/stout.h | 3 +++
>> 5 files changed, 112 insertions(+)
>>
>> diff --git a/arch/arm/mach-rmobile/Kconfig.32 b/arch/arm/mach-rmobile/Kconfig.32
>> index 076a019..a2e9e3d 100644
>> --- a/arch/arm/mach-rmobile/Kconfig.32
>> +++ b/arch/arm/mach-rmobile/Kconfig.32
>> @@ -76,6 +76,8 @@ config TARGET_LAGER
>> select SUPPORT_SPL
>> select USE_TINY_PRINTF
>> imply CMD_DM
>> + select CPU_V7_HAS_NONSEC
>> + select CPU_V7_HAS_VIRT
> Shouldn't this be a H2 CPU property instead of a board property ?
Probably yes, sounds reasonable. I will move these options under "config
R8A7790".
> Does this apply to M2* too , since it has CA15 cores as well ?
I think, yes. But, without PSCI support being implemented for M2*, I
think it is not worth to select these options for it as well.
>
>> config TARGET_KZM9G
>> bool "KZM9D board"
>> @@ -115,6 +117,8 @@ config TARGET_STOUT
>> select SUPPORT_SPL
>> select USE_TINY_PRINTF
>> imply CMD_DM
>> + select CPU_V7_HAS_NONSEC
>> + select CPU_V7_HAS_VIRT
>>
>> endchoice
>>
>> diff --git a/board/renesas/lager/lager.c b/board/renesas/lager/lager.c
>> index 062e88c..9b96cc4 100644
>> --- a/board/renesas/lager/lager.c
>> +++ b/board/renesas/lager/lager.c
>> @@ -76,6 +76,53 @@ int board_early_init_f(void)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +#define TIMER_BASE 0xE6080000
>> +#define TIMER_CNTCR 0x00
>> +#define TIMER_CNTFID0 0x20
>> +
>> +/*
>> + * Taking into the account that arch timer is only configurable in secure mode
>> + * and we are going to enter kernel/hypervisor in a non-secure mode, update
>> + * arch timer right now to avoid possible issues. Make sure arch timer is
>> + * enabled and configured to use proper frequency.
>> + */
>> +static void rcar_gen2_timer_init(void)
>> +{
>> + u32 freq = RMOBILE_XTAL_CLK / 2;
>> +
>> + /*
>> + * Update the arch timer if it is either not running, or is not at the
>> + * right frequency.
>> + */
>> + if ((readl(TIMER_BASE + TIMER_CNTCR) & 1) == 0 ||
>> + readl(TIMER_BASE + TIMER_CNTFID0) != freq) {
> What is this check about ?
Actually, this code for updating arch timer was borrowed from Linux
almost as is.
Code in Linux uses this check in order to update timer only if required
(either timer disabled or uses wrong freq).
This check avoids abort in Linux if loader has already updated timer and
switched to non-secure mode.
But here in U-Boot, while we are still in secure mode, we can safely
remove this check and always update the timer. Shall I remove this check?
>
>> + /* Update registers with correct frequency */
>> + writel(freq, TIMER_BASE + TIMER_CNTFID0);
>> + asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
>> +
>> + /* Make sure arch timer is started by setting bit 0 of CNTCR */
>> + writel(1, TIMER_BASE + TIMER_CNTCR);
> Shouldn't this be in the timer driver instead ?
Which timer driver? Probably, I missed something, but I didn't find any
arch timer driver being used for Gen2.
I see that TMU timer (arch/sh/lib/time.c) is used as a system timer, but
it is yet another IP.
Would it be correct, if I move arch timer updating code to
arch/arm/mach-rmobile directory?
Actually, at the same location the corresponding code lives in Linux:
https://elixir.bootlin.com/linux/v5.0-rc5/source/arch/arm/mach-shmobile/setup-rcar-gen2.c#L61
>
>> + }
>> +}
>> +
>> +/*
>> + * In order not to break compilation if someone decides to build with PSCI
>> + * support disabled, keep these dummy functions.
>> + */
> That's currently the only use-case.
Shall I drop this comment and dummy functions below?
>
>> +void smp_set_core_boot_addr(unsigned long addr, int corenr)
>> +{
>> +}
>> +
>> +void smp_kick_all_cpus(void)
>> +{
>> +}
>> +
>> +void smp_waitloop(unsigned int previous_address)
>> +{
>> +}
>> +#endif
>> +
>> #define ETHERNET_PHY_RESET 185 /* GPIO 5 31 */
>>
>> int board_init(void)
>> @@ -89,6 +136,10 @@ int board_init(void)
>> mdelay(10);
>> gpio_direction_output(ETHERNET_PHY_RESET, 1);
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
> Define empty function in case the macro is not set instead of ifdefing
> every place that calls the rcar_gen2_timer_init() function.
Agree, will do
>
>> + rcar_gen2_timer_init();
>> +#endif
>> +
>> return 0;
>> }
>>
>> diff --git a/board/renesas/stout/stout.c b/board/renesas/stout/stout.c
>> index 85e30db..8d034a8 100644
>> --- a/board/renesas/stout/stout.c
>> +++ b/board/renesas/stout/stout.c
>> @@ -77,6 +77,53 @@ int board_early_init_f(void)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
>> +#define TIMER_BASE 0xE6080000
>> +#define TIMER_CNTCR 0x00
>> +#define TIMER_CNTFID0 0x20
>> +
>> +/*
>> + * Taking into the account that arch timer is only configurable in secure mode
>> + * and we are going to enter kernel/hypervisor in a non-secure mode, update
>> + * arch timer right now to avoid possible issues. Make sure arch timer is
>> + * enabled and configured to use proper frequency.
>> + */
>> +static void rcar_gen2_timer_init(void)
>> +{
> Why is this stuff in board code ? It should be in driver code / core
> code. And there should be only one copy of it.
Completely agree about the only one copy. When we move this code out of
Stout/Lager board files, we will have only one of it.
>
>> + u32 freq = RMOBILE_XTAL_CLK / 2;
>> +
>> + /*
>> + * Update the arch timer if it is either not running, or is not at the
>> + * right frequency.
>> + */
>> + if ((readl(TIMER_BASE + TIMER_CNTCR) & 1) == 0 ||
>> + readl(TIMER_BASE + TIMER_CNTFID0) != freq) {
>> + /* Update registers with correct frequency */
>> + writel(freq, TIMER_BASE + TIMER_CNTFID0);
>> + asm volatile("mcr p15, 0, %0, c14, c0, 0" : : "r" (freq));
>> +
>> + /* Make sure arch timer is started by setting bit 0 of CNTCR */
>> + writel(1, TIMER_BASE + TIMER_CNTCR);
>> + }
>> +}
>> +
>> +/*
>> + * In order not to break compilation if someone decides to build with PSCI
>> + * support disabled, keep these dummy functions.
>> + */
>> +void smp_set_core_boot_addr(unsigned long addr, int corenr)
>> +{
>> +}
>> +
>> +void smp_kick_all_cpus(void)
>> +{
>> +}
>> +
>> +void smp_waitloop(unsigned int previous_address)
>> +{
>> +}
>> +#endif
>> +
>> #define ETHERNET_PHY_RESET 123 /* GPIO 3 31 */
>>
>> int board_init(void)
>> @@ -92,6 +139,10 @@ int board_init(void)
>> mdelay(20);
>> gpio_direction_output(ETHERNET_PHY_RESET, 1);
>>
>> +#ifdef CONFIG_ARMV7_NONSEC
>> + rcar_gen2_timer_init();
>> +#endif
>> +
>> return 0;
>> }
>>
>> diff --git a/include/configs/lager.h b/include/configs/lager.h
>> index 89c5d01..d8a0b01 100644
>> --- a/include/configs/lager.h
>> +++ b/include/configs/lager.h
>> @@ -48,4 +48,7 @@
>> #define CONFIG_SH_SCIF_CLK_FREQ 65000000
>> #endif
>>
>> +/* The PERIPHBASE in the CBAR register is wrong, so override it */
>> +#define CONFIG_ARM_GIC_BASE_ADDRESS 0xf1000000
>> +
>> #endif /* __LAGER_H */
>> diff --git a/include/configs/stout.h b/include/configs/stout.h
>> index 93d9805..7edb9d7 100644
>> --- a/include/configs/stout.h
>> +++ b/include/configs/stout.h
>> @@ -56,4 +56,7 @@
>> #define CONFIG_SH_SCIF_CLK_FREQ 52000000
>> #endif
>>
>> +/* The PERIPHBASE in the CBAR register is wrong, so override it */
>> +#define CONFIG_ARM_GIC_BASE_ADDRESS 0xf1000000
>> +
>> #endif /* __STOUT_H */
>>
>
--
Regards,
Oleksandr Tyshchenko
More information about the U-Boot
mailing list