| Message ID | 20221104100741.2176307-8-wei.chen@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/arm: Add Armv8-R64 MPU support to Xen - Part#1 | expand |
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@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? > 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,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2022年11月7日 3:45 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@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.chen@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
On 09/11/2022 06:46, Wei Chen wrote: > Hi Julien, Hi Wei, > >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2022年11月7日 3:45 >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org >> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand >> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk >> <Volodymyr_Babchuk@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. FAOD, this is not what I had in mind. Instead, it was to provide helper which for !HAS_MPU would call fixmap and for HAS_MPU would do the work to map the region in the MPU. [...] >>> 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. Do you mean you added some stubs for PMAP? If so, I would not expect any caller for the pmap() to be used for the MPU. Therefore, why would they be necessary? Cheers,
Hi Julien, > -----Original Message----- > From: Julien Grall <julien@xen.org> > Sent: 2022年11月10日 2:30 > To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org > Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand > Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > <Volodymyr_Babchuk@epam.com> > Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU > systems > > > > On 09/11/2022 06:46, Wei Chen wrote: > > Hi Julien, > > Hi Wei, > > > > >> -----Original Message----- > >> From: Julien Grall <julien@xen.org> > >> Sent: 2022年11月7日 3:45 > >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org > >> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; > Bertrand > >> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk > >> <Volodymyr_Babchuk@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. > > FAOD, this is not what I had in mind. Instead, it was to provide helper > which for !HAS_MPU would call fixmap and for HAS_MPU would do the work > to map the region in the MPU. > Sorry, I am still confused about this comment, did you mean we can provider Some generic helpers like: early_map_console / eary_map_guest_memory. For non-MPU system, we still can call fixmap in these callers, but for MPU system, we have to map the region to MPU region? > [...] > > >>> 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. > > Do you mean you added some stubs for PMAP? If so, I would not expect any > caller for the pmap() to be used for the MPU. Therefore, why would they > be necessary? > No, I mean if we want to make pmap can be built successfully for MPU, we have to implement some stubs like: fix_to_virt, xen_fixmap and write_pte, to make compiling success. But just as you said, we would not expect MPU to use any PMAP function, so we have not implemented them for MPU. Instead we disable PMAP for MPU. Cheer, Wei Chen > Cheers, > > -- > Julien Grall
On 11/11/2022 07:56, Wei Chen wrote: > Hi Julien, > >> -----Original Message----- >> From: Julien Grall <julien@xen.org> >> Sent: 2022年11月10日 2:30 >> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org >> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; Bertrand >> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk >> <Volodymyr_Babchuk@epam.com> >> Subject: Re: [PATCH v6 07/11] xen/arm: implement FIXMAP_ADDR for MPU >> systems >> >> >> >> On 09/11/2022 06:46, Wei Chen wrote: >>> Hi Julien, >> >> Hi Wei, >> >>> >>>> -----Original Message----- >>>> From: Julien Grall <julien@xen.org> >>>> Sent: 2022年11月7日 3:45 >>>> To: Wei Chen <Wei.Chen@arm.com>; xen-devel@lists.xenproject.org >>>> Cc: nd <nd@arm.com>; Stefano Stabellini <sstabellini@kernel.org>; >> Bertrand >>>> Marquis <Bertrand.Marquis@arm.com>; Volodymyr Babchuk >>>> <Volodymyr_Babchuk@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. >> >> FAOD, this is not what I had in mind. Instead, it was to provide helper >> which for !HAS_MPU would call fixmap and for HAS_MPU would do the work >> to map the region in the MPU. >> > > Sorry, I am still confused about this comment, did you mean we can provider > Some generic helpers like: early_map_console / eary_map_guest_memory. > For non-MPU system, we still can call fixmap in these callers, but for > MPU system, we have to map the region to MPU region? Yes. Cheers,
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 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 */
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. 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@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(-)