diff mbox series

[v4,05/13] xen/arm: Move MMU related definitions from config.h to mmu/layout.h

Message ID 20230801034419.2047541-6-Henry.Wang@arm.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Split MMU code as the prepration of MPU work | expand

Commit Message

Henry Wang Aug. 1, 2023, 3:44 a.m. UTC 
From: Wei Chen <wei.chen@arm.com>

Xen defines some global configuration macros for Arm in config.h.
However there are some address layout related definitions that are
defined for MMU systems only, and these definitions could not be
used by MPU systems. Adding ifdefs with CONFIG_HAS_MPU to gate these
definitions will result in a messy and hard-to-read/maintain code.

So move MMU related definitions to a new file, i.e. mmu/layout.h to
avoid spreading "#ifdef" everywhere.

Signed-off-by: Wei Chen <wei.chen@arm.com>
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
Signed-off-by: Henry Wang <Henry.Wang@arm.com>
---
v4:
- Rebase on top of latest staging to pick the recent UBSAN change
  to the layout.
- Use #ifdef CONFIG_HAS_MMU instead of #ifndef CONFIG_HAS_MPU, add
  a #else case.
- Rework commit message.
v3:
- name the new header layout.h
v2:
- Remove duplicated FIXMAP definitions from config_mmu.h
---
 xen/arch/arm/include/asm/config.h     | 132 +----------------------
 xen/arch/arm/include/asm/mmu/layout.h | 146 ++++++++++++++++++++++++++
 2 files changed, 149 insertions(+), 129 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/mmu/layout.h

Comments

Julien Grall Aug. 9, 2023, 12:19 p.m. UTC | #1 
Hi Henry,

On 01/08/2023 04:44, Henry Wang wrote:
> From: Wei Chen <wei.chen@arm.com>
> 
> Xen defines some global configuration macros for Arm in config.h.
> However there are some address layout related definitions that are
> defined for MMU systems only, and these definitions could not be
> used by MPU systems. Adding ifdefs with CONFIG_HAS_MPU to gate these

You haven't yet introduced HAS_MPU. So I would suggest to write: 'Adding 
ifdefs to differentiate the MPU from MMU layout will result in a ...'.

> definitions will result in a messy and hard-to-read/maintain code.
> 
> So move MMU related definitions to a new file, i.e. mmu/layout.h to
> avoid spreading "#ifdef" everywhere.

AFAICT, you are moving all memory layout definition in a separate file. 
So I would say it explicitely rather than using "MMU related definitions".

> 
> Signed-off-by: Wei Chen <wei.chen@arm.com>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>

With the comments addressed:

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,
Henry Wang Aug. 10, 2023, 7:41 a.m. UTC | #2 
Hi Julien,

> On Aug 9, 2023, at 20:19, Julien Grall <julien@xen.org> wrote:
> 
> Hi Henry,
> 
> On 01/08/2023 04:44, Henry Wang wrote:
>> From: Wei Chen <wei.chen@arm.com>
>> Xen defines some global configuration macros for Arm in config.h.
>> However there are some address layout related definitions that are
>> defined for MMU systems only, and these definitions could not be
>> used by MPU systems. Adding ifdefs with CONFIG_HAS_MPU to gate these
> 
> You haven't yet introduced HAS_MPU. So I would suggest to write: 'Adding ifdefs to differentiate the MPU from MMU layout will result in a ...'.
> 
>> definitions will result in a messy and hard-to-read/maintain code.
>> So move MMU related definitions to a new file, i.e. mmu/layout.h to
>> avoid spreading "#ifdef" everywhere.
> 
> AFAICT, you are moving all memory layout definition in a separate file. So I would say it explicitely rather than using "MMU related definitions".
> 
>> Signed-off-by: Wei Chen <wei.chen@arm.com>
>> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> 
> With the comments addressed:
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Thanks, I’ve taken this tag with above two comments fixed.

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/config.h b/xen/arch/arm/include/asm/config.h
index 83cbf6b0cb..a3cde7f2d7 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -71,136 +71,10 @@ 
 #include <xen/const.h>
 #include <xen/page-size.h>
 
