On Fri, Aug 31, 2018 at 3:21 PM Andy Lutomirski <l...@kernel.org> wrote: > > #ifdef CONFIG_X86_64 > # define cpu_current_top_of_stack (cpu_tss_rw + TSS_sp1) > +# define rsp_scratch (cpu_tss_rw + TSS_sp2) > #endif
Ugh. The above gets used by *assembler* code. I was really confused by how this: > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -59,8 +59,6 @@ > #include <asm/unistd_32_ia32.h> > #endif > > -__visible DEFINE_PER_CPU(unsigned long, rsp_scratch); > - could continue to work despite the accesses to "rsp_scratch" still remaining in the asm files. Can yu humor me, and just not do something quite that subtle. I must have missed this the first time around. Please get rid of the define, and just make the asm code spell out what it actually does. We already do that for TSS_sp0 for the normal case: movq PER_CPU_VAR(cpu_tss_rw + TSS_sp0), %rsp so I think this should just change - movq %rsp, PER_CPU_VAR(rsp_scratch) + movq %rsp, PER_CPU_VAR(cpu_tss_rw + TSS_sp2) instead of having that subtle rsp_scratch thing. And honestly, I think we should strive to do the same thing with cpu_current_top_of_stack. There at least the #define currently makes sense (because on 32-bit, it's actually a percpu variable, on 64-bit it's that sp1 field). But wouldn't it be nice to just unify 32-bit and 64-bit in this respect, and get rid of that subtle difference? But regardless of whether we eventually do that kind of unification change, the cpu_current_top_of_stack #define has a _reason_ for it as things stand now. The new rsp_scratch thing does not. Just spell out what you're doing. Linus