On 12/03/2025 14:52, Luca Fancellu wrote:
>
>
> From: Penny Zheng <penny.zh...@arm.com>
>
> virt_to_maddr and maddr_to_virt are used widely in Xen code. So
> even there is no VMSA in MPU system, we keep the interface in MPU to
> don't change the existing common code.
>
> In order to do that, move the virt_to_maddr() definition to mmu/mm.h,
> instead for maddr_to_virt() it's more difficult to isolate it under mmu/
> so it will be protected by #ifdef CONFIG_MMU.
I don't understand this rationale. I did a quick test and moving maddr_to_virt
to mmu/mm.h works just fine.
>
> Finally implement virt_to_maddr() and maddr_to_virt() for MPU systems
> under mpu/mm.h, the MPU version of virt/maddr conversion is simple since
> VA==PA.
>
> While there, take the occasion to add emacs footer to mpu/mm.c.
>
> Signed-off-by: Penny Zheng <penny.zh...@arm.com>
> Signed-off-by: Wei Chen <wei.c...@arm.com>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> ---
> xen/arch/arm/include/asm/mm.h | 13 +++++++------
> xen/arch/arm/include/asm/mmu/mm.h | 7 +++++++
> xen/arch/arm/include/asm/mpu/mm.h | 27 +++++++++++++++++++++++++++
> xen/arch/arm/mpu/mm.c | 9 +++++++++
> 4 files changed, 50 insertions(+), 6 deletions(-)
> create mode 100644 xen/arch/arm/include/asm/mpu/mm.h
>
> diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
> index a0d8e5afe977..e7767cdab493 100644
> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -16,8 +16,10 @@
>
> #if defined(CONFIG_MMU)
> # include <asm/mmu/mm.h>
> -#elif !defined(CONFIG_MPU)
> -# error "Unknown memory management layout"
> +#elif defined(CONFIG_MPU)
> +# include <asm/mpu/mm.h>
> +#else
> +#error "Unknown memory management layout"
> #endif
>
> /* Align Xen to a 2 MiB boundary. */
> @@ -261,10 +263,7 @@ static inline void __iomem *ioremap_wc(paddr_t start,
> size_t len)
> /* Page-align address and convert to frame number format */
> #define paddr_to_pfn_aligned(paddr) paddr_to_pfn(PAGE_ALIGN(paddr))
>
> -#define virt_to_maddr(va) ({ \
> - vaddr_t va_ = (vaddr_t)(va); \
> - (paddr_t)((va_to_par(va_) & PADDR_MASK & PAGE_MASK) | (va_ &
> ~PAGE_MASK)); \
> -})
> +#if defined(CONFIG_MMU)
>
> #ifdef CONFIG_ARM_32
> /**
> @@ -310,6 +309,8 @@ static inline void *maddr_to_virt(paddr_t ma)
> }
> #endif
>
> +#endif /* CONFIG_MMU */
> +
> /*
> * Translate a guest virtual address to a machine address.
> * Return the fault information if the translation has failed else 0.
> diff --git a/xen/arch/arm/include/asm/mmu/mm.h
> b/xen/arch/arm/include/asm/mmu/mm.h
> index f5a00558c47b..5ff2071133ee 100644
> --- a/xen/arch/arm/include/asm/mmu/mm.h
> +++ b/xen/arch/arm/include/asm/mmu/mm.h
> @@ -2,6 +2,8 @@
> #ifndef __ARM_MMU_MM_H__
> #define __ARM_MMU_MM_H__
>
> +#include <asm/page.h>
> +
> /* Non-boot CPUs use this to find the correct pagetables. */
> extern uint64_t init_ttbr;
>
> @@ -14,6 +16,11 @@ extern unsigned long directmap_base_pdx;
>
> #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
>
> +#define virt_to_maddr(va) ({
> \
> + vaddr_t va_ = (vaddr_t)(va);
> \
> + (paddr_t)((va_to_par(va_) & PADDR_MASK & PAGE_MASK) | (va_ &
> ~PAGE_MASK)); \
> +})
> +
> /*
> * Print a walk of a page table or p2m
> *
> diff --git a/xen/arch/arm/include/asm/mpu/mm.h
> b/xen/arch/arm/include/asm/mpu/mm.h
> new file mode 100644
> index 000000000000..57f1e558fd44
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/mpu/mm.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ARM_MPU_MM__
Missing _H? Should be: __ARM_MPU_MM_H__
> +#define __ARM_MPU_MM__
> +
> +#include <xen/macros.h>
I guess you also need xen/types.h
> +
> +#define virt_to_maddr(va) ({ \
> + (paddr_t)va; \
> +})
Why multiline? Also, shouldn't we take PA bits into account?
I'd imagine:
((paddr_t)((vaddr_t)(va) & PADDR_MASK))
> +
> +/* On MPU systems there is no translation, ma == va. */
> +static inline void *maddr_to_virt(paddr_t ma)
> +{
> + return _p(ma);
Why do we need to cast paddr_t to unsigned long before casting to void?
Why not:
return (void *)(ma);
> +}
> +
> +#endif /* __ARM_MPU_MM__ */
__ARM_MPU_MM_H__
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/mpu/mm.c b/xen/arch/arm/mpu/mm.c
> index 0b8748e57598..a11e017d8a96 100644
> --- a/xen/arch/arm/mpu/mm.c
> +++ b/xen/arch/arm/mpu/mm.c
> @@ -13,3 +13,12 @@ static void __init __maybe_unused build_assertions(void)
> */
> BUILD_BUG_ON(PAGE_SIZE != SZ_4K);
> }
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --
> 2.34.1
>
~Michal
