[Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions

[Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions

[Andrew Cooper]

The boot/directmap pagetables, high Xen pagetables, and use of BOOT_FS all
come with arbitrary limitations to Xen's total size.  Remove these limits.

Testing of the EFI build indicates that there is still an issue lurking
somewhere:

  (XEN) ----[ Xen-4.14-unstable  x86_64  debug=y   Not tainted ]----
  (XEN) CPU:    0
  (XEN) RIP:    e008:[<ffff82d0802557fa>] drivers/char/ns16550.c#ns16550_poll+0x1d/0x21
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d0802557fa>] R drivers/char/ns16550.c#ns16550_poll+0x1d/0x21
  (XEN)    [<ffff82d080247446>] F common/timer.c#execute_timer+0x49/0x64
  (XEN)    [<ffff82d080247c24>] F common/timer.c#timer_softirq_action+0xa2/0x276
  (XEN)    [<ffff82d080243b81>] F common/softirq.c#__do_softirq+0x71/0x9a
  (XEN)    [<ffff82d080243bdd>] F process_pending_softirqs+0x33/0x37
  (XEN)    [<ffff82d0812bd1be>] F __cpu_up+0x652/0x719
  (XEN)    [<ffff82d080204874>] F cpu_up+0x75/0xe3
  (XEN)    [<ffff82d08162ad43>] F __start_xen+0x251a/0x29b1
  (XEN)
  (XEN)
  (XEN) ****************************************
  (XEN) Panic on CPU 0:
  (XEN) FATAL TRAP: vector = 6 (invalid opcode)
  (XEN) ****************************************

which is run_in_exception_handler() not being recognised as BUGFRAME_run_fn.
Therefore, I've left the linker assert in place for now.

Andrew Cooper (5):
  x86/boot: Create the l2_xenmap[] mappings dynamically
  x86/boot: Size the boot/directmap mappings dynamically
  x86/boot: Drop explicit %fs uses
  x86/boot: Simplify pagetable manipulation loops
  x86/boot: Drop sym_fs()

 xen/arch/x86/boot/head.S       | 145 +++++++++++++++++++++++------------------
 xen/arch/x86/boot/trampoline.S |   1 -
 xen/arch/x86/boot/x86_64.S     |  23 +++----
 xen/arch/x86/efi/efi-boot.h    |  37 +++++++++--
 xen/arch/x86/xen.lds.S         |   3 +
 5 files changed, 126 insertions(+), 83 deletions(-)

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 1/5] x86/boot: Create the l2_xenmap[] mappings dynamically

[Andrew Cooper]

The build-time construction of l2_xenmap[] imposes an arbitrary limit of 16M
total, which is a limit looking to be lifted.

Move l2_xenmap[] into the BSS, and adjust both the BIOS and EFI paths to fill
it in dynamically, based on the final linked size of Xen.

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:
 * Rewrite several comments
---
 xen/arch/x86/boot/head.S    | 14 ++++++++++++++
 xen/arch/x86/boot/x86_64.S  | 23 ++++++++---------------
 xen/arch/x86/efi/efi-boot.h | 14 ++++++++++++++
 xen/arch/x86/xen.lds.S      |  3 +++
 4 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index c5acbf56ae..ef9f562505 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -668,6 +668,20 @@ trampoline_setup:
         add     %esi,sym_fs(__page_tables_start)-8(,%ecx,8)
 2:      loop    1b
 
