* Chang S. Bae <chang.seok....@intel.com> wrote:
> Resending the patchset that was posted before [6]. > > Given feedbacks from [1], it was suggested to separate two parts > and to (re-)submit this patchset first. > > To facilitate FSGSBASE, Andy's FS/GS base read fix is first > ordered, then some helper functions and refactoring work > are included. Cleanup for the vDSO initialization is > for preparing per-CPU base finder that will be used in the > paranoid entry, when FSGSBASE enabled. > > Changes from V4 [5]: > * Change patch ordering; putting the fix first before introducing > the helper functions > * Cleanup further for vDSO CPU initialization codes > > Changes from V3 [4]: > * Unify CPU number initialization > > Changes from V2 [3]: > * Bisect the CPU number initialization patch > * Drop patches for introducing i386 CPU_NUMBER and switching > write_rdtscp_aux() to use wrmsr_safe() > > Changes from V1 [2]: > * Rename the x86-64 CPU_NUMBER segment from PER_CPU > * Add i386 CPU_NUMBER equivalent to x86-64 at GDT entry 23 > * Add additional helper function to store CPU number > * Switch write_rdtscp_aux() to use wrmsr_safe() > > [1] FSGSBASE patch set V2: > https://lore.kernel.org/patchwork/cover/912063/ > [2] infrastructure for FSGSBASE > V1: https://lore.kernel.org/patchwork/cover/913139/ > [3] V2: https://lore.kernel.org/patchwork/cover/913644/ > [4] V3: https://lore.kernel.org/patchwork/cover/949775/ > [5] V4: https://lore.kernel.org/patchwork/cover/951712/ > [6] V5: https://lore.kernel.org/patchwork/cover/956526/ > > Andy Lutomirski (1): > x86/arch_prctl/64: Make ptrace read FS/GS base accurately > > Chang S. Bae (7): > x86/fsgsbase/64: Introduce FS/GS base helper functions > x86/fsgsbase/64: Make ptrace use FS/GS base helpers > x86/fsgsbase/64: Use FS/GS base helpers in core dump > x86/fsgsbase/64: Factor out load FS/GS segments from __switch_to > x86/segments/64: Rename PER_CPU segment to CPU_NUMBER > x86/vdso: Introduce helper functions for CPU and node number > x86/vdso: Move out the CPU initialization > > arch/x86/entry/vdso/vgetcpu.c | 9 +- > arch/x86/entry/vdso/vma.c | 38 +-------- > arch/x86/include/asm/elf.h | 6 +- > arch/x86/include/asm/fsgsbase.h | 47 +++++++++++ > arch/x86/include/asm/segment.h | 46 +++++++++- > arch/x86/include/asm/vgtod.h | 26 ------ > arch/x86/kernel/cpu/common.c | 24 ++++++ > arch/x86/kernel/process_64.c | 183 > +++++++++++++++++++++++++++++++--------- > arch/x86/kernel/ptrace.c | 28 ++---- > 9 files changed, 270 insertions(+), 137 deletions(-) > create mode 100644 arch/x86/include/asm/fsgsbase.h Looks mostly good, here's a couple of details I noticed before I almost applied the series: - Please use a single, unified name-space for all the new FS/GS helpers, such as: x86_fsbase_read_cpu() x86_fsbase_read_task() x86_fsbase_write_cpu() x86_fsbase_write_cpu_inactive() x86_fsbase_write_task() x86_gsbase_read_cpu() x86_gsbase_read_task() x86_gsbase_write_task() x86_fsgsbase_load() # was: load_fsgs() x86_fsgsbase_read_task() # was: task_seg_base() etc. - Please port patch #7 to latest -tip, it's conflicting with: e78e5a91456f: x86/vdso: Fix lsl operand order The resolution is to preserve the modified code. Also some style nits: - Please fix the inconsistent capitalization of 'CPU' in various places, comments, descriptions, etc. - Comments should start with a capital letter, like sentences do. Right now there's a mixed style of: + /* + * If performance here mattered, we could protect the LDT + * with RCU. This is a slow path, though, so we can just + * take the mutex. + */ + /* + * set the selector to 0 as a notion, that the segment base is + * overwritten, which will be checked for skipping the segment load + * during context switch. + */ - Please refer to functions in changelogs and comments with fn_name(), not just fn_name. For example: x86/fsgsbase/64: Factor out load FS/GS segments from __switch_to But there's more examples. - Please read your own patches once again and fix typos in comments and changelogs, such as: + * correctly wrt barrier() and to keep gcc from cleverly Thanks, Ingo