* [Xen-devel] [PATCH v2 1/5] x86/boot: Create the l2_xenmap[] mappings dynamically
2020-01-17 20:42 [Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions Andrew Cooper
@ 2020-01-17 20:42 ` Andrew Cooper
2020-01-20 10:29 ` Jan Beulich
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 2/5] x86/boot: Size the boot/directmap " Andrew Cooper
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2020-01-17 20:42 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Xen-devel] [PATCH v2 2/5] x86/boot: Size the boot/directmap mappings dynamically
2020-01-17 20:42 [Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions Andrew Cooper
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 1/5] x86/boot: Create the l2_xenmap[] mappings dynamically Andrew Cooper
@ 2020-01-17 20:42 ` Andrew Cooper
2020-01-20 10:30 ` Jan Beulich
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses Andrew Cooper
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2020-01-17 20:42 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
... 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses
2020-01-17 20:42 [Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions Andrew Cooper
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 1/5] x86/boot: Create the l2_xenmap[] mappings dynamically Andrew Cooper
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 2/5] x86/boot: Size the boot/directmap " Andrew Cooper
@ 2020-01-17 20:42 ` Andrew Cooper
2020-01-20 10:35 ` Jan Beulich
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops Andrew Cooper
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 5/5] x86/boot: Drop sym_fs() Andrew Cooper
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2020-01-17 20:42 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses Andrew Cooper
@ 2020-01-20 10:35 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2020-01-20 10:35 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops
2020-01-17 20:42 [Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions Andrew Cooper
` (2 preceding siblings ...)
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 3/5] x86/boot: Drop explicit %fs uses Andrew Cooper
@ 2020-01-17 20:42 ` Andrew Cooper
2020-01-20 10:46 ` Jan Beulich
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 5/5] x86/boot: Drop sym_fs() Andrew Cooper
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2020-01-17 20:42 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops Andrew Cooper
@ 2020-01-20 10:46 ` Jan Beulich
2020-01-22 15:43 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2020-01-20 10:46 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
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
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops
2020-01-20 10:46 ` Jan Beulich
@ 2020-01-22 15:43 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2020-01-22 15:43 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Xen-devel] [PATCH v2 5/5] x86/boot: Drop sym_fs()
2020-01-17 20:42 [Xen-devel] [PATCH v2 0/5] x86: Remove more 16M total-size restrictions Andrew Cooper
` (3 preceding siblings ...)
2020-01-17 20:42 ` [Xen-devel] [PATCH v2 4/5] x86/boot: Simplify pagetable manipulation loops Andrew Cooper
@ 2020-01-17 20:42 ` Andrew Cooper
2020-01-20 11:39 ` Jan Beulich
4 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2020-01-17 20:42 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
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
^ permalink raw reply related [flat|nested] 12+ messages in thread