+        /* Map Xen into the higher mappings using 2M superpages. */
+        lea     _PAGE_PSE + PAGE_HYPERVISOR_RWX + sym_esi(_start), %eax
+        mov     $sym_offs(_start),   %ecx   /* %eax = PTE to write ^      */
+        mov     $sym_offs(_end - 1), %edx
+        shr     $L2_PAGETABLE_SHIFT, %ecx   /* %ecx = First slot to write */
+        shr     $L2_PAGETABLE_SHIFT, %edx   /* %edx = Final slot to write */
+
+1:      mov     %eax, sym_offs(l2_xenmap)(%esi, %ecx, 8)
+        add     $1, %ecx
+        add     $1 << L2_PAGETABLE_SHIFT, %eax
+
+        cmp     %edx, %ecx
+        jbe     1b
+
         /*
          * Map Xen into the directmap (needed for early-boot pagetable
          * handling/walking), and identity map Xen into bootmap (needed for
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index aabf561b23..e63bece460 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -43,6 +43,14 @@ multiboot_ptr:
 GLOBAL(stack_start)
         .quad   cpu0_stack + STACK_SIZE - CPUINFO_sizeof
 
+        .section .bss.page_aligned, "aw", @nobits
+        .align PAGE_SIZE, 0
+
+/* L2 mapping the Xen text/data/bss region.  Uses 1x 4k page. */
+GLOBAL(l2_xenmap)
+        .fill L2_PAGETABLE_ENTRIES, 8, 0
+        .size l2_xenmap, . - l2_xenmap
+
         .section .data.page_aligned, "aw", @progbits
         .align PAGE_SIZE, 0
 /*
@@ -80,21 +88,6 @@ GLOBAL(l2_directmap)
         .fill 4 * L2_PAGETABLE_ENTRIES - 1, 8, 0
         .size l2_directmap, . - l2_directmap
 
-/*
- * L2 mapping the 1GB Xen text/data/bss region.  At boot it maps 16MB from
- * __image_base__, and is modified when Xen relocates itself.  Uses 1x 4k
- * page.
- */
-GLOBAL(l2_xenmap)
-        .quad 0
-        idx = 1
-        .rept 7
-        .quad sym_offs(__image_base__) + (idx << L2_PAGETABLE_SHIFT) + (PAGE_HYPERVISOR_RWX | _PAGE_PSE)
-        idx = idx + 1
-        .endr
-        .fill L2_PAGETABLE_ENTRIES - 8, 8, 0
-        .size l2_xenmap, . - l2_xenmap
-
 /* L2 mapping the fixmap.  Uses 1x 4k page. */
 l2_fixmap:
         idx = 0
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 50d1499867..ce07aedf45 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -585,6 +585,20 @@ static void __init efi_arch_memory_setup(void)
     if ( !efi_enabled(EFI_LOADER) )
         return;
 
+    /*
+     * Map Xen into the higher mappings, using 2M superpages.
+     *
+     * NB: We are currently in physical mode, so a RIP-relative relocation
+     * against _start/_end result in our arbitrary placement by the bootloader
+     * in memory, rather than the intended high mappings position.  Subtract
+     * xen_phys_start to get the appropriate slots in l2_xenmap[].
+     */
+    for ( i =  l2_table_offset((UINTN)_start   - xen_phys_start);
+          i <= l2_table_offset((UINTN)_end - 1 - xen_phys_start); ++i )
+        l2_xenmap[i] =
+            l2e_from_paddr(xen_phys_start + (i << L2_PAGETABLE_SHIFT),
+                           PAGE_HYPERVISOR_RWX | _PAGE_PSE);
+
     /* 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_directmap) / L2_PAGETABLE_ENTRIES) < 4);
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 29ef507432..07c6448dbb 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -360,6 +360,9 @@ ASSERT(__2M_rwdata_end <= XEN_VIRT_END - XEN_VIRT_START + __XEN_VIRT_START -
 ASSERT(kexec_reloc_size - kexec_reloc <= PAGE_SIZE, "kexec_reloc is too large")
 #endif
 
+/* The Multiboot setup paths relies on this to simplify superpage PTE creation. */
+ASSERT(IS_ALIGNED(_start,            MB(2)), "_start misaligned")
+
 ASSERT(IS_ALIGNED(__2M_text_end,     SECTION_ALIGN), "__2M_text_end misaligned")
 ASSERT(IS_ALIGNED(__2M_rodata_start, SECTION_ALIGN), "__2M_rodata_start misaligned")
 ASSERT(IS_ALIGNED(__2M_rodata_end,   SECTION_ALIGN), "__2M_rodata_end misaligned")
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 2/5] x86/boot: Size the boot/directmap mappings dynamically

[Andrew Cooper]

... rather than presuming that 16M will do.  On the EFI side, use
l2e_add_flags() to reduce the code-generation overhead of using
l2e_from_paddr() twice.

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:
 * Drop adjustment to the linker script.  There are more 16M issues to find.
---
 xen/arch/x86/boot/head.S    | 21 +++++++++++++--------
 xen/arch/x86/efi/efi-boot.h | 23 ++++++++++++++++++-----
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ef9f562505..0137ee99a4 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -687,14 +687,19 @@ trampoline_setup:
          * 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_RWX|_PAGE_PSE)(%ebx),%eax
-        shr     $(L2_PAGETABLE_SHIFT-3),%ebx
-        mov     $8,%ecx
-1:      mov     %eax,sym_fs(l2_bootmap)-8(%ebx,%ecx,8)
-        mov     %eax,sym_fs(l2_directmap)-8(%ebx,%ecx,8)
-        sub     $(1<<L2_PAGETABLE_SHIFT),%eax
-        loop    1b
+        lea     sym_esi(_start), %ecx
+        lea     -1 + sym_esi(_end), %edx
+        lea     _PAGE_PSE + PAGE_HYPERVISOR_RWX(%ecx), %eax /* PTE to write. */
+        shr     $L2_PAGETABLE_SHIFT, %ecx                   /* First slot to write. */
+        shr     $L2_PAGETABLE_SHIFT, %edx                   /* Final slot to write. */
+
+1:      mov     %eax, sym_offs(l2_bootmap)  (%esi, %ecx, 8)
+        mov     %eax, sym_offs(l2_directmap)(%esi, %ecx, 8)
+        add     $1, %ecx
+        add     $1 << L2_PAGETABLE_SHIFT, %eax
+
+        cmp     %edx, %ecx
+        jbe     1b
 
         /* Initialize L3 boot-map page directory entries. */
         lea     __PAGE_HYPERVISOR+(L2_PAGETABLE_ENTRIES*8)*3+sym_esi(l2_bootmap),%eax
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index ce07aedf45..bf7b0a61dc 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -611,15 +611,28 @@ static void __init efi_arch_memory_setup(void)
      * 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.
+     *
+     * NB: We are currently in physical mode, so a RIP-relative relocation
+     * against _start/_end gets their real position in memory, which are the
+     * appropriate l2 slots to map.
      */
-    for ( i = 0; i < 8; ++i )
+#define l2_4G_offset(a)                                                 \
+    (((UINTN)(a) >> L2_PAGETABLE_SHIFT) & (4 * L2_PAGETABLE_ENTRIES - 1))
+
+    for ( i  = l2_4G_offset(_start);
+          i <= l2_4G_offset(_end - 1); ++i )
     {
-        unsigned int slot = (xen_phys_start >> L2_PAGETABLE_SHIFT) + i;
-        paddr_t addr = slot << L2_PAGETABLE_SHIFT;
+        l2_pgentry_t pte = l2e_from_paddr(i << L2_PAGETABLE_SHIFT,
+                                          __PAGE_HYPERVISOR | _PAGE_PSE);
+
+        l2_bootmap[i] = pte;
+
+        /* Bootmap RWX/Non-global.  Directmap RW/Global. */
+        l2e_add_flags(pte, PAGE_HYPERVISOR);
 
-        l2_directmap[slot] = l2e_from_paddr(addr, PAGE_HYPERVISOR|_PAGE_PSE);
-        l2_bootmap[slot] = l2e_from_paddr(addr, __PAGE_HYPERVISOR|_PAGE_PSE);
+        l2_directmap[i] = pte;
     }
+#undef l2_4G_offset
 }
 
 static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses

[Andrew Cooper]

The trampoline relocation code uses %fs for accessing Xen, and this comes with
an arbitrary 16M limitation.  We could adjust the limit, but the boot code is
a confusing mix of %ds/%esi-based and %fs-based accesses, and the use of %fs
is longer to encode, and incurs an address generation overhead.

Rewrite the logic to use %ds, for better consistency with the surrounding
code, and a marginal performance improvement.

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>
---
 xen/arch/x86/boot/head.S | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 0137ee99a4..1deeae2f2a 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -718,23 +718,27 @@ trampoline_setup:
         mov     %edx, sym_esi(l2_bootmap)
 
         /* Apply relocations to bootstrap trampoline. */
-        mov     sym_fs(trampoline_phys),%edx
-        mov     $sym_offs(__trampoline_rel_start),%edi
+        mov     sym_esi(trampoline_phys), %edx
+        lea     sym_esi(__trampoline_rel_start), %edi
+        lea     sym_esi(__trampoline_rel_stop), %ecx
 1:
-        mov     %fs:(%edi),%eax
-        add     %edx,%fs:(%edi,%eax)
+        mov     (%edi), %eax
+        add     %edx, (%edi, %eax)
         add     $4,%edi
-        cmp     $sym_offs(__trampoline_rel_stop),%edi
+
+        cmp     %ecx, %edi
         jb      1b
 
         /* Patch in the trampoline segment. */
         shr     $4,%edx
