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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230607193834.GC3110@yjiang5-mobl.amr.corp.intel.com>
Date:   Wed, 7 Jun 2023 12:38:34 -0700
From:   Yunhong Jiang <yunhong.jiang@...ux.intel.com>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Evgeniy Baskov <baskov@...ras.ru>,
        Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexey Khoroshilov <khoroshilov@...ras.ru>,
        Peter Jones <pjones@...hat.com>,
        Gerd Hoffmann <kraxel@...hat.com>,
        Dave Young <dyoung@...hat.com>,
        Mario Limonciello <mario.limonciello@....com>,
        Kees Cook <keescook@...omium.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Joerg Roedel <jroedel@...e.de>
Subject: Re: [PATCH v5 08/20] x86/decompressor: Use standard calling
 convention for trampoline

On Wed, Jun 07, 2023 at 09:23:30AM +0200, Ard Biesheuvel wrote:
> Update the trampoline code so its arguments are passed via RDI and RSI,
> which matches the ordinary SysV calling convention for x86_64. This will
> allow this code to be called directly from C.
> 
> Acked-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---
>  arch/x86/boot/compressed/head_64.S | 30 +++++++++-----------
>  arch/x86/boot/compressed/pgtable.h |  2 +-
>  2 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index af45ddd8297a4a07..a387cd80964e1a1e 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -443,9 +443,9 @@ SYM_CODE_START(startup_64)
>  	movq	%r15, %rdi		/* pass struct boot_params pointer */
>  	call	paging_prepare
>  
> -	/* Save the trampoline address in RCX */
> -	movq	%rax, %rcx
> -
> +	/* Pass the trampoline address and boolean flag as args #1 and #2 */
> +	movq	%rax, %rdi
> +	movq	%rdx, %rsi
>  	leaq	TRAMPOLINE_32BIT_CODE_OFFSET(%rax), %rax
>  	call	*%rax
>  
> @@ -534,11 +534,11 @@ SYM_FUNC_END(.Lrelocated)
>  /*
>   * This is the 32-bit trampoline that will be copied over to low memory.
>   *
> - * ECX contains the base address of the trampoline memory.
> - * Non zero RDX means trampoline needs to enable 5-level paging.
> + * EDI contains the base address of the trampoline memory.
> + * Non-zero ESI means trampoline needs to enable 5-level paging.
>   */
>  SYM_CODE_START(trampoline_32bit_src)

After the whole patchset, this function now only switch the paging level, is my
understanding correct? After all, it's converted to toggle_la57 directly in the
followed patches. If that's the case, would it makes sense to rename it
correspondingly?

Also, to align with the toggle_la57, would we make the first parameter as just
page table, instead of trampoline memory address?

> -	popq	%rdi
> +	popq	%r8
>  	/* Switch to compatibility mode (CS.L = 0 CS.D = 1) via far return */
>  	pushq	$__KERNEL32_CS
>  	leaq	0f(%rip), %rax
> @@ -552,7 +552,7 @@ SYM_CODE_START(trampoline_32bit_src)
>  	movl	%eax, %ss
>  
>  	/* Set up new stack */
> -	leal	TRAMPOLINE_32BIT_STACK_END(%ecx), %esp
> +	leal	TRAMPOLINE_32BIT_STACK_END(%edi), %esp
>  
>  	/* Disable paging */
>  	movl	%cr0, %eax
> @@ -560,7 +560,7 @@ SYM_CODE_START(trampoline_32bit_src)
>  	movl	%eax, %cr0
>  
>  	/* Check what paging mode we want to be in after the trampoline */
> -	testl	%edx, %edx
> +	testl	%esi, %esi
>  	jz	1f
>  
>  	/* We want 5-level paging: don't touch CR3 if it already points to 5-level page tables */
> @@ -575,21 +575,17 @@ SYM_CODE_START(trampoline_32bit_src)
>  	jz	3f
>  2:
>  	/* Point CR3 to the trampoline's new top level page table */
> -	leal	TRAMPOLINE_32BIT_PGTABLE_OFFSET(%ecx), %eax
> +	leal	TRAMPOLINE_32BIT_PGTABLE_OFFSET(%edi), %eax
>  	movl	%eax, %cr3
>  3:
>  	/* Set EFER.LME=1 as a precaution in case hypervsior pulls the rug */
> -	pushl	%ecx
> -	pushl	%edx
>  	movl	$MSR_EFER, %ecx
>  	rdmsr
>  	btsl	$_EFER_LME, %eax
>  	/* Avoid writing EFER if no change was made (for TDX guest) */
>  	jc	1f
>  	wrmsr
> -1:	popl	%edx
> -	popl	%ecx
> -
> +1:
>  #ifdef CONFIG_X86_MCE
>  	/*
>  	 * Preserve CR4.MCE if the kernel will enable #MC support.
> @@ -606,14 +602,14 @@ SYM_CODE_START(trampoline_32bit_src)
>  
>  	/* Enable PAE and LA57 (if required) paging modes */
>  	orl	$X86_CR4_PAE, %eax
> -	testl	%edx, %edx
> +	testl	%esi, %esi
>  	jz	1f
>  	orl	$X86_CR4_LA57, %eax
>  1:
>  	movl	%eax, %cr4
>  
>  	/* Calculate address of paging_enabled() once we are executing in the trampoline */
> -	leal	.Lpaging_enabled - trampoline_32bit_src + TRAMPOLINE_32BIT_CODE_OFFSET(%ecx), %eax
> +	leal	.Lpaging_enabled - trampoline_32bit_src + TRAMPOLINE_32BIT_CODE_OFFSET(%edi), %eax
>  
>  	/* Prepare the stack for far return to Long Mode */
>  	pushl	$__KERNEL_CS
> @@ -630,7 +626,7 @@ SYM_CODE_END(trampoline_32bit_src)
>  	.code64
>  SYM_FUNC_START_LOCAL_NOALIGN(.Lpaging_enabled)
>  	/* Return from the trampoline */
> -	jmp	*%rdi
> +	jmp	*%r8
>  SYM_FUNC_END(.Lpaging_enabled)
>  
>  	/*
> diff --git a/arch/x86/boot/compressed/pgtable.h b/arch/x86/boot/compressed/pgtable.h
> index 91dbb99203fbce2d..4e8cef135226bcbb 100644
> --- a/arch/x86/boot/compressed/pgtable.h
> +++ b/arch/x86/boot/compressed/pgtable.h
> @@ -14,7 +14,7 @@
>  
>  extern unsigned long *trampoline_32bit;
>  
> -extern void trampoline_32bit_src(void *return_ptr);
> +extern void trampoline_32bit_src(void *trampoline, bool enable_5lvl);
>  
>  #endif /* __ASSEMBLER__ */
>  #endif /* BOOT_COMPRESSED_PAGETABLE_H */
> -- 
> 2.39.2
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