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 <l...@intel.com>

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-...@lists.01.org

Attachment: .config.gz
Description: application/gzip

Reply via email to