Hi Julien, > -----Original Message----- > From: Julien Grall <jul...@xen.org> > Sent: 2022年11月7日 3:45 > To: Wei Chen <wei.c...@arm.com>; xen-devel@lists.xenproject.org > Cc: nd <n...@arm.com>; Stefano Stabellini <sstabell...@kernel.org>; Bertrand > Marquis <bertrand.marq...@arm.com>; Volodymyr Babchuk > <volodymyr_babc...@epam.com> > Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU > systems > > 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. >
I agree, if we don't mind to add some CONFIG_HAS_MPU in some generic code. I also prefer this way. Frankly, I really think FIXMAP is awkward in MPU System. > > > > 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.c...@arm.com> > > --- > > 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? Yes, in our internal testing, open PMAP for MPU will cause building failed, except we add some new stubs for MPU system. > > > 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
