Re: [Xen-devel] [PATCH v2] xen/arm: Switch OMAP5 secondary cores into hyp mode

[Julien Grall <julien.grall@xxxxxxx>]

Hi Denis,

On 6/28/19 8:42 PM, Denis Obrezkov wrote:
> This function allows xen to bring secondary CPU cores into non-secure
> HYP mode. This is done by using a Secure Monitor call.
As I pointed out in the previous version, the current code is likely 
working on some omap5 platform. So your commit message should provide 
some information on why this is necessary and on which platform (i.e 
beagleboard x15).
It would be nice to also have summary of why it is fine to extend to all 
the omap5 platform. For reminder, below the analysis I wrote earlier on:
I am trying to understand how this ever worked. omap5_smp_init is called 
by two sets of platforms (based on compatible):
   - ti,dra7: there were some hacks in U-boot to avoid the SMC. If I am 
right, then I would not bother to support hacked U-boot.
   - ti,omap5: [1] suggest that U-boot do the switch for us but it is 
not clear whether this is upstreamed. @Chen, I know you did the port a 
long time ago. Do you recall how this worked?
Linux seems to use the smc on any dra7 and omap54xx. So maybe we can use 
safely here.
> Signed-off-by: Denis Obrezkov <denisobrezkov@xxxxxxxxx>
> ---
> Changes in v2:
> - move code to platform specific file
> ---
>   xen/arch/arm/platforms/omap5.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/platforms/omap5.c b/xen/arch/arm/platforms/omap5.c
> index aee24e4d28..79764a6cd6 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -23,6 +23,17 @@
>   #include <xen/vmap.h>
>   #include <asm/io.h>
>   
> +void omap5_init_secondary(void);
> +asm (
> +        ".text                              \n\t"
> +        "omap5_init_secondary:              \n\t"
> +        "        ldr   r12, =0x102          \n\t" /* API_HYP_ENTRY */
> +        "        adr   r0, omap5_hyp        \n\t"
> +        "        smc   #0                   \n\t"
> +        "omap5_hyp:                         \n\t"
> +        "        b     init_secondary       \n\t"
> +        );
> +
>   static uint16_t num_den[8][2] = {
>       {         0,          0 },  /* not used */
>       {  26 *  64,  26 *  125 },  /* 12.0 Mhz */
> @@ -128,8 +139,9 @@ static int __init omap5_smp_init(void)
>       }
>   
>       printk("Set AuxCoreBoot1 to %"PRIpaddr" (%p)\n",
> -           __pa(init_secondary), init_secondary);
> -    writel(__pa(init_secondary), wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
> +           __pa(omap5_init_secondary), omap5_init_secondary);
> +    writel(__pa(omap5_init_secondary),
> +           wugen_base + OMAP_AUX_CORE_BOOT_1_OFFSET);
>   
>       printk("Set AuxCoreBoot0 to 0x20\n");
>       writel(0x20, wugen_base + OMAP_AUX_CORE_BOOT_0_OFFSET);
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel