Re: [PATCH v6 3/5] xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping
Hi Bertrand, On 03/03/2023 10:35, Bertrand Marquis wrote: Hi Julien, On 2 Mar 2023, at 15:59, Julien Grall wrote: From: Julien Grall In follow-up patches we will need to have part of Xen identity mapped in order to safely switch the TTBR. On some platform, the identity mapping may have to start at 0. If we always keep the identity region mapped, NULL pointer dereference would lead to access to valid mapping. It would be possible to relocate Xen to avoid clashing with address 0. However the identity mapping is only meant to be used in very limited places. Therefore it would be better to keep the identity region invalid for most of the time. Two new external helpers are introduced: - arch_setup_page_tables() will setup the page-tables so it is easy to create the mapping afterwards. - update_identity_mapping() will create/remove the identity mapping Signed-off-by: Julien Grall In Arm internal CI this patch (or maybe an other in the serie) made one of our test crash on qemu-arm64. Thanks for the report. I managed to reproduce it by tweaking the QEMU command line option I was using: 42sh> qemu/build/qemu-system-aarch64 -machine virt,gic-version=3 -machine virtualization=true -cpu cortex-a57 -smp 4 -m 2048 -serial mon:stdio -serial null -nographic -kernel xen/xen/xen The problem is in patch #2 because I didn't adjust the address of the vmap/frametable areas. So they effectively are still right in the middle of the reserved region for identity mapping. I will update patch #2. I am also thinking to add a check in xen_pt_update() to ensure no-one can create a non 1:1 mapping in the reserved area for identity mapping. Cheers, -- Julien Grall
Re: [PATCH v6 3/5] xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping
Hi Julien, > On 2 Mar 2023, at 15:59, Julien Grall wrote: > > From: Julien Grall > > In follow-up patches we will need to have part of Xen identity mapped in > order to safely switch the TTBR. > > On some platform, the identity mapping may have to start at 0. If we always > keep the identity region mapped, NULL pointer dereference would lead to > access to valid mapping. > > It would be possible to relocate Xen to avoid clashing with address 0. > However the identity mapping is only meant to be used in very limited > places. Therefore it would be better to keep the identity region invalid > for most of the time. > > Two new external helpers are introduced: >- arch_setup_page_tables() will setup the page-tables so it is > easy to create the mapping afterwards. >- update_identity_mapping() will create/remove the identity mapping > > Signed-off-by: Julien Grall In Arm internal CI this patch (or maybe an other in the serie) made one of our test crash on qemu-arm64. The error log is here after. Cheers Bertrand - UART enabled - - Boot CPU booting - - Current EL 0008 - - Initialize CPU - - Turning on paging - - Zero BSS - - Ready - (XEN) Checking for initrd in /chosen (XEN) RAM: 4000 - bfff (XEN) (XEN) MODULE[0]: 4020 - 403590c8 Xen (XEN) MODULE[1]: 4800 - 48008f20 Device Tree (XEN) MODULE[2]: 4500 - 463e3200 Kernel (XEN) (XEN) (XEN) Command line: console=dtuart dtuart=/pl011@900 dom0_mem=512m (XEN) Domain heap initialised (XEN) Booting using Device Tree (XEN) Platform: Generic System (XEN) Looking for dtuart at "/pl011@900", options "" Xen 4.18-unstable (XEN) Xen version 4.18-unstable (@eu-west-1.compute.internal) (aarch64-poky-linux-gcc (GCC) 11.3.0) debug=y 2023-03-02 (XEN) Latest ChangeSet: (XEN) build-id: cc83f93cef7a75d303680ba1f98e756c07df5497 (XEN) Processor: 411fd070: "ARM Limited", variant: 0x1, part 0xd07,rev 0x0 (XEN) 64-bit Execution: (XEN) Processor Features: 01000222 (XEN) Exception Levels: EL3:No EL2:64+32 EL1:64+32 EL0:64+32 (XEN) Extensions: FloatingPoint AdvancedSIMD GICv3-SysReg (XEN) Debug Features: 10305106 (XEN) Auxiliary Features: (XEN) Memory Model Features: 1124 (XEN) ISA Features: 00011120 (XEN) 32-bit Execution: (XEN) Processor Features: 0131:10011001 (XEN) Instruction Sets: AArch32 A32 Thumb Thumb-2 Jazelle (XEN) Extensions: GenericTimer (XEN) Debug Features: 03010066 (XEN) Auxiliary Features: (XEN) Memory Model Features: 10101105 4000 (XEN) 0126 02102211 (XEN) ISA Features: 02101110 13112111 21232042 (XEN) 01112131 00011142 00011121 (XEN) Using SMC Calling Convention v1.0 (XEN) Using PSCI v0.2 (XEN) SMP: Allowing 4 CPUs (XEN) enabled workaround for: ARM erratum 832075 (XEN) enabled workaround for: ARM erratum 834220 (XEN) enabled workaround for: ARM erratum 1319367 (XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 62500 KHz (XEN) GICv3 initialization: (XEN) gic_dist_addr=0x000800 (XEN) gic_maintenance_irq=25 (XEN) gic_rdist_stride=0 (XEN) gic_rdist_regions=1 (XEN) redistributor regions: (XEN) - region 0: 0x00080a - 0x000900 (XEN) GICv3: 256 lines, (IID 043b). (XEN) GICv3: CPU0: Found redistributor in region 0 @4001c000 (XEN) XSM Framework v1.0.1 initialized (XEN) Initialising XSM SILO mode (XEN) Using scheduler: SMP Credit Scheduler rev2 (credit2) (XEN) Initializing Credit2 scheduler (XEN) load_precision_shift: 18 (XEN) load_window_shift: 30 (XEN) underload_balance_tolerance: 0 (XEN) overload_balance_tolerance: -3 (XEN) runqueues arrangement: socket (XEN) cap enforcement granularity: 10ms (XEN) load tracking window length 1073741824 ns (XEN) Allocated console ring of 32 KiB. (XEN) CPU0: Guest atomics will try 1 times before pausing the domain (XEN) Bringing up CPU1 (XEN) arch/arm/mm.c:874: Changing MFN for a valid entry is not allowed (0x8284 -> 0x40200). (XEN) Xen WARN at arch/arm/mm.c:874 (XEN) [ Xen-4.18-unstable arm64 debug=y Not tainted ] (XEN) CPU: 0 (XEN) PC: 02272438 mm.c#xen_pt_update+0x6d0/0x82c (XEN) LR: 02272438 (XEN) SP: 0230fc60 (XEN) CPSR: 8249 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 02318038 X1: 0002 X2: (XEN) X3: 0003 X4: 022be956 X5: 0002 (XEN) X6: 80007ffe9360 X7: fefefefefefefefe X8: (XEN) X9: 0080 X10: 7f7f7f7f7f7f7f7f X11: 0101010101010101 (XEN) X12: 0008 X13: 0030 X14: 0005 (XEN) X15: X16: fff8 X17: 00
RE: [PATCH v6 3/5] xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping
Hi Julien, > -Original Message- > Subject: [PATCH v6 3/5] xen/arm64: mm: Introduce helpers to > prepare/enable/disable the identity mapping > > From: Julien Grall > > In follow-up patches we will need to have part of Xen identity mapped in > order to safely switch the TTBR. > > On some platform, the identity mapping may have to start at 0. If we always > keep the identity region mapped, NULL pointer dereference would lead to > access to valid mapping. > > It would be possible to relocate Xen to avoid clashing with address 0. > However the identity mapping is only meant to be used in very limited > places. Therefore it would be better to keep the identity region invalid > for most of the time. > > Two new external helpers are introduced: > - arch_setup_page_tables() will setup the page-tables so it is > easy to create the mapping afterwards. > - update_identity_mapping() will create/remove the identity mapping > > Signed-off-by: Julien Grall Reviewed-by: Henry Wang Kind regards, Henry
[PATCH v6 3/5] xen/arm64: mm: Introduce helpers to prepare/enable/disable the identity mapping
From: Julien Grall
In follow-up patches we will need to have part of Xen identity mapped in
order to safely switch the TTBR.
On some platform, the identity mapping may have to start at 0. If we always
keep the identity region mapped, NULL pointer dereference would lead to
access to valid mapping.
It would be possible to relocate Xen to avoid clashing with address 0.
However the identity mapping is only meant to be used in very limited
places. Therefore it would be better to keep the identity region invalid
for most of the time.
Two new external helpers are introduced:
- arch_setup_page_tables() will setup the page-tables so it is
easy to create the mapping afterwards.
- update_identity_mapping() will create/remove the identity mapping
Signed-off-by: Julien Grall
Changes in v6:
- Correctly check the placement of the identity mapping (take
2).
- Fix typoes
Changes in v5:
- The reserved area for the identity mapping is 2TB (so 4 slots)
rather than 512GB.
Changes in v4:
- Fix typo in a comment
- Clarify which page-tables are updated
Changes in v2:
- Remove the arm32 part
- Use a different logic for the boot page tables and runtime
one because Xen may be running in a different place.
---
xen/arch/arm/arm64/Makefile | 1 +
xen/arch/arm/arm64/mm.c | 130
xen/arch/arm/include/asm/arm32/mm.h | 4 +
xen/arch/arm/include/asm/arm64/mm.h | 13 +++
xen/arch/arm/include/asm/config.h | 2 +
xen/arch/arm/include/asm/setup.h| 11 +++
xen/arch/arm/mm.c | 6 +-
7 files changed, 165 insertions(+), 2 deletions(-)
create mode 100644 xen/arch/arm/arm64/mm.c
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 6d507da0d44d..28481393e98f 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -10,6 +10,7 @@ obj-y += entry.o
obj-y += head.o
obj-y += insn.o
obj-$(CONFIG_LIVEPATCH) += livepatch.o
+obj-y += mm.o
obj-y += smc.o
obj-y += smpboot.o
obj-y += traps.o
diff --git a/xen/arch/arm/arm64/mm.c b/xen/arch/arm/arm64/mm.c
new file mode 100644
index ..56b9e9b8d3ef
--- /dev/null
+++ b/xen/arch/arm/arm64/mm.c
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include
+#include
+
+#include
+
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
+static DEFINE_PAGE_TABLE(xen_first_id);
+static DEFINE_PAGE_TABLE(xen_second_id);
+static DEFINE_PAGE_TABLE(xen_third_id);
+
+/*
+ * The identity mapping may start at physical address 0. So we don't want
+ * to keep it mapped longer than necessary.
+ *
+ * When this is called, we are still using the boot_pgtable.
+ *
+ * We need to prepare the identity mapping for both the boot page tables
+ * and runtime page tables.
+ *
+ * The logic to create the entry is slightly different because Xen may
+ * be running at a different location at runtime.
+ */
+static void __init prepare_boot_identity_mapping(void)
+{
+paddr_t id_addr = virt_to_maddr(_start);
+lpae_t pte;
+DECLARE_OFFSETS(id_offsets, id_addr);
+
+/*
+ * We will be re-using the boot ID tables. They may not have been
+ * zeroed but they should be unlinked. So it is fine to use
+ * clear_page().
+ */
+clear_page(boot_first_id);
+clear_page(boot_second_id);
+clear_page(boot_third_id);
+
+if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
+panic("Cannot handle ID mapping above 2TB\n");
+
+/* Link first ID table */
+pte = mfn_to_xen_entry(virt_to_mfn(boot_first_id), MT_NORMAL);
+pte.pt.table = 1;
+pte.pt.xn = 0;
+
+write_pte(&boot_pgtable[id_offsets[0]], pte);
+
+/* Link second ID table */
+pte = mfn_to_xen_entry(virt_to_mfn(boot_second_id), MT_NORMAL);
+pte.pt.table = 1;
+pte.pt.xn = 0;
+
+write_pte(&boot_first_id[id_offsets[1]], pte);
+
+/* Link third ID table */
+pte = mfn_to_xen_entry(virt_to_mfn(boot_third_id), MT_NORMAL);
+pte.pt.table = 1;
+pte.pt.xn = 0;
+
+write_pte(&boot_second_id[id_offsets[2]], pte);
+
+/* The mapping in the third table will be created at a later stage */
+}
+
+static void __init prepare_runtime_identity_mapping(void)
+{
+paddr_t id_addr = virt_to_maddr(_start);
+lpae_t pte;
+DECLARE_OFFSETS(id_offsets, id_addr);
+
+if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
+panic("Cannot handle ID mapping above 2TB\n");
+
+/* Link first ID table */
+pte = pte_of_xenaddr((vaddr_t)xen_first_id);
+pte.pt.table = 1;
+pte.pt.xn = 0;
+
+write_pte(&xen_pgtable[id_offsets[0]], pte);
+
+/* Link second ID table */
+pte = pte_of_xenaddr((vaddr_t)xen_second_id);
+pte.pt.table = 1;
+pte.pt.xn = 0;
+
+write_pte(&xen_first_id[id_offsets[1]], pte);
+
+/* Link third ID table */
+
