lkml.org 
[lkml]   [2017]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/4] x86/boot/compressed/64: Introduce place_trampoline()
On Fri, Nov 10, 2017 at 10:29:33AM +0100, Ingo Molnar wrote:
>
> * Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote:
>
> > --- a/arch/x86/boot/compressed/head_64.S
> > +++ b/arch/x86/boot/compressed/head_64.S
> > @@ -315,6 +315,18 @@ ENTRY(startup_64)
> > * The first step is go into compatibility mode.
> > */
> >
> > + /*
> > + * Find suitable place for trampoline and populate it.
> > + * The address will be stored in RCX.
> > + *
> > + * RSI holds real mode data and need to be preserved across
> > + * a function call.
> > + */
> > + pushq %rsi
> > + call place_trampoline
> > + popq %rsi
> > + movq %rax, %rcx
> > +
> > /* Clear additional page table */
> > leaq lvl5_pgtable(%rbx), %rdi
> > xorq %rax, %rax
>
> So in the final version of this code we now have:
>
> pushq %rsi
> call need_to_enabled_l5
> popq %rsi
>
> /* If need_to_enabled_l5() 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.
> *
> * 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.
> *
> * We use trampoline in lower memory to handle situation when
> * bootloader put the kernel image above 4G.
> *
> * The first step is go into compatibility mode.
> */
>
> /*
> * Find suitable place for trampoline and populate it.
> * The address will be stored in RCX.
> *
> * RSI holds real mode data and need to be preserved across
> * a function call.
> */
> pushq %rsi
> call place_trampoline
> popq %rsi
> movq %rax, %rcx
>
> Firstly, the 'need_to_enabled_l5' name sucks because it includes a typo, but also
> because the prefix is way too generic.
>
> Something like:
>
> l5_paging_required()
>
> would read a lot better - and would also provide a namespace for all L5 paging
> related functions.
>
> Secondly, couldn't this be combined into a single .c function, named accordingly:
>
> l5_paging_prepare()
>
> which would return true if L5 paging is available and should be enabled. In this
> case the trampoline copying function would be called in C, by l5_paging_prepare().
>
> This further reduces the amount of assembly code.

Makes sense.

--
Kirill A. Shutemov

\
 
 \ /
  Last update: 2017-11-10 10:58    [W:1.052 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site