[PATCHv4 0/4] x86: 5-level related changes into decompression code

[PATCHv4 0/4] x86: 5-level related changes into decompression code

[Kirill A. Shutemov]

Hi,

Here's few changes to x86 decompression code.

The first patch fixes build regression introduced by my recent patch.
It only triggers when KASLR disabled and GCC version < 5. I haven't
noticed this before.

The second patch is pure cosmetic change: give file with KASLR helpers
a proper name.

The last two patches bring support of booting into 5-level paging mode if
a bootloader put the kernel above 4G.

Patch 3/4 handles allocation of space for trampoline and gets it prepared.
Patch 4/4 gets trampoline used.

Please review and consider applying.

Kirill A. Shutemov (4):
  x86/boot/compressed/64: Fix build with GCC < 5
  x86/boot/compressed/64: Rename pagetable.c to kaslr_64.c
  x86/boot/compressed/64: Introduce place_trampoline()
  x86/boot/compressed/64: Handle 5-level paging boot if kernel is above
    4G

 arch/x86/boot/compressed/Makefile                  |   2 +-
 arch/x86/boot/compressed/head_64.S                 | 130 +++++++++++++--------
 .../boot/compressed/{pagetable.c => kaslr_64.c}    |   3 -
 arch/x86/boot/compressed/pgtable.h                 |  18 +++
 arch/x86/boot/compressed/pgtable_64.c              |  75 ++++++++++--
 5 files changed, 163 insertions(+), 65 deletions(-)
 rename arch/x86/boot/compressed/{pagetable.c => kaslr_64.c} (97%)
 create mode 100644 arch/x86/boot/compressed/pgtable.h

-- 
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCHv4 1/4] x86/boot/compressed/64: Fix build with GCC < 5

[Kirill A. Shutemov]

0-day reported this build error:

   arch/x86/boot/compressed/pgtable_64.o: In function `l5_paging_required':
   pgtable_64.c:(.text+0x22): undefined reference to `__force_order'

The issue is only with GCC < 5 and when KASLR is disabled. Newer GCC
works fine.

__force_order is used by special_insns.h asm code to force instruction
serialization.

It doesn't actually referenced from the code, but GCC < 5 with -fPIE
would still generate undefined symbol.

I didn't noticed this before and failed to move __force_order definition
from pagetable.c (which compiles only with KASLR enabled) to
pgtable_64.c.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Fixes: 10c9a5346f72 ("x86/boot/compressed/64: Detect and handle 5-level paging at boot-time")
Cc: stable@vger.kernel.org
---
 arch/x86/boot/compressed/pagetable.c  |  3 ---
 arch/x86/boot/compressed/pgtable_64.c | 11 +++++++++++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