-/*
- * ARM32 layout:
- *   0  -   2M   Unmapped
- *   2M -  10M   Xen text, data, bss
- *  10M -  12M   Fixmap: special-purpose 4K mapping slots
- *  12M -  16M   Early boot mapping of FDT
- *  16M -  18M   Livepatch vmap (if compiled in)
- *
- *  32M - 128M   Frametable: 32 bytes per page for 12GB of RAM
- * 256M -   1G   VMAP: ioremap and early_ioremap use this virtual address
- *                    space
- *
- *   1G -   2G   Xenheap: always-mapped memory
- *   2G -   4G   Domheap: on-demand-mapped
- *
- * ARM64 layout:
- * 0x0000000000000000 - 0x000001ffffffffff (2TB, L0 slots [0..3])
- *
- *  Reserved to identity map Xen
- *
- * 0x0000020000000000 - 0x0000027fffffffff (512GB, L0 slot [4])
- *  (Relative offsets)
- *   0  -   2M   Unmapped
- *   2M -  10M   Xen text, data, bss
- *  10M -  12M   Fixmap: special-purpose 4K mapping slots
- *  12M -  16M   Early boot mapping of FDT
- *  16M -  18M   Livepatch vmap (if compiled in)
- *
- *   1G -   2G   VMAP: ioremap and early_ioremap
- *
- *  32G -  64G   Frametable: 56 bytes per page for 2TB of RAM
- *
- * 0x0000028000000000 - 0x00007fffffffffff (125TB, L0 slots [5..255])
- *  Unused
- *
- * 0x0000800000000000 - 0x000084ffffffffff (5TB, L0 slots [256..265])
- *  1:1 mapping of RAM
- *
- * 0x0000850000000000 - 0x0000ffffffffffff (123TB, L0 slots [266..511])
- *  Unused
- */
-
-#ifdef CONFIG_ARM_32
-#define XEN_VIRT_START          _AT(vaddr_t, MB(2))
+#ifdef CONFIG_HAS_MMU
+#include <asm/mmu/layout.h>
 #else
-
-#define SLOT0_ENTRY_BITS  39
-#define SLOT0(slot) (_AT(vaddr_t,slot) << SLOT0_ENTRY_BITS)
-#define SLOT0_ENTRY_SIZE  SLOT0(1)
-
-#define XEN_VIRT_START          (SLOT0(4) + _AT(vaddr_t, MB(2)))
-#endif
-
-/*
- * Reserve enough space so both UBSAN and GCOV can be enabled together
- * plus some slack for future growth.
- */
-#define XEN_VIRT_SIZE           _AT(vaddr_t, MB(8))
-#define XEN_NR_ENTRIES(lvl)     (XEN_VIRT_SIZE / XEN_PT_LEVEL_SIZE(lvl))
-
-#define FIXMAP_VIRT_START       (XEN_VIRT_START + XEN_VIRT_SIZE)
-#define FIXMAP_VIRT_SIZE        _AT(vaddr_t, MB(2))
-
-#define FIXMAP_ADDR(n)          (FIXMAP_VIRT_START + (n) * PAGE_SIZE)
-
-#define BOOT_FDT_VIRT_START     (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE)
-#define BOOT_FDT_VIRT_SIZE      _AT(vaddr_t, MB(4))
-
-#ifdef CONFIG_LIVEPATCH
-#define LIVEPATCH_VMAP_START    (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
-#define LIVEPATCH_VMAP_SIZE    _AT(vaddr_t, MB(2))
-#endif
-
-#define HYPERVISOR_VIRT_START  XEN_VIRT_START
-
-#ifdef CONFIG_ARM_32
-
-#define CONFIG_SEPARATE_XENHEAP 1
-
-#define FRAMETABLE_VIRT_START  _AT(vaddr_t, MB(32))
-#define FRAMETABLE_SIZE        MB(128-32)
-#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
-
-#define VMAP_VIRT_START        _AT(vaddr_t, MB(256))
-#define VMAP_VIRT_SIZE         _AT(vaddr_t, GB(1) - MB(256))
-
-#define XENHEAP_VIRT_START     _AT(vaddr_t, GB(1))
-#define XENHEAP_VIRT_SIZE      _AT(vaddr_t, GB(1))
-
-#define DOMHEAP_VIRT_START     _AT(vaddr_t, GB(2))
-#define DOMHEAP_VIRT_SIZE      _AT(vaddr_t, GB(2))
-
-#define DOMHEAP_ENTRIES        1024  /* 1024 2MB mapping slots */
-
-/* Number of domheap pagetable pages required at the second level (2MB mappings) */
-#define DOMHEAP_SECOND_PAGES (DOMHEAP_VIRT_SIZE >> FIRST_SHIFT)
-
-/*
- * The temporary area is overlapping with the domheap area. This may
- * be used to create an alias of the first slot containing Xen mappings
- * when turning on/off the MMU.
- */
-#define TEMPORARY_AREA_FIRST_SLOT    (first_table_offset(DOMHEAP_VIRT_START))
-
-/* Calculate the address in the temporary area */
-#define TEMPORARY_AREA_ADDR(addr)                           \
-     (((addr) & ~XEN_PT_LEVEL_MASK(1)) |                    \
-      (TEMPORARY_AREA_FIRST_SLOT << XEN_PT_LEVEL_SHIFT(1)))
-
-#define TEMPORARY_XEN_VIRT_START    TEMPORARY_AREA_ADDR(XEN_VIRT_START)
-
-#else /* ARM_64 */
-
-#define IDENTITY_MAPPING_AREA_NR_L0  4
-
-#define VMAP_VIRT_START  (SLOT0(4) + GB(1))
-#define VMAP_VIRT_SIZE   GB(1)
-
-#define FRAMETABLE_VIRT_START  (SLOT0(4) + GB(32))
-#define FRAMETABLE_SIZE        GB(32)
-#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
-
-#define DIRECTMAP_VIRT_START   SLOT0(256)
-#define DIRECTMAP_SIZE         (SLOT0_ENTRY_SIZE * (266 - 256))
-#define DIRECTMAP_VIRT_END     (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE - 1)
-
-#define XENHEAP_VIRT_START     directmap_virt_start
-
-#define HYPERVISOR_VIRT_END    DIRECTMAP_VIRT_END
-
+# error "Unknown memory management layout"
 #endif
 
 #define NR_hypercalls 64