-        mov     $sym_offs(__trampoline_seg_start),%edi
+        lea     sym_esi(__trampoline_seg_start), %edi
+        lea     sym_esi(__trampoline_seg_stop), %ecx
 1:
-        mov     %fs:(%edi),%eax
-        mov     %dx,%fs:(%edi,%eax)
+        mov     (%edi), %eax
+        mov     %dx, (%edi, %eax)
         add     $4,%edi
-        cmp     $sym_offs(__trampoline_seg_stop),%edi
+
+        cmp     %ecx, %edi
         jb      1b
 
         /* Do not parse command line on EFI platform here. */
@@ -760,9 +764,9 @@ trampoline_setup:
         push    %eax
 
         /* Copy bootstrap trampoline to low memory, below 1MB. */
-        mov     $sym_offs(trampoline_start),%esi
+        lea     sym_esi(trampoline_start), %esi
         mov     $((trampoline_end - trampoline_start) / 4),%ecx
-        rep movsl %fs:(%esi),%es:(%edi)
+        rep movsl
 
         /* Jump into the relocated trampoline. */
         lret
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops

[Andrew Cooper]

For __page_tables_{start,end} and L3 bootmap initialisation, the logic is
unnecesserily complicated owing to its attempt to use the LOOP instruction,
which results in an off-by-8 memory address owing to LOOP's termination
condition.

Rewrite both loops for improved clarity and speed.

Misc notes:
 * TEST $IMM, MEM can't macrofuse.  The loop has 0x1200 iterations, so pull
   the $_PAGE_PRESENT constant out into a spare register to turn the TEST into
   its %REG, MEM form, which can macrofuse.
 * Avoid the use of %fs-relative references.  %esi-relative is the more common
   form in the code, and doesn't suffer an address generation overhead.
 * Avoid LOOP.  CMP/JB isn't microcoded and faster to execute in all cases.
 * For a 4 interation trivial loop, even compilers unroll these.  The
   generated code size is a fraction larger, but this is init and the asm is
   far easier to follow.
 * Reposition the l2=>l1 bootmap construction so the asm reads in pagetable
   level order.

No functional change.

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>
---
 xen/arch/x86/boot/head.S | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 1deeae2f2a..1acaf817ba 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -662,11 +662,17 @@ trampoline_setup:
         mov     %edx,sym_fs(boot_tsc_stamp)+4
 
         /* Relocate pagetables to point at Xen's current location in memory. */
-        mov     $((__page_tables_end-__page_tables_start)/8),%ecx
-1:      testl   $_PAGE_PRESENT,sym_fs(__page_tables_start)-8(,%ecx,8)
+        mov     $_PAGE_PRESENT, %edx
+        lea     sym_esi(__page_tables_start), %eax
+        lea     sym_esi(__page_tables_end), %edi
+
+1:      testb   %dl, (%eax)  /* if page present */
         jz      2f
-        add     %esi,sym_fs(__page_tables_start)-8(,%ecx,8)
-2:      loop    1b
+        add     %esi, (%eax) /* pte += base */
+2:      add     $8, %eax
+
+        cmp     %edi, %eax
+        jb      1b
 
         /* Map Xen into the higher mappings using 2M superpages. */
         lea     _PAGE_PSE + PAGE_HYPERVISOR_RWX + sym_esi(_start), %eax
@@ -701,22 +707,27 @@ trampoline_setup:
         cmp     %edx, %ecx
         jbe     1b
 
-        /* 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)
-        sub     $(L2_PAGETABLE_ENTRIES*8),%eax
-        loop    1b
-
-        /* Map the permanent trampoline page into l{1,2}_bootmap[]. */
+        /* Map 4x l2_bootmap[] into l3_bootmap[0...3] */
+        lea     __PAGE_HYPERVISOR + sym_esi(l2_bootmap), %eax
+        mov     $PAGE_SIZE, %edx
+        mov     %eax, 0  + sym_esi(l3_bootmap)
+        add     %edx, %eax
+        mov     %eax, 8  + sym_esi(l3_bootmap)
+        add     %edx, %eax
+        mov     %eax, 16 + sym_esi(l3_bootmap)
+        add     %edx, %eax
+        mov     %eax, 24 + sym_esi(l3_bootmap)
+
+        /* Map l1_bootmap[] into l2_bootmap[0]. */
+        lea     __PAGE_HYPERVISOR + sym_esi(l1_bootmap), %eax
+        mov     %eax, sym_esi(l2_bootmap)
+
+        /* Map the permanent trampoline page into l1_bootmap[]. */
         mov     sym_esi(trampoline_phys), %ecx
         lea     __PAGE_HYPERVISOR_RX(%ecx), %edx /* %edx = PTE to write  */
         shr     $PAGE_SHIFT, %ecx                /* %ecx = Slot to write */
         mov     %edx, sym_offs(l1_bootmap)(%esi, %ecx, 8)
 
