[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 20/28] xen/arm32: head: Remove 1:1 mapping as soon as it is not used
Hi Stefano,
On 8/22/19 7:17 PM, Stefano Stabellini wrote:
On Mon, 12 Aug 2019, Julien Grall wrote:
The 1:1 mapping may clash with other parts of the Xen virtual memory
layout. At the moment, Xen is handling the clash by only creating a
mapping to the runtime virtual address before enabling the MMU.
The rest of the mappings (such as the fixmap) will be mapped after the
MMU is enabled. However, the code doing the mapping is not safe as it
replace mapping without using the Break-Before-Make sequence.
As the 1:1 mapping can be anywhere in the memory, it is easier to remove
all the entries added as soon as the 1:1 mapping is not used rather than
adding the Break-Before-Make sequence everywhere.
It is difficult to track where exactly the 1:1 mapping was created
without a full rework of create_page_tables(). Instead, introduce a new
function remove_identity_mapping() will look where is the top-level entry
for the 1:1 mapping and remove it.
The new function is only called for the boot CPU. Secondary CPUs will
switch directly to the runtime page-tables so there are no need to
remove the 1:1 mapping. Note that this still doesn't make the Secondary
CPUs path safe but it is not making it worst.
Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
---
It is very likely we will need to re-introduce the 1:1 mapping to cater
secondary CPUs boot and suspend/resume. For now, the attempt is to make
boot CPU path fully Arm Arm compliant.
Changes in v3:
- Remove unused label
- Avoid harcoding slots
Changes in v2:
- Patch added
---
xen/arch/arm/arm32/head.S | 86 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 69 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 8f945d318a..34fc9ffcee 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -32,6 +32,10 @@
#define PT_UPPER(x) (PT_##x & 0xf00)
#define PT_LOWER(x) (PT_##x & 0x0ff)
+/* Convenience defines to get slot used by Xen mapping. */
+#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
+#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
+
#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
#include EARLY_PRINTK_INC
#endif
@@ -158,6 +162,13 @@ past_zImage:
ldr r0, =primary_switched
mov pc, r0
primary_switched:
+ /*
+ * The 1:1 map may clash with other parts of the Xen virtual memory
+ * layout. As it is not used anymore, remove it completely to
+ * avoid having to worry about replacing existing mapping
+ * afterwards.
+ */
+ bl remove_identity_mapping
bl setup_fixmap
#ifdef CONFIG_EARLY_PRINTK
/* Use a virtual address to access the UART. */
@@ -474,12 +485,63 @@ enable_mmu:
mov pc, lr
ENDPROC(enable_mmu)
-setup_fixmap:
+/*
+ * Remove the 1:1 map for the page-tables. It is not easy to keep track
+ * where the 1:1 map was mapped, so we will look for the top-level entry
+ * exclusive to the 1:1 map and remove it.
+ *
+ * Inputs:
+ * r9 : paddr(start)
+ *
+ * Clobbers r0 - r3
+ */
+remove_identity_mapping:
+ /* r2:r3 := invalid page-table entry */
+ mov r2, #0x0
+ mov r3, #0x0
/*
- * Now we can install the fixmap and dtb mappings, since we
- * don't need the 1:1 map any more
+ * Find the first slot used. Remove the entry for the first
+ * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT,
+ * the 1:1 mapping was done in the second table.
May I suggest something more similar to the arm64 version as a comment?
Something like:
"Find the first slot used. Remove the entry for the first table if the
slot is not XEN_FIRST_SLOT. In case of XEN_FIRST_SLOT, we have to look
at the second table as the slot is shared with the XEN_VIRT_START
mapping."
I have answered that on the arm64 version. Let's continue the
conversation there.
*/
- dsb
+ lsr r1, r9, #FIRST_SHIFT
+ mov_w r0, LPAE_ENTRY_MASK
ldr?
What's wrong with the mov_w? Ok it is two instructions but... the
constant will be stored in a literal and therefore induce a memory load
(see patch #8).
+ and r1, r1, r0 /* r1 := first slot */
+ cmp r1, #XEN_FIRST_SLOT
NIT: align
align of?
+ beq 1f
+ /* It is not in slot 0, remove the entry */
+ ldr r0, =boot_pgtable /* r0 := root table */
+ lsl r1, r1, #3 /* r1 := Slot offset */
+ strd r2, r3, [r0, r1]
+ b identity_mapping_removed
+
+1:
+ /*
+ * Find the second slot used. Remove the entry for the first
+ * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT,
+ * it means the 1:1 mapping was not created.
+ */
+ lsr r1, r9, #SECOND_SHIFT
+ mov_w r0, LPAE_ENTRY_MASK
+ and r1, r1, r0 /* r1 := second slot */
+ cmp r1, #XEN_SECOND_SLOT
+ beq identity_mapping_removed
+ /* It is not in slot 1, remove the entry */
+ ldr r0, =boot_second /* r0 := second table */
+ lsl r1, r1, #3 /* r1 := Slot offset */
+ strd r2, r3, [r0, r1]
+
+identity_mapping_removed:
+ /* See asm-arm/arm32/flushtlb.h for the explanation of the sequence. */
+ dsb nshst
+ mcr CP32(r0, TLBIALLH)
+ dsb nsh
+ isb
As for the arm64 patch, I would like to understand dsb ishst vs. dsb
nshst.
I have answered that on the arm64 version. Let's continue the
conversation there.
+ mov pc, lr
+ENDPROC(remove_identity_mapping)
+
+setup_fixmap:
#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */
/* Add UART to the fixmap table */
ldr r1, =xen_fixmap /* r1 := vaddr (xen_fixmap) */
@@ -489,7 +551,6 @@ setup_fixmap:
orr r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART
*/
mov r3, #0x0
strd r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first
fixmap's slot */
-1:
/* Map fixmap into boot_second */
ldr r1, =boot_second /* r1 := vaddr (boot_second) */
@@ -501,19 +562,10 @@ setup_fixmap:
mov r4, r4, lsr #(SECOND_SHIFT - 3) /* r4 := Slot for FIXMAP(0) */
mov r3, #0x0
strd r2, r3, [r1, r4] /* Map it in the fixmap's slot */
-#endif
-
- /*
- * Flush the TLB in case the 1:1 mapping happens to clash with
- * the virtual addresses used by the fixmap or DTB.
- */
- dsb /* Ensure any page table updates made
above
- * have occurred. */
- isb
- mcr CP32(r0, TLBIALLH) /* Flush hypervisor TLB */
- dsb /* Ensure completion of TLB flush */
- isb
+ /* Ensure any page table updates made above have occurred. */
+ dsb nshst
same here
I have answered that on the arm64 version. Let's continue the
conversation there.
+#endif
mov pc, lr
ENDPROC(setup_fixmap)
--
2.11.0
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
