linux-kernel - Re: [PATCH 3/4] x86/boot/compressed/64: Introduce place

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171110092933.gt4f3ofhjl4fpuqt@gmail.com>
Date:   Fri, 10 Nov 2017 10:29:33 +0100
From:   Ingo Molnar <mingo@...nel.org>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>, x86@...nel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        Andy Lutomirski <luto@...capital.net>,
        Cyrill Gorcunov <gorcunov@...nvz.org>,
        Borislav Petkov <bp@...e.de>, Andi Kleen <ak@...ux.intel.com>,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] x86/boot/compressed/64: Introduce place_trampoline()


* Kirill A. Shutemov <kirill.shutemov@...ux.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.

Thanks,

	Ingo