Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 24 Jul 2019 11:14:38 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Matt Mullins <mmullins@fb.com>
Cc:     tglx@linutronix.de, luto@kernel.org,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] x86/entry/32: pass cr2 to do_async_page_fault
Message-ID: <20190724091438.GA31363@hirez.programming.kicks-ass.net>
References: <20190724042058.24506-1-mmullins@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190724042058.24506-1-mmullins@fb.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jul 23, 2019 at 09:20:58PM -0700, Matt Mullins wrote:
> Commit a0d14b8909de ("x86/mm, tracing: Fix CR2 corruption") added the
> address parameter to do_async_page_fault, but does not pass it from the
> 32-bit entry point.  To plumb it through, factor-out
> common_exception_read_cr2 in the same fashion as common_exception, and
> uses it from both page_fault and async_page_fault.
> 
> For a 32-bit KVM guest, this fixes:
> 
> [    1.148669][    T1] Run /sbin/init as init process
> [    1.153328][    T1] Starting init: /sbin/init exists but couldn't execute it (error -14)
> 
> Fixes: a0d14b8909de ("x86/mm, tracing: Fix CR2 corruption")
> Signed-off-by: Matt Mullins <mmullins@fb.com>

Argh. Sorry for overlooking that one.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

There's some potential for sharing between common_exception and your
common_exception_read_cr2, I'll see if I can do that without making a
mess of it.

Also; FB uses 32bit guests?!?

> ---
>  arch/x86/entry/entry_32.S | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index 2bb986f305ac..4f86928246e7 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -1443,8 +1443,12 @@ BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
>  
>  ENTRY(page_fault)
>  	ASM_CLAC
> -	pushl	$0; /* %gs's slot on the stack */
> +	pushl	$do_page_fault
> +	jmp	common_exception_read_cr2
> +END(page_fault)
>  
> +common_exception_read_cr2:
> +	/* the function address is in %gs's slot on the stack */
>  	SAVE_ALL switch_stacks=1 skip_gs=1
>  
>  	ENCODE_FRAME_POINTER
> @@ -1452,6 +1456,7 @@ ENTRY(page_fault)
>  
>  	/* fixup %gs */
>  	GS_TO_REG %ecx
> +	movl	PT_GS(%esp), %edi
>  	REG_TO_PTGS %ecx
>  	SET_KERNEL_GS %ecx
>  
> @@ -1463,9 +1468,9 @@ ENTRY(page_fault)
>  
>  	TRACE_IRQS_OFF
>  	movl	%esp, %eax			# pt_regs pointer
> -	call	do_page_fault
> +	CALL_NOSPEC %edi
>  	jmp	ret_from_exception
> -END(page_fault)
> +END(common_exception_read_cr2)
>  
>  common_exception:
>  	/* the function address is in %gs's slot on the stack */
> @@ -1595,7 +1600,7 @@ END(general_protection)
>  ENTRY(async_page_fault)
>  	ASM_CLAC
>  	pushl	$do_async_page_fault
> -	jmp	common_exception
> +	jmp	common_exception_read_cr2
>  END(async_page_fault)
>  #endif
>  
> -- 
> 2.17.1
>