* Re: [patch 05/22] genirq/msi: Fixup includes
From: Thomas Gleixner @ 2021-11-30 22:41 UTC (permalink / raw)
To: Cédric Le Goater, LKML
Cc: linux-hyperv, linux-pci, Paul Mackerras, sparclinux, Wei Liu,
Ashok Raj, Marc Zygnier, x86, Christian Borntraeger,
Bjorn Helgaas, Jason Gunthorpe, xen-devel, ath11k, Kevin Tian,
Heiko Carstens, Alex Williamson, Megha Dey, Juergen Gross,
Thomas Bogendoerfer, Greg Kroah-Hartman, linux-mips, linuxppc-dev
In-Reply-To: <87czmhb8gq.ffs@tglx>
On Tue, Nov 30 2021 at 23:10, Thomas Gleixner wrote:
> On Tue, Nov 30 2021 at 22:48, Cédric Le Goater wrote:
>> On 11/29/21 22:38, Thomas Gleixner wrote:
>>> On Mon, Nov 29 2021 at 08:33, Cédric Le Goater wrote:
>>> thanks for having a look. I fixed up this and other fallout and pushed out an
>>> updated series (all 4 parts) to:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel msi
>>
>> pSeries fails to allocate MSIs starting with this patch :
>>
>> [PATCH 049/101] powerpc/pseries/msi: Let core code check for contiguous ...
>>
>> I will dig in later on.
>
> Let me stare at the core function..
It's not the core function. It's the patch above and I'm a moron.
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -359,9 +359,6 @@ static int rtas_prepare_msi_irqs(struct
if (quota && quota < nvec)
return quota;
- if (type == PCI_CAP_ID_MSIX)
- return -EINVAL;
-
/*
* Firmware currently refuse any non power of two allocation
* so we round up if the quota will allow it.
^ permalink raw reply
* Re: [PATCH 0/2] of: remove reserved regions count restriction
From: Mike Rapoport @ 2021-11-30 21:07 UTC (permalink / raw)
To: Rob Herring
Cc: Kirill A. Shutemov, Mark Rutland, Kefeng Wang, Rich Felker,
Jinyang He, David Hildenbrand, Lee Jones, linux-kernel,
Max Filippov, Anup Patel, Guo Ren, Guo Ren, Calvin Zhang,
Nick Kossifidis, Vladimir Isaev, Tiezhu Yang, Vincent Chen,
Will Deacon, Markus Elfring, Vitaly Wool, Jonas Bonn, devicetree,
linux-snps-arc, uclinux-h8-devel, Yoshinori Sato, Palmer Dabbelt,
linux-sh, Rafael J. Wysocki, Russell King, Ley Foon Tan,
Geert Uytterhoeven, Aneesh Kumar K.V, Catalin Marinas,
Ganesh Goudar, David Brazdil, linux-riscv, Guenter Roeck,
Alexander Sverdlin, Thierry Reding, Albert Ou, Arnd Bergmann,
Anshuman Khandual, linux-xtensa, Vineet Gupta, Andreas Oetken,
Stefan Kristiansson, Russell King (Oracle), linux-csky,
Christophe JAILLET, Greentime Hu, Paul Walmsley, Stafford Horne,
Andy Shevchenko, linux-arm-kernel, Andrey Konovalov,
Christophe Leroy, Chris Zankel, Thomas Bogendoerfer, linux-mips,
Alexandre Ghiti, Nick Hu, Atish Patra, Greg Kroah-Hartman,
Randy Dunlap, Frank Rowand, Serge Semin, Dinh Nguyen,
Zhang Yunkai, Palmer Dabbelt, Souptick Joarder, Marc Zyngier,
Mauri Sandberg, Paul Mackerras, Andrew Morton, linuxppc-dev,
openrisc
In-Reply-To: <YaVrahm+ysoQRGKe@robh.at.kernel.org>
On Mon, Nov 29, 2021 at 06:08:10PM -0600, Rob Herring wrote:
> On Sun, Nov 21, 2021 at 08:43:47AM +0200, Mike Rapoport wrote:
> > On Fri, Nov 19, 2021 at 03:58:17PM +0800, Calvin Zhang wrote:
> > > The count of reserved regions in /reserved-memory was limited because
> > > the struct reserved_mem array was defined statically. This series sorts
> > > out reserved memory code and allocates that array from early allocator.
> > >
> > > Note: reserved region with fixed location must be reserved before any
> > > memory allocation. While struct reserved_mem array should be allocated
> > > after allocator is activated. We make early_init_fdt_scan_reserved_mem()
> > > do reservation only and add another call to initialize reserved memory.
> > > So arch code have to change for it.
> >
> > I think much simpler would be to use the same constant for sizing
> > memblock.reserved and reserved_mem arrays.
>
> Do those arrays get shrunk? Or do we waste the memory forever?
Memblock arrays don't get shrunk, but they are __init unless an architecture
chose to keep them after boot, but most architectures that use device tree
actually keep memblock structures.
> Maybe we can copy and shrink the initial array? Though I suspect struct
> reserved_mem pointers have already been given out.
I'm not sure. It seems that reserved_mem pointers are given out at initcall
time and AFAIU the reserved_mem array is created somewhere around
setup_arch(). So maybe it's possible to copy and shrink the initial array.
> >
> > If there is too much reserved regions in the device tree, reserving them in
> > memblock will fail anyway because memblock also starts with static array
> > for memblock.reserved, so doing one pass with memblock_reserve() and
> > another to set up reserved_mem wouldn't help anyway.
> >
> > > I'm only familiar with arm and arm64 architectures. Approvals from arch
> > > maintainers are required. Thank you all.
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [patch 05/22] genirq/msi: Fixup includes
From: Thomas Gleixner @ 2021-11-30 22:10 UTC (permalink / raw)
To: Cédric Le Goater, LKML
Cc: linux-hyperv, linux-pci, Paul Mackerras, sparclinux, Wei Liu,
Ashok Raj, Marc Zygnier, x86, Christian Borntraeger,
Bjorn Helgaas, Jason Gunthorpe, xen-devel, ath11k, Kevin Tian,
Heiko Carstens, Alex Williamson, Megha Dey, Juergen Gross,
Thomas Bogendoerfer, Greg Kroah-Hartman, linux-mips, linuxppc-dev
In-Reply-To: <524d9b84-caa8-dd6f-bb5e-9fc906d279c0@kaod.org>
On Tue, Nov 30 2021 at 22:48, Cédric Le Goater wrote:
> On 11/29/21 22:38, Thomas Gleixner wrote:
>> On Mon, Nov 29 2021 at 08:33, Cédric Le Goater wrote:
>> thanks for having a look. I fixed up this and other fallout and pushed out an
>> updated series (all 4 parts) to:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel msi
>
> pSeries fails to allocate MSIs starting with this patch :
>
> [PATCH 049/101] powerpc/pseries/msi: Let core code check for contiguous ...
>
> I will dig in later on.
Let me stare at the core function..
^ permalink raw reply
* Re: [patch 05/22] genirq/msi: Fixup includes
From: Cédric Le Goater @ 2021-11-30 21:48 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: linux-hyperv, linux-pci, Paul Mackerras, sparclinux, Wei Liu,
Ashok Raj, Marc Zygnier, x86, Christian Borntraeger,
Bjorn Helgaas, Jason Gunthorpe, xen-devel, ath11k, Kevin Tian,
Heiko Carstens, Alex Williamson, Megha Dey, Juergen Gross,
Thomas Bogendoerfer, Greg Kroah-Hartman, linux-mips, linuxppc-dev
In-Reply-To: <87tufud4m3.ffs@tglx>
On 11/29/21 22:38, Thomas Gleixner wrote:
> Cedric,
>
> On Mon, Nov 29 2021 at 08:33, Cédric Le Goater wrote:
>> On 11/27/21 02:18, Thomas Gleixner wrote:
>>> Remove the kobject.h include from msi.h as it's not required and add a
>>> sysfs.h include to the core code instead.
>>>
>>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>>
>>
>> This patch breaks compile on powerpc :
>>
>> CC arch/powerpc/kernel/msi.o
>> In file included from ../arch/powerpc/kernel/msi.c:7:
>> ../include/linux/msi.h:410:65: error: ‘struct cpumask’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
>> 410 | int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask,
>> | ^~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Below is fix you can merge in patch 5.
>
> thanks for having a look. I fixed up this and other fallout and pushed out an
> updated series (all 4 parts) to:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel msi
pSeries fails to allocate MSIs starting with this patch :
[PATCH 049/101] powerpc/pseries/msi: Let core code check for contiguous ...
I will dig in later on.
C.
^ permalink raw reply
* Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()
From: Bill Wendling @ 2021-11-30 18:44 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kbuild-all, kernel test robot, llvm, Nick Desaulniers,
linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <CAGG=3QX4k6MZ1qkT+sVAroJeBpbZBnOJauM_uJsu2uV1vnVObQ@mail.gmail.com>
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 "*&")
^ permalink raw reply
* Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()
From: Bill Wendling @ 2021-11-30 18:38 UTC (permalink / raw)
To: Nathan Chancellor
Cc: kbuild-all, kernel test robot, llvm, Nick Desaulniers,
linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <YaZqs2tPxMzhgkAW@archlinux-ax161>
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
^ permalink raw reply
* Re: [PATCH v5 5/5] powerpc/inst: Optimise copy_inst_from_kernel_nofault()
From: Nathan Chancellor @ 2021-11-30 18:17 UTC (permalink / raw)
To: Michael Ellerman
Cc: kbuild-all, kernel test robot, llvm, Nick Desaulniers,
linux-kernel, Paul Mackerras, Bill Wendling, linuxppc-dev
In-Reply-To: <87a6hlq408.fsf@mpe.ellerman.id.au>
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
^ permalink raw reply
* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2021-11-30 15:39 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE
--- Comment #49 from Erhard F. (erhard_f@mailbox.org) ---
As the other KASAN inconsistencies mentioned here got ironed out by Christophe
(patches available but not in stable yet) and this "KASAN hit at raid6_pq: BUG:
Unable to handle kernel data access at 0x00f0fd0d" does no longer show up with
OUTLINE/INLINE KASAN on 5.15.5 or 5.16-rc3 I think it is safe to close this bug
as obsolete.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v2 0/6] KEXEC_SIG with appended signature
From: Heiko Carstens @ 2021-11-30 15:28 UTC (permalink / raw)
To: Michal Suchanek
Cc: Nayna, Mimi Zohar, David Howells, keyrings, Paul Mackerras,
Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
Christian Borntraeger, Serge E. Hallyn, Vasily Gorbik, linux-s390,
linux-crypto, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
Philipp Rudo, Frank van der Linden, kexec, linux-kernel,
Luis Chamberlain, Sven Schnelle, linux-security-module,
Jessica Yu, linux-integrity, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1637862358.git.msuchanek@suse.de>
On Thu, Nov 25, 2021 at 07:02:38PM +0100, Michal Suchanek wrote:
> Hello,
>
> This is resend of the KEXEC_SIG patchset.
>
> The first patch is new because it'a a cleanup that does not require any
> change to the module verification code.
>
> The second patch is the only one that is intended to change any
> functionality.
>
> The rest only deduplicates code but I did not receive any review on that
> part so I don't know if it's desirable as implemented.
>
> The first two patches can be applied separately without the rest.
>
> Thanks
>
> Michal
>
> Michal Suchanek (6):
> s390/kexec_file: Don't opencode appended signature check.
> powerpc/kexec_file: Add KEXEC_SIG support.
> kexec_file: Don't opencode appended signature verification.
> module: strip the signature marker in the verification function.
> module: Use key_being_used_for for log messages in
> verify_appended_signature
> module: Move duplicate mod_check_sig users code to mod_parse_sig
>
> arch/powerpc/Kconfig | 11 +++++
> arch/powerpc/kexec/elf_64.c | 14 ++++++
> arch/s390/kernel/machine_kexec_file.c | 42 ++----------------
> crypto/asymmetric_keys/asymmetric_type.c | 1 +
> include/linux/module_signature.h | 1 +
> include/linux/verification.h | 4 ++
> kernel/module-internal.h | 2 -
> kernel/module.c | 12 +++--
> kernel/module_signature.c | 56 +++++++++++++++++++++++-
> kernel/module_signing.c | 33 +++++++-------
> security/integrity/ima/ima_modsig.c | 22 ++--------
> 11 files changed, 113 insertions(+), 85 deletions(-)
For all patches which touch s390:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH v2] powerpc/powermac: Add missing lockdep_register_key()
From: Erhard F. @ 2021-11-30 15:30 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <34b423d68d170676fc3367594d17d1ca5c3844a4.1638282630.git.christophe.leroy@csgroup.eu>
On Tue, 30 Nov 2021 15:30:42 +0100
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> KeyWest i2c @0xf8001003 irq 42 /uni-n@f8000000/i2c@f8001000
> BUG: key c2d00cbc has not been registered!
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(1)
> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4801 lockdep_init_map_type+0x4c0/0xb4c
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.5-gentoo-PowerMacG4 #9
> NIP: c01a9428 LR: c01a9428 CTR: 00000000
> REGS: e1033cf0 TRAP: 0700 Not tainted (5.15.5-gentoo-PowerMacG4)
> MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24002002 XER: 00000000
>
> GPR00: c01a9428 e1033db0 c2d1cf20 00000016 00000004 00000001 c01c0630 e1033a73
> GPR08: 00000000 00000000 00000000 e1033db0 24002004 00000000 f8729377 00000003
> GPR16: c1829a9c 00000000 18305357 c1416fc0 c1416f80 c006ac60 c2d00ca8 c1416f00
> GPR24: 00000000 c21586f0 c2160000 00000000 c2d00cbc c2170000 c216e1a0 c2160000
> NIP [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
> LR [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
> Call Trace:
> [e1033db0] [c01a9428] lockdep_init_map_type+0x4c0/0xb4c (unreliable)
> [e1033df0] [c1c177b8] kw_i2c_add+0x334/0x424
> [e1033e20] [c1c18294] pmac_i2c_init+0x9ec/0xa9c
> [e1033e80] [c1c1a790] smp_core99_probe+0xbc/0x35c
> [e1033eb0] [c1c03cb0] kernel_init_freeable+0x190/0x5a4
> [e1033f10] [c000946c] kernel_init+0x28/0x154
> [e1033f30] [c0035148] ret_from_kernel_thread+0x14/0x1c
>
> Add missing lockdep_register_key()
>
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=200055
> Fixes: 9e607f72748d ("i2c_powermac: shut up lockdep warning")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v2: Added lockdep_register_key() at two other places.
> ---
> arch/powerpc/platforms/powermac/low_i2c.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
> index f77a59b5c2e1..df89d916236d 100644
> --- a/arch/powerpc/platforms/powermac/low_i2c.c
> +++ b/arch/powerpc/platforms/powermac/low_i2c.c
> @@ -582,6 +582,7 @@ static void __init kw_i2c_add(struct pmac_i2c_host_kw *host,
> bus->close = kw_i2c_close;
> bus->xfer = kw_i2c_xfer;
> mutex_init(&bus->mutex);
> + lockdep_register_key(&bus->lock_key);
> lockdep_set_class(&bus->mutex, &bus->lock_key);
> if (controller == busnode)
> bus->flags = pmac_i2c_multibus;
> @@ -810,6 +811,7 @@ static void __init pmu_i2c_probe(void)
> bus->hostdata = bus + 1;
> bus->xfer = pmu_i2c_xfer;
> mutex_init(&bus->mutex);
> + lockdep_register_key(&bus->lock_key);
> lockdep_set_class(&bus->mutex, &bus->lock_key);
> bus->flags = pmac_i2c_multibus;
> list_add(&bus->link, &pmac_i2c_busses);
> @@ -933,6 +935,7 @@ static void __init smu_i2c_probe(void)
> bus->hostdata = bus + 1;
> bus->xfer = smu_i2c_xfer;
> mutex_init(&bus->mutex);
> + lockdep_register_key(&bus->lock_key);
> lockdep_set_class(&bus->mutex, &bus->lock_key);
> bus->flags = 0;
> list_add(&bus->link, &pmac_i2c_busses);
> --
> 2.33.1
>
Yes, that did the trick! The lockdep spalt is gone now. Many thanks Christophe!
I'll close the original bugzilla report once this gets into stable.
Regards,
Erhard
^ permalink raw reply
* [PATCH AUTOSEL 5.10 36/43] xen: flag hvc_xen to be not essential for system boot
From: Sasha Levin @ 2021-11-30 14:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Juergen Gross, Sasha Levin, gregkh, linuxppc-dev, jbeulich,
Boris Ostrovsky, jirislaby
In-Reply-To: <20211130145022.945517-1-sashal@kernel.org>
From: Juergen Gross <jgross@suse.com>
[ Upstream commit 0239143490a9fa1344955dde93527b09f5576dac ]
The Xen pv console driver is not essential for boot. Set the respective
flag.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Link: https://lore.kernel.org/r/20211022064800.14978-4-jgross@suse.com
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/hvc/hvc_xen.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 92c9a476defc9..55a435e329470 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -511,6 +511,7 @@ static struct xenbus_driver xencons_driver = {
.remove = xencons_remove,
.resume = xencons_resume,
.otherend_changed = xencons_backend_changed,
+ .not_essential = true,
};
#endif /* CONFIG_HVC_XEN_FRONTEND */
--
2.33.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 50/68] xen: flag hvc_xen to be not essential for system boot
From: Sasha Levin @ 2021-11-30 14:46 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Juergen Gross, Sasha Levin, gregkh, linuxppc-dev, jbeulich,
Boris Ostrovsky, jirislaby
In-Reply-To: <20211130144707.944580-1-sashal@kernel.org>
From: Juergen Gross <jgross@suse.com>
[ Upstream commit 0239143490a9fa1344955dde93527b09f5576dac ]
The Xen pv console driver is not essential for boot. Set the respective
flag.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Link: https://lore.kernel.org/r/20211022064800.14978-4-jgross@suse.com
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/tty/hvc/hvc_xen.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index f0bf01ea069ae..71e0dd2c0ce5b 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -522,6 +522,7 @@ static struct xenbus_driver xencons_driver = {
.remove = xencons_remove,
.resume = xencons_resume,
.otherend_changed = xencons_backend_changed,
+ .not_essential = true,
};
#endif /* CONFIG_HVC_XEN_FRONTEND */
--
2.33.0
^ permalink raw reply related
* [PATCH v2] powerpc/powermac: Add missing lockdep_register_key()
From: Christophe Leroy @ 2021-11-30 14:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Erhard Furtner, linuxppc-dev, linux-kernel
KeyWest i2c @0xf8001003 irq 42 /uni-n@f8000000/i2c@f8001000
BUG: key c2d00cbc has not been registered!
------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(1)
WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4801 lockdep_init_map_type+0x4c0/0xb4c
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.5-gentoo-PowerMacG4 #9
NIP: c01a9428 LR: c01a9428 CTR: 00000000
REGS: e1033cf0 TRAP: 0700 Not tainted (5.15.5-gentoo-PowerMacG4)
MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24002002 XER: 00000000
GPR00: c01a9428 e1033db0 c2d1cf20 00000016 00000004 00000001 c01c0630 e1033a73
GPR08: 00000000 00000000 00000000 e1033db0 24002004 00000000 f8729377 00000003
GPR16: c1829a9c 00000000 18305357 c1416fc0 c1416f80 c006ac60 c2d00ca8 c1416f00
GPR24: 00000000 c21586f0 c2160000 00000000 c2d00cbc c2170000 c216e1a0 c2160000
NIP [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
LR [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
Call Trace:
[e1033db0] [c01a9428] lockdep_init_map_type+0x4c0/0xb4c (unreliable)
[e1033df0] [c1c177b8] kw_i2c_add+0x334/0x424
[e1033e20] [c1c18294] pmac_i2c_init+0x9ec/0xa9c
[e1033e80] [c1c1a790] smp_core99_probe+0xbc/0x35c
[e1033eb0] [c1c03cb0] kernel_init_freeable+0x190/0x5a4
[e1033f10] [c000946c] kernel_init+0x28/0x154
[e1033f30] [c0035148] ret_from_kernel_thread+0x14/0x1c
Add missing lockdep_register_key()
Reported-by: Erhard Furtner <erhard_f@mailbox.org>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=200055
Fixes: 9e607f72748d ("i2c_powermac: shut up lockdep warning")
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Added lockdep_register_key() at two other places.
---
arch/powerpc/platforms/powermac/low_i2c.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
index f77a59b5c2e1..df89d916236d 100644
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -582,6 +582,7 @@ static void __init kw_i2c_add(struct pmac_i2c_host_kw *host,
bus->close = kw_i2c_close;
bus->xfer = kw_i2c_xfer;
mutex_init(&bus->mutex);
+ lockdep_register_key(&bus->lock_key);
lockdep_set_class(&bus->mutex, &bus->lock_key);
if (controller == busnode)
bus->flags = pmac_i2c_multibus;
@@ -810,6 +811,7 @@ static void __init pmu_i2c_probe(void)
bus->hostdata = bus + 1;
bus->xfer = pmu_i2c_xfer;
mutex_init(&bus->mutex);
+ lockdep_register_key(&bus->lock_key);
lockdep_set_class(&bus->mutex, &bus->lock_key);
bus->flags = pmac_i2c_multibus;
list_add(&bus->link, &pmac_i2c_busses);
@@ -933,6 +935,7 @@ static void __init smu_i2c_probe(void)
bus->hostdata = bus + 1;
bus->xfer = smu_i2c_xfer;
mutex_init(&bus->mutex);
+ lockdep_register_key(&bus->lock_key);
lockdep_set_class(&bus->mutex, &bus->lock_key);
bus->flags = 0;
list_add(&bus->link, &pmac_i2c_busses);
--
2.33.1
^ permalink raw reply related
* Re: [PATCH] powerpc/modules: Don't WARN on first module allocation tentative
From: Christophe Leroy @ 2021-11-30 14:21 UTC (permalink / raw)
To: Erhard F.; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20211130150651.4aefba87@yea>
Le 30/11/2021 à 15:06, Erhard F. a écrit :
> On Tue, 30 Nov 2021 11:10:43 +0100
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>> module_alloc() first tries to allocate module text within
>> 24 bits direct jump from kernel text, and tries a wider
>> allocation if first one fails.
>>
>> When first allocation fails the following is observed in kernel logs:
>>
>> vmap allocation for size 2400256 failed: use vmalloc=<size> to increase size
>> systemd-udevd: vmalloc error: size 2395133, vm_struct allocation failed, mode:0xcc0(GFP_KERNEL), nodemask=(null)
>> CPU: 0 PID: 127 Comm: systemd-udevd Tainted: G W 5.15.5-gentoo-PowerMacG4 #9
>> Call Trace:
>> [e2a53a50] [c0ba0048] dump_stack_lvl+0x80/0xb0 (unreliable)
>> [e2a53a70] [c0540128] warn_alloc+0x11c/0x2b4
>> [e2a53b50] [c0531be8] __vmalloc_node_range+0xd8/0x64c
>> [e2a53c10] [c00338c0] module_alloc+0xa0/0xac
>> [e2a53c40] [c027a368] load_module+0x2ae0/0x8148
>> [e2a53e30] [c027fc78] sys_finit_module+0xfc/0x130
>> [e2a53f30] [c0035098] ret_from_syscall+0x0/0x28
>> --- interrupt: c00 at 0x25df10
>> NIP: 0025df10 LR: 00416180 CTR: 00000000
>> REGS: e2a53f40 TRAP: 0c00 Tainted: G W (5.15.5-gentoo-PowerMacG4)
>> MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 2822242c XER: 20000000
>>
>> GPR00: 00000161 afa8f060 a73a6160 00000011 0041dfa5 00000000 00000011 00000000
>> GPR08: 00000000 20000000 0000007a afa8efe0 002113f8 0072f924 00000000 00000000
>> GPR16: 00000005 afa8f1fc afa8f1e8 00000000 22222422 00000000 009603b0 22222422
>> GPR24: 00000000 00000000 0041dfa5 008a5fc0 00020000 00000000 00444cc4 009958f0
>> NIP [0025df10] 0x25df10
>> LR [00416180] 0x416180
>> --- interrupt: c00
>> Mem-Info:
>> active_anon:36 inactive_anon:4417 isolated_anon:0
>> active_file:3496 inactive_file:10535 isolated_file:0
>> unevictable:0 dirty:59 writeback:0
>> slab_reclaimable:3672 slab_unreclaimable:20540
>> mapped:5700 shmem:166 pagetables:185 bounce:0
>> kernel_misc_reclaimable:0
>> free:438282 free_pcp:1121 free_cma:0
>> Node 0 active_anon:144kB inactive_anon:17668kB active_file:13984kB inactive_file:42140kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:22800kB dirty:404kB writeback:0kB shmem:664kB writeback_tmp:0kB kernel_stack:1352kB pagetables:740kB all_unreclaimable? no
>> DMA free:267900kB min:2488kB low:3108kB high:3728kB reserved_highatomic:4096KB active_anon:0kB inactive_anon:0kB active_file:4016kB inactive_file:4224kB unevictable:0kB writepending:76kB present:524288kB managed:389432kB mlocked:0kB bounce:0kB free_pcp:2480kB local_pcp:1176kB free_cma:0kB
>> lowmem_reserve[]: 0 0 1536 1536
>> HighMem free:1485036kB min:512kB low:3032kB high:5552kB reserved_highatomic:0KB active_anon:136kB inactive_anon:17612kB active_file:9976kB inactive_file:37912kB unevictable:0kB writepending:364kB present:1572864kB managed:1572864kB mlocked:0kB bounce:0kB free_pcp:1776kB local_pcp:404kB free_cma:0kB
>> BTRFS: selftest: sectorsize: 4096 nodesize: 8192
>> lowmem_reserve[]: 0 0 0 0
>> BTRFS: selftest: running btrfs free space cache tests
>> DMA: 283*4kB (UMH) 282*8kB (UMH) 242*16kB (UMEH) 422*32kB (UMH) 187*64kB (UMH) 139*128kB (UME) 118*256kB (UM) 87*512kB (UME) 61*1024kB (UME) 27*2048kB (U) 6*4096kB (U) = 267612kB
>> BTRFS: selftest: running extent only tests
>> HighMem: 1*4kB (M) 1*8kB (M) 92*16kB (M) 43*32kB (UM) 21*64kB (M) 7*128kB (M) 3*256kB (UM) 1*512kB (U) 2*1024kB (UM) 1*2048kB (U) 360*4096kB (UM) = 1485036kB
>> BTRFS: selftest: running bitmap only tests
>> 14218 total pagecache pages
>> 0 pages in swap cache
>> Swap cache stats: add 0, delete 0, find 0/0
>> Free swap = 8388604kB
>> Total swap = 8388604kB
>> 524288 pages RAM
>> 393216 pages HighMem/MovableOnly
>> 33714 pages reserved
>>
>> Add __GFP_NOWARN flag to first allocation so that no warning appears
>> when it fails.
>>
>> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
>> Fixes: 2ec13df16704 ("powerpc/modules: Load modules closer to kernel text")
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/kernel/module.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
>> index ed04a3ba66fe..40a583e9d3c7 100644
>> --- a/arch/powerpc/kernel/module.c
>> +++ b/arch/powerpc/kernel/module.c
>> @@ -90,16 +90,17 @@ int module_finalize(const Elf_Ehdr *hdr,
>> }
>>
>> static __always_inline void *
>> -__module_alloc(unsigned long size, unsigned long start, unsigned long end)
>> +__module_alloc(unsigned long size, unsigned long start, unsigned long end, bool nowarn)
>> {
>> pgprot_t prot = strict_module_rwx_enabled() ? PAGE_KERNEL : PAGE_KERNEL_EXEC;
>> + gfp_t gfp = GFP_KERNEL | (nowarn ? __GFP_NOWARN : 0);
>>
>> /*
>> * Don't do huge page allocations for modules yet until more testing
>> * is done. STRICT_MODULE_RWX may require extra work to support this
>> * too.
>> */
>> - return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, prot,
>> + return __vmalloc_node_range(size, 1, start, end, gfp, prot,
>> VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
>> NUMA_NO_NODE, __builtin_return_address(0));
>> }
>> @@ -114,13 +115,13 @@ void *module_alloc(unsigned long size)
>>
>> /* First try within 32M limit from _etext to avoid branch trampolines */
>> if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
>> - ptr = __module_alloc(size, limit, MODULES_END);
>> + ptr = __module_alloc(size, limit, MODULES_END, true);
>>
>> if (!ptr)
>> - ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
>> + ptr = __module_alloc(size, MODULES_VADDR, MODULES_END, false);
>>
>> return ptr;
>> #else
>> - return __module_alloc(size, VMALLOC_START, VMALLOC_END);
>> + return __module_alloc(size, VMALLOC_START, VMALLOC_END, false);
>> #endif
>> }
>> --
>> 2.33.1
>
> The patch applies on 5.15.5 but I still get this vmalloc error. Looks a bit different though:
>
> [..]
> Running code patching self-tests ...
> vmap allocation for size 33562624 failed: use vmalloc=<size> to increase size
> swapper/0: vmalloc error: size 33558528, vm_struct allocation failed, mode:0xcc0(GFP_KERNEL), nodemask=(null)
> CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.15.5-PowerMacG4+ #2
> Call Trace:
> [f1033ba0] [c0b9fff0] dump_stack_lvl+0x80/0xb0 (unreliable)
> [f1033bc0] [c05400d0] warn_alloc+0x11c/0x2b4
> [f1033ca0] [c0531b90] __vmalloc_node_range+0xd8/0x64c
> [f1033d60] [c05319a8] __vmalloc_node+0xec/0xf4
> [f1033da0] [c1c0f998] test_code_patching+0x72c/0xd50
This one is something different.
The one I fixed with this patch is the one that was almost at the end of
your dmesg, it was related to the loading of some module.
The one that remains comes from test_code_patching() and is a real
failure that is due to LOWMEM being to high. This one should disappear
when you reduce LOWMEM.
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/modules: Don't WARN on first module allocation tentative
From: Erhard F. @ 2021-11-30 14:06 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <93c9b84d6ec76aaf7b4f03468e22433a6d308674.1638267035.git.christophe.leroy@csgroup.eu>
On Tue, 30 Nov 2021 11:10:43 +0100
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> module_alloc() first tries to allocate module text within
> 24 bits direct jump from kernel text, and tries a wider
> allocation if first one fails.
>
> When first allocation fails the following is observed in kernel logs:
>
> vmap allocation for size 2400256 failed: use vmalloc=<size> to increase size
> systemd-udevd: vmalloc error: size 2395133, vm_struct allocation failed, mode:0xcc0(GFP_KERNEL), nodemask=(null)
> CPU: 0 PID: 127 Comm: systemd-udevd Tainted: G W 5.15.5-gentoo-PowerMacG4 #9
> Call Trace:
> [e2a53a50] [c0ba0048] dump_stack_lvl+0x80/0xb0 (unreliable)
> [e2a53a70] [c0540128] warn_alloc+0x11c/0x2b4
> [e2a53b50] [c0531be8] __vmalloc_node_range+0xd8/0x64c
> [e2a53c10] [c00338c0] module_alloc+0xa0/0xac
> [e2a53c40] [c027a368] load_module+0x2ae0/0x8148
> [e2a53e30] [c027fc78] sys_finit_module+0xfc/0x130
> [e2a53f30] [c0035098] ret_from_syscall+0x0/0x28
> --- interrupt: c00 at 0x25df10
> NIP: 0025df10 LR: 00416180 CTR: 00000000
> REGS: e2a53f40 TRAP: 0c00 Tainted: G W (5.15.5-gentoo-PowerMacG4)
> MSR: 0000d032 <EE,PR,ME,IR,DR,RI> CR: 2822242c XER: 20000000
>
> GPR00: 00000161 afa8f060 a73a6160 00000011 0041dfa5 00000000 00000011 00000000
> GPR08: 00000000 20000000 0000007a afa8efe0 002113f8 0072f924 00000000 00000000
> GPR16: 00000005 afa8f1fc afa8f1e8 00000000 22222422 00000000 009603b0 22222422
> GPR24: 00000000 00000000 0041dfa5 008a5fc0 00020000 00000000 00444cc4 009958f0
> NIP [0025df10] 0x25df10
> LR [00416180] 0x416180
> --- interrupt: c00
> Mem-Info:
> active_anon:36 inactive_anon:4417 isolated_anon:0
> active_file:3496 inactive_file:10535 isolated_file:0
> unevictable:0 dirty:59 writeback:0
> slab_reclaimable:3672 slab_unreclaimable:20540
> mapped:5700 shmem:166 pagetables:185 bounce:0
> kernel_misc_reclaimable:0
> free:438282 free_pcp:1121 free_cma:0
> Node 0 active_anon:144kB inactive_anon:17668kB active_file:13984kB inactive_file:42140kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:22800kB dirty:404kB writeback:0kB shmem:664kB writeback_tmp:0kB kernel_stack:1352kB pagetables:740kB all_unreclaimable? no
> DMA free:267900kB min:2488kB low:3108kB high:3728kB reserved_highatomic:4096KB active_anon:0kB inactive_anon:0kB active_file:4016kB inactive_file:4224kB unevictable:0kB writepending:76kB present:524288kB managed:389432kB mlocked:0kB bounce:0kB free_pcp:2480kB local_pcp:1176kB free_cma:0kB
> lowmem_reserve[]: 0 0 1536 1536
> HighMem free:1485036kB min:512kB low:3032kB high:5552kB reserved_highatomic:0KB active_anon:136kB inactive_anon:17612kB active_file:9976kB inactive_file:37912kB unevictable:0kB writepending:364kB present:1572864kB managed:1572864kB mlocked:0kB bounce:0kB free_pcp:1776kB local_pcp:404kB free_cma:0kB
> BTRFS: selftest: sectorsize: 4096 nodesize: 8192
> lowmem_reserve[]: 0 0 0 0
> BTRFS: selftest: running btrfs free space cache tests
> DMA: 283*4kB (UMH) 282*8kB (UMH) 242*16kB (UMEH) 422*32kB (UMH) 187*64kB (UMH) 139*128kB (UME) 118*256kB (UM) 87*512kB (UME) 61*1024kB (UME) 27*2048kB (U) 6*4096kB (U) = 267612kB
> BTRFS: selftest: running extent only tests
> HighMem: 1*4kB (M) 1*8kB (M) 92*16kB (M) 43*32kB (UM) 21*64kB (M) 7*128kB (M) 3*256kB (UM) 1*512kB (U) 2*1024kB (UM) 1*2048kB (U) 360*4096kB (UM) = 1485036kB
> BTRFS: selftest: running bitmap only tests
> 14218 total pagecache pages
> 0 pages in swap cache
> Swap cache stats: add 0, delete 0, find 0/0
> Free swap = 8388604kB
> Total swap = 8388604kB
> 524288 pages RAM
> 393216 pages HighMem/MovableOnly
> 33714 pages reserved
>
> Add __GFP_NOWARN flag to first allocation so that no warning appears
> when it fails.
>
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Fixes: 2ec13df16704 ("powerpc/modules: Load modules closer to kernel text")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/module.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
> index ed04a3ba66fe..40a583e9d3c7 100644
> --- a/arch/powerpc/kernel/module.c
> +++ b/arch/powerpc/kernel/module.c
> @@ -90,16 +90,17 @@ int module_finalize(const Elf_Ehdr *hdr,
> }
>
> static __always_inline void *
> -__module_alloc(unsigned long size, unsigned long start, unsigned long end)
> +__module_alloc(unsigned long size, unsigned long start, unsigned long end, bool nowarn)
> {
> pgprot_t prot = strict_module_rwx_enabled() ? PAGE_KERNEL : PAGE_KERNEL_EXEC;
> + gfp_t gfp = GFP_KERNEL | (nowarn ? __GFP_NOWARN : 0);
>
> /*
> * Don't do huge page allocations for modules yet until more testing
> * is done. STRICT_MODULE_RWX may require extra work to support this
> * too.
> */
> - return __vmalloc_node_range(size, 1, start, end, GFP_KERNEL, prot,
> + return __vmalloc_node_range(size, 1, start, end, gfp, prot,
> VM_FLUSH_RESET_PERMS | VM_NO_HUGE_VMAP,
> NUMA_NO_NODE, __builtin_return_address(0));
> }
> @@ -114,13 +115,13 @@ void *module_alloc(unsigned long size)
>
> /* First try within 32M limit from _etext to avoid branch trampolines */
> if (MODULES_VADDR < PAGE_OFFSET && MODULES_END > limit)
> - ptr = __module_alloc(size, limit, MODULES_END);
> + ptr = __module_alloc(size, limit, MODULES_END, true);
>
> if (!ptr)
> - ptr = __module_alloc(size, MODULES_VADDR, MODULES_END);
> + ptr = __module_alloc(size, MODULES_VADDR, MODULES_END, false);
>
> return ptr;
> #else
> - return __module_alloc(size, VMALLOC_START, VMALLOC_END);
> + return __module_alloc(size, VMALLOC_START, VMALLOC_END, false);
> #endif
> }
> --
> 2.33.1
The patch applies on 5.15.5 but I still get this vmalloc error. Looks a bit different though:
[..]
Running code patching self-tests ...
vmap allocation for size 33562624 failed: use vmalloc=<size> to increase size
swapper/0: vmalloc error: size 33558528, vm_struct allocation failed, mode:0xcc0(GFP_KERNEL), nodemask=(null)
CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 5.15.5-PowerMacG4+ #2
Call Trace:
[f1033ba0] [c0b9fff0] dump_stack_lvl+0x80/0xb0 (unreliable)
[f1033bc0] [c05400d0] warn_alloc+0x11c/0x2b4
[f1033ca0] [c0531b90] __vmalloc_node_range+0xd8/0x64c
[f1033d60] [c05319a8] __vmalloc_node+0xec/0xf4
[f1033da0] [c1c0f998] test_code_patching+0x72c/0xd50
[f1033de0] [c0008994] do_one_initcall+0x2a0/0x5ac
[f1033eb0] [c1c040b0] kernel_init_freeable+0x590/0x5a4
[f1033f10] [c000946c] kernel_init+0x28/0x154
[f1033f30] [c0035148] ret_from_kernel_thread+0x14/0x1c
Mem-Info:
active_anon:0 inactive_anon:0 isolated_anon:0
active_file:0 inactive_file:0 isolated_file:0
unevictable:0 dirty:0 writeback:0
slab_reclaimable:1154 slab_unreclaimable:4207
mapped:0 shmem:0 pagetables:0 bounce:0
kernel_misc_reclaimable:0
free:474202 free_pcp:630 free_cma:0
Node 0 active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB writeback_tmp:0kB kernel_stack:848kB pagetables:0kB all_unreclaimable? no
DMA free:587836kB min:3140kB low:3924kB high:4708kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:786432kB managed:616760kB mlocked:0kB bounce:0kB free_pcp:1864kB local_pcp:1460kB free_cma:0kB
lowmem_reserve[]: 0 0 1280 1280
HighMem free:1308972kB min:512kB low:2180kB high:3848kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:1310720kB managed:1310720kB mlocked:0kB bounce:0kB free_pcp:656kB local_pcp:52kB free_cma:0kB
lowmem_reserve[]: 0 0 0 0
DMA: 5*4kB (UM) 1*8kB (M) 4*16kB (M) 3*32kB (UME) 4*64kB (ME) 3*128kB (ME) 5*256kB (UME) 6*512kB (UME) 3*1024kB (M) 3*2048kB (ME) 140*4096kB (M) = 587836kB
HighMem: 3*4kB (U) 2*8kB (U) 5*16kB (U) 0*32kB 1*64kB (U) 1*128kB (U) 0*256kB 0*512kB 0*1024kB 1*2048kB (U) 319*4096kB (M) = 1308972kB
0 total pagecache pages
0 pages in swap cache
Swap cache stats: add 0, delete 0, find 0/0
Free swap = 0kB
Total swap = 0kB
524288 pages RAM
327680 pages HighMem/MovableOnly
42418 pages reserved
code-patching: test failed at line 598
[...]
Updated /sys/kernel/debug/kernel_page_tables with the patch applied and default LOWMEM_SIZE=0x30000000 in the original bug report: https://bugzilla.kernel.org/show_bug.cgi?id=205099
Regards,
Erhard
^ permalink raw reply
* Re: [PATCH] powerpc/powermac: Add missing lockdep_register_key()
From: Christophe Leroy @ 2021-11-30 14:06 UTC (permalink / raw)
To: Erhard F.; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20211130145318.5fcdf633@yea>
Le 30/11/2021 à 14:53, Erhard F. a écrit :
> On Tue, 30 Nov 2021 10:32:42 +0100
> Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>> KeyWest i2c @0xf8001003 irq 42 /uni-n@f8000000/i2c@f8001000
>> BUG: key c2d00cbc has not been registered!
>> ------------[ cut here ]------------
>> DEBUG_LOCKS_WARN_ON(1)
>> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4801 lockdep_init_map_type+0x4c0/0xb4c
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.5-gentoo-PowerMacG4 #9
>> NIP: c01a9428 LR: c01a9428 CTR: 00000000
>> REGS: e1033cf0 TRAP: 0700 Not tainted (5.15.5-gentoo-PowerMacG4)
>> MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24002002 XER: 00000000
>>
>> GPR00: c01a9428 e1033db0 c2d1cf20 00000016 00000004 00000001 c01c0630 e1033a73
>> GPR08: 00000000 00000000 00000000 e1033db0 24002004 00000000 f8729377 00000003
>> GPR16: c1829a9c 00000000 18305357 c1416fc0 c1416f80 c006ac60 c2d00ca8 c1416f00
>> GPR24: 00000000 c21586f0 c2160000 00000000 c2d00cbc c2170000 c216e1a0 c2160000
>> NIP [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
>> LR [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
>> Call Trace:
>> [e1033db0] [c01a9428] lockdep_init_map_type+0x4c0/0xb4c (unreliable)
>> [e1033df0] [c1c177b8] kw_i2c_add+0x334/0x424
>> [e1033e20] [c1c18294] pmac_i2c_init+0x9ec/0xa9c
>> [e1033e80] [c1c1a790] smp_core99_probe+0xbc/0x35c
>> [e1033eb0] [c1c03cb0] kernel_init_freeable+0x190/0x5a4
>> [e1033f10] [c000946c] kernel_init+0x28/0x154
>> [e1033f30] [c0035148] ret_from_kernel_thread+0x14/0x1c
>>
>> Add missing lockdep_register_key()
>>
>> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/platforms/powermac/low_i2c.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
>> index f77a59b5c2e1..de34fa34c42d 100644
>> --- a/arch/powerpc/platforms/powermac/low_i2c.c
>> +++ b/arch/powerpc/platforms/powermac/low_i2c.c
>> @@ -582,6 +582,7 @@ static void __init kw_i2c_add(struct pmac_i2c_host_kw *host,
>> bus->close = kw_i2c_close;
>> bus->xfer = kw_i2c_xfer;
>> mutex_init(&bus->mutex);
>> + lockdep_register_key(&bus->lock_key);
>> lockdep_set_class(&bus->mutex, &bus->lock_key);
>> if (controller == busnode)
>> bus->flags = pmac_i2c_multibus;
>> --
>> 2.33.1
>>
>
> The patch applied on 5.15.5 but unfortunately was not enough to fix the lockdep bug.
>
> Originally was bug #200055 (https://bugzilla.kernel.org/show_bug.cgi?id=200055).
>
Is that still exactly the same backtrace , ie still pointing to
kw_i2c_add() ?
Because I see from original bug that in fact the offending commit added
lockdep_set_class() at 3 places. So it is likely that
lockdep_register_key() also needs to be added at the two other places.
So if the call trace now gives pmu_i2c_probe() or smu_i2c_probe() then
you could try and add lockdep_register_key() also there and see if it
fixes things.
Christophe
^ permalink raw reply
* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2021-11-30 13:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
--- Comment #48 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 299795
--> https://bugzilla.kernel.org/attachment.cgi?id=299795&action=edit
kernel_page_tables (5.15.5 + patch, INLINE KASAN, LOWMEM_SIZE=0x30000000,
PowerMac G4 DP)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 205099] KASAN hit at raid6_pq: BUG: Unable to handle kernel data access at 0x00f0fd0d
From: bugzilla-daemon @ 2021-11-30 13:58 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-205099-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=205099
--- Comment #47 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 299793
--> https://bugzilla.kernel.org/attachment.cgi?id=299793&action=edit
kernel_page_tables (5.15.5 + patch, OUTLINE KASAN, LOWMEM_SIZE=0x30000000,
PowerMac G4 DP)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH] powerpc/32s: Fix shift-out-of-bounds in KASAN init
From: Erhard F. @ 2021-11-30 13:45 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <15cbc3439d4ad988b225e2119ec99502a5cc6ad3.1638261744.git.christophe.leroy@csgroup.eu>
On Tue, 30 Nov 2021 09:42:37 +0100
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> ================================================================================
> UBSAN: shift-out-of-bounds in arch/powerpc/mm/kasan/book3s_32.c:22:23
> shift exponent -1 is negative
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.5-gentoo-PowerMacG4 #9
> Call Trace:
> [c214be60] [c0ba0048] dump_stack_lvl+0x80/0xb0 (unreliable)
> [c214be80] [c0b99288] ubsan_epilogue+0x10/0x5c
> [c214be90] [c0b98fe0] __ubsan_handle_shift_out_of_bounds+0x94/0x138
> [c214bf00] [c1c0f010] kasan_init_region+0xd8/0x26c
> [c214bf30] [c1c0ed84] kasan_init+0xc0/0x198
> [c214bf70] [c1c08024] setup_arch+0x18/0x54c
> [c214bfc0] [c1c037f0] start_kernel+0x90/0x33c
> [c214bff0] [00003610] 0x3610
> ================================================================================
>
> This happens when the directly mapped memory is a power of 2.
>
> Fix it by checking the shift and set the result to 0 when shift is -1
>
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215169
> Fixes: 7974c4732642 ("powerpc/32s: Implement dedicated kasan_init_region()")
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/mm/kasan/book3s_32.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/kasan/book3s_32.c b/arch/powerpc/mm/kasan/book3s_32.c
> index 202bd260a009..35b287b0a8da 100644
> --- a/arch/powerpc/mm/kasan/book3s_32.c
> +++ b/arch/powerpc/mm/kasan/book3s_32.c
> @@ -19,7 +19,8 @@ int __init kasan_init_region(void *start, size_t size)
> block = memblock_alloc(k_size, k_size_base);
>
> if (block && k_size_base >= SZ_128K && k_start == ALIGN(k_start, k_size_base)) {
> - int k_size_more = 1 << (ffs(k_size - k_size_base) - 1);
> + int shift = ffs(k_size - k_size_base);
> + int k_size_more = shift ? 1 << (shift - 1) : 0;
>
> setbat(-1, k_start, __pa(block), k_size_base, PAGE_KERNEL);
> if (k_size_more >= SZ_128K)
> --
> 2.33.1
>
Tested Christophes patch applied on 5.15.5 which works fine. Thanks!
I'll close the bugzilla report once it gets into stable.
Regards
Erhard
^ permalink raw reply
* Re: [PATCH] powerpc/powermac: Add missing lockdep_register_key()
From: Erhard F. @ 2021-11-30 13:53 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <69e4f55565bb45ebb0843977801b245af0c666fe.1638264741.git.christophe.leroy@csgroup.eu>
On Tue, 30 Nov 2021 10:32:42 +0100
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> KeyWest i2c @0xf8001003 irq 42 /uni-n@f8000000/i2c@f8001000
> BUG: key c2d00cbc has not been registered!
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(1)
> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:4801 lockdep_init_map_type+0x4c0/0xb4c
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.5-gentoo-PowerMacG4 #9
> NIP: c01a9428 LR: c01a9428 CTR: 00000000
> REGS: e1033cf0 TRAP: 0700 Not tainted (5.15.5-gentoo-PowerMacG4)
> MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24002002 XER: 00000000
>
> GPR00: c01a9428 e1033db0 c2d1cf20 00000016 00000004 00000001 c01c0630 e1033a73
> GPR08: 00000000 00000000 00000000 e1033db0 24002004 00000000 f8729377 00000003
> GPR16: c1829a9c 00000000 18305357 c1416fc0 c1416f80 c006ac60 c2d00ca8 c1416f00
> GPR24: 00000000 c21586f0 c2160000 00000000 c2d00cbc c2170000 c216e1a0 c2160000
> NIP [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
> LR [c01a9428] lockdep_init_map_type+0x4c0/0xb4c
> Call Trace:
> [e1033db0] [c01a9428] lockdep_init_map_type+0x4c0/0xb4c (unreliable)
> [e1033df0] [c1c177b8] kw_i2c_add+0x334/0x424
> [e1033e20] [c1c18294] pmac_i2c_init+0x9ec/0xa9c
> [e1033e80] [c1c1a790] smp_core99_probe+0xbc/0x35c
> [e1033eb0] [c1c03cb0] kernel_init_freeable+0x190/0x5a4
> [e1033f10] [c000946c] kernel_init+0x28/0x154
> [e1033f30] [c0035148] ret_from_kernel_thread+0x14/0x1c
>
> Add missing lockdep_register_key()
>
> Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/platforms/powermac/low_i2c.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/powermac/low_i2c.c b/arch/powerpc/platforms/powermac/low_i2c.c
> index f77a59b5c2e1..de34fa34c42d 100644
> --- a/arch/powerpc/platforms/powermac/low_i2c.c
> +++ b/arch/powerpc/platforms/powermac/low_i2c.c
> @@ -582,6 +582,7 @@ static void __init kw_i2c_add(struct pmac_i2c_host_kw *host,
> bus->close = kw_i2c_close;
> bus->xfer = kw_i2c_xfer;
> mutex_init(&bus->mutex);
> + lockdep_register_key(&bus->lock_key);
> lockdep_set_class(&bus->mutex, &bus->lock_key);
> if (controller == busnode)
> bus->flags = pmac_i2c_multibus;
> --
> 2.33.1
>
The patch applied on 5.15.5 but unfortunately was not enough to fix the lockdep bug.
Originally was bug #200055 (https://bugzilla.kernel.org/show_bug.cgi?id=200055).
Regards,
Erhard
^ permalink raw reply
* Re: [PATCH 1/2] tools/perf: Include global and local variants for p_stage_cyc sort key
From: Athira Rajeev @ 2021-11-30 13:29 UTC (permalink / raw)
To: Jiri Olsa
Cc: maddy, rnsastry, Arnaldo Carvalho de Melo, linux-perf-users,
Jiri Olsa, kjain, Namhyung Kim, linuxppc-dev
In-Reply-To: <YaUJwxHPTz1J7K1e@krava>
> On 29-Nov-2021, at 10:41 PM, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Nov 25, 2021 at 08:18:50AM +0530, Athira Rajeev wrote:
>> Sort key p_stage_cyc is used to present the latency
>> cycles spend in pipeline stages. perf tool has local
>> p_stage_cyc sort key to display this info. There is no
>> global variant available for this sort key. local variant
>> shows latency in a sinlge sample, whereas, global value
>> will be useful to present the total latency (sum of
>> latencies) in the hist entry. It represents latency
>> number multiplied by the number of samples.
>>
>> Add global (p_stage_cyc) and local variant
>> (local_p_stage_cyc) for this sort key. Use the
>> local_p_stage_cyc as default option for "mem" sort mode.
>> Also add this to list of dynamic sort keys.
>>
>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>> Reported-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>> tools/perf/util/hist.c | 4 +++-
>> tools/perf/util/hist.h | 3 ++-
>> tools/perf/util/sort.c | 34 +++++++++++++++++++++++++---------
>> tools/perf/util/sort.h | 3 ++-
>> 4 files changed, 32 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index b776465e04ef..0a8033b09e28 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -211,7 +211,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>> hists__new_col_len(hists, HISTC_MEM_BLOCKED, 10);
>> hists__new_col_len(hists, HISTC_LOCAL_INS_LAT, 13);
>> hists__new_col_len(hists, HISTC_GLOBAL_INS_LAT, 13);
>> - hists__new_col_len(hists, HISTC_P_STAGE_CYC, 13);
>> + hists__new_col_len(hists, HISTC_LOCAL_P_STAGE_CYC, 13);
>> + hists__new_col_len(hists, HISTC_GLOBAL_P_STAGE_CYC, 13);
>> +
>> if (symbol_conf.nanosecs)
>> hists__new_col_len(hists, HISTC_TIME, 16);
>> else
>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>> index 5343b62476e6..2752ce681108 100644
>> --- a/tools/perf/util/hist.h
>> +++ b/tools/perf/util/hist.h
>> @@ -75,7 +75,8 @@ enum hist_column {
>> HISTC_MEM_BLOCKED,
>> HISTC_LOCAL_INS_LAT,
>> HISTC_GLOBAL_INS_LAT,
>> - HISTC_P_STAGE_CYC,
>> + HISTC_LOCAL_P_STAGE_CYC,
>> + HISTC_GLOBAL_P_STAGE_CYC,
>> HISTC_NR_COLS, /* Last entry */
>> };
>>
>> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> index e9216a292a04..e978f7883e07 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -37,7 +37,7 @@ const char default_parent_pattern[] = "^sys_|^do_page_fault";
>> const char *parent_pattern = default_parent_pattern;
>> const char *default_sort_order = "comm,dso,symbol";
>> const char default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles";
>> -const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,p_stage_cyc";
>> +const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked,blocked,local_ins_lat,local_p_stage_cyc";
>> const char default_top_sort_order[] = "dso,symbol";
>> const char default_diff_sort_order[] = "dso,symbol";
>> const char default_tracepoint_sort_order[] = "trace";
>> @@ -46,8 +46,8 @@ const char *field_order;
>> regex_t ignore_callees_regex;
>> int have_ignore_callees = 0;
>> enum sort_mode sort__mode = SORT_MODE__NORMAL;
>> -const char *dynamic_headers[] = {"local_ins_lat", "p_stage_cyc"};
>> -const char *arch_specific_sort_keys[] = {"p_stage_cyc"};
>> +const char *dynamic_headers[] = {"local_ins_lat", "ins_lat", "local_p_stage_cyc", "p_stage_cyc"};
>
> so you also add global ins_lat, right? will this change
> some default behaviour?
Hi Jiri,
By default, if the architecture doesn’t have a dynamic header entry for this sort key ( taken care by arch_perf_header_entry() function ),
It will use the default se_header value in perf report. So default behaviour will not be changed.
>
>> +const char *arch_specific_sort_keys[] = {"local_p_stage_cyc", "p_stage_cyc"};
>
> nit.. both dynamic_headers and arch_specific_sort_keys could be static right?
Sure, this is used only in util/sort.c
I will add the change to make these static in next version.
Thanks
Athira
>
> thanks,
> jirka
>
>>
>> /*
>> * Replaces all occurrences of a char used with the:
>> @@ -1392,22 +1392,37 @@ struct sort_entry sort_global_ins_lat = {
>> };
>>
>> static int64_t
>> -sort__global_p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>> +sort__p_stage_cyc_cmp(struct hist_entry *left, struct hist_entry *right)
>> {
>> return left->p_stage_cyc - right->p_stage_cyc;
>> }
>>
>> +static int hist_entry__global_p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>> + size_t size, unsigned int width)
>> +{
>> + return repsep_snprintf(bf, size, "%-*u", width,
>> + he->p_stage_cyc * he->stat.nr_events);
>> +}
>> +
>> +
>> static int hist_entry__p_stage_cyc_snprintf(struct hist_entry *he, char *bf,
>> size_t size, unsigned int width)
>> {
>> return repsep_snprintf(bf, size, "%-*u", width, he->p_stage_cyc);
>> }
>>
>> -struct sort_entry sort_p_stage_cyc = {
>> - .se_header = "Pipeline Stage Cycle",
>> - .se_cmp = sort__global_p_stage_cyc_cmp,
>> +struct sort_entry sort_local_p_stage_cyc = {
>> + .se_header = "Local Pipeline Stage Cycle",
>> + .se_cmp = sort__p_stage_cyc_cmp,
>> .se_snprintf = hist_entry__p_stage_cyc_snprintf,
>> - .se_width_idx = HISTC_P_STAGE_CYC,
>> + .se_width_idx = HISTC_LOCAL_P_STAGE_CYC,
>> +};
>> +
>> +struct sort_entry sort_global_p_stage_cyc = {
>> + .se_header = "Pipeline Stage Cycle",
>> + .se_cmp = sort__p_stage_cyc_cmp,
>> + .se_snprintf = hist_entry__global_p_stage_cyc_snprintf,
>> + .se_width_idx = HISTC_GLOBAL_P_STAGE_CYC,
>> };
>>
>> struct sort_entry sort_mem_daddr_sym = {
>> @@ -1858,7 +1873,8 @@ static struct sort_dimension common_sort_dimensions[] = {
>> DIM(SORT_CODE_PAGE_SIZE, "code_page_size", sort_code_page_size),
>> DIM(SORT_LOCAL_INS_LAT, "local_ins_lat", sort_local_ins_lat),
>> DIM(SORT_GLOBAL_INS_LAT, "ins_lat", sort_global_ins_lat),
>> - DIM(SORT_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_p_stage_cyc),
>> + DIM(SORT_LOCAL_PIPELINE_STAGE_CYC, "local_p_stage_cyc", sort_local_p_stage_cyc),
>> + DIM(SORT_GLOBAL_PIPELINE_STAGE_CYC, "p_stage_cyc", sort_global_p_stage_cyc),
>> };
>>
>> #undef DIM
>> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
>> index 3c7518378d62..83abe5e6812a 100644
>> --- a/tools/perf/util/sort.h
>> +++ b/tools/perf/util/sort.h
>> @@ -235,7 +235,8 @@ enum sort_type {
>> SORT_CODE_PAGE_SIZE,
>> SORT_LOCAL_INS_LAT,
>> SORT_GLOBAL_INS_LAT,
>> - SORT_PIPELINE_STAGE_CYC,
>> + SORT_LOCAL_PIPELINE_STAGE_CYC,
>> + SORT_GLOBAL_PIPELINE_STAGE_CYC,
>>
>> /* branch stack specific sort keys */
>> __SORT_BRANCH_STACK,
>> --
>> 2.27.0
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.
From: Nathan Lynch @ 2021-11-30 12:39 UTC (permalink / raw)
To: mahesh; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <20211130093144.zpjcxnbkz3jsxfql@in.ibm.com>
Mahesh J Salgaonkar <mahesh@linux.ibm.com> writes:
> On 2021-11-29 22:53:41 Mon, Nathan Lynch wrote:
>> Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
>> > When certain PHB HW failure causes phyp to recover PHB, it marks the PE
>> > state as temporarily unavailable until recovery is complete. This also
>> > triggers an EEH handler in Linux which needs to notify drivers, and perform
>> > recovery. But before notifying the driver about the pci error it uses
>> > get_adapter_state()->get-sesnor-state() operation of the hotplug_slot to
>> > determine if the slot contains a device or not. if the slot is empty, the
>> > recovery is skipped entirely.
>> >
>> > However on certain PHB failures, the rtas call get-sesnor-state() returns
>> > extended busy error (9902) until PHB is recovered by phyp. Once PHB is
>> > recovered, the get-sensor-state() returns success with correct presence
>> > status. The rtas call interface rtas_get_sensor() loops over the rtas call
>> > on extended delay return code (9902) until the return value is either
>> > success (0) or error (-1). This causes the EEH handler to get stuck for ~6
>> > seconds before it could notify that the pci error has been detected and
>> > stop any active operations.
>>
>> I am curious whether you see any difference with "powerpc/rtas:
>> rtas_busy_delay() improvements" which was recently applied. It will
>> cause the the calling task to sleep in response to a 990x status instead
>> of immediately retrying:
>
> If it is still sleeping it may not help, however I will give a try.
Thanks. My thought is that with the old behavior of rtas_busy_delay(),
the repeated retries (potentially hundreds or thousands of
get-sensor-state calls) that occur before finally sleeping may be
prolonging the ongoing PHB recovery.
^ permalink raw reply
* [PATCH 2/2] powerpc/32: Remove _ENTRY() macro
From: Christophe Leroy @ 2021-11-30 12:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <68932ec2ba6b868d35006b96e90f0890f3da3c05.1638273868.git.christophe.leroy@csgroup.eu>
_ENTRY() is now redundant with _GLOBAL(). Remove it.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/ppc_asm.h | 4 ----
arch/powerpc/kernel/head_40x.S | 18 +++++++++---------
arch/powerpc/kernel/head_44x.S | 4 ++--
arch/powerpc/kernel/head_8xx.S | 4 ++--
arch/powerpc/kernel/head_book3s_32.S | 8 ++++----
arch/powerpc/kernel/head_fsl_booke.S | 6 +++---
6 files changed, 20 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 35544ba93352..ea90ef9da207 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -192,10 +192,6 @@ GLUE(.,name):
#else /* 32-bit */
-#define _ENTRY(n) \
- .globl n; \
-n:
-
#define _GLOBAL(n) \
.globl n; \
n:
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index 7d72ee5ab387..af63eb7deb69 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -52,8 +52,8 @@
* This is all going to change RSN when we add bi_recs....... -- Dan
*/
__HEAD
-_ENTRY(_stext);
-_ENTRY(_start);
+_GLOBAL(_stext);
+_GLOBAL(_start);
mr r31,r3 /* save device tree ptr */
@@ -81,19 +81,19 @@ turn_on_mmu:
*/
. = 0xc0
crit_save:
-_ENTRY(crit_r10)
+_GLOBAL(crit_r10)
.space 4
-_ENTRY(crit_r11)
+_GLOBAL(crit_r11)
.space 4
-_ENTRY(crit_srr0)
+_GLOBAL(crit_srr0)
.space 4
-_ENTRY(crit_srr1)
+_GLOBAL(crit_srr1)
.space 4
-_ENTRY(crit_r1)
+_GLOBAL(crit_r1)
.space 4
-_ENTRY(crit_dear)
+_GLOBAL(crit_dear)
.space 4
-_ENTRY(crit_esr)
+_GLOBAL(crit_esr)
.space 4
/*
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index 02d2928d1e01..6170a804e181 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -52,8 +52,8 @@
*
*/
__HEAD
-_ENTRY(_stext);
-_ENTRY(_start);
+_GLOBAL(_stext);
+_GLOBAL(_start);
/*
* Reserve a word at a fixed location to store the address
* of abatron_pteptrs
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 0d073b9fd52c..0b05f2be66b9 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -53,8 +53,8 @@
#define PAGE_SHIFT_8M 23
__HEAD
-_ENTRY(_stext);
-_ENTRY(_start);
+_GLOBAL(_stext);
+_GLOBAL(_start);
/* MPC8xx
* This port was done on an MBX board with an 860. Right now I only
diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index dae813539851..26a56cd26967 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -50,13 +50,13 @@
mtspr SPRN_DBAT##n##L,RB
__HEAD
-_ENTRY(_stext);
+_GLOBAL(_stext);
/*
* _start is defined this way because the XCOFF loader in the OpenFirmware
* on the powermac expects the entry point to be a procedure descriptor.
*/
-_ENTRY(_start);
+_GLOBAL(_start);
/*
* These are here for legacy reasons, the kernel used to
* need to look like a coff function entry for the pmac
@@ -781,7 +781,7 @@ relocate_kernel:
* r3 = dest addr, r4 = source addr, r5 = copy limit, r6 = start offset
* on exit, r3, r4, r5 are unchanged, r6 is updated to be >= r5.
*/
-_ENTRY(copy_and_flush)
+_GLOBAL(copy_and_flush)
addi r5,r5,-4
addi r6,r6,-4
4: li r0,L1_CACHE_BYTES/4
@@ -1075,7 +1075,7 @@ BEGIN_MMU_FTR_SECTION
END_MMU_FTR_SECTION_IFSET(MMU_FTR_USE_HIGH_BATS)
blr
-_ENTRY(update_bats)
+_GLOBAL(update_bats)
lis r4, 1f@h
ori r4, r4, 1f@l
tophys(r4, r4)
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 0a9a0f301474..29badf8176ed 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -54,8 +54,8 @@
*
*/
__HEAD
-_ENTRY(_stext);
-_ENTRY(_start);
+_GLOBAL(_stext);
+_GLOBAL(_start);
/*
* Reserve a word at a fixed location to store the address
* of abatron_pteptrs
@@ -154,7 +154,7 @@ _ENTRY(_start);
* if needed
*/
-_ENTRY(__early_start)
+_GLOBAL(__early_start)
LOAD_REG_ADDR_PIC(r20, kernstart_virt_addr)
lwz r20,0(r20)
--
2.33.1
^ permalink raw reply related
* [PATCH 1/2] powerpc/32: Remove remaining .stabs annotations
From: Christophe Leroy @ 2021-11-30 12:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel, kernel test robot
STABS debug format has been superseded long time ago by DWARF.
Remove the few remaining .stabs annotations from old 32 bits code.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/ppc_asm.h | 1 -
arch/powerpc/kernel/head_book3s_32.S | 3 ---
arch/powerpc/lib/checksum_32.S | 3 ---
arch/powerpc/lib/copy_32.S | 3 ---
4 files changed, 10 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index 7be24048b8d1..35544ba93352 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -197,7 +197,6 @@ GLUE(.,name):
n:
#define _GLOBAL(n) \
- .stabs __stringify(n:F-1),N_FUN,0,0,n;\
.globl n; \
n:
diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index 68e5c0a7e99d..dae813539851 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -50,9 +50,6 @@
mtspr SPRN_DBAT##n##L,RB
__HEAD
- .stabs "arch/powerpc/kernel/",N_SO,0,0,0f
- .stabs "head_book3s_32.S",N_SO,0,0,0f
-0:
_ENTRY(_stext);
/*
diff --git a/arch/powerpc/lib/checksum_32.S b/arch/powerpc/lib/checksum_32.S
index 27d9070617df..4541e8e29467 100644
--- a/arch/powerpc/lib/checksum_32.S
+++ b/arch/powerpc/lib/checksum_32.S
@@ -116,9 +116,6 @@ EXPORT_SYMBOL(__csum_partial)
EX_TABLE(8 ## n ## 7b, fault);
.text
- .stabs "arch/powerpc/lib/",N_SO,0,0,0f
- .stabs "checksum_32.S",N_SO,0,0,0f
-0:
CACHELINE_BYTES = L1_CACHE_BYTES
LG_CACHELINE_BYTES = L1_CACHE_SHIFT
diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
index a3bcf4786e4a..3e9c27c46331 100644
--- a/arch/powerpc/lib/copy_32.S
+++ b/arch/powerpc/lib/copy_32.S
@@ -57,9 +57,6 @@
EX_TABLE(8 ## n ## 7b,9 ## n ## 1b)
.text
- .stabs "arch/powerpc/lib/",N_SO,0,0,0f
- .stabs "copy_32.S",N_SO,0,0,0f
-0:
CACHELINE_BYTES = L1_CACHE_BYTES
LG_CACHELINE_BYTES = L1_CACHE_SHIFT
--
2.33.1
^ permalink raw reply related
* Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
From: Joakim Tjernlund @ 2021-11-30 11:56 UTC (permalink / raw)
To: regressions@leemhuis.info, leoyang.li@nxp.com,
Eugene_Bordenkircher@selinc.com, linux-usb@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Cc: gregkh@linuxfoundation.org, balbi@kernel.org
In-Reply-To: <MWHPR2201MB15205A333F1F610D332038AC91669@MWHPR2201MB1520.namprd22.prod.outlook.com>
On Mon, 2021-11-29 at 23:48 +0000, Eugene Bordenkircher wrote:
> Agreed,
>
> We are happy pick up the torch on this, but I'd like to try and hear from Joakim first before we do. The patch set is his, so I'd like to give him the opportunity. I think he's the only one that can add a truly proper description as well because he mentioned that this includes a "few more fixes" than just the one we ran into. I'd rather hear from him than try to reverse engineer what was being addressed.
>
> Joakim, if you are still watching the thread, would you like to take a stab at it? If I don't hear from you in a couple days, we'll pick up the torch and do what we can.
>
I am far away from this now and still on 4.19. I don't mind if you tweak tweak the patches for better "upstreamability"
Regards
Joakim
> Eugene T. Bordenkircher
>
> -----Original Message-----
> From: Leo Li <leoyang.li@nxp.com>
> Sent: Monday, November 29, 2021 3:37 PM
> To: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>; Thorsten Leemhuis <regressions@leemhuis.info>; jocke@infinera.com <joakim.tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-usb@vger.kernel.org
> Cc: gregkh@linuxfoundation.org; balbi@kernel.org
> Subject: RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list leads to unrecoverable loop.
>
> [Caution - External]
>
> > -----Original Message-----
> > From: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>
> > Sent: Monday, November 29, 2021 11:25 AM
> > To: Thorsten Leemhuis <regressions@leemhuis.info>; jocke@infinera.com
> > <joakim.tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> > usb@vger.kernel.org
> > Cc: Leo Li <leoyang.li@nxp.com>; gregkh@linuxfoundation.org;
> > balbi@kernel.org
> > Subject: RE: bug: usb: gadget: FSL_UDC_CORE Corrupted request list
> > leads to unrecoverable loop.
> >
> > The final result of our testing is that the patch set posted seems to
> > address all known defects in the Linux kernel. The mentioned
> > additional problems are entirely caused by the antivirus solution on
> > the windows box. The antivirus solution blocks the disconnect
> > messages from reaching the RNDIS driver so it has no idea the USB
> > device went away. There is nothing we can do to address this in the Linux kernel.
>
> Thanks for the confirmation.
>
> >
> > I propose we move forward with the patchset.
>
> I think that we should proceed to merge the patchset but it seems to need some cleanup for coding style issues and better description before submitted formally.
>
> >
> > Eugene T. Bordenkircher
> >
> > -----Original Message-----
> > From: Thorsten Leemhuis <regressions@leemhuis.info>
> > Sent: Thursday, November 25, 2021 5:59 AM
> > To: Eugene Bordenkircher <Eugene_Bordenkircher@selinc.com>; Thorsten
> > Leemhuis <regressions@leemhuis.info>; Joakim Tjernlund
> > <Joakim.Tjernlund@infinera.com>; linuxppc-dev@lists.ozlabs.org; linux-
> > usb@vger.kernel.org
> > Cc: leoyang.li@nxp.com; gregkh@linuxfoundation.org; balbi@kernel.org
> > Subject: Re: bug: usb: gadget: FSL_UDC_CORE Corrupted request list
> > leads to unrecoverable loop.
> >
> > Hi, this is your Linux kernel regression tracker speaking.
> >
> > Top-posting for once, to make this easy to process for everyone:
> >
> > Li Yang and Felipe Balbi: how to move on with this? It's quite an old
> > regression, but nevertheless it is one and thus should be fixed. Part
> > of my position is to make that happen and thus remind developers and
> > maintainers about this until the regression is resolved.
> >
> > Ciao, Thorsten
> >
> > On 16.11.21 20:11, Eugene Bordenkircher wrote:
> > > On 02.11.21 22:15, Joakim Tjernlund wrote:
> > > > On Sat, 2021-10-30 at 14:20 +0000, Joakim Tjernlund wrote:
> > > > > On Fri, 2021-10-29 at 17:14 +0000, Eugene Bordenkircher wrote:
> > > >
> > > > > > We've discovered a situation where the FSL udc driver
> > (drivers/usb/gadget/udc/fsl_udc_core.c) will enter a loop iterating
> > over the request queue, but the queue has been corrupted at some point
> > so it loops infinitely. I believe we have narrowed into the offending
> > code, but we are in need of assistance trying to find an appropriate
> > fix for the problem. The identified code appears to be in all
> > versions of the Linux kernel the driver exists in.
> > > > > >
> > > > > > The problem appears to be when handling a USB_REQ_GET_STATUS
> > request. The driver gets this request and then calls the
> > ch9getstatus() function. In this function, it starts a request by
> > "borrowing" the per device status_req, filling it in, and then queuing
> > it with a call to list_add_tail() to add the request to the endpoint
> > queue. Right before it exits the function however, it's calling
> > ep0_prime_status(), which is filling out that same status_req
> > structure and then queuing it with another call to list_add_tail() to
> > add the request to the endpoint queue. This adds two instances of the
> > exact same LIST_HEAD to the endpoint queue, which breaks the list
> > since the prev and next pointers end up pointing to the wrong things.
> > This ends up causing a hard loop the next time nuke() gets called, which happens on the next setup IRQ.
> > > > > >
> > > > > > I'm not sure what the appropriate fix to this problem is, mostly
> > > > > > due to
> > my lack of expertise in USB and this driver stack. The code has been
> > this way in the kernel for a very long time, which suggests that it
> > has been working, unless USB_REQ_GET_STATUS requests are never made.
> > This further suggests that there is something else going on that I don't understand.
> > Deleting the call to ep0_prime_status() and the following ep0stall()
> > call appears, on the surface, to get the device working again, but may
> > have side effects that I'm not seeing.
> > > > > >
> > > > > > I'm hopeful someone in the community can help provide some
> > information on what I may be missing or help come up with a solution
> > to the problem. A big thank you to anyone who would like to help out.
> > > > >
> > > > > Run into this to a while ago. Found the bug and a few more fixes.
> > > > > This is against 4.19 so you may have to tweak them a bit.
> > > > > Feel free to upstream them.
> > > >
> > > > Curious, did my patches help? Good to known once we upgrade as well.
> > >
> > > There's good news and bad news.
> > >
> > > The good news is that this appears to stop the driver from entering
> > > an infinite loop, which prevents the Linux system from locking up
> > > and never recovering. So I'm willing to say we've made the behavior
> > > better.
> > >
> > > The bad news is that once we get past this point, there is new bad
> > > behavior. What is on top of this driver in our system is the RNDIS
> > > gadget driver communicating to a Laptop running Win10 -1809.
> > > Everything appears to work fine with the Linux system until there is
> > > a USB disconnect. After the disconnect, the Linux side appears to
> > > continue on just fine, but the Windows side doesn't seem to
> > > recognize the disconnect, which causes the USB driver on that side
> > > to hang forever and eventually blue screen the box. This doesn't happen on
> > > all machines, just a select few. I think we can isolate the
> > > behavior to a specific antivirus/security software driver that is
> > > inserting itself into the USB stack and filtering the disconnect
> > > message, but we're still proving that.
> > >
> > > I'm about 90% certain this is a different problem and we can call
> > > this patchset good, at least for our test setup. My only hesitation
> > > is if the Linux side is sending a set of responses that are
> > > confusing the Windows side (specifically this antivirus) or not.
> > > I'd be content calling that a separate defect though and letting
> > > this one close up with that patchset.
> >
> > P.S.: As a Linux kernel regression tracker I'm getting a lot of
> > reports on my table. I can only look briefly into most of them.
> > Unfortunately therefore I sometimes will get things wrong or miss something important.
> > I hope that's not the case here; if you think it is, don't hesitate to
> > tell me about it in a public reply. That's in everyone's interest, as
> > what I wrote above might be misleading to everyone reading this; any
> > suggestion I gave they thus might sent someone reading this down the
> > wrong rabbit hole, which none of us wants.
> >
> > BTW, I have no personal interest in this issue, which is tracked using
> > regzbot, my Linux kernel regression tracking bot
> > (https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Furldefense.com%2Fv3%2F__https%3A%2F%2Feur01.safelinks.protection.outloo&data=04%7C01%7Cjoakim.tjernlund%40infinera.com%7Cb302ff817a8f4b3184c408d9b392bd1c%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C0%7C637738265108962168%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=D2TOGHLaeLnnmbJQG5VEY3CQ66GKtkpBOkFZ16WeW%2F4%3D&reserved=0
> > k.com/?url=https*3A*2F*2Furld__;JSUl!!O7uE89YCNVw!a6nsIMfn544OIzmshw3H
> > bMBVcbwor4cV2Q5OsST7-86jy_YZKvDsN-558Ris4wh8Zawz4puN$
> > efense.com%2Fv3%2F__https%3A%2F%2Flinux-
> > regtracking.leemhuis.info%2Fregzbot%2F__%3B!!O7uE89YCNVw!aHa5_mLM
> > nBeDjINlAtV19tBHm-
> > He9jbusXucMA5h7oonHvNFwYpOHAaaqqewPOuGK9HAzJUz%24&data
> > =04%7C01%7Cleoyang.li%40nxp.com%7C859ce1560a7344729cea08d9b35d2e
> > 67%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6377380350721308
> > 84%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luM
> > zIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ONQZyAKXNgok
> > 6LgYvnaAL7LVY%2B5Wl7pXglZDqWUJZMc%3D&reserved=0 ). I'm only
> > posting this mail to get things rolling again and hence don't need to
> > be CC on all further activities wrt to this regression.
> >
> > #regzbot title: usb: fsl_udc_core: corrupted request list leads to
> > unrecoverable loop
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox