Messages in this thread | | | From | Andy Lutomirski <> | Date | Fri, 17 Apr 2020 08:52:06 -0700 | Subject | Re: [PATCH v9 00/17] Enable FSGSBASE instructions |
| |
tip-On Fri, Apr 17, 2020 at 6:30 AM Sasha Levin <sashal@kernel.org> wrote: > > On Mon, Apr 13, 2020 at 05:32:05PM -0700, Andi Kleen wrote: > >> Is my attempt at understanding the current situation correct? > > > >Yes. > > > >Nothing breaks, and it's a nice improvement for context switch > >performance, in NMI/PMU performance, and also gives user space two free > >registers to play around with. > > Thomas, Andy, > > Could you list your outstanding objections to this patchset? I know it > might be rehashing stuff you've already mentioned in this thread but I > think that there's a disconnect between folks and it'll help with > restarting everything. >
My outstanding objections are:
1. The previous submission was broken. This should obviously be fixed.
2. The issues documented here need to be addressed:
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=56f2ab41b652251f336a0f471b1033afeaedd161
3. Adding FSGSBASE fundamentally changes the user ABI, and the changes will be observable. This means that something could break, especially in the case where ptrace is in use and the tracee uses the new instructions. The old behavior cannot sanely be preserved with FSGSBASE enabled. This isn't a showstopper, but whoever resubmits this thing needs to document what changes and what use cases might break. I'm hopeful that the only thing that will break is actual human beings using a tool like gdb to manually poke at the registers. This is fine -- the behavior of the registers is different and human beings debugging need to be aware of this. But the existing automated stuff that gdb, lldb, etc do needs to continue working. This especially includes using gdb to force the tracee to call a function, e.g. 'p function()'.
4. The exising ptrace API does not provide a sane way to ask what the base value associated with a selector would be. This means that, under the natural way to make FSGSBASE and ptrace work together (e.g. as implemented in the previous submission), the tracer has no good way to emulate 'MOV [whatever], %gs' in the tracee.
Now maybe no one cares about #4. I certainly have the impression that the *gdb developers* don't care. But gdb isn't exactly a good example of a piece of software that tries to work correctly when dealing with unusual software. Maybe other things like rr will care more. It might be nice to avoid a situation where a piece of careful software (like rr?) can support kernel 5.y, but breaks in 5.y+1 because of FSGSBASE, and only starts working again in 5.y+3 because we added the ptrace API that's needed.
So maybe the first version should have a PTRACE_LOAD_SEGMENT that sticks a selector in FS or GS and changes the base accordingly, even if no current userspace has spoken up and said they need it. And a selftest.
--Andy
| |