LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: kernel test robot <xiaolong.ye@intel.com>,
	Ingo Molnar <mingo@kernel.org>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Matthias Kaehlcke <mka@chromium.org>,
	Alexander Potapenko <glider@google.com>,
	Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Dmitriy Vyukov <dvyukov@google.com>,
	Miguel Bernal Marin <miguel.bernal.marin@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>, LKP <lkp@01.org>
Subject: Re: [lkp-robot] [x86/asm] f5caf621ee: PANIC:double_fault
Date: Thu, 28 Sep 2017 14:10:32 -0500
Message-ID: <20170928191032.5fhnyrark5ebov4c@treble> (raw)
In-Reply-To: <20170928170121.qpyyfjijuwdkfx7g@treble>

On Thu, Sep 28, 2017 at 12:01:21PM -0500, Josh Poimboeuf wrote:
> On Thu, Sep 28, 2017 at 11:44:22AM -0500, Josh Poimboeuf wrote:
> > Agreed, changing it to "unsigned long" and "rsp" will probably fix it.
> > 
> > I had made it "unsigned int" because of a clang issue with "unsigned
> > long":
> > 
> >     CC      arch/x86/entry/vdso/vdso32/vclock_gettime.o
> >   In file included from arch/x86/entry/vdso/vdso32/vclock_gettime.c:32:
> >   In file included from arch/x86/entry/vdso/vdso32/../vclock_gettime.c:15:
> >   In file included from ./arch/x86/include/asm/vgtod.h:5:
> >   In file included from ./include/linux/clocksource.h:12:
> >   In file included from ./include/linux/timex.h:56:
> >   In file included from ./include/uapi/linux/timex.h:56:
> >   In file included from ./include/linux/time.h:5:
> >   In file included from ./include/linux/seqlock.h:35:
> >   In file included from ./include/linux/spinlock.h:50:
> >   In file included from ./include/linux/preempt.h:10:
> >   In file included from ./include/linux/list.h:8:
> >   In file included from ./include/linux/kernel.h:10:
> >   In file included from ./include/linux/bitops.h:37:
> >   In file included from ./arch/x86/include/asm/bitops.h:16:
> >   In file included from ./arch/x86/include/asm/alternative.h:9:
> >   ./arch/x86/include/asm/asm.h:142:42: error: register 'rsp' unsuitable for global register variables on this target
> >   register unsigned long __asm_call_sp asm("rsp");
> > 
> > And I think we saw the same error in the realmode code.
> > 
> > So we may need to tweak the macro a bit.
> 
> Going to try the following patch.
> 
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index c1eadbaf1115..30c3c9ac784a 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -11,10 +11,12 @@
>  # define __ASM_FORM_COMMA(x) " " #x ","
>  #endif
>  
> -#ifdef CONFIG_X86_32
> +#ifndef __x86_64__
> +/* 32 bit */
>  # define __ASM_SEL(a,b) __ASM_FORM(a)
>  # define __ASM_SEL_RAW(a,b) __ASM_FORM_RAW(a)
>  #else
> +/* 64 bit */
>  # define __ASM_SEL(a,b) __ASM_FORM(b)
>  # define __ASM_SEL_RAW(a,b) __ASM_FORM_RAW(b)
>  #endif
> @@ -139,7 +141,7 @@
>   * gets set up by the containing function.  If you forget to do this, objtool
>   * may print a "call without frame pointer save/setup" warning.
>   */
> -register unsigned int __asm_call_sp asm("esp");
> +register unsigned long __asm_call_sp asm(_ASM_SP);
>  #define ASM_CALL_CONSTRAINT "+r" (__asm_call_sp)
>  #endif

Confirmed that this patch works on both compilers, and fixes GCC 4.4.

GCC 4.4 before:

  ffffffff8147461d:       89 e0                   mov    %esp,%eax
  ffffffff8147461f:       4c 89 f7                mov    %r14,%rdi
  ffffffff81474622:       4c 89 fe                mov    %r15,%rsi
  ffffffff81474625:       ba 20 00 00 00          mov    $0x20,%edx
  ffffffff8147462a:       89 c4                   mov    %eax,%esp
  ffffffff8147462c:       e8 bf 52 05 00          callq  ffffffff814c98f0 <copy_user_generic_unrolled>

after:

  ffffffff8147461e:       48 89 e0                mov    %rsp,%rax
  ffffffff81474621:       4c 89 f7                mov    %r14,%rdi
  ffffffff81474624:       4c 89 fe                mov    %r15,%rsi
  ffffffff81474627:       ba 20 00 00 00          mov    $0x20,%edx
  ffffffff8147462c:       48 89 c4                mov    %rax,%rsp
  ffffffff8147462f:       e8 cc 52 05 00          callq  ffffffff814c9900 <copy_user_generic_unrolled>

It still has the "back up and restore the stack pointer just for the fun
of it" thing, but at least the corruption is gone.

Will finalize the patch and send it along to tip.

-- 
Josh

  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-28  7:47 kernel test robot
2017-09-28  7:59 ` Ingo Molnar
2017-09-28  8:18   ` Peter Zijlstra
2017-09-28  8:49     ` Ingo Molnar
2017-09-28 11:49       ` Peter Zijlstra
2017-09-28 16:21 ` Linus Torvalds
2017-09-28 16:44   ` Josh Poimboeuf
2017-09-28 17:01     ` Josh Poimboeuf
2017-09-28 19:10       ` Josh Poimboeuf [this message]
2017-09-28 21:58         ` [PATCH] x86/asm: Fix inline asm call constraints for GCC 4.4 Josh Poimboeuf
2017-09-28 23:53           ` Linus Torvalds
2017-09-29  1:40             ` Josh Poimboeuf
2017-09-29  8:01               ` Ingo Molnar
2017-09-29 10:32                 ` Ye Xiaolong
2017-09-29  7:51             ` Ingo Molnar
2017-09-29 15:29               ` Arnd Bergmann
2017-09-29  9:27           ` [tip:x86/urgent] " tip-bot for Josh Poimboeuf
2017-09-29 11:18           ` tip-bot for Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170928191032.5fhnyrark5ebov4c@treble \
    --to=jpoimboe@redhat.com \
    --cc=arnd@arndb.de \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@01.org \
    --cc=luto@kernel.org \
    --cc=miguel.bernal.marin@linux.intel.com \
    --cc=mingo@kernel.org \
    --cc=mka@chromium.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git