Messages in this thread | | | From | Andy Lutomirski <> | Date | Wed, 18 Apr 2018 09:53:16 -0700 | Subject | Re: [PATCH] x86/entry/64/compat: Preserve r8-r11 in int $0x80 |
| |
On Tue, Apr 17, 2018 at 8:00 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote: > This means that the new behavior is there for some 8 years already. > Whoever was impacted by it, probably already switched to the new ABI. > > Current ABI is "weaker", it allows kernel to save fewer registers. > > Which is generally a good thing, since saving/restoring things cost > cycles, and sometimes painful on entry paths where you may desperately > need a scratch register or two. (Recall this one? - > ... > movq %rsp, PER_CPU_VAR(rsp_scratch) > movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp > /* Construct struct pt_regs on stack */ > pushq $__USER_DS /* pt_regs->ss */ > pushq PER_CPU_VAR(rsp_scratch) /* pt_regs->sp */ > ... > wouldn't it be _great_ if one of GPRs would be available here > to hold userspace %rsp? > )
But this is the int $0x80 entry, which uses the stack sanely and doesn't have this problem at all.
> > If userspace needs some registers saved, it's trivial for it to have: > > push reg1 > push reg2 > int 0x80 > pop reg2 > pop reg1 > > OTOH if userspace _does not_ need some registers saved, > but they are defined as saved by the entrypoint ABI, then save/restore work > is done every time, even when not needed. > > Thus, I propose to retain the current behavior.
The problems are:
1. If you look up how to do int $0x80, every answer you get doesn't mention any clobbers. The code works on x86_32 and seems to work on x86_64. I think we should make it actually work.
2. It's very easy to make this mistake and get away with it for a long time, and the failure modes are hard to debug. gcc doesn't allocate r8..r11 that often, and there are plenty of contexts (near end of a leaf function) where r8..r11 are dead even if they were allocated. So there is probably a decent body of code out there that makes this mistake and is okay for now. But if anyone ever compiles it with LTO, it's reasonably likely to go boom.
So I think we should fix it in the interest of avoiding weird bugs.
| |