Re: [PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S

[Ayan Kumar Halder <ayankuma@xxxxxxx>]

Hi Henry,

On 28/08/2023 02:32, Henry Wang wrote:
> CAUTION: This message has originated from an External Source. Please use proper 
> judgment and caution when opening attachments, clicking links, or responding to 
> this email.
>
>
> Currently mmu_init_secondary_cpu() only enforces the page table
> should not contain mapping that are both Writable and eXecutables
> after boot. To ease the arch/arm/mm.c split work, fold this function
> to head.S.
>
> Introduce assembly macro pt_enforce_wxn for both arm32 and arm64.
> For arm64, the macro is called at the end of enable_secondary_cpu_mm().
> For arm32, the macro is called before secondary CPUs jumping into
> the C world.
>
> Signed-off-by: Henry Wang <Henry.Wang@xxxxxxx>
> ---
> v6:
> - New patch.
> ---
>   xen/arch/arm/arm32/head.S     | 20 ++++++++++++++++++++
>   xen/arch/arm/arm64/mmu/head.S | 21 +++++++++++++++++++++
>   xen/arch/arm/include/asm/mm.h |  2 --
>   xen/arch/arm/mm.c             |  6 ------
>   xen/arch/arm/smpboot.c        |  2 --
>   5 files changed, 41 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 33b038e7e0..39218cf15f 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -83,6 +83,25 @@
>           isb
>   .endm
>
> +/*
> + * Enforce Xen page-tables do not contain mapping that are both
> + * Writable and eXecutables.
> + *
> + * This should be called on each secondary CPU.
> + */
> +.macro pt_enforce_wxn tmp
> +        mrc   CP32(\tmp, HSCTLR)
> +        orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
> +        dsb
> +        mcr   CP32(\tmp, HSCTLR)
> +        /*
> +         * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
> +         * before flushing the TLBs.
> +         */
> +        isb
> +        flush_xen_tlb_local \tmp
> +.endm
> +
>   /*
>    * Common register usage in this file:
>    *   r0  -
> @@ -254,6 +273,7 @@ secondary_switched:
>           /* Use a virtual address to access the UART. */
>           mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
>   #endif
> +        pt_enforce_wxn
>   
Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU 
related functionality are bundled together?
Also AFAIU, mov_w has not effect on pt_enforce_wxn().

So that I can create a function "enable_secondary_cpu_mm()" - similar to 
one you introduced for arm64
/* This will contain all the MMU related function for secondary cpu */

enable_secondary_cpu_mm:

bl    create_page_tables

mov_w lr, secondary_switched

....

flush_xen_tlb_local r0

pt_enforce_wxn r0

ENDPROC(enable_secondary_cpu_mm)


- Ayan

>           PRINT("- Ready -\r\n")
>           /* Jump to C world */
>           mov_w r2, start_secondary
> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> index a5271e3880..25028bdf07 100644
> --- a/xen/arch/arm/arm64/mmu/head.S
> +++ b/xen/arch/arm/arm64/mmu/head.S
> @@ -31,6 +31,25 @@
>           isb
>   .endm
>
> +/*
> + * Enforce Xen page-tables do not contain mapping that are both
> + * Writable and eXecutables.
> + *
> + * This should be called on each secondary CPU.
> + */
> +.macro pt_enforce_wxn tmp
> +       mrs   \tmp, SCTLR_EL2
> +       orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
> +       dsb   sy
> +       msr   SCTLR_EL2, \tmp
> +       /*
> +        * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
> +        * before flushing the TLBs.
> +        */
> +       isb
> +       flush_xen_tlb_local
> +.endm
> +
>   /*
>    * Macro to find the slot number at a given page-table level
>    *
> @@ -308,6 +327,8 @@ ENTRY(enable_secondary_cpu_mm)
>           bl    enable_mmu
>           mov   lr, x5
>
> +        pt_enforce_wxn x0
> +
>           /* Return to the virtual address requested by the caller. */
>           ret
>   ENDPROC(enable_secondary_cpu_mm)
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index bf2fe26f9e..a66aa219b1 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -216,8 +216,6 @@ extern void remove_early_mappings(void);
>   /* Allocate and initialise pagetables for a secondary CPU. Sets init_ttbr to 
> the
>    * new page table */
>   extern int init_secondary_pagetables(int cpu);
> -/* Switch secondary CPUS to its own pagetables and finalise MMU setup */
> -extern void mmu_init_secondary_cpu(void);
>   /*
>    * For Arm32, set up the direct-mapped xenheap: up to 1GB of contiguous,
>    * always-mapped memory. Base must be 32MB aligned and size a multiple of 
> 32MB.
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index f3ef0da0e3..3ee74542ba 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -322,12 +322,6 @@ void __init setup_pagetables(unsigned long 
> boot_phys_offset)
>   #endif
>   }
>
> -/* MMU setup for secondary CPUS (which already have paging enabled) */
> -void mmu_init_secondary_cpu(void)
> -{
> -    xen_pt_enforce_wnx();
> -}
> -
>   #ifdef CONFIG_ARM_32
>   /*
>    * Set up the direct-mapped xenheap:
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index e107b86b7b..ade2c77cf9 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -359,8 +359,6 @@ void start_secondary(void)
>        */
>       update_system_features(&current_cpu_data);
>
> -    mmu_init_secondary_cpu();
> -
>       gic_init_secondary_cpu();
>
>       set_current(idle_vcpu[cpuid]);
> --
> 2.25.1