Re: [RFC][PATCH 5/7] x86/speculation: Change __FILL_RETURN_BUFFER to work with objtool

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Thu, Apr 16, 2020 at 05:07:57PM +0200, Peter Zijlstra wrote:
> From: Alexandre Chartre <alexandre.chartre@oracle.com>
> 
> Change __FILL_RETURN_BUFFER so that the stack state is deterministically
> defined for each iteration and that objtool can have an accurate view
> of the stack.
> 
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lkml.kernel.org/r/20200414103618.12657-8-alexandre.chartre@oracle.com
> ---
>  arch/x86/include/asm/nospec-branch.h |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -4,6 +4,7 @@
>  #define _ASM_X86_NOSPEC_BRANCH_H_
>  
>  #include <linux/static_key.h>
> +#include <linux/frame.h>
>  
>  #include <asm/alternative.h>
>  #include <asm/alternative-asm.h>
> @@ -46,12 +47,14 @@
>  #define __FILL_RETURN_BUFFER(reg, nr, sp)	\
>  	mov	$(nr/2), reg;			\
>  771:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>  	call	772f;				\
>  773:	/* speculation trap */			\
>  	pause;					\
>  	lfence;					\
>  	jmp	773b;				\
>  772:						\
> +	ANNOTATE_INTRA_FUNCTION_CALL		\
>  	call	774f;				\
>  775:	/* speculation trap */			\
>  	pause;					\
> @@ -59,8 +62,8 @@
>  	jmp	775b;				\
>  774:						\
>  	dec	reg;				\
> -	jnz	771b;				\
> -	add	$(BITS_PER_LONG/8) * nr, sp;
> +	add	$(BITS_PER_LONG/8) * 2, sp;	\
> +	jnz	771b;
>  
>  #ifdef __ASSEMBLY__

Are we still planning to warn about stack changes inside an alternative?
If so then this would still fail...

In this case I think it should be safe, but I'm not sure how we can
ensure that will always be the case for other alternatives.

And do the ORC entries actually work for this?  As far as I can tell,
they would be associated with the .altinstructions section and not
.text, so it wouldn't work.

Also, does objtool not warn about the unreachable speculation traps?
I'm guessing it doesn't notice unreachable alternatives... which I guess
is probably fine.

-- 
Josh