Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 5/8] x86/speculation: Change FILL_RETURN_BUFFER to work
 with objtool
To:     Peter Zijlstra <peterz@infradead.org>, jpoimboe@redhat.com
Cc:     linux-kernel@vger.kernel.org, jthierry@redhat.com,
        tglx@linutronix.de, x86@kernel.org, mbenes@suse.cz
References: <20200423125013.452964352@infradead.org>
 <20200423125042.876058319@infradead.org>
From:   Alexandre Chartre <alexandre.chartre@oracle.com>
Message-ID: <1e0e7e76-622d-0162-2234-767176d5adbc@oracle.com>
Date:   Thu, 23 Apr 2020 17:45:08 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.6.0
MIME-Version: 1.0
In-Reply-To: <20200423125042.876058319@infradead.org>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 4/23/20 2:47 PM, Peter Zijlstra wrote:
> Change FILL_RETURN_BUFFER and vmexit_fill_RSB() so that objtool groks
> them and can generate correct ORC unwind information for them.
> 
>   - Since ORC is alternative invariant; that is, all alternatives
>     should have the same ORC entries, the __FILL_RETURN_BUFFER body
>     can not be part of an alternative.
> 
>     Therefore, move it out of the alternative and keep the alternative
>     as a sort of jump_label around it.
> 
>   - Use the ANNOTATE_INTRA_FUNCTION_CALL annotation to white-list
>     these 'funny' call instructions to nowhere.
> 
>   - Use UNWIND_HINT_EMPTY to 'fill' the speculation traps, otherwise
>     objtool will consider them unreachable.
> 
>   - Move the RSP adjustment into the loop, such that the loop has a
>     deterministic stack layout.
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>   arch/x86/include/asm/nospec-branch.h |   16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)


Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>

alex.

> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -4,11 +4,13 @@
>   #define _ASM_X86_NOSPEC_BRANCH_H_
>   
>   #include <linux/static_key.h>
> +#include <linux/frame.h>
>   
>   #include <asm/alternative.h>
>   #include <asm/alternative-asm.h>
>   #include <asm/cpufeatures.h>
>   #include <asm/msr-index.h>
> +#include <asm/unwind_hints.h>
>   
>   /*
>    * This should be used immediately before a retpoline alternative. It tells
> @@ -46,21 +48,25 @@
>   #define __FILL_RETURN_BUFFER(reg, nr, sp)	\
>   	mov	$(nr/2), reg;			\
>   771:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>   	call	772f;				\
>   773:	/* speculation trap */			\
> +	UNWIND_HINT_EMPTY;			\
>   	pause;					\
>   	lfence;					\
>   	jmp	773b;				\
>   772:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>   	call	774f;				\
>   775:	/* speculation trap */			\
> +	UNWIND_HINT_EMPTY;			\
>   	pause;					\
>   	lfence;					\
>   	jmp	775b;				\
>   774:						\
> +	add	$(BITS_PER_LONG/8) * 2, sp;	\
>   	dec	reg;				\
> -	jnz	771b;				\
> -	add	$(BITS_PER_LONG/8) * nr, sp;
> +	jnz	771b;
>   
>   #ifdef __ASSEMBLY__
>   
> @@ -137,10 +143,8 @@
>     */
>   .macro FILL_RETURN_BUFFER reg:req nr:req ftr:req
>   #ifdef CONFIG_RETPOLINE
> -	ANNOTATE_NOSPEC_ALTERNATIVE
> -	ALTERNATIVE "jmp .Lskip_rsb_\@",				\
> -		__stringify(__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP))	\
> -		\ftr
> +	ALTERNATIVE "jmp .Lskip_rsb_\@", "", \ftr
> +	__FILL_RETURN_BUFFER(\reg,\nr,%_ASM_SP)
>   .Lskip_rsb_\@:
>   #endif
>   .endm
> 
>