Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1620963 > unrolled thread

Re: [PATCH 3/8] x86/boot/64: Add support of additional page table level during early boot

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot Ingo Molnar <mingo@kernel.org> - 2017-04-11 09:10 +0200
    Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-11 13:00 +0200
      Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot Ingo Molnar <mingo@kernel.org> - 2017-04-11 13:30 +0200
        Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2017-04-11 13:50 +0200
          Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot Andi Kleen <ak@linux.intel.com> - 2017-04-11 16:10 +0200
            Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-12 12:20 +0200
              Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot Ingo Molnar <mingo@kernel.org> - 2017-04-17 12:40 +0200
                Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-18 11:10 +0200
                  Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-18 12:20 +0200
                    Re: [PATCH 3/8] x86/boot/64: Add support of additional page table  level during early boot "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-04-18 13:20 +0200

#1620963 — Re: [PATCH 3/8] x86/boot/64: Add support of additional page table level during early boot

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

> This patch adds support for 5-level paging during early boot.
> It generalizes boot for 4- and 5-level paging on 64-bit systems with
> compile-time switch between them.
> 
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  arch/x86/boot/compressed/head_64.S          | 23 ++++++++++++---
>  arch/x86/include/asm/pgtable_64.h           |  2 ++
>  arch/x86/include/uapi/asm/processor-flags.h |  2 ++
>  arch/x86/kernel/head64.c                    | 44 +++++++++++++++++++++++++----
>  arch/x86/kernel/head_64.S                   | 29 +++++++++++++++----
>  5 files changed, 85 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index d2ae1f821e0c..3ed26769810b 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -122,9 +122,12 @@ ENTRY(startup_32)
>  	addl	%ebp, gdt+2(%ebp)
>  	lgdt	gdt(%ebp)
>  
> -	/* Enable PAE mode */
> +	/* Enable PAE and LA57 mode */
>  	movl	%cr4, %eax
>  	orl	$X86_CR4_PAE, %eax
> +#ifdef CONFIG_X86_5LEVEL
> +	orl	$X86_CR4_LA57, %eax
> +#endif
>  	movl	%eax, %cr4
>  
>   /*
> @@ -136,13 +139,24 @@ ENTRY(startup_32)
>  	movl	$(BOOT_INIT_PGT_SIZE/4), %ecx
>  	rep	stosl
>  
> +	xorl	%edx, %edx
> +
> +	/* Build Top Level */
> +	leal	pgtable(%ebx,%edx,1), %edi
> +	leal	0x1007 (%edi), %eax
> +	movl	%eax, 0(%edi)
> +
> +#ifdef CONFIG_X86_5LEVEL
>  	/* Build Level 4 */
> -	leal	pgtable + 0(%ebx), %edi
> +	addl	$0x1000, %edx
> +	leal	pgtable(%ebx,%edx), %edi
>  	leal	0x1007 (%edi), %eax
>  	movl	%eax, 0(%edi)
> +#endif
>  
>  	/* Build Level 3 */
> -	leal	pgtable + 0x1000(%ebx), %edi
> +	addl	$0x1000, %edx
> +	leal	pgtable(%ebx,%edx), %edi
>  	leal	0x1007(%edi), %eax
>  	movl	$4, %ecx
>  1:	movl	%eax, 0x00(%edi)
> @@ -152,7 +166,8 @@ ENTRY(startup_32)
>  	jnz	1b
>  
>  	/* Build Level 2 */
> -	leal	pgtable + 0x2000(%ebx), %edi
> +	addl	$0x1000, %edx
> +	leal	pgtable(%ebx,%edx), %edi
>  	movl	$0x00000183, %eax
>  	movl	$2048, %ecx
>  1:	movl	%eax, 0(%edi)

I realize that you had difficulties converting this to C, but it's not going to 
get any easier in the future either, with one more paging mode/level added!

If you are stuck on where it breaks I'd suggest doing it gradually: first add a 
trivial .c, build and link it in and call it separately. Then once that works, 
move functionality from asm to C step by step and test it at every step.

