Re: [Xen-devel] [PATCH v2 29/35] xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 22 Jul 2019, Julien Grall wrote:
> The assembly switch to the runtime PT is only necessary for the
> secondary CPUs. So move the code in the secondary CPUs path.
> 
> While this is definitely not compliant with the Arm Arm as we are
> switching between two differents set of page-tables without turning off
> the MMU. Turning off the MMU is impossible here as the ID map may clash
> with other mappings in the runtime page-tables. This will require more
> rework to avoid the problem. So for now add a TODO in the code.
> 
> Finally, the code is currently assume that r5 will be properly set to 0
> before hand. This is done by create_page_tables() which is called quite
> early in the boot process. There are a risk this may be oversight in the
> future and therefore breaking secondary CPUs boot. Instead, set r5 to 0
> just before using it.
> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> 
> ---
>     Changes in v2:
>         - Patch added
> ---
>  xen/arch/arm/arm32/head.S | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 4081a52dfa..6dc6032498 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -201,6 +201,26 @@ GLOBAL(init_secondary)
>          mov   pc, r0
>  secondary_switched:
>          bl    setup_fixmap
> +
> +        /*
> +         * Non-boot CPUs need to move on to the proper pagetables, which were
> +         * setup in init_secondary_pagetables.
> +         *
> +         * XXX: This is not compliant with the Arm Arm.
> +         */
> +        ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
> +        mov   r5, #0

Why do we need to zero r5? Shouldn't ldrd overwrite r5 anyway?


> +        ldrd  r4, r5, [r4]           /* Actual value */
> +        dsb
> +        mcrr  CP64(r4, r5, HTTBR)
> +        dsb
> +        isb
> +        mcr   CP32(r0, TLBIALLH)     /* Flush hypervisor TLB */
> +        mcr   CP32(r0, ICIALLU)      /* Flush I-cache */
> +        mcr   CP32(r0, BPIALL)       /* Flush branch predictor */
> +        dsb                          /* Ensure completion of TLB+BP flush */
> +        isb
> +
>          b     launch
>  ENDPROC(init_secondary)
>  
> @@ -504,28 +524,6 @@ ENDPROC(setup_fixmap)
>  launch:
>          PRINT("- Ready -\r\n")
>  
> -        /* The boot CPU should go straight into C now */
> -        teq   r12, #0
> -        beq   1f
> -
> -        /*
> -         * Non-boot CPUs need to move on to the proper pagetables, which were
> -         * setup in init_secondary_pagetables.
> -         */
> -
> -        ldr   r4, =init_ttbr         /* VA of HTTBR value stashed by CPU 0 */
> -        ldrd  r4, r5, [r4]           /* Actual value */
> -        dsb
> -        mcrr  CP64(r4, r5, HTTBR)
> -        dsb
> -        isb
> -        mcr   CP32(r0, TLBIALLH)     /* Flush hypervisor TLB */
> -        mcr   CP32(r0, ICIALLU)      /* Flush I-cache */
> -        mcr   CP32(r0, BPIALL)       /* Flush branch predictor */
> -        dsb                          /* Ensure completion of TLB+BP flush */
> -        isb
> -
> -1:
>          ldr   r0, =init_data
>          add   r0, #INITINFO_stack    /* Find the boot-time stack */
>          ldr   sp, [r0]
> -- 
> 2.11.0
> 

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