index 6bd51de4475c..250826ac216e 100644
--- a/arch/x86/boot/compressed/pagetable.c
+++ b/arch/x86/boot/compressed/pagetable.c
@@ -38,9 +38,6 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
-/* Used by pgtable.h asm code to force instruction serialization. */
-unsigned long __force_order;
-
 /* Used to track our page table allocation area. */
 struct alloc_pgt_data {
 	unsigned char *pgt_buf;
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 7bcf03b376da..491fa2d08bca 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -1,5 +1,16 @@
 #include <asm/processor.h>
 
+/*
+ * __force_order is used by special_insns.h asm code to force instruction
+ * serialization.
+ *
+ * It doesn't actually referenced from the code, but GCC < 5 with -fPIE
+ * would still generate undefined symbol.
+ *
+ * Let's workaround this by defining the variable.
+ */
+unsigned long __force_order;
+
 int l5_paging_required(void)
 {
 	/* Check if leaf 7 is supported. */
-- 
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 1/4] x86/boot/compressed/64: Fix build with GCC < 5

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> 0-day reported this build error:
> 
>    arch/x86/boot/compressed/pgtable_64.o: In function `l5_paging_required':
>    pgtable_64.c:(.text+0x22): undefined reference to `__force_order'
> 
> The issue is only with GCC < 5 and when KASLR is disabled. Newer GCC
> works fine.
> 
> __force_order is used by special_insns.h asm code to force instruction
> serialization.

s/is used by special_insns.h asm code
 /is used by the special_insns.h asm code

> It doesn't actually referenced from the code, but GCC < 5 with -fPIE
> would still generate undefined symbol.

s/It doesn't actually referenced from the code
 /It isn't actually referenced from the code

s/would still generate undefined symbol.
 /would still generate an undefined symbol.

> I didn't noticed this before and failed to move __force_order definition
> from pagetable.c (which compiles only with KASLR enabled) to
> pgtable_64.c.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Fixes: 10c9a5346f72 ("x86/boot/compressed/64: Detect and handle 5-level paging at boot-time")
> Cc: stable@vger.kernel.org
> ---
>  arch/x86/boot/compressed/pagetable.c  |  3 ---
>  arch/x86/boot/compressed/pgtable_64.c | 11 +++++++++++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/pagetable.c
> index 6bd51de4475c..250826ac216e 100644
> --- a/arch/x86/boot/compressed/pagetable.c
> +++ b/arch/x86/boot/compressed/pagetable.c
> @@ -38,9 +38,6 @@
>  #define __PAGE_OFFSET __PAGE_OFFSET_BASE
>  #include "../../mm/ident_map.c"
>  
> -/* Used by pgtable.h asm code to force instruction serialization. */
> -unsigned long __force_order;
> -
>  /* Used to track our page table allocation area. */
>  struct alloc_pgt_data {
>  	unsigned char *pgt_buf;
> diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> index 7bcf03b376da..491fa2d08bca 100644
> --- a/arch/x86/boot/compressed/pgtable_64.c
> +++ b/arch/x86/boot/compressed/pgtable_64.c
> @@ -1,5 +1,16 @@
>  #include <asm/processor.h>
>  
> +/*
> + * __force_order is used by special_insns.h asm code to force instruction
> + * serialization.

s/is used by special_insns.h asm code
 /is used by the special_insns.h asm code

> + *
> + * It doesn't actually referenced from the code, but GCC < 5 with -fPIE
> + * would still generate undefined symbol.

s/It doesn't actually referenced from the code
 /It isn't actually referenced from the code

s/would still generate undefined symbol.
 /would still generate an undefined symbol.

> + *
> + * Let's workaround this by defining the variable.

s/Let's workaround
 /Let's work around

Also, for the title:

s/Fix build with GCC < 5
 /Fix the build with GCC < 5

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCHv4 2/4] x86/boot/compressed/64: Rename pagetable.c to kaslr_64.c

[Kirill A. Shutemov]

The name of the file -- pagetable.c -- is misleading: it only contains
helpers used for KASLR in 64-bin mode.

Let's rename the file to reflect its content.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/boot/compressed/Makefile                    | 2 +-
 arch/x86/boot/compressed/{pagetable.c => kaslr_64.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename arch/x86/boot/compressed/{pagetable.c => kaslr_64.c} (100%)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index f25e1530e064..1f734cd98fd3 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -78,7 +78,7 @@ vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
 vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
 vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/kaslr.o
 ifdef CONFIG_X86_64
-	vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/pagetable.o
+	vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/kaslr_64.o
 	vmlinux-objs-y += $(obj)/mem_encrypt.o
 	vmlinux-objs-y += $(obj)/pgtable_64.o
 endif
diff --git a/arch/x86/boot/compressed/pagetable.c b/arch/x86/boot/compressed/kaslr_64.c
similarity index 100%
rename from arch/x86/boot/compressed/pagetable.c
rename to arch/x86/boot/compressed/kaslr_64.c
-- 
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 2/4] x86/boot/compressed/64: Rename pagetable.c to kaslr_64.c

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> The name of the file -- pagetable.c -- is misleading: it only contains
> helpers used for KASLR in 64-bin mode.

s/64-bin mode
 /64-bit mode

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Kirill A. Shutemov]

If a bootloader enables 64-bit mode with 4-level paging, we might need to
switch over to 5-level paging. The switching requires disabling paging.
It works fine if kernel itself is loaded below 4G.

But if the bootloader put the kernel above 4G (not sure if anybody does
this), we would loose control as soon as paging is disabled as code
becomes unreachable.

To handle the situation, we need a trampoline in lower memory that would
take care about switching on 5-level paging.

Apart from the trampoline code itself we also need a place to store top
level page table in lower memory as we don't have a way to load 64-bit
value into CR3 from 32-bit mode. We only really need 8-bytes there as we
only use the very first entry of the page table. But we allocate whole
page anyway.

We cannot have code in the same page as page table because there's
hazard that a CPU would read page table speculatively and get confused
seeing garbage.

We also need a small stack in the trampoline to re-enable long mode via
long return. But stack and code can share the page just fine.

This patch introduces paging_prepare() that checks if we need to enable
5-level paging and then finds a right spot in lower memory for the
trampoline. Then it copies trampoline code there and setups new top
level page table for 5-level paging.

At this point we do all the preparation, but not yet use trampoline.
It will be done in the following patch.

The trampoline will be used even on 4-level paging machine. This way we
will get better coverage and keep trampoline code in shape.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/boot/compressed/head_64.S    | 42 +++++++++++------------
 arch/x86/boot/compressed/pgtable.h    | 18 ++++++++++
 arch/x86/boot/compressed/pgtable_64.c | 64 +++++++++++++++++++++++++++++------
 3 files changed, 91 insertions(+), 33 deletions(-)
 create mode 100644 arch/x86/boot/compressed/pgtable.h

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index fc313e29fe2c..92d47b1bf10a 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -304,20 +304,6 @@ ENTRY(startup_64)
 	/* Set up the stack */
 	leaq	boot_stack_end(%rbx), %rsp
 
-#ifdef CONFIG_X86_5LEVEL
-	/*
-	 * Check if we need to enable 5-level paging.
-	 * RSI holds real mode data and need to be preserved across
-	 * a function call.
-	 */
-	pushq	%rsi
-	call	l5_paging_required
-	popq	%rsi
-
-	/* If l5_paging_required() returned zero, we're done here. */
-	cmpq	$0, %rax
-	je	lvl5
-
 	/*
 	 * At this point we are in long mode with 4-level paging enabled,
 	 * but we want to enable 5-level paging.
@@ -325,12 +311,28 @@ ENTRY(startup_64)
 	 * The problem is that we cannot do it directly. Setting LA57 in
 	 * long mode would trigger #GP. So we need to switch off long mode
 	 * first.
+	 */
+
+	/*
+	 * paging_prepare() would setup trampoline and check if we need to
+	 * enable 5-level paging.
 	 *
-	 * NOTE: This is not going to work if bootloader put us above 4G
-	 * limit.
+	 * Address of the trampoline is returned in RAX. The bit 0 is used
+	 * to encode if we need to enabled 5-level paging.
 	 *
-	 * The first step is go into compatibility mode.
+	 * RSI holds real mode data and need to be preserved across
+	 * a function call.
 	 */
+	pushq	%rsi
+	call	paging_prepare
+	popq	%rsi
+
+	/* Save trampoline address in RCX */
+	movq	%rax, %rcx
+	andq	$~1, %rcx
+
+	testq	$1, %rax
+	jz	lvl5
 
 	/* Clear additional page table */
 	leaq	lvl5_pgtable(%rbx), %rdi
@@ -352,7 +354,6 @@ ENTRY(startup_64)
 	pushq	%rax
 	lretq
 lvl5:
-#endif
 
 	/* Zero EFLAGS */
 	pushq	$0
@@ -490,7 +491,7 @@ relocated:
 	jmp	*%rax
 
 	.code32
-#ifdef CONFIG_X86_5LEVEL
+ENTRY(trampoline_32bit_src)
 compatible_mode:
 	/* Setup data and stack segments */
 	movl	$__KERNEL_DS, %eax
@@ -526,7 +527,6 @@ compatible_mode:
 	movl	%eax, %cr0
 
 	lret
-#endif
 
 no_longmode:
 	/* This isn't an x86-64 CPU so hang */
@@ -585,7 +585,5 @@ boot_stack_end:
 	.balign 4096
 pgtable:
 	.fill BOOT_PGT_SIZE, 1, 0
-#ifdef CONFIG_X86_5LEVEL
 lvl5_pgtable:
 	.fill PAGE_SIZE, 1, 0
-#endif
diff --git a/arch/x86/boot/compressed/pgtable.h b/arch/x86/boot/compressed/pgtable.h
new file mode 100644
index 000000000000..1a88f58227c3
--- /dev/null
+++ b/arch/x86/boot/compressed/pgtable.h
@@ -0,0 +1,18 @@
+#ifndef BOOT_COMPRESSED_PAGETABLE_H
+#define BOOT_COMPRESSED_PAGETABLE_H
+
+#define TRAMPOLINE_32BIT_SIZE		(2 * PAGE_SIZE)
+
+#define TRAMPOLINE_32BIT_PGTABLE_OFF	0
+
+#define TRAMPOLINE_32BIT_CODE_OFF	PAGE_SIZE
+#define TRAMPOLINE_32BIT_CODE_SIZE	0x60
+
+#define TRAMPOLINE_32BIT_STACK_END	TRAMPOLINE_32BIT_SIZE
+
+#ifndef __ASSEMBLER__
+
+extern void (*trampoline_32bit_src)(void *return_ptr);
+
+#endif /* __ASSEMBLER__ */
+#endif /* BOOT_COMPRESSED_PAGETABLE_H */
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 491fa2d08bca..f72f87c2be8c 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -1,4 +1,6 @@
 #include <asm/processor.h>
+#include "pgtable.h"
+#include "../string.h"
 
 /*
  * __force_order is used by special_insns.h asm code to force instruction
@@ -11,19 +13,59 @@
  */
 unsigned long __force_order;
 
-int l5_paging_required(void)
+#define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
+#define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
+
+unsigned long paging_prepare(void)
 {
-	/* Check if leaf 7 is supported. */
-	if (native_cpuid_eax(0) < 7)
-		return 0;
+	unsigned long bios_start, ebda_start, trampoline_start, *trampoline;
+	int l5_required = 0;
+
+	/* Check if la57 is desired and supported */
+	if (IS_ENABLED(CONFIG_X86_5LEVEL) && native_cpuid_eax(0) >= 7 &&
+			(native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31))))
+		l5_required = 1;
+
+	/*
+	 * Find a suitable spot for the trampoline.
+	 * Code is based on reserve_bios_regions().
+	 */
+
+	ebda_start = *(unsigned short *)0x40e << 4;
+	bios_start = *(unsigned short *)0x413 << 10;
+
+	if (bios_start < BIOS_START_MIN || bios_start > BIOS_START_MAX)
+		bios_start = BIOS_START_MAX;
+
+	if (ebda_start > BIOS_START_MIN && ebda_start < bios_start)
+		bios_start = ebda_start;
+
+	/* Place the trampoline just below the end of low memory, aligned to 4k */
+	trampoline_start = bios_start - TRAMPOLINE_32BIT_SIZE;
+	trampoline_start = round_down(trampoline_start, PAGE_SIZE);
+
+	trampoline = (unsigned long *)trampoline_start;
+
+	/* Clear trampoline memory first */
+	memset(trampoline, 0, TRAMPOLINE_32BIT_SIZE);
 
-	/* Check if la57 is supported. */
-	if (!(native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31))))
-		return 0;
+	/* Copy trampoline code in place */
+	memcpy(trampoline + TRAMPOLINE_32BIT_CODE_OFF / sizeof(unsigned long),
+			&trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
 
-	/* Check if 5-level paging has already been enabled. */
-	if (native_read_cr4() & X86_CR4_LA57)
-		return 0;
+	/*
+	 * For 5-level paging, setup current CR3 as the first and
+	 * the only entry in a new top level page table.
+	 *
+	 * For 4-level paging, trampoline wouldn't touch CR3.
+	 * KASLR relies on CR3 pointing to _pgtable.
+	 * See initialize_identity_maps.
+	 */
+	if (l5_required) {
+		trampoline[TRAMPOLINE_32BIT_PGTABLE_OFF] =
+			__native_read_cr3() + _PAGE_TABLE_NOENC;
+	}
 
-	return 1;
+	/* Bit 0 is used to encode if 5-level paging is required */
+	return trampoline_start | l5_required;
 }