diff --git a/xen/arch/arm/include/asm/mmu/layout.h b/xen/arch/arm/include/asm/mmu/layout.h
new file mode 100644
index 0000000000..da6be276ac
--- /dev/null
+++ b/xen/arch/arm/include/asm/mmu/layout.h
@@ -0,0 +1,146 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ARM_MMU_LAYOUT_H__
+#define __ARM_MMU_LAYOUT_H__
+
+/*
+ * ARM32 layout:
+ *   0  -   2M   Unmapped
+ *   2M -  10M   Xen text, data, bss
+ *  10M -  12M   Fixmap: special-purpose 4K mapping slots
+ *  12M -  16M   Early boot mapping of FDT
+ *  16M -  18M   Livepatch vmap (if compiled in)
+ *
+ *  32M - 128M   Frametable: 32 bytes per page for 12GB of RAM
+ * 256M -   1G   VMAP: ioremap and early_ioremap use this virtual address
+ *                    space
+ *
+ *   1G -   2G   Xenheap: always-mapped memory
+ *   2G -   4G   Domheap: on-demand-mapped
+ *
+ * ARM64 layout:
+ * 0x0000000000000000 - 0x000001ffffffffff (2TB, L0 slots [0..3])
+ *
+ *  Reserved to identity map Xen
+ *
+ * 0x0000020000000000 - 0x0000027fffffffff (512GB, L0 slot [4])
+ *  (Relative offsets)
+ *   0  -   2M   Unmapped
+ *   2M -  10M   Xen text, data, bss
+ *  10M -  12M   Fixmap: special-purpose 4K mapping slots
+ *  12M -  16M   Early boot mapping of FDT
+ *  16M -  18M   Livepatch vmap (if compiled in)
+ *
+ *   1G -   2G   VMAP: ioremap and early_ioremap
+ *
+ *  32G -  64G   Frametable: 56 bytes per page for 2TB of RAM
+ *
+ * 0x0000028000000000 - 0x00007fffffffffff (125TB, L0 slots [5..255])
+ *  Unused
+ *
+ * 0x0000800000000000 - 0x000084ffffffffff (5TB, L0 slots [256..265])
+ *  1:1 mapping of RAM
+ *
+ * 0x0000850000000000 - 0x0000ffffffffffff (123TB, L0 slots [266..511])
+ *  Unused
+ */
+
+#ifdef CONFIG_ARM_32
+#define XEN_VIRT_START          _AT(vaddr_t, MB(2))
+#else
+
+#define SLOT0_ENTRY_BITS  39
+#define SLOT0(slot) (_AT(vaddr_t,slot) << SLOT0_ENTRY_BITS)
+#define SLOT0_ENTRY_SIZE  SLOT0(1)
+
+#define XEN_VIRT_START          (SLOT0(4) + _AT(vaddr_t, MB(2)))
+#endif
+
+/*
+ * Reserve enough space so both UBSAN and GCOV can be enabled together
+ * plus some slack for future growth.
+ */
+#define XEN_VIRT_SIZE           _AT(vaddr_t, MB(8))
+#define XEN_NR_ENTRIES(lvl)     (XEN_VIRT_SIZE / XEN_PT_LEVEL_SIZE(lvl))
+
+#define FIXMAP_VIRT_START       (XEN_VIRT_START + XEN_VIRT_SIZE)
+#define FIXMAP_VIRT_SIZE        _AT(vaddr_t, MB(2))
+
+#define FIXMAP_ADDR(n)          (FIXMAP_VIRT_START + (n) * PAGE_SIZE)
+
+#define BOOT_FDT_VIRT_START     (FIXMAP_VIRT_START + FIXMAP_VIRT_SIZE)
+#define BOOT_FDT_VIRT_SIZE      _AT(vaddr_t, MB(4))
+
+#ifdef CONFIG_LIVEPATCH
+#define LIVEPATCH_VMAP_START    (BOOT_FDT_VIRT_START + BOOT_FDT_VIRT_SIZE)
+#define LIVEPATCH_VMAP_SIZE    _AT(vaddr_t, MB(2))
+#endif
+
+#define HYPERVISOR_VIRT_START  XEN_VIRT_START
+
+#ifdef CONFIG_ARM_32
+
+#define CONFIG_SEPARATE_XENHEAP 1
+
+#define FRAMETABLE_VIRT_START  _AT(vaddr_t, MB(32))
+#define FRAMETABLE_SIZE        MB(128-32)
+#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
+
+#define VMAP_VIRT_START        _AT(vaddr_t, MB(256))
+#define VMAP_VIRT_SIZE         _AT(vaddr_t, GB(1) - MB(256))
+
+#define XENHEAP_VIRT_START     _AT(vaddr_t, GB(1))
+#define XENHEAP_VIRT_SIZE      _AT(vaddr_t, GB(1))
+
+#define DOMHEAP_VIRT_START     _AT(vaddr_t, GB(2))
+#define DOMHEAP_VIRT_SIZE      _AT(vaddr_t, GB(2))
+
+#define DOMHEAP_ENTRIES        1024  /* 1024 2MB mapping slots */
+
+/* Number of domheap pagetable pages required at the second level (2MB mappings) */
+#define DOMHEAP_SECOND_PAGES (DOMHEAP_VIRT_SIZE >> FIRST_SHIFT)
+
+/*
+ * The temporary area is overlapping with the domheap area. This may
+ * be used to create an alias of the first slot containing Xen mappings
+ * when turning on/off the MMU.
+ */
+#define TEMPORARY_AREA_FIRST_SLOT    (first_table_offset(DOMHEAP_VIRT_START))
+
+/* Calculate the address in the temporary area */
+#define TEMPORARY_AREA_ADDR(addr)                           \
+     (((addr) & ~XEN_PT_LEVEL_MASK(1)) |                    \
+      (TEMPORARY_AREA_FIRST_SLOT << XEN_PT_LEVEL_SHIFT(1)))
+
+#define TEMPORARY_XEN_VIRT_START    TEMPORARY_AREA_ADDR(XEN_VIRT_START)
+
+#else /* ARM_64 */
+
+#define IDENTITY_MAPPING_AREA_NR_L0  4
+
+#define VMAP_VIRT_START  (SLOT0(4) + GB(1))
+#define VMAP_VIRT_SIZE   GB(1)
+
+#define FRAMETABLE_VIRT_START  (SLOT0(4) + GB(32))
+#define FRAMETABLE_SIZE        GB(32)
+#define FRAMETABLE_NR          (FRAMETABLE_SIZE / sizeof(*frame_table))
+
+#define DIRECTMAP_VIRT_START   SLOT0(256)
+#define DIRECTMAP_SIZE         (SLOT0_ENTRY_SIZE * (266 - 256))
+#define DIRECTMAP_VIRT_END     (DIRECTMAP_VIRT_START + DIRECTMAP_SIZE - 1)
+
+#define XENHEAP_VIRT_START     directmap_virt_start
+
+#define HYPERVISOR_VIRT_END    DIRECTMAP_VIRT_END
+
+#endif
+
+#endif /* __ARM_MMU_LAYOUT_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */