lkml.org 
[lkml]   [2017]   [Dec]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: PROBLEM: consolidated IDT invalidation causes kexec to reboot
Thanks for that!

On Mon, Dec 25, 2017 at 06:40:14AM -0800, Andy Lutomirski wrote:
> On Sat, Dec 23, 2017 at 7:30 PM, Linus Torvalds
> <torvalds@linux-foundation.org> 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()"?
> >
> >> Is this expected behaviour?
> >
> > No. The code literally seems identical. The only difference is
> >
> > (a) where the 0 limit comes from
> >
> > (b) perhaps build flags and whether it is inlined or not due to being
> > in a different file
> >
> > and neither of those should matter, but maybe they do.
> >
> > Which is why I'd like you to actually look at the generated code and
> > see if you can see any difference..
> >
>
> This is presumably the same call-tracing-without-TLS-working problem.
> idt_invalidate() is out-of-line and is compiled with full tracing on,
> and we're calling it from a context without TLS working (it's
> explicitly disabled in load_segments()) in machine_kexec_32.c. The
> right fix is probably to inline idt_invalidate() and to add a comment.
>

This works.. I went back to the troublesome commit e802a51 and
modified it as follows:

--------------------------------------------------------
make idt_invalidate static inline in header file

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index 33aff45..87ca363 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -504,6 +504,17 @@ static inline void load_current_idt(void)
load_idt((const struct desc_ptr *)&idt_descr);
}

-extern void idt_invalidate(void *addr);
+
+/**
+ * idt_invalidate - Invalidate interrupt descriptor table
+ * @addr: The virtual address of the 'invalid' IDT
+ */
+static inline void idt_invalidate(void *addr)
+{
+ struct desc_ptr idt = { .address = (unsigned long) addr, .size = 0 };
+
+ load_idt(&idt);
+}
+

#endif /* _ASM_X86_DESC_H */
diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index cd4658c..fec44c6 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -25,13 +25,3 @@ const struct desc_ptr debug_idt_descr = {
};
#endif

-/**
- * idt_invalidate - Invalidate interrupt descriptor table
- * @addr: The virtual address of the 'invalid' IDT
- */
-void idt_invalidate(void *addr)
-{
- struct desc_ptr idt = { .address = (unsigned long) addr, .size = 0 };
-
- load_idt(&idt);
-}
--------------------------------------------------------
kexec now works as expected; tested repeatedly, both with direct
execution and crash triggering.

I had to google 'inline function' :)).

> Also, why idt_invalidate(phys_to_virt(0))? That makes rather little
> sense to me.
>
> --Andy

\
 
 \ /
  Last update: 2017-12-25 22:28    [W:0.096 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site