I've applied the first two patches of this series, but we really should convert 
this assembly bit to C too.

Thanks,

	Ingo

[toc] | [next] | [standalone]

#1621148

On Tue, Apr 11, 2017 at 09:02:03AM +0200, Ingo Molnar wrote:
> I realize that you had difficulties converting this to C, but it's not going to 
> get any easier in the future either, with one more paging mode/level added!
> 
> If you are stuck on where it breaks I'd suggest doing it gradually: first add a 
> trivial .c, build and link it in and call it separately. Then once that works, 
> move functionality from asm to C step by step and test it at every step.

I've described the specific issue with converting this code to C in cover
letter: how to make compiler to generate 32-bit code for a specific
function or translation unit, without breaking linking afterwards (-m32
break it).

I would be glad to convert it, but I'm stuck.

Do you have an idea how to get around the issue.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1621168

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

> On Tue, Apr 11, 2017 at 09:02:03AM +0200, Ingo Molnar wrote:
> > I realize that you had difficulties converting this to C, but it's not going to 
> > get any easier in the future either, with one more paging mode/level added!
> > 
> > If you are stuck on where it breaks I'd suggest doing it gradually: first add a 
> > trivial .c, build and link it in and call it separately. Then once that works, 
> > move functionality from asm to C step by step and test it at every step.
> 
> I've described the specific issue with converting this code to C in cover
> letter: how to make compiler to generate 32-bit code for a specific
> function or translation unit, without breaking linking afterwards (-m32
> break it).

Have you tried putting it into a separate .c file, and building it 32-bit?

I think arch/x86/entry/vdso/Makefile contains an example of how to build 32-bit 
code even on 64-bit kernels.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1621185

On Tue, Apr 11, 2017 at 01:28:45PM +0200, Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> 
> > On Tue, Apr 11, 2017 at 09:02:03AM +0200, Ingo Molnar wrote:
> > > I realize that you had difficulties converting this to C, but it's not going to 
> > > get any easier in the future either, with one more paging mode/level added!
> > > 
> > > If you are stuck on where it breaks I'd suggest doing it gradually: first add a 
> > > trivial .c, build and link it in and call it separately. Then once that works, 
> > > move functionality from asm to C step by step and test it at every step.
> > 
> > I've described the specific issue with converting this code to C in cover
> > letter: how to make compiler to generate 32-bit code for a specific
> > function or translation unit, without breaking linking afterwards (-m32
> > break it).
> 
> Have you tried putting it into a separate .c file, and building it 32-bit?

Yes, I have. The patch below fails linking:

ld: i386 architecture of input file `arch/x86/boot/compressed/head64.o' is incompatible with i386:x86-64 output

> 
> I think arch/x86/entry/vdso/Makefile contains an example of how to build 32-bit 
> code even on 64-bit kernels.

I'll look closer (building proccess it's rather complicated), but my
understanding is that VDSO is stand-alone binary and doesn't really links
with the rest of the kernel, rather included as blob, no?

Andy, may be you have an idea?

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 44163e8c3868..8c1acacf408e 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -76,6 +76,8 @@ 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-y += $(obj)/head64.o
+$(obj)/head64.o: KBUILD_CFLAGS := -m32 -D__KERNEL__ -O2
 endif
 
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
diff --git a/arch/x86/boot/compressed/head64.c b/arch/x86/boot/compressed/head64.c
new file mode 100644
index 000000000000..42e1d64a15f4
--- /dev/null
+++ b/arch/x86/boot/compressed/head64.c
@@ -0,0 +1,3 @@
+void __startup32(void)
+{
+}
-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1621310

> I'll look closer (building proccess it's rather complicated), but my
> understanding is that VDSO is stand-alone binary and doesn't really links
> with the rest of the kernel, rather included as blob, no?
> 
> Andy, may be you have an idea?

There isn't any way I know of to directly link them together. The ELF 
format wasn't designed for that. You would need to merge blobs and then use
manual jump vectors, like the 16bit startup code does. It would be likely
complicated and ugly.

-Andi

[toc] | [prev] | [next] | [standalone]

#1621970

On Tue, Apr 11, 2017 at 07:09:07AM -0700, Andi Kleen wrote:
> > I'll look closer (building proccess it's rather complicated), but my
> > understanding is that VDSO is stand-alone binary and doesn't really links
> > with the rest of the kernel, rather included as blob, no?
> > 
> > Andy, may be you have an idea?
> 
> There isn't any way I know of to directly link them together. The ELF 
> format wasn't designed for that. You would need to merge blobs and then use
> manual jump vectors, like the 16bit startup code does. It would be likely
> complicated and ugly.

Ingo, can we proceed without coverting this assembly to C?

I'm committed to convert it to C later if we'll find reasonable solution
to the issue.

We're pretty late into release cycle. It would be nice to give the whole
thing time in tip/master and -next before the merge window.

Can I repost part 4?

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1624630

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

> On Tue, Apr 11, 2017 at 07:09:07AM -0700, Andi Kleen wrote:
> > > I'll look closer (building proccess it's rather complicated), but my
> > > understanding is that VDSO is stand-alone binary and doesn't really links
> > > with the rest of the kernel, rather included as blob, no?
> > > 
> > > Andy, may be you have an idea?
> > 
> > There isn't any way I know of to directly link them together. The ELF 
> > format wasn't designed for that. You would need to merge blobs and then use
> > manual jump vectors, like the 16bit startup code does. It would be likely
> > complicated and ugly.
> 
> Ingo, can we proceed without coverting this assembly to C?
> 
> I'm committed to convert it to C later if we'll find reasonable solution
> to the issue.

So one way to do it would be to build it standalone as a .o, then add it not to 
the regular kernel objects link target (as you found out it's not possible to link 
32-bit and 64-bit objects), but to link it in a manual fashion, as part of 
vmlinux.bin.all-y in arch/x86/boot/compressed/Makefile.

But there would be other complications with this approach, such as we'd have to 
add a size field and there might be symbol linking problems ...

Another, pretty hacky way would be to generate a .S from the .c, then post-process 
the .S and essentially generate today's 32-bit .S from it.

Probably not worth the trouble.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1625161

On Mon, Apr 17, 2017 at 12:32:25PM +0200, Ingo Molnar wrote:
> 
> * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> 
> > On Tue, Apr 11, 2017 at 07:09:07AM -0700, Andi Kleen wrote:
> > > > I'll look closer (building proccess it's rather complicated), but my
> > > > understanding is that VDSO is stand-alone binary and doesn't really links
> > > > with the rest of the kernel, rather included as blob, no?
> > > > 
> > > > Andy, may be you have an idea?
> > > 
> > > There isn't any way I know of to directly link them together. The ELF 
> > > format wasn't designed for that. You would need to merge blobs and then use
> > > manual jump vectors, like the 16bit startup code does. It would be likely
> > > complicated and ugly.
> > 
> > Ingo, can we proceed without coverting this assembly to C?
> > 
> > I'm committed to convert it to C later if we'll find reasonable solution
> > to the issue.
> 
> So one way to do it would be to build it standalone as a .o, then add it not to 
> the regular kernel objects link target (as you found out it's not possible to link 
> 32-bit and 64-bit objects), but to link it in a manual fashion, as part of 
> vmlinux.bin.all-y in arch/x86/boot/compressed/Makefile.
> 
> But there would be other complications with this approach, such as we'd have to 
> add a size field and there might be symbol linking problems ...
> 
> Another, pretty hacky way would be to generate a .S from the .c, then post-process 
> the .S and essentially generate today's 32-bit .S from it.
> 
> Probably not worth the trouble.

So, do I need to do anything else to get part 4 applied?

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1625216

On Tue, Apr 18, 2017 at 11:59:26AM +0300, Kirill A. Shutemov wrote:
> On Mon, Apr 17, 2017 at 12:32:25PM +0200, Ingo Molnar wrote:
> > 
> > * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > 
> > > On Tue, Apr 11, 2017 at 07:09:07AM -0700, Andi Kleen wrote:
> > > > > I'll look closer (building proccess it's rather complicated), but my
> > > > > understanding is that VDSO is stand-alone binary and doesn't really links
> > > > > with the rest of the kernel, rather included as blob, no?
> > > > > 
> > > > > Andy, may be you have an idea?
> > > > 
> > > > There isn't any way I know of to directly link them together. The ELF 
> > > > format wasn't designed for that. You would need to merge blobs and then use
> > > > manual jump vectors, like the 16bit startup code does. It would be likely
> > > > complicated and ugly.
> > > 
> > > Ingo, can we proceed without coverting this assembly to C?
> > > 
> > > I'm committed to convert it to C later if we'll find reasonable solution
> > > to the issue.
> > 
> > So one way to do it would be to build it standalone as a .o, then add it not to 
> > the regular kernel objects link target (as you found out it's not possible to link 
> > 32-bit and 64-bit objects), but to link it in a manual fashion, as part of 
> > vmlinux.bin.all-y in arch/x86/boot/compressed/Makefile.
> > 
> > But there would be other complications with this approach, such as we'd have to 
> > add a size field and there might be symbol linking problems ...
> > 
> > Another, pretty hacky way would be to generate a .S from the .c, then post-process 
> > the .S and essentially generate today's 32-bit .S from it.
> > 
> > Probably not worth the trouble.
> 
> So, do I need to do anything else to get part 4 applied?

Doh!

I've just realized we don't really need to enable 5-level paging in
decompression code. Leaving 4-level paging there works perfectly fine.

I'll drop changes to arch/x86/boot/compressed/head_64.S and resubmit the
patchset.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1625260

On Tue, Apr 18, 2017 at 01:15:34PM +0300, Kirill A. Shutemov wrote:
> On Tue, Apr 18, 2017 at 11:59:26AM +0300, Kirill A. Shutemov wrote:
> > On Mon, Apr 17, 2017 at 12:32:25PM +0200, Ingo Molnar wrote:
> > > 
> > > * Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > > 
> > > > On Tue, Apr 11, 2017 at 07:09:07AM -0700, Andi Kleen wrote:
> > > > > > I'll look closer (building proccess it's rather complicated), but my
> > > > > > understanding is that VDSO is stand-alone binary and doesn't really links
> > > > > > with the rest of the kernel, rather included as blob, no?
> > > > > > 
> > > > > > Andy, may be you have an idea?
> > > > > 
> > > > > There isn't any way I know of to directly link them together. The ELF 
> > > > > format wasn't designed for that. You would need to merge blobs and then use
> > > > > manual jump vectors, like the 16bit startup code does. It would be likely
> > > > > complicated and ugly.
> > > > 
> > > > Ingo, can we proceed without coverting this assembly to C?
> > > > 
> > > > I'm committed to convert it to C later if we'll find reasonable solution
> > > > to the issue.
> > > 
> > > So one way to do it would be to build it standalone as a .o, then add it not to 
> > > the regular kernel objects link target (as you found out it's not possible to link 
> > > 32-bit and 64-bit objects), but to link it in a manual fashion, as part of 
> > > vmlinux.bin.all-y in arch/x86/boot/compressed/Makefile.
> > > 
> > > But there would be other complications with this approach, such as we'd have to 
> > > add a size field and there might be symbol linking problems ...
> > > 
> > > Another, pretty hacky way would be to generate a .S from the .c, then post-process 
> > > the .S and essentially generate today's 32-bit .S from it.
> > > 
> > > Probably not worth the trouble.
> > 
> > So, do I need to do anything else to get part 4 applied?
> 
> Doh!
> 
> I've just realized we don't really need to enable 5-level paging in
> decompression code. Leaving 4-level paging there works perfectly fine.
> 
> I'll drop changes to arch/x86/boot/compressed/head_64.S and resubmit the
> patchset.

No. This breaks KASLR. Decompression code has to use 5-level paging to
keep KASLR working.

So, v4 of part 4 is up-to-date.

Sorry for noise.

-- 
 Kirill A. Shutemov

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web