-- 
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> If a bootloader enables 64-bit mode with 4-level paging, we might need to
> switch over to 5-level paging. The switching requires disabling paging.
> It works fine if kernel itself is loaded below 4G.

s/The switching requires disabling paging.
 /The switching requires the disabling of paging.

> But if the bootloader put the kernel above 4G (not sure if anybody does
> this), we would loose control as soon as paging is disabled as code
> becomes unreachable.

Yeah, so instead of the double 'as' which is syntactically right but a bit 
confusing to read, how about something like:

  But if the bootloader put the kernel above 4G (not sure if anybody does
  this), we would loose control as soon as paging is disabled, because the
  code becomes unreachable to the CPU.

?

> To handle the situation, we need a trampoline in lower memory that would
> take care about switching on 5-level paging.

s/would take care about
 /would take care of

> Apart from the trampoline code itself we also need a place to store top
> level page table in lower memory as we don't have a way to load 64-bit
> value into CR3 from 32-bit mode. We only really need 8-bytes there as we
> only use the very first entry of the page table. But we allocate whole
> page anyway.

s/64-bit value
 /64-bit values

s/into CR3 from 32-bit mode
 /into CR3 in 32-bit mode

s/We only really need 8-bytes there
 /We only really need 8 bytes there

s/But we allocate whole page anyway.
 /But we allocate a whole page anyway.

> We cannot have code in the same page as page table because there's
> hazard that a CPU would read page table speculatively and get confused
> seeing garbage.

How about:

  We cannot have the code in the same page as the page table because there's
  a risk that a CPU would read the page table speculatively and get confused
  by seeing garbage. It's never a good idea to have junk in PTE entries
  visible to the CPU.

? (Assuming it's the PTEs that are stored in that page.)

> We also need a small stack in the trampoline to re-enable long mode via
> long return. But stack and code can share the page just fine.

BTW., I'm not sure this is necessarily a good idea: it means writable+executable 
memory, which we generally try to avoid. How complicated would it be to have them 
separate?

> This patch introduces paging_prepare() that checks if we need to enable
> 5-level paging and then finds a right spot in lower memory for the
> trampoline. Then it copies trampoline code there and setups new top
> level page table for 5-level paging.

s/Then it copies trampoline code there
 /Then it copies the trampoline code there

s/and setups new top level page table
 /and sets up the new top level page table

> At this point we do all the preparation, but not yet use trampoline.
> It will be done in the following patch.

s/but not yet use trampoline
 /but don't use trampoline yet

> The trampoline will be used even on 4-level paging machine. This way we
> will get better coverage and keep trampoline code in shape.

s/even on 4-level paging machine
 /even on 4-level paging machines

s/better coverage
 /better test coverage

s/and keep trampoline code in shape
 /and keep the trampoline code in shape

> +	/*
> +	 * paging_prepare() would setup trampoline and check if we need to
> +	 * enable 5-level paging.

s/would setup trampoline
 /would set up the trampoline

>  	 *
> +	 * Address of the trampoline is returned in RAX. The bit 0 is used
> +	 * to encode if we need to enabled 5-level paging.
>  	 *
> +	 * RSI holds real mode data and need to be preserved across
> +	 * a function call.
>  	 */


s/The bit 0
 /Bit 0

s/if we need to enabled 5-level paging
 /if we need to enable 5-level paging

> +	/* Save trampoline address in RCX */

s/Save trampoline address
 /Save the trampoline address

>  	/* Setup data and stack segments */

While at it:

s/Setup
 /Set up

> +	/* Check if la57 is desired and supported */

Please capitalize LA57 consistently!

> +	/*
> +	 * Find a suitable spot for the trampoline.
> +	 * Code is based on reserve_bios_regions().

s/Code is based on
 /This code is based on

> +	/*
> +	 * For 5-level paging, setup current CR3 as the first and
> +	 * the only entry in a new top level page table.

s/setup
 /set up

> +	 *
> +	 * For 4-level paging, trampoline wouldn't touch CR3.
> +	 * KASLR relies on CR3 pointing to _pgtable.
> +	 * See initialize_identity_maps.

Please refer to functions with '...()'

> +	 */
> +	if (l5_required) {
> +		trampoline[TRAMPOLINE_32BIT_PGTABLE_OFF] =
> +			__native_read_cr3() + _PAGE_TABLE_NOENC;

Please don't break lines nonsensically ...

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Matthew Wilcox]

On Thu, Dec 07, 2017 at 07:30:48AM +0100, Ingo Molnar wrote:
> > But if the bootloader put the kernel above 4G (not sure if anybody does
> > this), we would loose control as soon as paging is disabled as code
> > becomes unreachable.
> 
> Yeah, so instead of the double 'as' which is syntactically right but a bit 
> confusing to read, how about something like:
> 
>   But if the bootloader put the kernel above 4G (not sure if anybody does
>   this), we would loose control as soon as paging is disabled, because the
>   code becomes unreachable to the CPU.

btw, it's "lose control".

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Ingo Molnar]