-        lea     __PAGE_HYPERVISOR + sym_esi(l1_bootmap), %edx
-        mov     %edx, sym_esi(l2_bootmap)
-
         /* Apply relocations to bootstrap trampoline. */
         mov     sym_esi(trampoline_phys), %edx
         lea     sym_esi(__trampoline_rel_start), %edi
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v2 5/5] x86/boot: Drop sym_fs()

[Andrew Cooper]

All remaining users of sym_fs() can trivially be switched to using sym_esi()
instead.  This is shorter to encode and faster to execute.

This removes the final uses of %fs during boot, which allows us to drop
BOOT_FS from the trampoline GDT, which drops an 16M arbitrary limit on Xen's
compiled size.

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>
---
 xen/arch/x86/boot/head.S       | 41 ++++++++++++++---------------------------
 xen/arch/x86/boot/trampoline.S |  1 -
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 1acaf817ba..aea6744c80 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -14,14 +14,12 @@
 
 #define sym_offs(sym)     ((sym) - __XEN_VIRT_START)
 #define sym_esi(sym)      sym_offs(sym)(%esi)
-#define sym_fs(sym)       %fs:sym_offs(sym)
 
 #define BOOT_CS32        0x0008
 #define BOOT_CS64        0x0010
 #define BOOT_DS          0x0018
 #define BOOT_PSEUDORM_CS 0x0020
 #define BOOT_PSEUDORM_DS 0x0028
-#define BOOT_FS          0x0030
 
 #define MB2_HT(name)      (MULTIBOOT2_HEADER_TAG_##name)
 #define MB2_TT(name)      (MULTIBOOT2_TAG_TYPE_##name)
@@ -555,24 +553,13 @@ trampoline_bios_setup:
 trampoline_setup:
         /*
          * Called on legacy BIOS and EFI platforms.
-         *
-         * Set the BOOT_FS descriptor base address to %esi.
          */
-        mov     %esi, %edx
-        shr     $16, %edx
-        mov     %si, BOOT_FS + 2 + sym_esi(trampoline_gdt) /* Bits  0-15 */
-        mov     %dl, BOOT_FS + 4 + sym_esi(trampoline_gdt) /* Bits 16-23 */
-        mov     %dh, BOOT_FS + 7 + sym_esi(trampoline_gdt) /* Bits 24-31 */
-
-        /* Load %fs to allow for access to Xen data. */
-        mov     $BOOT_FS, %edx
-        mov     %edx, %fs
 
         /* Save Xen image load base address for later use. */
-        mov     %esi,sym_fs(xen_phys_start)
-        mov     %esi,sym_fs(trampoline_xen_phys_start)
+        mov     %esi, sym_esi(xen_phys_start)
+        mov     %esi, sym_esi(trampoline_xen_phys_start)
 
-        mov     sym_fs(trampoline_phys),%ecx
+        mov     sym_esi(trampoline_phys), %ecx
 
         /* Get bottom-most low-memory stack address. */
         add     $TRAMPOLINE_SPACE,%ecx
@@ -583,13 +570,13 @@ trampoline_setup:
         push    %eax                /* Magic number. */
         call    reloc
 #ifdef CONFIG_PVH_GUEST
-        cmpb    $0, sym_fs(pvh_boot)
+        cmpb    $0, sym_esi(pvh_boot)
         je      1f
-        mov     %eax, sym_fs(pvh_start_info_pa)
+        mov     %eax, sym_esi(pvh_start_info_pa)
         jmp     2f
 #endif
 1:
-        mov     %eax, sym_fs(multiboot_ptr)
+        mov     %eax, sym_esi(multiboot_ptr)
 2:
 
         /*
@@ -613,7 +600,7 @@ trampoline_setup:
          * Do not zero BSS on EFI platform here.
          * It was initialized earlier.
          */
