On Thu, Nov 23, 2017 at 3:44 PM, Thomas Gleixner <t...@linutronix.de> wrote: > On Tue, 21 Nov 2017, Andy Lutomirski wrote: >> The asm isn't exactly beautiful, > > Delightful euphemism :) > >> but I think that fully refactoring >> it can wait. > >> @@ -560,6 +560,14 @@ END(irq_entries_start) >> .macro interrupt func >> cld >> ALLOC_PT_GPREGS_ON_STACK >> + >> + testb $3, CS(%rsp) >> + jz 1f >> + SWAPGS >> + call switch_to_thread_stack >> + SWAPGS > > I'm surely missing something subtle, but the register saving does really > not care on which GS it is. This swapgs orgy looks odd.
You're mostly right. switch_to_thread_stack uses PER_CPU_VAR(cpu_current_top_of_stack), which definitely cares about which GS it's on, but there's still no legitimate reason for the SWAPGS orgy. I'll fix it. > >> +1: >> + >> SAVE_C_REGS >> SAVE_EXTRA_REGS >> ENCODE_FRAME_POINTER >> @@ -827,6 +835,33 @@ apicinterrupt IRQ_WORK_VECTOR >> irq_work_interrupt smp_irq_work_interrupt >> */ >> #define CPU_TSS_IST(x) PER_CPU_VAR(cpu_tss) + (TSS_ist + ((x) - 1) * 8) >> >> +/* >> + * Switch to the thread stack. This is called with the IRET frame and >> + * orig_ax in pt_regs and the rest of pt_regs allocated, but with all GPRs >> + * in the CPU registers. > > That took several attempts to grok why you left ALLOC_PT_GPRES_ON_STACK in > place in the interrupts macro above. > > In theory it would be sufficient to push %rdi on the entry stack and > operate from there, but it spares only the 'addq %rsp'. Not worth the > trouble of dealing with different register offsets. Hrm. There wasn't actually a good reason for that. I got rid of it.
