Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[kernel test robot]

Hi Christophe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.16-rc3 next-20211129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc cross compiling tool for clang build
        # apt-get install binutils-powerpc-linux-gnu
        # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
        git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/asm-offsets.c:71:
   In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
                   *inst = ppc_inst(val);
                                    ^~~
   arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
   #define ppc_inst(x) (x)
                        ^
   arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
           unsigned int val, suffix;
                           ^
                            = 0
   1 warning generated.
   <stdin>:1559:2: warning: syscall futex_waitv not implemented [-W#warnings]
   #warning syscall futex_waitv not implemented
    ^
   1 warning generated.
   arch/powerpc/kernel/vdso32/gettimeofday.S:72:8: error: unsupported directive '.stabs'
   .stabs "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x; _restgpr_31_x:
          ^
   arch/powerpc/kernel/vdso32/gettimeofday.S:73:8: error: unsupported directive '.stabs'
   .stabs "_rest32gpr_31_x:F-1",36,0,0,_rest32gpr_31_x; .globl _rest32gpr_31_x; _rest32gpr_31_x:
          ^
   make[2]: *** [arch/powerpc/kernel/vdso32/Makefile:55: arch/powerpc/kernel/vdso32/gettimeofday.o] Error 1
   make[2]: Target 'include/generated/vdso32-offsets.h' not remade because of errors.
   make[1]: *** [arch/powerpc/Makefile:421: vdso_prepare] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:219: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/val +165 arch/powerpc/include/asm/inst.h

   152	
   153	static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
   154	{
   155		unsigned int val, suffix;
   156	
   157		if (unlikely(!is_kernel_addr((unsigned long)src)))
   158			return -ERANGE;
   159	
   160		__get_kernel_nofault(&val, src, u32, Efault);
   161		if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
   162			__get_kernel_nofault(&suffix, src + 1, u32, Efault);
   163			*inst = ppc_inst_prefix(val, suffix);
   164		} else {
 > 165			*inst = ppc_inst(val);
   166		}
   167		return 0;
   168	Efault:
   169		return -EFAULT;
   170	}
   171	

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

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Christophe Leroy]

Le 29/11/2021 à 23:55, kernel test robot a écrit :
> Hi Christophe,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on powerpc/next]
> [also build test WARNING on v5.16-rc3 next-20211129]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # install powerpc cross compiling tool for clang build
>          # apt-get install binutils-powerpc-linux-gnu
>          # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
>          git remote add linux-review https://github.com/0day-ci/linux
>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
>          git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
>          # save the config file to linux build tree
>          mkdir build_dir
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>                     *inst = ppc_inst(val);
>                                      ^~~
>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>     #define ppc_inst(x) (x)
>                          ^
>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>             unsigned int val, suffix;
>                             ^
>                              = 0

I can't understand what's wrong here.

We have

	__get_kernel_nofault(&val, src, u32, Efault);
	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
		*inst = ppc_inst_prefix(val, suffix);
	} else {
		*inst = ppc_inst(val);
	}

With

#define __get_kernel_nofault(dst, src, type, err_label)			\
	__get_user_size_goto(*((type *)(dst)),				\
		(__force type __user *)(src), sizeof(type), err_label)


And

#define __get_user_size_goto(x, ptr, size, label)				\
do {										\
	BUILD_BUG_ON(size > sizeof(x));						\
	switch (size) {								\
	case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break;	\
	case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break;	\
	case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break;	\
	case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label);  break;	\
	default: x = 0; BUILD_BUG();						\
	}									\
} while (0)

And

#define __get_user_asm_goto(x, addr, label, op)			\
	asm_volatile_goto(					\
		"1:	"op"%U1%X1 %0, %1	# get_user\n"	\
		EX_TABLE(1b, %l2)				\
		: "=r" (x)					\
		: "m<>" (*addr)				\
		:						\
		: label)


I see no possibility, no alternative path where val wouldn't be set. The 
asm clearly has *addr as an output param so it is always set.

