Re: [PATCH v11 12/18] x86/fsgsbase/64: move save_fsgs to header file

From: kbuild test robot
Date: Sat May 09 2020 - 20:42:49 EST

Hi Sasha,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/asm]
[also build test ERROR on tip/auto-latest linus/master tip/x86/core v5.7-rc4 next-20200508]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Sasha-Levin/Enable-FSGSBASE-instructions/20200510-032805
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 2ce0d7f9766f0e49bb54f149c77bae89464932fb
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@xxxxxxxxx>

All error/warnings (new ones prefixed by >>):

In file included from arch/x86/include/uapi/asm/ptrace.h:6:0,
from arch/x86/include/asm/ptrace.h:7,
from arch/x86/include/asm/math_emu.h:5,
from arch/x86/include/asm/processor.h:13,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from arch/x86/kernel/process.c:6:
>> arch/x86/include/uapi/asm/ptrace-abi.h:16:12: error: expected identifier before numeric constant
#define FS 9
^
>> arch/x86/kernel/process.h:42:2: note: in expansion of macro 'FS'
FS,
^~
In file included from arch/x86/kernel/process.c:46:0:
arch/x86/kernel/process.h: In function 'save_base_legacy':
>> arch/x86/kernel/process.h:85:18: error: 'struct thread_struct' has no member named 'fsbase'
prev_p->thread.fsbase = 0;
^
>> arch/x86/kernel/process.h:87:18: error: 'struct thread_struct' has no member named 'gsbase'
prev_p->thread.gsbase = 0;
^
In file included from arch/x86/include/asm/ptrace.h:5:0,
from arch/x86/include/asm/math_emu.h:5,
from arch/x86/include/asm/processor.h:13,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:51,
from include/linux/mmzone.h:8,
from include/linux/gfp.h:6,
from include/linux/mm.h:10,
from arch/x86/kernel/process.c:6:
arch/x86/kernel/process.h: In function 'save_fsgs':
>> arch/x86/kernel/process.h:93:30: error: 'struct thread_struct' has no member named 'fsindex'
savesegment(fs, task->thread.fsindex);
^
arch/x86/include/asm/segment.h:368:32: note: in definition of macro 'savesegment'
asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
^~~~~
>> arch/x86/kernel/process.h:94:30: error: 'struct thread_struct' has no member named 'gsindex'
savesegment(gs, task->thread.gsindex);
^
arch/x86/include/asm/segment.h:368:32: note: in definition of macro 'savesegment'
asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
^~~~~
In file included from arch/x86/kernel/process.c:46:0:
arch/x86/kernel/process.h:101:15: error: 'struct thread_struct' has no member named 'fsbase'
task->thread.fsbase = rdfsbase();
^
>> arch/x86/kernel/process.h:101:25: error: implicit declaration of function 'rdfsbase'; did you mean 'rb_erase'? [-Werror=implicit-function-declaration]
task->thread.fsbase = rdfsbase();
^~~~~~~~
rb_erase
arch/x86/kernel/process.h:102:15: error: 'struct thread_struct' has no member named 'gsbase'
task->thread.gsbase = x86_gsbase_read_cpu_inactive();
^
>> arch/x86/kernel/process.h:102:25: error: implicit declaration of function 'x86_gsbase_read_cpu_inactive' [-Werror=implicit-function-declaration]
task->thread.gsbase = x86_gsbase_read_cpu_inactive();
^~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/x86/kernel/process.h:104:38: error: 'struct thread_struct' has no member named 'fsindex'
save_base_legacy(task, task->thread.fsindex, FS);
^
arch/x86/kernel/process.h:105:38: error: 'struct thread_struct' has no member named 'gsindex'
save_base_legacy(task, task->thread.gsindex, GS);
^
cc1: some warnings being treated as errors

vim +85 arch/x86/kernel/process.h

40
41 enum which_selector {
> 42 FS,
43 GS
44 };
45
46 /*
47 * Saves the FS or GS base for an outgoing thread if FSGSBASE extensions are
48 * not available. The goal is to be reasonably fast on non-FSGSBASE systems.
49 * It's forcibly inlined because it'll generate better code and this function
50 * is hot.
51 */
52 static __always_inline void save_base_legacy(struct task_struct *prev_p,
53 unsigned short selector,
54 enum which_selector which)
55 {
56 if (likely(selector == 0)) {
57 /*
58 * On Intel (without X86_BUG_NULL_SEG), the segment base could
59 * be the pre-existing saved base or it could be zero. On AMD
60 * (with X86_BUG_NULL_SEG), the segment base could be almost
61 * anything.
62 *
63 * This branch is very hot (it's hit twice on almost every
64 * context switch between 64-bit programs), and avoiding
65 * the RDMSR helps a lot, so we just assume that whatever
66 * value is already saved is correct. This matches historical
67 * Linux behavior, so it won't break existing applications.
68 *
69 * To avoid leaking state, on non-X86_BUG_NULL_SEG CPUs, if we
70 * report that the base is zero, it needs to actually be zero:
71 * see the corresponding logic in load_seg_legacy.
72 */
73 } else {
74 /*
75 * If the selector is 1, 2, or 3, then the base is zero on
76 * !X86_BUG_NULL_SEG CPUs and could be anything on
77 * X86_BUG_NULL_SEG CPUs. In the latter case, Linux
78 * has never attempted to preserve the base across context
79 * switches.
80 *
81 * If selector > 3, then it refers to a real segment, and
82 * saving the base isn't necessary.
83 */
84 if (which == FS)
> 85 prev_p->thread.fsbase = 0;
86 else
> 87 prev_p->thread.gsbase = 0;
88 }
89 }
90
91 static __always_inline void save_fsgs(struct task_struct *task)
92 {
> 93 savesegment(fs, task->thread.fsindex);
> 94 savesegment(gs, task->thread.gsindex);
95 if (static_cpu_has(X86_FEATURE_FSGSBASE)) {
96 /*
97 * If FSGSBASE is enabled, we can't make any useful guesses
98 * about the base, and user code expects us to save the current
99 * value. Fortunately, reading the base directly is efficient.
100 */
> 101 task->thread.fsbase = rdfsbase();
> 102 task->thread.gsbase = x86_gsbase_read_cpu_inactive();

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip