[PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[Josh Poimboeuf]

The kbuild test robot reported this objtool warning [1]:

  arch/x86/kvm/emulate.o: warning: objtool: fastop()+0x69: call without frame pointer save/setup

The issue seems to be caused by CONFIG_PROFILE_ALL_BRANCHES.  With that
option, for some reason gcc decides not to create a stack frame in
fastop() before doing the inline asm call, which can result in a bad
stack trace.

Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by
listing the stack pointer as an output operand for the inline asm
statement.

This change has no effect for !CONFIG_PROFILE_ALL_BRANCHES.

[1] https://lists.01.org/pipermail/kbuild-all/2016-March/018249.html

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kvm/emulate.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 0f62943..a2f24af 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -5110,13 +5110,17 @@ static void fetch_possible_mmx_operand(struct x86_emulate_ctxt *ctxt,
 
 static int fastop(struct x86_emulate_ctxt *ctxt, void (*fop)(struct fastop *))
 {
+	register void *__sp asm(_ASM_SP);
 	ulong flags = (ctxt->eflags & EFLAGS_MASK) | X86_EFLAGS_IF;
+
 	if (!(ctxt->d & ByteOp))
 		fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
+
 	asm("push %[flags]; popf; call *%[fastop]; pushf; pop %[flags]\n"
 	    : "+a"(ctxt->dst.val), "+d"(ctxt->src.val), [flags]"+D"(flags),
-	      [fastop]"+S"(fop)
+	      [fastop]"+S"(fop), "+r"(__sp)
 	    : "c"(ctxt->src2.val));
+
 	ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
 	if (!fop) /* exception is returned in fop variable */
 		return emulate_de(ctxt);
-- 
2.4.3

Re: [PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[Josh Poimboeuf]

Hi Ingo,

Ping?

On Wed, Mar 09, 2016 at 12:59:50PM -0600, Josh Poimboeuf wrote:
> The kbuild test robot reported this objtool warning [1]:
> 
>   arch/x86/kvm/emulate.o: warning: objtool: fastop()+0x69: call without frame pointer save/setup
> 
> The issue seems to be caused by CONFIG_PROFILE_ALL_BRANCHES.  With that
> option, for some reason gcc decides not to create a stack frame in
> fastop() before doing the inline asm call, which can result in a bad
> stack trace.
> 
> Force a stack frame to be created if CONFIG_FRAME_POINTER is enabled by
> listing the stack pointer as an output operand for the inline asm
> statement.
> 
> This change has no effect for !CONFIG_PROFILE_ALL_BRANCHES.
> 
> [1] https://lists.01.org/pipermail/kbuild-all/2016-March/018249.html
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  arch/x86/kvm/emulate.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 0f62943..a2f24af 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -5110,13 +5110,17 @@ static void fetch_possible_mmx_operand(struct x86_emulate_ctxt *ctxt,
>  
>  static int fastop(struct x86_emulate_ctxt *ctxt, void (*fop)(struct fastop *))
>  {
> +	register void *__sp asm(_ASM_SP);
>  	ulong flags = (ctxt->eflags & EFLAGS_MASK) | X86_EFLAGS_IF;
> +
>  	if (!(ctxt->d & ByteOp))
>  		fop += __ffs(ctxt->dst.bytes) * FASTOP_SIZE;
> +
>  	asm("push %[flags]; popf; call *%[fastop]; pushf; pop %[flags]\n"
>  	    : "+a"(ctxt->dst.val), "+d"(ctxt->src.val), [flags]"+D"(flags),
> -	      [fastop]"+S"(fop)
> +	      [fastop]"+S"(fop), "+r"(__sp)
>  	    : "c"(ctxt->src2.val));
> +
>  	ctxt->eflags = (ctxt->eflags & ~EFLAGS_MASK) | (flags & EFLAGS_MASK);
>  	if (!fop) /* exception is returned in fop variable */
>  		return emulate_de(ctxt);
> -- 
> 2.4.3
> 

-- 
Josh

Re: [PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[Ingo Molnar]

* Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> Hi Ingo,
> 
> Ping?

I think this should go via the KVM tree.

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[Josh Poimboeuf]

Hi Paolo,

Would you be willing to merge this patch?

On Fri, Apr 15, 2016 at 11:40:40AM +0200, Ingo Molnar wrote:
> 
> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> 
> > Hi Ingo,
> > 
> > Ping?
> 
> I think this should go via the KVM tree.
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>
> 
> Thanks,
> 
> 	Ingo

-- 
Josh

Re: [PATCH] x86/kvm: Add stack frame dependency to fastop() inline asm

[Paolo Bonzini]

On 25/04/2016 18:11, Josh Poimboeuf wrote:
> Hi Paolo,
> 
> Would you be willing to merge this patch?

Yes, will be in 4.7.

Paolo

> On Fri, Apr 15, 2016 at 11:40:40AM +0200, Ingo Molnar wrote:
>>
>> * Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>>
>>> Hi Ingo,
>>>
>>> Ping?
>>
>> I think this should go via the KVM tree.
>>
>> Acked-by: Ingo Molnar <mingo@kernel.org>
>>
>> Thanks,
>>
>> 	Ingo
>