-        cmpb    $0,sym_fs(efi_platform)
+        cmpb    $0, sym_esi(efi_platform)
         jnz     1f
 
         /*
@@ -632,7 +619,7 @@ trampoline_setup:
         /* Interrogate CPU extended features via CPUID. */
         mov     $1, %eax
         cpuid
-        mov     %ecx, sym_fs(boot_cpu_data) + CPUINFO_FEATURE_OFFSET(X86_FEATURE_HYPERVISOR)
+        mov     %ecx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_HYPERVISOR) + sym_esi(boot_cpu_data)
 
         mov     $0x80000000,%eax
         cpuid
@@ -644,7 +631,7 @@ trampoline_setup:
         jbe     1f
         mov     $0x80000001,%eax
         cpuid
-1:      mov     %edx, sym_fs(boot_cpu_data) + CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM)
+1:      mov     %edx, CPUINFO_FEATURE_OFFSET(X86_FEATURE_LM) + sym_esi(boot_cpu_data)
 
         /* Check for NX. Adjust EFER setting if available. */
         bt      $cpufeat_bit(X86_FEATURE_NX), %edx
@@ -658,8 +645,8 @@ trampoline_setup:
 
         /* Stash TSC to calculate a good approximation of time-since-boot */
         rdtsc
-        mov     %eax,sym_fs(boot_tsc_stamp)
-        mov     %edx,sym_fs(boot_tsc_stamp)+4
+        mov     %eax,     sym_esi(boot_tsc_stamp)
+        mov     %edx, 4 + sym_esi(boot_tsc_stamp)
 
         /* Relocate pagetables to point at Xen's current location in memory. */
         mov     $_PAGE_PRESENT, %edx
@@ -753,11 +740,11 @@ trampoline_setup:
         jb      1b
 
         /* Do not parse command line on EFI platform here. */
-        cmpb    $0,sym_fs(efi_platform)
+        cmpb    $0, sym_esi(efi_platform)
         jnz     1f
 
         /* Bail if there is no command line to parse. */
-        mov     sym_fs(multiboot_ptr),%ebx
+        mov     sym_esi(multiboot_ptr), %ebx
         testl   $MBI_CMDLINE,MB_flags(%ebx)
         jz      1f
 
@@ -768,7 +755,7 @@ trampoline_setup:
 
 1:
         /* Switch to low-memory stack which lives at the end of trampoline region. */
-        mov     sym_fs(trampoline_phys),%edi
+        mov     sym_esi(trampoline_phys), %edi
         lea     TRAMPOLINE_SPACE+TRAMPOLINE_STACK_SPACE(%edi),%esp
         lea     trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
         pushl   $BOOT_CS32
diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S
index 6b403a6d1a..18c6638924 100644
--- a/xen/arch/x86/boot/trampoline.S
+++ b/xen/arch/x86/boot/trampoline.S
@@ -129,7 +129,6 @@ gdt_48:
         .quad   0x00cf93000000ffff /* 0x0018: ring 0 data */
         .quad   0x00009b000000ffff /* 0x0020: real-mode code @ BOOT_TRAMPOLINE */
         .quad   0x000093000000ffff /* 0x0028: real-mode data @ BOOT_TRAMPOLINE */
-        .quad   0x00c0930000000fff /* 0x0030: ring 0 Xen data, 16M @ XEN */
 .Ltrampoline_gdt_end:
 
         /* Relocations for trampoline Real Mode segments. */
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 1/5] x86/boot: Create the l2_xenmap[] mappings dynamically

[Jan Beulich]

On 17.01.2020 21:42, Andrew Cooper wrote:
> The build-time construction of l2_xenmap[] imposes an arbitrary limit of 16M
> total, which is a limit looking to be lifted.
> 
> Move l2_xenmap[] into the BSS, and adjust both the BIOS and EFI paths to fill
> it in dynamically, based on the final linked size of Xen.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

> v2:
>  * Rewrite several comments

This has helped indeed, thanks.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 2/5] x86/boot: Size the boot/directmap mappings dynamically

[Jan Beulich]

On 17.01.2020 21:42, Andrew Cooper wrote:
> ... rather than presuming that 16M will do.  On the EFI side, use
> l2e_add_flags() to reduce the code-generation overhead of using
> l2e_from_paddr() twice.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses

[Jan Beulich]

