Re: [PATCH v6 11/11] xen/arm64: add setup_fixmap and remove_identity_mapping for MPU

[Julien Grall <julien@xxxxxxx>]

Hi,

On 04/11/2022 10:07, Wei Chen wrote:
> From: Penny Zheng <penny.zheng@xxxxxxx>
>
> setup_fixmap and remove_identity_mapping are two functions that
> are used in Xen boot-time code flow. We implement these two
> functions for MPU system, in this case, the code flow in head.S
> doesn't need to use #ifdef to gate MPU/MMU code.
>
> In MMU system, setup_fixmap is used for Xen to map some essentail
> data or devices in boot-time. For MPU system, we still have this
> requirement, we map the early UART to MPU protection region when
> earlyprintk is enabled. This also means PRINT can't be used after
> turning on MPU but before setup_fixmap. This restriction is the
> same as MMU system.
>
> For remove_identity_mapping, we just need an empty function to
> make head.S code flow happy.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> ---
>   xen/arch/arm/arm64/head_mpu.S                 | 49 +++++++++++++++++++
>   .../arm/include/asm/platforms/fvp_baser.h     |  4 ++
>   2 files changed, 53 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/head_mpu.S b/xen/arch/arm/arm64/head_mpu.S
> index 5a1b03e293..336c0a630f 100644
> --- a/xen/arch/arm/arm64/head_mpu.S
> +++ b/xen/arch/arm/arm64/head_mpu.S
> @@ -20,13 +20,20 @@
>   /*
>    * In boot stage, we will use 1 MPU region:
>    * Region#0: Normal memory for Xen text + data + bss (2MB)
> + * Region#1: Device memory for EARLY UART, size is defined
> + *           by platform's EARLY_UART_SIZE
>    */
>   #define BOOT_NORMAL_REGION_IDX  0x0
> +#define BOOT_DEVICE_REGION_IDX  0x1
>   
>   /* MPU normal memory attributes. */
>   #define PRBAR_NORMAL_MEM        0x30    /* SH=11 AP=00 XN=00 */
>   #define PRLAR_NORMAL_MEM        0x0f    /* NS=0 ATTR=111 EN=1 */
>   
> +/* MPU device memory attributes. */
> +#define PRBAR_DEVICE_MEM        0x20    /* SH=10 AP=00 XN=00 */
> +#define PRLAR_DEVICE_MEM        0x09    /* NS=0 ATTR=100 EN=1 */
> +
>   .macro write_pr, sel, prbar, prlar
>       msr   PRSELR_EL2, \sel
>       dsb   sy
> @@ -69,6 +76,48 @@ ENTRY(prepare_early_mappings)
>       ret
>   ENDPROC(prepare_early_mappings)
>   
> +/*
> + * In MMU system, setup_fixmap is used for Xen to map some essential data
> + * or devices in boot-time. In order to be consistent with MMU system, we
> + * inherit the function name for MPU system.
> + * setup_fixmap of MPU system will:
> + * - Map the early UART to MPU protection region when earlyprintk is
> + *   enabled (The PRINT can't be used after turning on MPU but before
> + *   setup_fixmap).
For the MMU, we have this restriction because the fixmap could clash 
with the identity mapping. I don't think there are such restrictions for 
the MPU and therefore it seems strange to pertain the same behavior.
In fact, I have plan to get rid of this restriction even for the MMU. So 
better this restriction is not spread if we can.
> + *
> + * Clobbers x0 - x3
> + */
> +ENTRY(setup_fixmap)
> +#ifdef CONFIG_EARLY_PRINTK
> +    /* Map early uart to MPU device region for early printk. */
> +    mov x0, #BOOT_DEVICE_REGION_IDX
> +    ldr x1, =CONFIG_EARLY_UART_BASE_ADDRESS
> +    and x1, x1, #MPU_REGION_MASK
> +    mov x3, #PRBAR_DEVICE_MEM
> +    orr x1, x1, x3
> +
> +    ldr x2, =CONFIG_EARLY_UART_BASE_ADDRESS
> +    ldr x3, =(CONFIG_EARLY_UART_BASE_ADDRESS + EARLY_UART_SIZE - 1)
> +    add x2, x2, x3
> +    and x2, x2, #MPU_REGION_MASK
> +    mov x3, #PRLAR_DEVICE_MEM
> +    orr x2, x2, x3
> +
> +    /*
> +     * Write to MPU protection region:
> +     * x0 for pr_sel, x1 for prbar x2 for prlar
> +     */
> +    write_pr x0, x1, x2
> +#endif
> +
> +    ret
> +ENDPROC(setup_fixmap)
> +
> +/* Stub of remove_identity_mapping for MPU systems */
> +ENTRY(remove_identity_mapping)
> +    ret
> +ENDPROC(remove_identity_mapping)
This stub could be avoided if you move the call to 
remove_identity_mapping in enable_mmu() as I did for arm32. See [1].
[1] https://lore.kernel.org/all/20221022150422.17707-3-julien@xxxxxxx/

Cheers,

--
Julien Grall