On Fri, Jun 30, 2017 at 6:11 AM, Josh Poimboeuf <jpoim...@redhat.com> wrote: > On Thu, Jun 29, 2017 at 10:41:44PM -0700, Andy Lutomirski wrote: >> On Thu, Jun 29, 2017 at 10:05 PM, Andy Lutomirski <l...@kernel.org> wrote: >> > Hmm. There's another option that might be considerably nicer, though: >> > put the IRQ stack at a known (at link time) position *in percpu >> > space*. (Presumably it already is -- I haven't checked.) Then we do: >> > >> > .macro ENTER_IRQ_STACK old_rsp >> > DEBUG_ENTRY_ASSERT_IRQS_OFF >> > movq %rsp, \old_rsp >> > incl PER_CPU_VAR(irq_count) >> > >> > /* >> > * Right now, if we just incremented irq_count to zero, we've >> > * claimed the IRQ stack but we haven't switched to it yet. >> > * Anything that can interrupt us here without using IST >> > * must be *extremely* careful to limit its stack usage. >> > */ >> > jnz .Lpush_old_rsp_\@ >> > movq \old_rsp, PER_CPU_VAR(top_word_in_irq_stack) >> > movq PER_CPU_VAR(irq_stack_ptr), %rsp >> > .Lpush_old_rsp_\@: >> > pushq \old_rsp >> > .endm >> > >> >> How about the two commits here (well, soon to be there once gitweb catches >> up): >> >> https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_irq_stack&id=0f56a55bb133cd53ccb78ca51378086296618322 >> >> If you like them, want to add them to your series? > > The second patch looks good to me, thanks. I can pick up the patches. > > A few comments about the first patch: > > > https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/commit/?h=x86/entry_irq_stack&id=3e2aa2102cc1c5e60d4a8637bff78d0478a55059 > > - It uses a '693:' label instead of '.Lirqs_off_\@:'
Touché! > > - There's a comment I don't follow: > > "Anything that can interrupt us here without using IST must be > *extremely* careful to limit its stack usage." > > What specifically could interrupt there without using IST? #DB, later on in the series. I'll update the comment. > > - Since do_softirq_own_stack() is a callable function, I think it still > needs to save rbp. Whoops. > > - Why change the "jmp error_exit" to "ret" in > xen_do_hypervisor_callback()? To match the other change I made there. I removed both.