On 17.01.2020 21:42, Andrew Cooper wrote:
> The trampoline relocation code uses %fs for accessing Xen, and this comes with
> an arbitrary 16M limitation.  We could adjust the limit, but the boot code is
> a confusing mix of %ds/%esi-based and %fs-based accesses, and the use of %fs
> is longer to encode, and incurs an address generation overhead.
> 
> Rewrite the logic to use %ds, for better consistency with the surrounding
> code, and a marginal performance improvement.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops

[Jan Beulich]

On 17.01.2020 21:42, Andrew Cooper wrote:
> For __page_tables_{start,end} and L3 bootmap initialisation, the logic is
> unnecesserily complicated owing to its attempt to use the LOOP instruction,
> which results in an off-by-8 memory address owing to LOOP's termination
> condition.
> 
> Rewrite both loops for improved clarity and speed.
> 
> Misc notes:
>  * TEST $IMM, MEM can't macrofuse.  The loop has 0x1200 iterations, so pull
>    the $_PAGE_PRESENT constant out into a spare register to turn the TEST into
>    its %REG, MEM form, which can macrofuse.
>  * Avoid the use of %fs-relative references.  %esi-relative is the more common
>    form in the code, and doesn't suffer an address generation overhead.
>  * Avoid LOOP.  CMP/JB isn't microcoded and faster to execute in all cases.
>  * For a 4 interation trivial loop, even compilers unroll these.  The
>    generated code size is a fraction larger, but this is init and the asm is
>    far easier to follow.
>  * Reposition the l2=>l1 bootmap construction so the asm reads in pagetable
>    level order.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two remarks/questions, but leaving it up to you whether
you want to adjust the code:

> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -662,11 +662,17 @@ trampoline_setup:
>          mov     %edx,sym_fs(boot_tsc_stamp)+4
>  
>          /* Relocate pagetables to point at Xen's current location in memory. */
> -        mov     $((__page_tables_end-__page_tables_start)/8),%ecx
> -1:      testl   $_PAGE_PRESENT,sym_fs(__page_tables_start)-8(,%ecx,8)
> +        mov     $_PAGE_PRESENT, %edx
> +        lea     sym_esi(__page_tables_start), %eax
> +        lea     sym_esi(__page_tables_end), %edi
> +
> +1:      testb   %dl, (%eax)  /* if page present */

When it's an immediate, using TESTB is generally helpful because
there's no (sign- or whatever-)extended immediate form of it.
When using a register, I think it would generally be better to
use native size, even if for register reads the partial register
access penalty may (today) be zero.

> @@ -701,22 +707,27 @@ trampoline_setup:
>          cmp     %edx, %ecx
>          jbe     1b
>  
> -        /* 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)
> -        sub     $(L2_PAGETABLE_ENTRIES*8),%eax
> -        loop    1b
> -
> -        /* Map the permanent trampoline page into l{1,2}_bootmap[]. */
> +        /* Map 4x l2_bootmap[] into l3_bootmap[0...3] */
> +        lea     __PAGE_HYPERVISOR + sym_esi(l2_bootmap), %eax
> +        mov     $PAGE_SIZE, %edx
> +        mov     %eax, 0  + sym_esi(l3_bootmap)
> +        add     %edx, %eax
> +        mov     %eax, 8  + sym_esi(l3_bootmap)
> +        add     %edx, %eax
> +        mov     %eax, 16 + sym_esi(l3_bootmap)
> +        add     %edx, %eax
> +        mov     %eax, 24 + sym_esi(l3_bootmap)

It took me a moment to realize the code is correct despite there
not being any mention of PAGE_SIZE between each of the MOVs. As
you don't view code size as a (primary) concern, perhaps worth
using

        add     $PAGE_SIZE, %eax

everywhere, the more that this has a special, ModR/M-less
encoding?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 5/5] x86/boot: Drop sym_fs()

[Jan Beulich]

On 17.01.2020 21:42, Andrew Cooper wrote:
> All remaining users of sym_fs() can trivially be switched to using sym_esi()
> instead.  This is shorter to encode and faster to execute.
> 
> This removes the final uses of %fs during boot, which allows us to drop
> BOOT_FS from the trampoline GDT, which drops an 16M arbitrary limit on Xen's
> compiled size.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops

[Andrew Cooper]

