[v2,2/4] x86/boot: Clean up l?_bootmap[] construction
diff mbox series

Message ID 20200109193241.14542-3-andrew.cooper3@citrix.com
State New
Headers show
Series
  • x86/boot: Remove mappings at 0
Related show

Commit Message

Andrew Cooper Jan. 9, 2020, 7:32 p.m. UTC
The need for Xen to be identity mapped into the bootmap is not obvious, and
differs between the MB and EFI boot paths.

The EFI side is further complicated by an attempt to cope with with l2_bootmap
only being 4k long.  This is undocumented, confusing, only works if Xen is the
single object wanting mapping.

The pageables are common to both the MB and EFI builds, so simplify the EFI
bootmap construction code to make exactly one identity-map of Xen, which now
makes the two paths consistent.  Comment both pieces of logic, explaining what
the mappings are needed for.

Finally, leave a linker assert covering the fact that plenty of code blindly
assumes that Xen is less that 16M.  This wants fixing in due course.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * Extra BUILD_BUG_ON()'s
---
 xen/arch/x86/boot/head.S    |  8 ++++++--
 xen/arch/x86/efi/efi-boot.h | 20 +++++++++++++-------
 xen/arch/x86/xen.lds.S      |  3 +++
 3 files changed, 22 insertions(+), 9 deletions(-)

Comments

Jan Beulich Jan. 10, 2020, 4:47 p.m. UTC | #1
On 09.01.2020 20:32, Andrew Cooper wrote:
> The need for Xen to be identity mapped into the bootmap is not obvious, and
> differs between the MB and EFI boot paths.
> 
> The EFI side is further complicated by an attempt to cope with with l2_bootmap
> only being 4k long.  This is undocumented, confusing, only works if Xen is the
> single object wanting mapping.
> 
> The pageables are common to both the MB and EFI builds, so simplify the EFI
> bootmap construction code to make exactly one identity-map of Xen, which now
> makes the two paths consistent.  Comment both pieces of logic, explaining what
> the mappings are needed for.
> 
> Finally, leave a linker assert covering the fact that plenty of code blindly
> assumes that Xen is less that 16M.  This wants fixing in due course.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Patch
diff mbox series

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index d152af4542..0b75d33a25 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -668,7 +668,11 @@  trampoline_setup:
         add     %esi,sym_fs(__page_tables_start)-8(,%ecx,8)
 2:      loop    1b
 
-        /* Initialize L2 boot-map/direct map page table entries (16MB). */
+        /*
+         * Map Xen into the directmap (needed for early-boot pagetable
+         * handling/walking), and identity map Xen into bootmap (needed for
+         * the transition into long mode), using 2M superpages.
+         */
         lea     sym_esi(start),%ebx
         lea     (1<<L2_PAGETABLE_SHIFT)*7+(PAGE_HYPERVISOR|_PAGE_PSE)(%ebx),%eax
         shr     $(L2_PAGETABLE_SHIFT-3),%ebx
@@ -678,7 +682,7 @@  trampoline_setup:
         sub     $(1<<L2_PAGETABLE_SHIFT),%eax
         loop    1b
 
-        /* Initialize L3 boot-map page directory entry. */
+        /* Initialize L3 boot-map page directory entries. */
         lea     __PAGE_HYPERVISOR+(L2_PAGETABLE_ENTRIES*8)*3+sym_esi(l2_bootmap),%eax
         mov     $4,%ecx
 1:      mov     %eax,sym_fs(l3_bootmap)-8(,%ecx,8)
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 9c036d5f4c..203a9d3bb2 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -585,21 +585,27 @@  static void __init efi_arch_memory_setup(void)
     if ( !efi_enabled(EFI_LOADER) )
         return;
 
-    /* Initialise L2 identity-map and boot-map page table entries (16MB). */
+    /* Check that there is at least 4G of mapping space in l2_*map[] */
+    BUILD_BUG_ON((sizeof(l2_bootmap)  / L2_PAGETABLE_ENTRIES) < 4);
+    BUILD_BUG_ON((sizeof(l2_identmap) / L2_PAGETABLE_ENTRIES) < 4);
+
+    /* Initialize L3 boot-map page directory entries. */
+    for ( i = 0; i < 4; ++i )
+        l3_bootmap[i] = l3e_from_paddr((UINTN)l2_bootmap + i * PAGE_SIZE,
+                                       __PAGE_HYPERVISOR);
+    /*
+     * Map Xen into the directmap (needed for early-boot pagetable
+     * handling/walking), and identity map Xen into bootmap (needed for the
+     * transition from the EFI pagetables to Xen), using 2M superpages.
+     */
     for ( i = 0; i < 8; ++i )
     {
         unsigned int slot = (xen_phys_start >> L2_PAGETABLE_SHIFT) + i;
         paddr_t addr = slot << L2_PAGETABLE_SHIFT;
 
         l2_identmap[slot] = l2e_from_paddr(addr, PAGE_HYPERVISOR|_PAGE_PSE);
-        slot &= L2_PAGETABLE_ENTRIES - 1;
         l2_bootmap[slot] = l2e_from_paddr(addr, __PAGE_HYPERVISOR|_PAGE_PSE);
     }
-    /* Initialise L3 boot-map page directory entries. */
-    l3_bootmap[l3_table_offset(xen_phys_start)] =
-        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
-    l3_bootmap[l3_table_offset(xen_phys_start + (8 << L2_PAGETABLE_SHIFT) - 1)] =
-        l3e_from_paddr((UINTN)l2_bootmap, __PAGE_HYPERVISOR);
 }
 
 static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 111edb5360..7f82f64078 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -381,3 +381,6 @@  ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE - MBI_SPACE_MIN,
     "not enough room for trampoline and mbi data")
 ASSERT((wakeup_stack - wakeup_stack_start) >= WAKEUP_STACK_MIN,
     "wakeup stack too small")
+
+/* Plenty of boot code assumes that Xen isn't larger than 16M. */
+ASSERT(_end - _start <= MB(16), "Xen too large for early-boot assumptions")