>     1 warning generated.
>     <stdin>:1559:2: warning: syscall futex_waitv not implemented [-W#warnings]
>     #warning syscall futex_waitv not implemented
>      ^
>     1 warning generated.
>     arch/powerpc/kernel/vdso32/gettimeofday.S:72:8: error: unsupported directive '.stabs'
>     .stabs "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x; _restgpr_31_x:
>            ^
>     arch/powerpc/kernel/vdso32/gettimeofday.S:73:8: error: unsupported directive '.stabs'
>     .stabs "_rest32gpr_31_x:F-1",36,0,0,_rest32gpr_31_x; .globl _rest32gpr_31_x; _rest32gpr_31_x:

How should we fix that ? Will clang support .stabs in the future ?


>            ^
>     make[2]: *** [arch/powerpc/kernel/vdso32/Makefile:55: arch/powerpc/kernel/vdso32/gettimeofday.o] Error 1
>     make[2]: Target 'include/generated/vdso32-offsets.h' not remade because of errors.
>     make[1]: *** [arch/powerpc/Makefile:421: vdso_prepare] Error 2
>     make[1]: Target 'prepare' not remade because of errors.
>     make: *** [Makefile:219: __sub-make] Error 2
>     make: Target 'prepare' not remade because of errors.
> 
> 
> vim +/val +165 arch/powerpc/include/asm/inst.h
> 
>     152	
>     153	static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
>     154	{
>     155		unsigned int val, suffix;
>     156	
>     157		if (unlikely(!is_kernel_addr((unsigned long)src)))
>     158			return -ERANGE;
>     159	
>     160		__get_kernel_nofault(&val, src, u32, Efault);
>     161		if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
>     162			__get_kernel_nofault(&suffix, src + 1, u32, Efault);
>     163			*inst = ppc_inst_prefix(val, suffix);
>     164		} else {
>   > 165			*inst = ppc_inst(val);
>     166		}
>     167		return 0;
>     168	Efault:
>     169		return -EFAULT;
>     170	}
>     171	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Michael Ellerman]

Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 29/11/2021 à 23:55, kernel test robot a écrit :
>> Hi Christophe,
>> 
>> I love your patch! Perhaps something to improve:
>> 
>> [auto build test WARNING on powerpc/next]
>> [also build test WARNING on v5.16-rc3 next-20211129]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>> 
>> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
>> config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
>> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # install powerpc cross compiling tool for clang build
>>          # apt-get install binutils-powerpc-linux-gnu
>>          # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
>>          git remote add linux-review https://github.com/0day-ci/linux
>>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
>>          git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
>>          # save the config file to linux build tree
>>          mkdir build_dir
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
>> 
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>> 
>> All warnings (new ones prefixed by >>):
>> 
>>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
>>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>>                     *inst = ppc_inst(val);
>>                                      ^~~
>>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>>     #define ppc_inst(x) (x)
>>                          ^
>>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>>             unsigned int val, suffix;
>>                             ^
>>                              = 0
>
> I can't understand what's wrong here.
>
> We have
>
> 	__get_kernel_nofault(&val, src, u32, Efault);
> 	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
> 		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
> 		*inst = ppc_inst_prefix(val, suffix);
> 	} else {
> 		*inst = ppc_inst(val);
> 	}
>
> With
>
> #define __get_kernel_nofault(dst, src, type, err_label)			\
> 	__get_user_size_goto(*((type *)(dst)),				\
> 		(__force type __user *)(src), sizeof(type), err_label)
>
>
> And
>
> #define __get_user_size_goto(x, ptr, size, label)				\
> do {										\
> 	BUILD_BUG_ON(size > sizeof(x));						\
> 	switch (size) {								\
> 	case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break;	\
> 	case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break;	\
> 	case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break;	\
> 	case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label);  break;	\
> 	default: x = 0; BUILD_BUG();						\
> 	}									\
> } while (0)
>
> And
>
> #define __get_user_asm_goto(x, addr, label, op)			\
> 	asm_volatile_goto(					\
> 		"1:	"op"%U1%X1 %0, %1	# get_user\n"	\
> 		EX_TABLE(1b, %l2)				\
> 		: "=r" (x)					\
> 		: "m<>" (*addr)				\
> 		:						\
> 		: label)
>
>
> I see no possibility, no alternative path where val wouldn't be set. The 
> asm clearly has *addr as an output param so it is always set.

I guess clang can't convince itself of that?

>>     1 warning generated.
>>     <stdin>:1559:2: warning: syscall futex_waitv not implemented [-W#warnings]
>>     #warning syscall futex_waitv not implemented
>>      ^
>>     1 warning generated.
>>     arch/powerpc/kernel/vdso32/gettimeofday.S:72:8: error: unsupported directive '.stabs'
>>     .stabs "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x; _restgpr_31_x:
>>            ^
>>     arch/powerpc/kernel/vdso32/gettimeofday.S:73:8: error: unsupported directive '.stabs'
>>     .stabs "_rest32gpr_31_x:F-1",36,0,0,_rest32gpr_31_x; .globl _rest32gpr_31_x; _rest32gpr_31_x:
>
> How should we fix that ? Will clang support .stabs in the future ?

I think we should drop any stabs annotations / support. AFAICT none of
the toolchains we support produce it anymore.

cheers

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Nathan Chancellor]

On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > Le 29/11/2021 à 23:55, kernel test robot a écrit :
> >> Hi Christophe,
> >> 
> >> I love your patch! Perhaps something to improve:
> >> 
> >> [auto build test WARNING on powerpc/next]
> >> [also build test WARNING on v5.16-rc3 next-20211129]
> >> [If your patch is applied to the wrong git tree, kindly drop us a note.
> >> And when submitting patch, we suggest to use '--base' as documented in
> >> https://git-scm.com/docs/git-format-patch]
> >> 
> >> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> >> config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
> >> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
> >> reproduce (this is a W=1 build):
> >>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >>          chmod +x ~/bin/make.cross
> >>          # install powerpc cross compiling tool for clang build
> >>          # apt-get install binutils-powerpc-linux-gnu
> >>          # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> >>          git remote add linux-review https://github.com/0day-ci/linux
> >>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> >>          git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> >>          # save the config file to linux build tree
> >>          mkdir build_dir
> >>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
> >> 
> >> If you fix the issue, kindly add following tag as appropriate
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> 
> >> All warnings (new ones prefixed by >>):
> >> 
> >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
> >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
> >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> >>                     *inst = ppc_inst(val);
> >>                                      ^~~
> >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
> >>     #define ppc_inst(x) (x)
> >>                          ^
> >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
> >>             unsigned int val, suffix;
> >>                             ^
> >>                              = 0
> >
> > I can't understand what's wrong here.
> >
> > We have
> >
> > 	__get_kernel_nofault(&val, src, u32, Efault);
> > 	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
> > 		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
> > 		*inst = ppc_inst_prefix(val, suffix);
> > 	} else {
> > 		*inst = ppc_inst(val);
> > 	}
> >
> > With
> >
> > #define __get_kernel_nofault(dst, src, type, err_label)			\
> > 	__get_user_size_goto(*((type *)(dst)),				\
> > 		(__force type __user *)(src), sizeof(type), err_label)
> >
> >
> > And
> >
> > #define __get_user_size_goto(x, ptr, size, label)				\
> > do {										\
> > 	BUILD_BUG_ON(size > sizeof(x));						\
> > 	switch (size) {								\
> > 	case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break;	\
> > 	case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break;	\
> > 	case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break;	\
> > 	case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label);  break;	\
> > 	default: x = 0; BUILD_BUG();						\
> > 	}									\
> > } while (0)
> >
> > And
> >
> > #define __get_user_asm_goto(x, addr, label, op)			\
> > 	asm_volatile_goto(					\
> > 		"1:	"op"%U1%X1 %0, %1	# get_user\n"	\
> > 		EX_TABLE(1b, %l2)				\
> > 		: "=r" (x)					\
> > 		: "m<>" (*addr)				\
> > 		:						\
> > 		: label)
> >
> >
> > I see no possibility, no alternative path where val wouldn't be set. The 
> > asm clearly has *addr as an output param so it is always set.
> 
> I guess clang can't convince itself of that?

A simplified reproducer:

$ cat test.c
static inline int copy_inst_from_kernel_nofault(unsigned int *inst,
                                                unsigned int *src)
{
        unsigned int val;

        asm goto("1: lwz %U1%X1 %0, %1   # get_user\n"
                 ".section __ex_table,\"a\";"
                 ".balign 4;"
                 ".long (1b) - . ;"
                 ".long (%l2) - . ;"
                 ".previous"
                 : "=r" (*(unsigned int *)(&val))
                 : "m<>" (*(unsigned int *)(src))
                 :
                 : Efault);

        *inst = val;
        return 0;

Efault:
        return -14; /* -EFAULT */
}

$ clang --target=powerpc-linux-gnu -Wuninitialized -fsyntax-only test.c
test.c:17:10: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
        *inst = val;
                ^~~
test.c:4:18: note: initialize the variable 'val' to silence this warning
        unsigned int val;
                        ^
                         = 0
1 warning generated.

It certainly looks like there is something wrong with how clang is
tracking the initialization of the variable because it looks to me like
val is only used in the fallthrough path, which happens after it is
initialized via lwz.  Perhaps something is wrong with the logic of
https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
being migrated from Bugzilla to GitHub Issues right now so I cannot file
this upstream at the moment).

Cheers,
Nathan

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Bill Wendling]

On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
> > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
> > >> Hi Christophe,
> > >>
> > >> I love your patch! Perhaps something to improve:
> > >>
> > >> [auto build test WARNING on powerpc/next]
> > >> [also build test WARNING on v5.16-rc3 next-20211129]
> > >> [If your patch is applied to the wrong git tree, kindly drop us a note.
> > >> And when submitting patch, we suggest to use '--base' as documented in
> > >> https://git-scm.com/docs/git-format-patch]
> > >>
> > >> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> > >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> > >> config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
> > >> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
> > >> reproduce (this is a W=1 build):
> > >>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >>          chmod +x ~/bin/make.cross
> > >>          # install powerpc cross compiling tool for clang build
> > >>          # apt-get install binutils-powerpc-linux-gnu
> > >>          # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> > >>          git remote add linux-review https://github.com/0day-ci/linux
> > >>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> > >>          git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> > >>          # save the config file to linux build tree
> > >>          mkdir build_dir
> > >>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
> > >>
> > >> If you fix the issue, kindly add following tag as appropriate
> > >> Reported-by: kernel test robot <lkp@intel.com>
> > >>
> > >> All warnings (new ones prefixed by >>):
> > >>
> > >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
> > >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
> > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> > >>                     *inst = ppc_inst(val);
> > >>                                      ^~~
> > >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
> > >>     #define ppc_inst(x) (x)
> > >>                          ^
> > >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
> > >>             unsigned int val, suffix;
> > >>                             ^
> > >>                              = 0
> > >
> > > I can't understand what's wrong here.
> > >
> > > We have
> > >
> > >     __get_kernel_nofault(&val, src, u32, Efault);
> > >     if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
> > >             __get_kernel_nofault(&suffix, src + 1, u32, Efault);
> > >             *inst = ppc_inst_prefix(val, suffix);
> > >     } else {
> > >             *inst = ppc_inst(val);
> > >     }
> > >
> > > With
> > >
> > > #define __get_kernel_nofault(dst, src, type, err_label)                     \
> > >     __get_user_size_goto(*((type *)(dst)),                          \
> > >             (__force type __user *)(src), sizeof(type), err_label)
> > >
> > >
> > > And
> > >
> > > #define __get_user_size_goto(x, ptr, size, label)                           \
> > > do {                                                                                \
> > >     BUILD_BUG_ON(size > sizeof(x));                                         \
> > >     switch (size) {                                                         \
> > >     case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break;  \
> > >     case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break; \
> > >     case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break; \
> > >     case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label);  break;      \
> > >     default: x = 0; BUILD_BUG();                                            \
> > >     }                                                                       \
> > > } while (0)
> > >
> > > And
> > >
> > > #define __get_user_asm_goto(x, addr, label, op)                     \
> > >     asm_volatile_goto(                                      \
> > >             "1:     "op"%U1%X1 %0, %1       # get_user\n"   \
> > >             EX_TABLE(1b, %l2)                               \
> > >             : "=r" (x)                                      \
> > >             : "m<>" (*addr)                         \
> > >             :                                               \
> > >             : label)
> > >
> > >
> > > I see no possibility, no alternative path where val wouldn't be set. The
> > > asm clearly has *addr as an output param so it is always set.
> >
> > I guess clang can't convince itself of that?
>
> A simplified reproducer:
>
> $ cat test.c
> static inline int copy_inst_from_kernel_nofault(unsigned int *inst,
>                                                 unsigned int *src)
> {
>         unsigned int val;
>
>         asm goto("1: lwz %U1%X1 %0, %1   # get_user\n"
>                  ".section __ex_table,\"a\";"
>                  ".balign 4;"
>                  ".long (1b) - . ;"
>                  ".long (%l2) - . ;"
>                  ".previous"
>                  : "=r" (*(unsigned int *)(&val))
>                  : "m<>" (*(unsigned int *)(src))
>                  :
>                  : Efault);
>
>         *inst = val;
>         return 0;
>
> Efault:
>         return -14; /* -EFAULT */
> }
>
> $ clang --target=powerpc-linux-gnu -Wuninitialized -fsyntax-only test.c
> test.c:17:10: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>         *inst = val;
>                 ^~~
> test.c:4:18: note: initialize the variable 'val' to silence this warning
>         unsigned int val;
>                         ^
>                          = 0
> 1 warning generated.
>
> It certainly looks like there is something wrong with how clang is
> tracking the initialization of the variable because it looks to me like
> val is only used in the fallthrough path, which happens after it is
> initialized via lwz.  Perhaps something is wrong with the logic of
> https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
> being migrated from Bugzilla to GitHub Issues right now so I cannot file
> this upstream at the moment).
>
If I remove the casts of "val" the warning doesn't appear. I suspect
that when I wrote that patch I forgot to remove those when checking.
#include "Captain_Picard_facepalm.h"

I'll look into it.

-bw

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Bill Wendling]

On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
>
> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
> > > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
> > > >> Hi Christophe,
> > > >>
> > > >> I love your patch! Perhaps something to improve:
> > > >>
> > > >> [auto build test WARNING on powerpc/next]
> > > >> [also build test WARNING on v5.16-rc3 next-20211129]
> > > >> [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > >> And when submitting patch, we suggest to use '--base' as documented in
> > > >> https://git-scm.com/docs/git-format-patch]
> > > >>
> > > >> url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> > > >> base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
> > > >> config: powerpc-randconfig-r023-20211129 (https://download.01.org/0day-ci/archive/20211130/202111300652.0yDBNvyJ-lkp@intel.com/config)
> > > >> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
> > > >> reproduce (this is a W=1 build):
> > > >>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > >>          chmod +x ~/bin/make.cross
> > > >>          # install powerpc cross compiling tool for clang build
> > > >>          # apt-get install binutils-powerpc-linux-gnu
> > > >>          # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> > > >>          git remote add linux-review https://github.com/0day-ci/linux
> > > >>          git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
> > > >>          git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
> > > >>          # save the config file to linux build tree
> > > >>          mkdir build_dir
> > > >>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare
> > > >>
> > > >> If you fix the issue, kindly add following tag as appropriate
> > > >> Reported-by: kernel test robot <lkp@intel.com>
> > > >>
> > > >> All warnings (new ones prefixed by >>):
> > > >>
> > > >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
> > > >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
> > > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> > > >>                     *inst = ppc_inst(val);
> > > >>                                      ^~~
> > > >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
> > > >>     #define ppc_inst(x) (x)
> > > >>                          ^
> > > >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
> > > >>             unsigned int val, suffix;
> > > >>                             ^
> > > >>                              = 0
> > > >
> > > > I can't understand what's wrong here.
> > > >
> > > > We have
> > > >
> > > >     __get_kernel_nofault(&val, src, u32, Efault);
> > > >     if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
> > > >             __get_kernel_nofault(&suffix, src + 1, u32, Efault);
> > > >             *inst = ppc_inst_prefix(val, suffix);
> > > >     } else {
> > > >             *inst = ppc_inst(val);
> > > >     }
> > > >
> > > > With
> > > >
> > > > #define __get_kernel_nofault(dst, src, type, err_label)                     \
> > > >     __get_user_size_goto(*((type *)(dst)),                          \
> > > >             (__force type __user *)(src), sizeof(type), err_label)
> > > >
> > > >
> > > > And
> > > >
> > > > #define __get_user_size_goto(x, ptr, size, label)                           \
> > > > do {                                                                                \
> > > >     BUILD_BUG_ON(size > sizeof(x));                                         \
> > > >     switch (size) {                                                         \
> > > >     case 1: __get_user_asm_goto(x, (u8 __user *)ptr, label, "lbz"); break;  \
> > > >     case 2: __get_user_asm_goto(x, (u16 __user *)ptr, label, "lhz"); break; \
> > > >     case 4: __get_user_asm_goto(x, (u32 __user *)ptr, label, "lwz"); break; \
> > > >     case 8: __get_user_asm2_goto(x, (u64 __user *)ptr, label);  break;      \
> > > >     default: x = 0; BUILD_BUG();                                            \
> > > >     }                                                                       \
> > > > } while (0)
> > > >
> > > > And
> > > >
> > > > #define __get_user_asm_goto(x, addr, label, op)                     \
> > > >     asm_volatile_goto(                                      \
> > > >             "1:     "op"%U1%X1 %0, %1       # get_user\n"   \
> > > >             EX_TABLE(1b, %l2)                               \
> > > >             : "=r" (x)                                      \
> > > >             : "m<>" (*addr)                         \
> > > >             :                                               \
> > > >             : label)
> > > >
> > > >
> > > > I see no possibility, no alternative path where val wouldn't be set. The
> > > > asm clearly has *addr as an output param so it is always set.
> > >
> > > I guess clang can't convince itself of that?
> >
> > A simplified reproducer:
> >
> > $ cat test.c
> > static inline int copy_inst_from_kernel_nofault(unsigned int *inst,
> >                                                 unsigned int *src)
> > {
> >         unsigned int val;
> >
> >         asm goto("1: lwz %U1%X1 %0, %1   # get_user\n"
> >                  ".section __ex_table,\"a\";"
> >                  ".balign 4;"
> >                  ".long (1b) - . ;"
> >                  ".long (%l2) - . ;"
> >                  ".previous"
> >                  : "=r" (*(unsigned int *)(&val))
> >                  : "m<>" (*(unsigned int *)(src))
> >                  :
> >                  : Efault);
> >
> >         *inst = val;
> >         return 0;
> >
> > Efault:
> >         return -14; /* -EFAULT */
> > }
> >
> > $ clang --target=powerpc-linux-gnu -Wuninitialized -fsyntax-only test.c
> > test.c:17:10: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> >         *inst = val;
> >                 ^~~
> > test.c:4:18: note: initialize the variable 'val' to silence this warning
> >         unsigned int val;
> >                         ^
> >                          = 0
> > 1 warning generated.
> >
> > It certainly looks like there is something wrong with how clang is
> > tracking the initialization of the variable because it looks to me like
> > val is only used in the fallthrough path, which happens after it is
> > initialized via lwz.  Perhaps something is wrong with the logic of
> > https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
> > being migrated from Bugzilla to GitHub Issues right now so I cannot file
> > this upstream at the moment).
> >
> If I remove the casts of "val" the warning doesn't appear. I suspect
> that when I wrote that patch I forgot to remove those when checking.
> #include "Captain_Picard_facepalm.h"
>
> I'll look into it.
>
Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Michael Ellerman]

Bill Wendling <morbo@google.com> writes:
> On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
>> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
>> > On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
>> > > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> > > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
...
>> > > >> All warnings (new ones prefixed by >>):
>> > > >>
>> > > >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
>> > > >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>> > > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>> > > >>                     *inst = ppc_inst(val);
>> > > >>                                      ^~~
>> > > >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>> > > >>     #define ppc_inst(x) (x)
>> > > >>                          ^
>> > > >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>> > > >>             unsigned int val, suffix;
>> > > >>                             ^
>> > > >>                              = 0
>> > > >
>> > > > I can't understand what's wrong here.
...
>> > > >
>> > > > I see no possibility, no alternative path where val wouldn't be set. The
>> > > > asm clearly has *addr as an output param so it is always set.
>> > >
>> > > I guess clang can't convince itself of that?
...
>> >
>> > It certainly looks like there is something wrong with how clang is
>> > tracking the initialization of the variable because it looks to me like
>> > val is only used in the fallthrough path, which happens after it is
>> > initialized via lwz.  Perhaps something is wrong with the logic of
>> > https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
>> > being migrated from Bugzilla to GitHub Issues right now so I cannot file
>> > this upstream at the moment).
>> >
>> If I remove the casts of "val" the warning doesn't appear. I suspect
>> that when I wrote that patch I forgot to remove those when checking.
>> #include "Captain_Picard_facepalm.h"
>>
>> I'll look into it.
>>
> Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")

I guess for now I'll just squash this in as a workaround?


diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index 631436f3f5c3..5b591c51fec9 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
 	if (unlikely(!is_kernel_addr((unsigned long)src)))
 		return -ERANGE;
 
+#ifdef CONFIG_CC_IS_CLANG
+	val = suffix = 0;
+#endif
 	__get_kernel_nofault(&val, src, u32, Efault);
 	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
 		__get_kernel_nofault(&suffix, src + 1, u32, Efault);



cheers

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Nathan Chancellor]

On Tue, Dec 07, 2021 at 02:37:26PM +1100, Michael Ellerman wrote:
> Bill Wendling <morbo@google.com> writes:
> > On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
> >> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >> > On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
> >> > > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> >> > > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
> ...
> >> > > >> All warnings (new ones prefixed by >>):
> >> > > >>
> >> > > >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
> >> > > >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
> >> > > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> >> > > >>                     *inst = ppc_inst(val);
> >> > > >>                                      ^~~
> >> > > >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
> >> > > >>     #define ppc_inst(x) (x)
> >> > > >>                          ^
> >> > > >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
> >> > > >>             unsigned int val, suffix;
> >> > > >>                             ^
> >> > > >>                              = 0
> >> > > >
> >> > > > I can't understand what's wrong here.
> ...
> >> > > >
> >> > > > I see no possibility, no alternative path where val wouldn't be set. The
> >> > > > asm clearly has *addr as an output param so it is always set.
> >> > >
> >> > > I guess clang can't convince itself of that?
> ...
> >> >
> >> > It certainly looks like there is something wrong with how clang is
> >> > tracking the initialization of the variable because it looks to me like
> >> > val is only used in the fallthrough path, which happens after it is
> >> > initialized via lwz.  Perhaps something is wrong with the logic of
> >> > https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
> >> > being migrated from Bugzilla to GitHub Issues right now so I cannot file
> >> > this upstream at the moment).
> >> >
> >> If I remove the casts of "val" the warning doesn't appear. I suspect
> >> that when I wrote that patch I forgot to remove those when checking.
> >> #include "Captain_Picard_facepalm.h"
> >>
> >> I'll look into it.
> >>
> > Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
> 
> I guess for now I'll just squash this in as a workaround?
> 
> 
> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> index 631436f3f5c3..5b591c51fec9 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
>  	if (unlikely(!is_kernel_addr((unsigned long)src)))
>  		return -ERANGE;

Could we add a version check to this and a link to our bug tracker:

/* https://github.com/ClangBuiltLinux/linux/issues/1521 */
#if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 140000

> +#ifdef CONFIG_CC_IS_CLANG
> +	val = suffix = 0;
> +#endif
>  	__get_kernel_nofault(&val, src, u32, Efault);
>  	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
>  		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
> 
> 
> 
> cheers

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Christophe Leroy]

Le 07/12/2021 à 05:48, Nathan Chancellor a écrit :
> On Tue, Dec 07, 2021 at 02:37:26PM +1100, Michael Ellerman wrote:
>> Bill Wendling <morbo@google.com> writes:
>>> On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
>>>> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
>>>>> On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
>>>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>>>> Le 29/11/2021 à 23:55, kernel test robot a écrit :
>> ...
>>>>>>>> All warnings (new ones prefixed by >>):
>>>>>>>>
>>>>>>>>      In file included from arch/powerpc/kernel/asm-offsets.c:71:
>>>>>>>>      In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>>>>>>>>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>>>>>>>>                      *inst = ppc_inst(val);
>>>>>>>>                                       ^~~
>>>>>>>>      arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>>>>>>>>      #define ppc_inst(x) (x)
>>>>>>>>                           ^
>>>>>>>>      arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>>>>>>>>              unsigned int val, suffix;
>>>>>>>>                              ^
>>>>>>>>                               = 0
>>>>>>>
>>>>>>> I can't understand what's wrong here.
>> ...
>>>>>>>
>>>>>>> I see no possibility, no alternative path where val wouldn't be set. The
>>>>>>> asm clearly has *addr as an output param so it is always set.
>>>>>>
>>>>>> I guess clang can't convince itself of that?
>> ...
>>>>>
>>>>> It certainly looks like there is something wrong with how clang is
>>>>> tracking the initialization of the variable because it looks to me like
>>>>> val is only used in the fallthrough path, which happens after it is
>>>>> initialized via lwz.  Perhaps something is wrong with the logic of
>>>>> https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
>>>>> being migrated from Bugzilla to GitHub Issues right now so I cannot file
>>>>> this upstream at the moment).
>>>>>
>>>> If I remove the casts of "val" the warning doesn't appear. I suspect
>>>> that when I wrote that patch I forgot to remove those when checking.
>>>> #include "Captain_Picard_facepalm.h"
>>>>
>>>> I'll look into it.
>>>>
>>> Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
>>
>> I guess for now I'll just squash this in as a workaround?
>>
>>
>> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
>> index 631436f3f5c3..5b591c51fec9 100644
>> --- a/arch/powerpc/include/asm/inst.h
>> +++ b/arch/powerpc/include/asm/inst.h
>> @@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
>>   	if (unlikely(!is_kernel_addr((unsigned long)src)))
>>   		return -ERANGE;
> 
> Could we add a version check to this and a link to our bug tracker:
> 
> /* https://github.com/ClangBuiltLinux/linux/issues/1521 */
> #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 140000

The robot reported the problem on:

compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 
df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)

Should it be CONFIG_CLANG_VERSION <= 140000 ?

> 
>> +#ifdef CONFIG_CC_IS_CLANG
>> +	val = suffix = 0;
>> +#endif
>>   	__get_kernel_nofault(&val, src, u32, Efault);
>>   	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
>>   		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
>>

Christophe

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Nathan Chancellor]

On Tue, Dec 07, 2021 at 05:45:08AM +0000, Christophe Leroy wrote:
> 
> 
> Le 07/12/2021 à 05:48, Nathan Chancellor a écrit :
> > On Tue, Dec 07, 2021 at 02:37:26PM +1100, Michael Ellerman wrote:
> >> Bill Wendling <morbo@google.com> writes:
> >>> On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
> >>>> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >>>>> On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
> >>>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> >>>>>>> Le 29/11/2021 à 23:55, kernel test robot a écrit :
> >> ...
> >>>>>>>> All warnings (new ones prefixed by >>):
> >>>>>>>>
> >>>>>>>>      In file included from arch/powerpc/kernel/asm-offsets.c:71:
> >>>>>>>>      In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
> >>>>>>>>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
> >>>>>>>>                      *inst = ppc_inst(val);
> >>>>>>>>                                       ^~~
> >>>>>>>>      arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
> >>>>>>>>      #define ppc_inst(x) (x)
> >>>>>>>>                           ^
> >>>>>>>>      arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
> >>>>>>>>              unsigned int val, suffix;
> >>>>>>>>                              ^
> >>>>>>>>                               = 0
> >>>>>>>
> >>>>>>> I can't understand what's wrong here.
> >> ...
> >>>>>>>
> >>>>>>> I see no possibility, no alternative path where val wouldn't be set. The
> >>>>>>> asm clearly has *addr as an output param so it is always set.
> >>>>>>
> >>>>>> I guess clang can't convince itself of that?
> >> ...
> >>>>>
> >>>>> It certainly looks like there is something wrong with how clang is
> >>>>> tracking the initialization of the variable because it looks to me like
> >>>>> val is only used in the fallthrough path, which happens after it is
> >>>>> initialized via lwz.  Perhaps something is wrong with the logic of
> >>>>> https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
> >>>>> being migrated from Bugzilla to GitHub Issues right now so I cannot file
> >>>>> this upstream at the moment).
> >>>>>
> >>>> If I remove the casts of "val" the warning doesn't appear. I suspect
> >>>> that when I wrote that patch I forgot to remove those when checking.
> >>>> #include "Captain_Picard_facepalm.h"
> >>>>
> >>>> I'll look into it.
> >>>>
> >>> Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
> >>
> >> I guess for now I'll just squash this in as a workaround?
> >>
> >>
> >> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
> >> index 631436f3f5c3..5b591c51fec9 100644
> >> --- a/arch/powerpc/include/asm/inst.h
> >> +++ b/arch/powerpc/include/asm/inst.h
> >> @@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
> >>   	if (unlikely(!is_kernel_addr((unsigned long)src)))
> >>   		return -ERANGE;
> > 
> > Could we add a version check to this and a link to our bug tracker:
> > 
> > /* https://github.com/ClangBuiltLinux/linux/issues/1521 */
> > #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 140000
> 
> The robot reported the problem on:
> 
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 
> df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
> 
> Should it be CONFIG_CLANG_VERSION <= 140000 ?

The robot tests clang from tip of tree, rebuilding every week or so. The
fix is getting ready to land so it will be released in 14.0.0 final. We
have always written tip of tree version checks with the expectation that
if people are testing tip of tree clang, they are frequently rebuilding.
If that is not true, they need to be using released/stable versions,
otherwise the model is broken.

If that is too problematic, we could add a version check to Kconfig
(cannot think of a great name for the config off the top of my head)
that checks for this issue and ifdef on that. That might be nice in
case another instance of this crops up in the future.

Cheers,
Nathan

> > 
> >> +#ifdef CONFIG_CC_IS_CLANG
> >> +	val = suffix = 0;
> >> +#endif
> >>   	__get_kernel_nofault(&val, src, u32, Efault);
> >>   	if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
> >>   		__get_kernel_nofault(&suffix, src + 1, u32, Efault);
> >>
> 
> Christophe

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Christophe Leroy]

Le 07/12/2021 à 07:41, Nathan Chancellor a écrit :
> On Tue, Dec 07, 2021 at 05:45:08AM +0000, Christophe Leroy wrote:
>>
>>
>> Le 07/12/2021 à 05:48, Nathan Chancellor a écrit :
>>> On Tue, Dec 07, 2021 at 02:37:26PM +1100, Michael Ellerman wrote:
>>>> Bill Wendling <morbo@google.com> writes:
>>>>> On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
>>>>>> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
>>>>>>> On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
>>>>>>>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>>>>>>>> Le 29/11/2021 à 23:55, kernel test robot a écrit :
>>>> ...
>>>>>>>>>> All warnings (new ones prefixed by >>):
>>>>>>>>>>
>>>>>>>>>>       In file included from arch/powerpc/kernel/asm-offsets.c:71:
>>>>>>>>>>       In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>>>>>>>>>>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>>>>>>>>>>                       *inst = ppc_inst(val);
>>>>>>>>>>                                        ^~~
>>>>>>>>>>       arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>>>>>>>>>>       #define ppc_inst(x) (x)
>>>>>>>>>>                            ^
>>>>>>>>>>       arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>>>>>>>>>>               unsigned int val, suffix;
>>>>>>>>>>                               ^
>>>>>>>>>>                                = 0
>>>>>>>>>
>>>>>>>>> I can't understand what's wrong here.
>>>> ...
>>>>>>>>>
>>>>>>>>> I see no possibility, no alternative path where val wouldn't be set. The
>>>>>>>>> asm clearly has *addr as an output param so it is always set.
>>>>>>>>
>>>>>>>> I guess clang can't convince itself of that?
>>>> ...
>>>>>>>
>>>>>>> It certainly looks like there is something wrong with how clang is
>>>>>>> tracking the initialization of the variable because it looks to me like
>>>>>>> val is only used in the fallthrough path, which happens after it is
>>>>>>> initialized via lwz.  Perhaps something is wrong with the logic of
>>>>>>> https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
>>>>>>> being migrated from Bugzilla to GitHub Issues right now so I cannot file
>>>>>>> this upstream at the moment).
>>>>>>>
>>>>>> If I remove the casts of "val" the warning doesn't appear. I suspect
>>>>>> that when I wrote that patch I forgot to remove those when checking.
>>>>>> #include "Captain_Picard_facepalm.h"
>>>>>>
>>>>>> I'll look into it.
>>>>>>
>>>>> Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
>>>>
>>>> I guess for now I'll just squash this in as a workaround?
>>>>
>>>>
>>>> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
>>>> index 631436f3f5c3..5b591c51fec9 100644
>>>> --- a/arch/powerpc/include/asm/inst.h
>>>> +++ b/arch/powerpc/include/asm/inst.h
>>>> @@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
>>>>    	if (unlikely(!is_kernel_addr((unsigned long)src)))
>>>>    		return -ERANGE;
>>>
>>> Could we add a version check to this and a link to our bug tracker:
>>>
>>> /* https://github.com/ClangBuiltLinux/linux/issues/1521 */
>>> #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 140000
>>
>> The robot reported the problem on:
>>
>> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
>> df08b2fe8b35cb63dfb3b49738a3494b9b4e6f8e)
>>
>> Should it be CONFIG_CLANG_VERSION <= 140000 ?
> 
> The robot tests clang from tip of tree, rebuilding every week or so. The
> fix is getting ready to land so it will be released in 14.0.0 final. We
> have always written tip of tree version checks with the expectation that
> if people are testing tip of tree clang, they are frequently rebuilding.
> If that is not true, they need to be using released/stable versions,
> otherwise the model is broken.
> 
> If that is too problematic, we could add a version check to Kconfig
> (cannot think of a great name for the config off the top of my head)
> that checks for this issue and ifdef on that. That might be nice in
> case another instance of this crops up in the future.
> 

It's fine for me. I didn't know robot was using prereleases with the 
same name as the future release.

Christophe

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[Michael Ellerman]

Nathan Chancellor <nathan@kernel.org> writes:
> On Tue, Dec 07, 2021 at 02:37:26PM +1100, Michael Ellerman wrote:
>> Bill Wendling <morbo@google.com> writes:
>> > On Tue, Nov 30, 2021 at 10:38 AM Bill Wendling <morbo@google.com> wrote:
>> >> On Tue, Nov 30, 2021 at 10:17 AM Nathan Chancellor <nathan@kernel.org> wrote:
>> >> > On Tue, Nov 30, 2021 at 10:25:43PM +1100, Michael Ellerman wrote:
>> >> > > Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> >> > > > Le 29/11/2021 à 23:55, kernel test robot a écrit :
>> ...
>> >> > > >> All warnings (new ones prefixed by >>):
>> >> > > >>
>> >> > > >>     In file included from arch/powerpc/kernel/asm-offsets.c:71:
>> >> > > >>     In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
>> >> > > >>>> arch/powerpc/include/asm/inst.h:165:20: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
>> >> > > >>                     *inst = ppc_inst(val);
>> >> > > >>                                      ^~~
>> >> > > >>     arch/powerpc/include/asm/inst.h:53:22: note: expanded from macro 'ppc_inst'
>> >> > > >>     #define ppc_inst(x) (x)
>> >> > > >>                          ^
>> >> > > >>     arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
>> >> > > >>             unsigned int val, suffix;
>> >> > > >>                             ^
>> >> > > >>                              = 0
>> >> > > >
>> >> > > > I can't understand what's wrong here.
>> ...
>> >> > > >
>> >> > > > I see no possibility, no alternative path where val wouldn't be set. The
>> >> > > > asm clearly has *addr as an output param so it is always set.
>> >> > >
>> >> > > I guess clang can't convince itself of that?
>> ...
>> >> >
>> >> > It certainly looks like there is something wrong with how clang is
>> >> > tracking the initialization of the variable because it looks to me like
>> >> > val is only used in the fallthrough path, which happens after it is
>> >> > initialized via lwz.  Perhaps something is wrong with the logic of
>> >> > https://reviews.llvm.org/D71314?  I've added Bill to CC (LLVM issues are
>> >> > being migrated from Bugzilla to GitHub Issues right now so I cannot file
>> >> > this upstream at the moment).
>> >> >
>> >> If I remove the casts of "val" the warning doesn't appear. I suspect
>> >> that when I wrote that patch I forgot to remove those when checking.
>> >> #include "Captain_Picard_facepalm.h"
>> >>
>> >> I'll look into it.
>> >>
>> > Small retraction. It's the "*(<cast>)&val" that's the issue. (I.e. the "*&")
>> 
>> I guess for now I'll just squash this in as a workaround?
>> 
>> 
>> diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
>> index 631436f3f5c3..5b591c51fec9 100644
>> --- a/arch/powerpc/include/asm/inst.h
>> +++ b/arch/powerpc/include/asm/inst.h
>> @@ -157,6 +157,9 @@ static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
>>  	if (unlikely(!is_kernel_addr((unsigned long)src)))
>>  		return -ERANGE;
>
> Could we add a version check to this and a link to our bug tracker:
>
> /* https://github.com/ClangBuiltLinux/linux/issues/1521 */
> #if defined(CONFIG_CC_IS_CLANG) && CONFIG_CLANG_VERSION < 140000

Yep, thanks I'll use that.

cheers

Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()

[kernel test robot]

Hi Christophe,

I love your patch! Perhaps something to improve:

[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.16-rc3 next-20211201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc64-randconfig-r021-20211201 (https://download.01.org/0day-ci/archive/20211201/202112011435.ttoYYtbC-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install powerpc64 cross compiling tool for clang build
        # apt-get install binutils-powerpc64-linux-gnu
        # https://github.com/0day-ci/linux/commit/fb7bff30cc0efc7e4df1b48bb69de1f325eee826
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christophe-Leroy/powerpc-inst-Refactor-___get_user_instr/20211130-015346
        git checkout fb7bff30cc0efc7e4df1b48bb69de1f325eee826
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc prepare

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:47:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(insl, (unsigned long p, void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:143:1: note: expanded from here
   __do_insl
   ^
   arch/powerpc/include/asm/io.h:558:56: note: expanded from macro '__do_insl'
   #define __do_insl(p, b, n)      readsl((PCI_IO_ADDR)_IO_BASE+(p), (b), (n))
                                          ~~~~~~~~~~~~~~~~~~~~~^
   In file included from arch/powerpc/kernel/asm-offsets.c:21:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:49:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsb, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:145:1: note: expanded from here
   __do_outsb
   ^
   arch/powerpc/include/asm/io.h:559:58: note: expanded from macro '__do_outsb'
   #define __do_outsb(p, b, n)     writesb((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from arch/powerpc/kernel/asm-offsets.c:21:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:51:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsw, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:147:1: note: expanded from here
   __do_outsw
   ^
   arch/powerpc/include/asm/io.h:560:58: note: expanded from macro '__do_outsw'
   #define __do_outsw(p, b, n)     writesw((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from arch/powerpc/kernel/asm-offsets.c:21:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:26:
   In file included from include/linux/kernel_stat.h:9:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from arch/powerpc/include/asm/hardirq.h:6:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/powerpc/include/asm/io.h:619:
   arch/powerpc/include/asm/io-defs.h:53:1: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
   DEF_PCI_AC_NORET(outsl, (unsigned long p, const void *b, unsigned long c),
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/include/asm/io.h:616:3: note: expanded from macro 'DEF_PCI_AC_NORET'
                   __do_##name al;                                 \
                   ^~~~~~~~~~~~~~
   <scratch space>:149:1: note: expanded from here
   __do_outsl
   ^
   arch/powerpc/include/asm/io.h:561:58: note: expanded from macro '__do_outsl'
   #define __do_outsl(p, b, n)     writesl((PCI_IO_ADDR)_IO_BASE+(p),(b),(n))
                                           ~~~~~~~~~~~~~~~~~~~~~^
   In file included from arch/powerpc/kernel/asm-offsets.c:71:
   In file included from arch/powerpc/kernel/../xmon/xmon_bpts.h:7:
   arch/powerpc/include/asm/inst.h:161:41: warning: variable 'val' is uninitialized when used here [-Wuninitialized]
           if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
                                                  ^~~
   arch/powerpc/include/asm/inst.h:155:18: note: initialize the variable 'val' to silence this warning
           unsigned int val, suffix;
                           ^
                            = 0
>> arch/powerpc/include/asm/inst.h:163:32: warning: variable 'suffix' is uninitialized when used here [-Wuninitialized]
                   *inst = ppc_inst_prefix(val, suffix);
                                                ^~~~~~
   arch/powerpc/include/asm/inst.h:62:69: note: expanded from macro 'ppc_inst_prefix'
   #define ppc_inst_prefix(x, y) ((ppc_inst_t){ .val = (x), .suffix = (y) })
                                                                       ^
   arch/powerpc/include/asm/inst.h:155:26: note: initialize the variable 'suffix' to silence this warning
           unsigned int val, suffix;
                                   ^
                                    = 0
   14 warnings generated.
   <stdin>:1559:2: warning: syscall futex_waitv not implemented [-W#warnings]
   #warning syscall futex_waitv not implemented
    ^
   1 warning generated.
   /usr/bin/ld: unrecognised emulation mode: elf64ppc
   Supported emulations: elf_x86_64 elf32_x86_64 elf_i386 elf_iamcu elf_l1om elf_k1om i386pep i386pe
   clang-14: error: linker command failed with exit code 1 (use -v to see invocation)
   make[2]: *** [arch/powerpc/kernel/vdso64/Makefile:44: arch/powerpc/kernel/vdso64/vdso64.so.dbg] Error 1
   make[2]: Target 'include/generated/vdso64-offsets.h' not remade because of errors.
   make[1]: *** [arch/powerpc/Makefile:422: vdso_prepare] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:219: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/suffix +163 arch/powerpc/include/asm/inst.h

   152	
   153	static inline int copy_inst_from_kernel_nofault(ppc_inst_t *inst, u32 *src)
   154	{
   155		unsigned int val, suffix;
   156	
   157		if (unlikely(!is_kernel_addr((unsigned long)src)))
   158			return -ERANGE;
   159	
   160		__get_kernel_nofault(&val, src, u32, Efault);
   161		if (IS_ENABLED(CONFIG_PPC64) && get_op(val) == OP_PREFIX) {
   162			__get_kernel_nofault(&suffix, src + 1, u32, Efault);
 > 163			*inst = ppc_inst_prefix(val, suffix);
   164		} else {
   165			*inst = ppc_inst(val);
   166		}
   167		return 0;
   168	Efault:
   169		return -EFAULT;
   170	}
   171	

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