On 20/01/2020 10:46, Jan Beulich wrote:
> On 17.01.2020 21:42, Andrew Cooper wrote:
>> For __page_tables_{start,end} and L3 bootmap initialisation, the logic is
>> unnecesserily complicated owing to its attempt to use the LOOP instruction,
>> which results in an off-by-8 memory address owing to LOOP's termination
>> condition.
>>
>> Rewrite both loops for improved clarity and speed.
>>
>> Misc notes:
>>  * TEST $IMM, MEM can't macrofuse.  The loop has 0x1200 iterations, so pull
>>    the $_PAGE_PRESENT constant out into a spare register to turn the TEST into
>>    its %REG, MEM form, which can macrofuse.
>>  * Avoid the use of %fs-relative references.  %esi-relative is the more common
>>    form in the code, and doesn't suffer an address generation overhead.
>>  * Avoid LOOP.  CMP/JB isn't microcoded and faster to execute in all cases.
>>  * For a 4 interation trivial loop, even compilers unroll these.  The
>>    generated code size is a fraction larger, but this is init and the asm is
>>    far easier to follow.
>>  * Reposition the l2=>l1 bootmap construction so the asm reads in pagetable
>>    level order.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with two remarks/questions, but leaving it up to you whether
> you want to adjust the code:
>
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -662,11 +662,17 @@ trampoline_setup:
>>          mov     %edx,sym_fs(boot_tsc_stamp)+4
>>  
>>          /* Relocate pagetables to point at Xen's current location in memory. */
>> -        mov     $((__page_tables_end-__page_tables_start)/8),%ecx
>> -1:      testl   $_PAGE_PRESENT,sym_fs(__page_tables_start)-8(,%ecx,8)
>> +        mov     $_PAGE_PRESENT, %edx
>> +        lea     sym_esi(__page_tables_start), %eax
>> +        lea     sym_esi(__page_tables_end), %edi
>> +
>> +1:      testb   %dl, (%eax)  /* if page present */
> When it's an immediate, using TESTB is generally helpful because
> there's no (sign- or whatever-)extended immediate form of it.
> When using a register, I think it would generally be better to
> use native size, even if for register reads the partial register
> access penalty may (today) be zero.

I don't think it is plausible that partial access penalties will be
introduced.  Partial merge penalties occur as a consequence of making
register reads consistent under renaming, and implicit zeroing behaviour
exists to remove merge penalties.

Any 32bit or larger register write results in allocating a fresh
physical register entry, filling it with the data provided, and updating
the register allocation table.

For 16bit or 8bit writes, either the physical register file needs to
support RMW updates to an architectural register, or an extra set of
uops are needed to perform the merge in the pipeline itself, before
making a 32bit writeback.

What matters in this case is the size of the memory access, and whether
8bit vs 32bit within the same cache line will ever be different.

However, we should switch to a 32bit access here, so we don't intermix
an 8bit read with a 32bit RMW.  Memory disambiguation speculation will
have an easier time of it on some parts, which will make an overall
difference.

>
>> @@ -701,22 +707,27 @@ trampoline_setup:
>>          cmp     %edx, %ecx
>>          jbe     1b
>>  
>> -        /* 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)
>> -        sub     $(L2_PAGETABLE_ENTRIES*8),%eax
>> -        loop    1b
>> -
>> -        /* Map the permanent trampoline page into l{1,2}_bootmap[]. */
>> +        /* Map 4x l2_bootmap[] into l3_bootmap[0...3] */
>> +        lea     __PAGE_HYPERVISOR + sym_esi(l2_bootmap), %eax
>> +        mov     $PAGE_SIZE, %edx
>> +        mov     %eax, 0  + sym_esi(l3_bootmap)
>> +        add     %edx, %eax
>> +        mov     %eax, 8  + sym_esi(l3_bootmap)
>> +        add     %edx, %eax
>> +        mov     %eax, 16 + sym_esi(l3_bootmap)
>> +        add     %edx, %eax
>> +        mov     %eax, 24 + sym_esi(l3_bootmap)
> It took me a moment to realize the code is correct despite there
> not being any mention of PAGE_SIZE between each of the MOVs. As
> you don't view code size as a (primary) concern, perhaps worth
> using
>
>         add     $PAGE_SIZE, %eax
>
> everywhere, the more that this has a special, ModR/M-less
> encoding?

I had it that way first time around.  Sadly, $PAGE_SIZE can't be
expressed as imm8, which is why I switched to using %edx.

I'm not overly fussed either way, so given the confusion, I'll switch
back to this form.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel