Messages in this thread Patch in this message | | | Date | Mon, 25 Dec 2017 16:29:34 -0500 | From | Alexandru Chirvasitu <> | Subject | Re: 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
| |