* Matthew Wilcox <willy@infradead.org> wrote:

> On Thu, Dec 07, 2017 at 07:30:48AM +0100, Ingo Molnar wrote:
> > > But if the bootloader put the kernel above 4G (not sure if anybody does
> > > this), we would loose control as soon as paging is disabled as code
> > > becomes unreachable.
> > 
> > Yeah, so instead of the double 'as' which is syntactically right but a bit 
> > confusing to read, how about something like:
> > 
> >   But if the bootloader put the kernel above 4G (not sure if anybody does
> >   this), we would loose control as soon as paging is disabled, because the
> >   code becomes unreachable to the CPU.
> 
> btw, it's "lose control".

Indeed!

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Kirill A. Shutemov]

On Thu, Dec 07, 2017 at 07:30:48AM +0100, Ingo Molnar wrote:
> > We also need a small stack in the trampoline to re-enable long mode via
> > long return. But stack and code can share the page just fine.
> 
> BTW., I'm not sure this is necessarily a good idea: it means writable+executable 
> memory, which we generally try to avoid. How complicated would it be to have them 
> separate?

It's trivial: you only need to bump TRAMPOLINE_32BIT_SIZE.

But it doesn't make much sense. We're running from indentity mapping: all
memory is r/w without NX bit set (and IA32_EFER.NXE is 0).

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 3/4] x86/boot/compressed/64: Introduce place_trampoline()

[Ingo Molnar]

* Kirill A. Shutemov <kirill@shutemov.name> wrote:

> On Thu, Dec 07, 2017 at 07:30:48AM +0100, Ingo Molnar wrote:
> > > We also need a small stack in the trampoline to re-enable long mode via
> > > long return. But stack and code can share the page just fine.
> > 
> > BTW., I'm not sure this is necessarily a good idea: it means writable+executable 
> > memory, which we generally try to avoid. How complicated would it be to have them 
> > separate?
> 
> It's trivial: you only need to bump TRAMPOLINE_32BIT_SIZE.
> 
> But it doesn't make much sense. We're running from indentity mapping: all
> memory is r/w without NX bit set (and IA32_EFER.NXE is 0).

Ok, fair enough!

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCHv4 4/4] x86/boot/compressed/64: Handle 5-level paging boot if kernel is above 4G

[Kirill A. Shutemov]

This patch addresses a shortcoming in current boot process on machines
that supports 5-level paging.

If a bootloader enables 64-bit mode with 4-level paging, we might need to
switch over to 5-level paging. The switching requires disabling paging.
It works fine if kernel itself is loaded below 4G.

But if the bootloader put the kernel above 4G (not sure if anybody does
this), we would loose control as soon as paging is disabled as code
becomes unreachable.

This patch implements a trampoline in lower memory to handle this
situation.

We only need the memory for very short time, until main kernel image
would setup its own page tables.

We go through trampoline even if we don't have to: if we're already in
5-level paging mode or if we don't need to switch to it. This way the
trampoline gets tested on every boot.

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
---
 arch/x86/boot/compressed/head_64.S | 100 ++++++++++++++++++++++++-------------
 1 file changed, 66 insertions(+), 34 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 92d47b1bf10a..b1a6750805aa 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -33,6 +33,7 @@
 #include <asm/processor-flags.h>
 #include <asm/asm-offsets.h>
 #include <asm/bootparam.h>
+#include "pgtable.h"
 
 /*
  * Locally defined symbols should be marked hidden:
@@ -306,11 +307,23 @@ ENTRY(startup_64)
 
 	/*
 	 * At this point we are in long mode with 4-level paging enabled,
-	 * but we want to enable 5-level paging.
+	 * but we might want to enable 5-level paging.
 	 *
-	 * The problem is that we cannot do it directly. Setting LA57 in
-	 * long mode would trigger #GP. So we need to switch off long mode
-	 * first.
+	 * The problem is that we cannot do it directly. Setting CR.LA57
+	 * in the long mode would trigger #GP. So we need to switch off
+	 * long mode and paging first.
+	 *
+	 * We also need a trampoline in lower memory to switch over from
+	 * 4- to 5-level paging for cases when bootloader put kernel above
+	 * 4G, but didn't enable 5-level paging for us.
+	 *
+	 * For the trampoline, we need top page table in lower memory as
+	 * we don't have a way to load 64-bit value into CR3 from 32-bit
+	 * mode.
+	 *
+	 * We go though the trampoline even if we don't have to: if we're
+	 * already in 5-level paging mode or if we don't need to switch to
+	 * it. This way the trampoline code gets tested on every boot.
 	 */
 
 	/*
@@ -329,31 +342,22 @@ ENTRY(startup_64)
 
 	/* Save trampoline address in RCX */
 	movq	%rax, %rcx
-	andq	$~1, %rcx
-
-	testq	$1, %rax
-	jz	lvl5
-
-	/* Clear additional page table */
-	leaq	lvl5_pgtable(%rbx), %rdi
-	xorq	%rax, %rax
-	movq	$(PAGE_SIZE/8), %rcx
-	rep	stosq
 
 	/*
-	 * Setup current CR3 as the first and only entry in a new top level
-	 * page table.
+	 * Load address of trampoline_return into RDI.
+	 * It will be used by trampoline to return to main code.
 	 */
-	movq	%cr3, %rdi
-	leaq	0x7 (%rdi), %rax
-	movq	%rax, lvl5_pgtable(%rbx)
+	leaq	trampoline_return(%rip), %rdi
 
 	/* Switch to compatibility mode (CS.L = 0 CS.D = 1) via far return */
 	pushq	$__KERNEL32_CS
-	leaq	compatible_mode(%rip), %rax
+	andq	$~1, %rax /* Clear bit 0: encodes if 5-level paging needed */
+	leaq	TRAMPOLINE_32BIT_CODE_OFF(%rax), %rax
 	pushq	%rax
 	lretq
-lvl5:
+trampoline_return:
+	/* Restore stack, 32-bit trampoline uses own stack */
+	leaq	boot_stack_end(%rbx), %rsp
 
 	/* Zero EFLAGS */
 	pushq	$0
@@ -491,36 +495,53 @@ relocated:
 	jmp	*%rax
 
 	.code32
+/*
+ * This is 32-bit trampoline that will be copied over to low memory.
+ *
+ * RDI contains return address (might be above 4G).
+ * ECX contains the base address of trampoline memory.
+ * Bit 0 of ECX encodes if 5-level paging is required.
+ */
 ENTRY(trampoline_32bit_src)
-compatible_mode:
 	/* Setup data and stack segments */
 	movl	$__KERNEL_DS, %eax
 	movl	%eax, %ds
 	movl	%eax, %ss
 
+	/* Save base address of trampoline in EDX, clearing bit 0 */
+	movl	%ecx, %edx
+	andl	$~1, %edx
+
+	/* Setup new stack */
+	leal	TRAMPOLINE_32BIT_STACK_END (%edx), %esp
+
 	/* Disable paging */
 	movl	%cr0, %eax
 	btrl	$X86_CR0_PG_BIT, %eax
 	movl	%eax, %cr0
 
-	/* Point CR3 to 5-level paging */
-	leal	lvl5_pgtable(%ebx), %eax
+	/* For 5-level paging, point CR3 to trampoline's new top level page table */
+	testl	$1, %ecx
+	jz	1f
+	leal	TRAMPOLINE_32BIT_PGTABLE_OFF (%edx), %eax
 	movl	%eax, %cr3
+1:
 
-	/* Enable PAE and LA57 mode */
+	/* Enable PAE and LA57 (if required) modes */
 	movl	%cr4, %eax
-	orl	$(X86_CR4_PAE | X86_CR4_LA57), %eax
+	orl	$X86_CR4_PAE, %eax
+	testl	$1, %ecx
+	jz	1f
+	orl	$X86_CR4_LA57, %eax
+1:
 	movl	%eax, %cr4
 
-	/* Calculate address we are running at */
-	call	1f
-1:	popl	%edi
-	subl	$1b, %edi
+	/* Calculate address of paging_enabled once we are in trampoline */
+	leal	paging_enabled - trampoline_32bit_src + TRAMPOLINE_32BIT_CODE_OFF (%edx), %eax
 
 	/* Prepare stack for far return to Long Mode */
 	pushl	$__KERNEL_CS
-	leal	lvl5(%edi), %eax
-	push	%eax
+	pushl	%eax
 
 	/* Enable paging back */
 	movl	$(X86_CR0_PG | X86_CR0_PE), %eax
@@ -528,6 +549,19 @@ compatible_mode:
 
 	lret
 
+	.code64
+paging_enabled:
+	/* Return from the trampoline */
+	jmp	*%rdi
+
+	/*
+	 * Bound size of trampoline code.
+	 * It would fail to compile if code of the trampoline would grow
+	 * beyond TRAMPOLINE_32BIT_CODE_SIZE bytes.
+	 */
+	.org	trampoline_32bit_src + TRAMPOLINE_32BIT_CODE_SIZE
+
+	.code32
 no_longmode:
 	/* This isn't an x86-64 CPU so hang */
 1:
@@ -585,5 +619,3 @@ boot_stack_end:
 	.balign 4096
 pgtable:
 	.fill BOOT_PGT_SIZE, 1, 0
-lvl5_pgtable:
-	.fill PAGE_SIZE, 1, 0
-- 
2.15.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCHv4 4/4] x86/boot/compressed/64: Handle 5-level paging boot if kernel is above 4G

[Ingo Molnar]

* Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:

> This patch addresses a shortcoming in current boot process on machines
> that supports 5-level paging.

s/in current boot process
 /in the current boot process

> If a bootloader enables 64-bit mode with 4-level paging, we might need to
> switch over to 5-level paging. The switching requires disabling paging.
> It works fine if kernel itself is loaded below 4G.

s/The switching requires disabling paging.
 /The switching requires the disabling of paging.

> But if the bootloader put the kernel above 4G (not sure if anybody does
> this), we would loose control as soon as paging is disabled as code
> becomes unreachable.

Same suggestion for this paragraph as for the previous patch.

> This patch implements a trampoline in lower memory to handle this
> situation.
> 
> We only need the memory for very short time, until main kernel image
> would setup its own page tables.

s/We only need the memory for very short time
 /We only need this memory for a very short time

s/until main kernel image would setup its own page tables.
 /until the main kernel image sets up its own page tables.

