public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Alexandru Chirvasitu <achirvasub@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	kernel list <linux-kernel@vger.kernel.org>,
	Andy Lutomirski <luto@kernel.org>, Borislav Petkov <bp@alien8.de>,
	Brian Gerst <brgerst@gmail.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>
Subject: Re: PROBLEM: consolidated IDT invalidation causes kexec to reboot
Date: Sun, 24 Dec 2017 12:28:17 +0100	[thread overview]
Message-ID: <20171224112817.4itmll6ru5i45t7n@gmail.com> (raw)
In-Reply-To: <20171224072832.GA959@chirva-void>


* Alexandru Chirvasitu <achirvasub@gmail.com> wrote:

> Thank you for the swift reply!
> 
> On Sat, Dec 23, 2017 at 07:30:21PM -0800, Linus Torvalds wrote:
> > On Sat, Dec 23, 2017 at 5:44 PM, Alexandru Chirvasitu
> > <achirvasub@gmail.com> wrote:
> > >
> > > For testing purposes, I've altered machine_kexec_32.c making the
> > > following toy commit. It naively undoes part of e802a51, solely to
> > > confirm that's where it goes awry in my setup.
> > 
> > That's really funky.
> > 
> > The idt_invalidate() seems to do *exactly* the same thing. It uses
> > "load_idt()" on an IDT with size 0, and the supplied address.
> > 
> > Can you disassemble your "set_idt()" code vs the "idt_invalidate()"?
> >
> 
> I seem to have done some such thing just now, but please excuse some
> poking around in the dark here (I've disassembled code exactly once
> before: yesterday, in answering a similar request for more info in
> another lkml thread..).
> 
> I'm actually not even certain the sequel is what you are asking.
> 
> I couldn't find the set_idt symbol in
> arch/x86/kernel/machine_kexec_32.o. Google seemed to think this has
> something to do with the 'static' marker for that function, so I made
> another commit differing from the previous one only in that it removes
> that marker (i.e. set_idt is now 'void' rather than 'static void').
> 
> I can now see that function with objdump. The relevant sections of
> objdump -D on the two files are:
> 
> --- arch/x86/kernel/machine_kexec_32.o ---
> 
> 00000180 <set_idt>:
>  180:   e8 fc ff ff ff          call   181 <set_idt+0x1>
>  185:   55                      push   %ebp
>  186:   89 e5                   mov    %esp,%ebp
>  188:   83 ec 0c                sub    $0xc,%esp
>  18b:   89 45 f8                mov    %eax,-0x8(%ebp)
>  18e:   66 89 55 f6             mov    %dx,-0xa(%ebp)
>  192:   8d 45 f6                lea    -0xa(%ebp),%eax
>  195:   65 8b 0d 14 00 00 00    mov    %gs:0x14,%ecx
>  19c:   89 4d fc                mov    %ecx,-0x4(%ebp)
>  19f:   31 c9                   xor    %ecx,%ecx
>  1a1:   ff 15 20 00 00 00       call   *0x20
>  1a7:   8b 45 fc                mov    -0x4(%ebp),%eax
>  1aa:   65 33 05 14 00 00 00    xor    %gs:0x14,%eax
>  1b1:   75 02                   jne    1b5 <set_idt+0x35>
>  1b3:   c9                      leave  
>  1b4:   c3                      ret    
>  1b5:   e8 fc ff ff ff          call   1b6 <set_idt+0x36>
>  1ba:   8d b6 00 00 00 00       lea    0x0(%esi),%esi
> 
> ----------------------------------------------
> 
> and
> 
> --- arch/x86/kernel/idt.o  ---
> 
> 00000000 <idt_invalidate>:
>    0:   e8 fc ff ff ff          call   1 <idt_invalidate+0x1>
>    5:   55                      push   %ebp
>    6:   89 e5                   mov    %esp,%ebp
>    8:   83 ec 0c                sub    $0xc,%esp
>    b:   65 8b 15 14 00 00 00    mov    %gs:0x14,%edx
>   12:   89 55 fc                mov    %edx,-0x4(%ebp)
>   15:   31 d2                   xor    %edx,%edx
>   17:   31 d2                   xor    %edx,%edx
>   19:   89 45 f8                mov    %eax,-0x8(%ebp)
>   1c:   8d 45 f6                lea    -0xa(%ebp),%eax
>   1f:   66 89 55 f6             mov    %dx,-0xa(%ebp)
>   23:   ff 15 20 00 00 00       call   *0x20
>   29:   8b 45 fc                mov    -0x4(%ebp),%eax
>   2c:   65 33 05 14 00 00 00    xor    %gs:0x14,%eax
>   33:   75 02                   jne    37 <idt_invalidate+0x37>
>   35:   c9                      leave  
>   36:   c3                      ret    
>   37:   e8 fc ff ff ff          call   38 <idt_invalidate+0x38>
> 
> -------------------------------
> 
> I've also checked again that this newer compilation still gives a
> well-behaved kexec.

So guessing from the disassembly your kernel config seems to have both function 
tracing and paravirt enabled - both can cause complications here, in particular 
paravirt may override load_idt().

( In the end execution should run through the same paravirt ops with and without 
  your patch, so I don't see how it can make a difference but it's easier to look 
  at the disassembly without the paravirt indirection - and your patch obviously 
  makes a difference so we are missing some detail. )

So to simplify analysis, could you disable both please - i.e. disable these if 
your .config has any of these set:

  CONFIG_HYPERVISOR_GUEST=y
  CONFIG_FUNCTION_TRACER=y
  CONFIG_STACK_TRACER=y
  CONFIG_FUNCTION_GRAPH_TRACER=y

The easiest way to disable these is to run this script over your .config, in the 
kernel source code directory where you are building your kernel:

  grep -vE 'CONFIG_HYPERVISOR_GUEST|CONFIG_FUNCTION_TRACER|CONFIG_STACK_TRACER|CONFIG_FUNCTION_GRAPH_TRACER' .config > .config2
  /bin/mv .config2 .config

... then run 'make oldconfig' and answer 'N' to every question. Double check the 
resulting .config via:

  grep -E 'CONFIG_HYPERVISOR_GUEST|CONFIG_FUNCTION_TRACER|CONFIG_STACK_TRACER|CONFIG_FUNCTION_GRAPH_TRACER' .config

which should output:

  # CONFIG_HYPERVISOR_GUEST is not set
  # CONFIG_FUNCTION_TRACER is not set
  # CONFIG_STACK_TRACER is not set

Also, could you send us your .config?

Thanks,

	Ingo

  reply	other threads:[~2017-12-24 11:28 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-24  1:44 PROBLEM: consolidated IDT invalidation causes kexec to reboot Alexandru Chirvasitu
2017-12-24  3:30 ` Linus Torvalds
2017-12-24  7:28   ` Alexandru Chirvasitu
2017-12-24 11:28     ` Ingo Molnar [this message]
2017-12-24 15:27       ` Alexandru Chirvasitu
2017-12-24 16:40         ` Alexandru Chirvasitu
2017-12-25  9:47         ` Ingo Molnar
2017-12-25 14:40   ` Andy Lutomirski
2017-12-25 21:29     ` Alexandru Chirvasitu
2017-12-26 18:51       ` Linus Torvalds
2017-12-26 19:26         ` hpa
2017-12-26 23:19         ` Alexandru Chirvasitu
2017-12-26 23:39           ` hpa
2017-12-26 23:40           ` hpa
2017-12-26 23:45           ` hpa
2017-12-27  2:16           ` Linus Torvalds
2017-12-27  2:25             ` hpa
2017-12-27  2:54               ` Linus Torvalds
2017-12-27  3:00                 ` hpa
2017-12-28  1:30                   ` Konrad Rzeszutek Wilk
2017-12-27  4:41             ` Alexandru Chirvasitu

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=20171224112817.4itmll6ru5i45t7n@gmail.com \
    --to=mingo@kernel.org \
    --cc=achirvasub@gmail.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox