Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU systems

[Julien Grall <julien@xxxxxxx>]

Hi Wei,

On 04/11/2022 10:07, Wei Chen wrote:
> FIXMAP is a special virtual address section for Xen to map some
> physical ram or device memory temporarily in initialization for
> MMU systems. FIXMAP_ADDR will return a virtual address by index
> for special purpose phys-to-virt mapping usage. For example,
> FIXMAP_ADDR(FIXMAP_CONSOLE) for early console mapping and
> FIXMAP_ADDR(FIXMAP_MISC) for copy_from_paddr.
To me, we are bending quite a bit the definition of the fixmap. There 
are not many use of the FIXMAP within the code and I think it would 
simply be better to abstract the use (or removing it when possible) and 
avoid defining FIXMAP_ADDR() & co for MPU.
> But in MPU systems, we can't map physical address to any virtual
> address. So we want the code that is using FIXMAP_ADDR to return
> the input physical address in MPU systems. So in MPU version,
> FIXMAP_ADDR will trim physical address to PAGE alignment. This
> will return an offset which is similar to MMU version FIXMAP_ADDR.
> But it's a physical offset got from input physical address, plus
> to an offset inside page (which is also got from physical address
> mask with PAGE_MASK). The caller can return the input physical
> address directly.
>
> As pmap depends on FIXAMP, so we disable pmap for Arm with MPU
> enabled systems.
>
> Signed-off-by: Wei Chen <wei.chen@xxxxxxx>
> ---
>   xen/arch/arm/Kconfig                  |  2 +-
>   xen/arch/arm/include/asm/config_mpu.h |  2 ++
>   xen/arch/arm/include/asm/fixmap.h     | 25 +++++++++++++++++++++++++
>   3 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index ac276307d6..1458ffa777 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -16,7 +16,7 @@ config ARM
>         select HAS_DEVICE_TREE
>         select HAS_PASSTHROUGH
>         select HAS_PDX
> -       select HAS_PMAP
> +       select HAS_PMAP if !HAS_MPU
I can't find any change of mm.c in this series. So surely this will 
break the build?
>         select IOMMU_FORCE_PT_SHARE
>   
>   config ARCH_DEFCONFIG
> diff --git a/xen/arch/arm/include/asm/config_mpu.h 
> b/xen/arch/arm/include/asm/config_mpu.h
> index 530abb8302..eee60dcffc 100644
> --- a/xen/arch/arm/include/asm/config_mpu.h
> +++ b/xen/arch/arm/include/asm/config_mpu.h
> @@ -24,4 +24,6 @@
>   
>   #define HYPERVISOR_VIRT_START  XEN_VIRT_START
>   
> +#define FIXMAP_ADDR(n)         (_AT(paddr_t, n) & (PAGE_MASK))
> +
>   #endif /* __ARM_CONFIG_MPU_H__ */
> diff --git a/xen/arch/arm/include/asm/fixmap.h 
> b/xen/arch/arm/include/asm/fixmap.h
> index d0c9a52c8c..1e338759e9 100644
> --- a/xen/arch/arm/include/asm/fixmap.h
> +++ b/xen/arch/arm/include/asm/fixmap.h
> @@ -7,6 +7,8 @@
>   #include <xen/acpi.h>
>   #include <xen/pmap.h>
>   
> +#ifndef CONFIG_HAS_MPU
> +
>   /* Fixmap slots */
>   #define FIXMAP_CONSOLE  0  /* The primary UART */
>   #define FIXMAP_MISC     1  /* Ephemeral mappings of hardware */
> @@ -45,4 +47,27 @@ static inline unsigned int virt_to_fix(vaddr_t vaddr)
>   
>   #endif /* __ASSEMBLY__ */
>   
> +#else
> +
> +/*
> + * FIXMAP_ADDR will trim physical address to PAGE alignment.
> + * This will return an offset which is similar to MMU version
> + * FIXMAP_ADDR.
> + * For example:
> + * EARLY_UART_VIRTUAL_ADDRESS is defined by:
> + *     (FIXMAP_ADDR(FIXMAP_CONSOLE) + \
> + *     (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
> + * With MPU version FIXMAP_CONSOLE and FIXMAP_ADDR definitions,
> + * EARLY_UART_VIRTUAL_ADDRESS can be restore to
> + * CONFIG_EARLY_UART_BASE_ADDRESS.
> + * In this case, we don't need to use #ifdef MPU in the code
> + * where are using FIXMAP_ADDR to make them to use physical
> + * address explicitily.
> + */
> +#ifdef CONFIG_EARLY_UART_BASE_ADDRESS
> +#define FIXMAP_CONSOLE         CONFIG_EARLY_UART_BASE_ADDRESS
> +#endif
> +
> +#endif /* CONFIG_HAS_MPU */
> +
>   #endif /* __ASM_FIXMAP_H */
Cheers,

--
Julien Grall