> We go through trampoline even if we don't have to: if we're already in
> 5-level paging mode or if we don't need to switch to it. This way the
> trampoline gets tested on every boot.

s/We go through trampoline
 /We go through the trampoline

>  	/*
>  	 * At this point we are in long mode with 4-level paging enabled,
> +	 * but we might want to enable 5-level paging.
>  	 *
> +	 * The problem is that we cannot do it directly. Setting CR.LA57
> +	 * in the long mode would trigger #GP. So we need to switch off
> +	 * long mode and paging first.

s/Setting CR.LA57 in the long mode
 /Setting CR.LA57 in long mode

Also, why only say 'CR', why not 'CR4'?

> +	 *
> +	 * We also need a trampoline in lower memory to switch over from
> +	 * 4- to 5-level paging for cases when bootloader put kernel above
> +	 * 4G, but didn't enable 5-level paging for us.

s/for cases when bootloader put kernel above 4G
 /for cases when the bootloader puts the kernel above 4G

> +	 *
> +	 * For the trampoline, we need top page table in lower memory as
> +	 * we don't have a way to load 64-bit value into CR3 from 32-bit
> +	 * mode.

s/we need top page table in lower memory
 /we need the top page table to reside in lower memory

s/load 64-bit value into CR3 from 32-bit mode
 /load 64-bit values into CR3 in 32-bit mode

> +	 *
> +	 * We go though the trampoline even if we don't have to: if we're
> +	 * already in 5-level paging mode or if we don't need to switch to
> +	 * it. This way the trampoline code gets tested on every boot.

>  	/*
> +	 * Load address of trampoline_return into RDI.
> +	 * It will be used by trampoline to return to main code.

s/Load address of trampoline_return
 /Load the address of trampoline_return

s/It will be used by trampoline to return to main code
 /It will be used by the trampoline to return to the main code

> +trampoline_return:
> +	/* Restore stack, 32-bit trampoline uses own stack */
> +	leaq	boot_stack_end(%rbx), %rsp

This phrasing would be a bit clearer:

	/* Restore the stack, the 32-bit trampoline uses its own stack */

> +/*
> + * This is 32-bit trampoline that will be copied over to low memory.

s/This is 32-bit trampoline that will be
 /This is the 32-bit trampoline that will be

> + *
> + * RDI contains return address (might be above 4G).

s/RDI contains return address (might be above 4G)
 /RDI contains the return address (might be above 4G)

> + * ECX contains the base address of trampoline memory.

s/of trampoline memory
 /of the trampoline memory

> +	/* For 5-level paging, point CR3 to trampoline's new top level page table */

s/point CR3 to trampoline's new top level page table
 /point CR3 to the trampoline's new top level page table

> +	testl	$1, %ecx
> +	jz	1f
> +	leal	TRAMPOLINE_32BIT_PGTABLE_OFF (%edx), %eax

BTW., could you please spell out 'OFFSET'? 'OFF' reminds me of the ON/OFF pattern. 
The constant is hopelessly long anyway ;-)

Also:

s/TRAMPOLINE_32BIT_PGTABLE_OFF (%edx)
 /TRAMPOLINE_32BIT_PGTABLE_OFF(%edx)

(This applies to the rest of the patch as well.)

> -	/* Enable PAE and LA57 mode */
> +	/* Enable PAE and LA57 (if required) modes */

A bit more clarity:

s/modes
 /paging modes

> +	/* Calculate address of paging_enabled once we are in trampoline */

Please use the same function name reference as I suggested for the previous patch.

s/once we are in trampoline
 /once we are executing in the trampoline

>  	/* Prepare stack for far return to Long Mode */

s/Prepare stack
 /Prepare the stack

>  	/* Enable paging back */

s/Enable paging back
 /Enable paging again

> +	.code64
> +paging_enabled:
> +	/* Return from the trampoline */
> +	jmp	*%rdi
> +
> +	/*
> +	 * Bound size of trampoline code.
> +	 * It would fail to compile if code of the trampoline would grow
> +	 * beyond TRAMPOLINE_32BIT_CODE_SIZE bytes.

How about:

	 * The trampoline code has a size limit.
	 * Make sure we fail to compile if the trampoline code grows
	 * beyond TRAMPOLINE_32BIT_CODE_SIZE bytes.

?

> +	.code32
>  no_longmode:
>  	/* This isn't an x86-64 CPU so hang */

While at it:

s/This isn't an x86-64 CPU so hang
 /This isn't an x86-64 CPU, so hang intentionally, we cannot continue:

Thanks,

	Ingo

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>