* Re: [PATCH 5.4 00/52] 5.4.153-rc2 review
From: Greg Kroah-Hartman @ 2021-10-12 9:28 UTC (permalink / raw)
To: Naresh Kamboju
Cc: Song Liu, Florian Fainelli, bpf, Johan Almbladh, Pavel Machek,
linuxppc-dev, open list, lkft-triage, Jon Hunter, Linus Torvalds,
linux-stable, patches, Andrew Morton, Shuah Khan, Naveen N. Rao,
Guenter Roeck
In-Reply-To: <CA+G9fYt3vmhvuoFJ6p49DHiFE60oBeWUwuSLrh7vXwr=8_rpfg@mail.gmail.com>
On Tue, Oct 12, 2021 at 01:04:54PM +0530, Naresh Kamboju wrote:
> On Tue, 12 Oct 2021 at 12:16, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > This is the start of the stable review cycle for the 5.4.153 release.
> > There are 52 patches in this series, all will be posted as a response
> > to this one. If anyone has any issues with these being applied, please
> > let me know.
> >
> > Responses should be made by Thu, 14 Oct 2021 06:44:25 +0000.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.153-rc2.gz
> > or in the git tree and branch at:
> > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.4.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
>
> stable rc 5.4.153-rc2 Powerpc build failed.
>
> In file included from arch/powerpc/net/bpf_jit64.h:11,
> from arch/powerpc/net/bpf_jit_comp64.c:19:
> arch/powerpc/net/bpf_jit_comp64.c: In function 'bpf_jit_build_body':
> arch/powerpc/net/bpf_jit.h:32:9: error: expected expression before 'do'
> 32 | do { if (d) { (d)[idx] = instr; } idx++; } while (0)
> | ^~
> arch/powerpc/net/bpf_jit.h:33:33: note: in expansion of macro 'PLANT_INSTR'
> 33 | #define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
> | ^~~~~~~~~~~
> arch/powerpc/net/bpf_jit_comp64.c:415:41: note: in expansion of macro 'EMIT'
> 415 | EMIT(PPC_LI(dst_reg, 0));
> | ^~~~
> arch/powerpc/net/bpf_jit.h:33:33: note: in expansion of macro 'PLANT_INSTR'
> 33 | #define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
> | ^~~~~~~~~~~
> arch/powerpc/net/bpf_jit.h:41:33: note: in expansion of macro 'EMIT'
> 41 | #define PPC_ADDI(d, a, i) EMIT(PPC_INST_ADDI |
> ___PPC_RT(d) | \
> | ^~~~
> arch/powerpc/net/bpf_jit.h:44:33: note: in expansion of macro 'PPC_ADDI'
> 44 | #define PPC_LI(r, i) PPC_ADDI(r, 0, i)
> | ^~~~~~~~
> arch/powerpc/net/bpf_jit_comp64.c:415:46: note: in expansion of macro 'PPC_LI'
> 415 | EMIT(PPC_LI(dst_reg, 0));
> | ^~~~~~
> make[3]: *** [scripts/Makefile.build:262:
> arch/powerpc/net/bpf_jit_comp64.o] Error 1
> make[3]: Target '__build' not remade because of errors.
>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
Ok, I'm just going to go delete this patch from the queue now...
Thanks for the quick report.
greg k-h
^ permalink raw reply
* Re: [PATCH v1 01/10] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
From: Arnd Bergmann @ 2021-10-12 8:02 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-arch, linux-ia64, Kees Cook, Arnd Bergmann, Helge Deller,
Linux Kernel Mailing List, James E.J. Bottomley, Linux-MM,
Paul Mackerras, Parisc List, Greg Kroah-Hartman, Andrew Morton,
linuxppc-dev
In-Reply-To: <878ryy7m6v.fsf@mpe.ellerman.id.au>
On Tue, Oct 12, 2021 at 9:10 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> > 'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h
> > But it was by mistake added outside of __KERNEL__ section,
> > therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
> > arch/powerpc/include/asm") moved it to uapi/asm/elf.h
>
> ... it's been visible to userspace since the first commit moved it, ~13
> years ago in 2008, v2.6.27.
>
> > Move it back into asm/elf.h, this brings it back in line with
> > IA64 and PARISC architectures.
>
> Removing it from the uapi header risks breaking userspace, I doubt
> anything uses it, but who knows.
>
> Given how long it's been there I think it's a bit risky to remove it :/
I would not be too worried about it. While we should absolutely
never break existing binaries, changing the visibility of internal
structures in header files only breaks compiling applications
that do rely on these entries, and they really should not be using
this in the first place.
Arnd
^ permalink raw reply
* Re: [PATCH 5.4 00/52] 5.4.153-rc2 review
From: Naresh Kamboju @ 2021-10-12 7:34 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Song Liu, Florian Fainelli, bpf, Johan Almbladh, Pavel Machek,
linuxppc-dev, open list, lkft-triage, Jon Hunter, Linus Torvalds,
linux-stable, patches, Andrew Morton, Shuah Khan, Naveen N. Rao,
Guenter Roeck
In-Reply-To: <20211012064436.577746139@linuxfoundation.org>
On Tue, 12 Oct 2021 at 12:16, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> This is the start of the stable review cycle for the 5.4.153 release.
> There are 52 patches in this series, all will be posted as a response
> to this one. If anyone has any issues with these being applied, please
> let me know.
>
> Responses should be made by Thu, 14 Oct 2021 06:44:25 +0000.
> Anything received after that time might be too late.
>
> The whole patch series can be found in one patch at:
> https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.4.153-rc2.gz
> or in the git tree and branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-5.4.y
> and the diffstat can be found below.
>
> thanks,
>
> greg k-h
stable rc 5.4.153-rc2 Powerpc build failed.
In file included from arch/powerpc/net/bpf_jit64.h:11,
from arch/powerpc/net/bpf_jit_comp64.c:19:
arch/powerpc/net/bpf_jit_comp64.c: In function 'bpf_jit_build_body':
arch/powerpc/net/bpf_jit.h:32:9: error: expected expression before 'do'
32 | do { if (d) { (d)[idx] = instr; } idx++; } while (0)
| ^~
arch/powerpc/net/bpf_jit.h:33:33: note: in expansion of macro 'PLANT_INSTR'
33 | #define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
| ^~~~~~~~~~~
arch/powerpc/net/bpf_jit_comp64.c:415:41: note: in expansion of macro 'EMIT'
415 | EMIT(PPC_LI(dst_reg, 0));
| ^~~~
arch/powerpc/net/bpf_jit.h:33:33: note: in expansion of macro 'PLANT_INSTR'
33 | #define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
| ^~~~~~~~~~~
arch/powerpc/net/bpf_jit.h:41:33: note: in expansion of macro 'EMIT'
41 | #define PPC_ADDI(d, a, i) EMIT(PPC_INST_ADDI |
___PPC_RT(d) | \
| ^~~~
arch/powerpc/net/bpf_jit.h:44:33: note: in expansion of macro 'PPC_ADDI'
44 | #define PPC_LI(r, i) PPC_ADDI(r, 0, i)
| ^~~~~~~~
arch/powerpc/net/bpf_jit_comp64.c:415:46: note: in expansion of macro 'PPC_LI'
415 | EMIT(PPC_LI(dst_reg, 0));
| ^~~~~~
make[3]: *** [scripts/Makefile.build:262:
arch/powerpc/net/bpf_jit_comp64.o] Error 1
make[3]: Target '__build' not remade because of errors.
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
--
Linaro LKFT
https://lkft.linaro.org
^ permalink raw reply
* Re: [PATCH v1 01/10] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
From: Michael Ellerman @ 2021-10-12 7:10 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
Kees Cook, Greg Kroah-Hartman
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <8ff3ec195d695033b652e9971fba2dc5528f7151.1633964380.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> 'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h
Agree, but ...
> It was initially in module_64.c and commit 2d291e902791 ("Fix compile
> failure with non modular builds") moved it into asm/elf.h
>
> But it was by mistake added outside of __KERNEL__ section,
> therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
> arch/powerpc/include/asm") moved it to uapi/asm/elf.h
... it's been visible to userspace since the first commit moved it, ~13
years ago in 2008, v2.6.27.
> Move it back into asm/elf.h, this brings it back in line with
> IA64 and PARISC architectures.
Removing it from the uapi header risks breaking userspace, I doubt
anything uses it, but who knows.
Given how long it's been there I think it's a bit risky to remove it :/
> diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
> index b8425e3cfd81..64b523848cd7 100644
> --- a/arch/powerpc/include/asm/elf.h
> +++ b/arch/powerpc/include/asm/elf.h
> @@ -176,4 +176,11 @@ do { \
> /* Relocate the kernel image to @final_address */
> void relocate(unsigned long final_address);
>
> +/* There's actually a third entry here, but it's unused */
> +struct ppc64_opd_entry
> +{
> + unsigned long funcaddr;
> + unsigned long r2;
> +};
> +
> #endif /* _ASM_POWERPC_ELF_H */
> diff --git a/arch/powerpc/include/uapi/asm/elf.h b/arch/powerpc/include/uapi/asm/elf.h
> index 860c59291bfc..308857123a08 100644
> --- a/arch/powerpc/include/uapi/asm/elf.h
> +++ b/arch/powerpc/include/uapi/asm/elf.h
> @@ -289,12 +289,4 @@ typedef elf_fpreg_t elf_vsrreghalf_t32[ELF_NVSRHALFREG];
> /* Keep this the last entry. */
> #define R_PPC64_NUM 253
>
> -/* There's actually a third entry here, but it's unused */
> -struct ppc64_opd_entry
> -{
> - unsigned long funcaddr;
> - unsigned long r2;
> -};
Rather than removing it we can make it uapi only with:
#ifndef __KERNEL__
/* There's actually a third entry here, but it's unused */
struct ppc64_opd_entry
{
unsigned long funcaddr;
unsigned long r2;
};
#endif
And then we can do whatever we want with the kernel internal version.
cheers
^ permalink raw reply
* Re: [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
From: Helge Deller @ 2021-10-12 6:48 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch, linux-ia64, Kees Cook, Arnd Bergmann,
Greg Kroah-Hartman, Helge Deller, linux-kernel,
James E.J. Bottomley, linux-mm, Paul Mackerras, linux-parisc,
Andrew Morton, linuxppc-dev
In-Reply-To: <09bcd71a-baae-92b7-4c89-c8d446396d1c@csgroup.eu>
* Christophe Leroy <christophe.leroy@csgroup.eu>:
>
>
> Le 12/10/2021 à 08:02, Helge Deller a écrit :
> > On 10/11/21 17:25, Christophe Leroy wrote:
> > > Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor'
> > > to know whether arch has function descriptors.
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> > > ---
> > > arch/ia64/include/asm/sections.h | 4 ++--
> > > arch/parisc/include/asm/sections.h | 6 ++++--
> > > arch/powerpc/include/asm/sections.h | 6 ++++--
> > > include/asm-generic/sections.h | 3 ++-
> > > 4 files changed, 12 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
> > > index 35f24e52149a..80f5868afb06 100644
> > > --- a/arch/ia64/include/asm/sections.h
> > > +++ b/arch/ia64/include/asm/sections.h
> > > @@ -7,6 +7,8 @@
> > > * David Mosberger-Tang <davidm@hpl.hp.com>
> > > */
> > >
> > > +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > +
> > > #include <linux/elf.h>
> > > #include <linux/uaccess.h>
> > > #include <asm-generic/sections.h>
> > > @@ -27,8 +29,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
> > > extern char __start_unwind[], __end_unwind[];
> > > extern char __start_ivt_text[], __end_ivt_text[];
> > >
> > > -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > -
> > > #undef dereference_function_descriptor
> > > static inline void *dereference_function_descriptor(void *ptr)
> > > {
> > > diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
> > > index bb52aea0cb21..2e781ee19b66 100644
> > > --- a/arch/parisc/include/asm/sections.h
> > > +++ b/arch/parisc/include/asm/sections.h
> > > @@ -2,6 +2,10 @@
> > > #ifndef _PARISC_SECTIONS_H
> > > #define _PARISC_SECTIONS_H
> > >
> > > +#ifdef CONFIG_64BIT
> > > +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > +#endif
> > > +
> > > /* nothing to see, move along */
> > > #include <asm-generic/sections.h>
> > >
> > > @@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
> > >
> > > #ifdef CONFIG_64BIT
> > >
> > > -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > -
> > > #undef dereference_function_descriptor
> > > void *dereference_function_descriptor(void *);
> > >
> > > diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
> > > index 32e7035863ac..b7f1ba04e756 100644
> > > --- a/arch/powerpc/include/asm/sections.h
> > > +++ b/arch/powerpc/include/asm/sections.h
> > > @@ -8,6 +8,10 @@
> > >
> > > #define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
> > >
> > > +#ifdef PPC64_ELF_ABI_v1
> > > +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > +#endif
> > > +
> > > #include <asm-generic/sections.h>
> > >
> > > extern bool init_mem_is_free;
> > > @@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
> > >
> > > #ifdef PPC64_ELF_ABI_v1
> > >
> > > -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> > > -
> > > #undef dereference_function_descriptor
> > > static inline void *dereference_function_descriptor(void *ptr)
> > > {
> > > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> > > index d16302d3eb59..1db5cfd69817 100644
> > > --- a/include/asm-generic/sections.h
> > > +++ b/include/asm-generic/sections.h
> > > @@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
> > > extern __visible const void __nosave_begin, __nosave_end;
> > >
> > > /* Function descriptor handling (if any). Override in asm/sections.h */
> > > -#ifndef dereference_function_descriptor
> > > +#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
> > > +#else
> >
> > why not
> > #ifndef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
> > instead of #if/#else ?
>
> To avoid changing it again in patch 6, or getting an ugly #ifndef/#else at
> the end.
Ok.
Building on parisc fails at multiple files like this:
CC mm/filemap.o
In file included from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/interrupt.h:21,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/trace_recursion.h:5,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/ftrace.h:10,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/perf_event.h:49,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/trace_events.h:10,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/trace/syscall.h:7,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/include/linux/syscalls.h:87,
from /home/cvs/parisc/git-kernel/linus-linux-2.6/init/initramfs.c:11:
/home/cvs/parisc/git-kernel/linus-linux-2.6/arch/parisc/include/asm/sections.h:7:9: error: unknown type name ‘Elf64_Fdesc’
7 | typedef Elf64_Fdesc funct_descr_t;
| ^~~~~~~~~~~
So, you still need e.g. this patch:
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index b041fb32a300..177983490e7c 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -4,6 +4,8 @@
#ifdef CONFIG_64BIT
#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
+#include <asm/elf.h>
+#include <linux/uaccess.h>
typedef Elf64_Fdesc funct_descr_t;
#endif
In general to save space I think it would be beneficial if the
dereference_kernel_function_descriptor() and
dereference_kernel_function_descriptor() functions would go as real
functions a c-file instead of just being inlined functions in the
asm-generic/sections.h header file.
Other than that it's a nice cleanup!
Helge
^ permalink raw reply related
* Re: [PATCH] powerpc: don't select KFENCE on platform PPC_FSL_BOOK3E
From: Michael Ellerman @ 2021-10-12 6:24 UTC (permalink / raw)
To: Liu Shixin, Marco Elver, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <77ce95e4-1af1-6536-5f0c-a573c648801a@huawei.com>
Liu Shixin <liushixin2@huawei.com> writes:
> kindly ping.
I was under the impression you were trying to debug why it wasn't
working with Christophe.
cheers
> On 2021/9/24 14:39, Liu Shixin wrote:
>> On platform PPC_FSL_BOOK3E, all lowmem is managed by tlbcam. That means
>> we didn't really map the kfence pool with page granularity. Therefore,
>> if KFENCE is enabled, the system will hit the following panic:
>>
>> BUG: Kernel NULL pointer dereference on read at 0x00000000
>> Faulting instruction address: 0xc01de598
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> BE PAGE_SIZE=4K SMP NR_CPUS=4 MPC8544 DS
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc3+ #298
>> NIP: c01de598 LR: c08ae9c4 CTR: 00000000
>> REGS: c0b4bea0 TRAP: 0300 Not tainted (5.12.0-rc3+)
>> MSR: 00021000 <CE,ME> CR: 24000228 XER: 20000000
>> DEAR: 00000000 ESR: 00000000
>> GPR00: c08ae9c4 c0b4bf60 c0ad64e0 ef720000 00021000 00000000 00000000 00000200
>> GPR08: c0ad5000 00000000 00000000 00000004 00000000 008fbb30 00000000 00000000
>> GPR16: 00000000 00000000 00000000 00000000 c0000000 00000000 00000000 00000000
>> GPR24: c08ca004 c08ca004 c0b6a0e0 c0b60000 c0b58f00 c0850000 c08ca000 ef720000
>> NIP [c01de598] kfence_protect+0x44/0x6c
>> LR [c08ae9c4] kfence_init+0xfc/0x2a4
>> Call Trace:
>> [c0b4bf60] [efffe160] 0xefffe160 (unreliable)
>> [c0b4bf70] [c08ae9c4] kfence_init+0xfc/0x2a4
>> [c0b4bfb0] [c0894d3c] start_kernel+0x3bc/0x574
>> [c0b4bff0] [c0000470] set_ivor+0x14c/0x188
>> Instruction dump:
>> 7c0802a6 8109d594 546a653a 90010014 54630026 39200000 7d48502e 2c0a0000
>> 41820010 554a0026 5469b53a 7d295214 <81490000> 38831000 554a003c 91490000
>> random: get_random_bytes called from print_oops_end_marker+0x40/0x78 with crng_init=0
>> ---[ end trace 0000000000000000 ]---
>>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> ---
>> arch/powerpc/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index d46db0bfb998..cffd57bcb5e4 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -185,7 +185,7 @@ config PPC
>> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
>> select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
>> select HAVE_ARCH_KGDB
>> - select HAVE_ARCH_KFENCE if PPC32
>> + select HAVE_ARCH_KFENCE if PPC32 && !PPC_FSL_BOOK3E
>> select HAVE_ARCH_MMAP_RND_BITS
>> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
>> select HAVE_ARCH_NVRAM_OPS
^ permalink raw reply
* Re: [PATCH] scsi: ibmvscsi: Use dma_alloc_coherent() instead of get_zeroed_page/dma_map_single()
From: kernel test robot @ 2021-10-12 6:12 UTC (permalink / raw)
To: Cai Huoqing
Cc: Tyrel Datwyler, kbuild-all, linux-scsi, Martin K. Petersen,
James E.J. Bottomley, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <20211010160121.539-1-caihuoqing@baidu.com>
[-- Attachment #1: Type: text/plain, Size: 5081 bytes --]
Hi Cai,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.15-rc5 next-20211011]
[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/Cai-Huoqing/scsi-ibmvscsi-Use-dma_alloc_coherent-instead-of-get_zeroed_page-dma_map_single/20211011-000515
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
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
# https://github.com/0day-ci/linux/commit/a1533699f9b84980097fc59d047b5292c1abab1b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Cai-Huoqing/scsi-ibmvscsi-Use-dma_alloc_coherent-instead-of-get_zeroed_page-dma_map_single/20211011-000515
git checkout a1533699f9b84980097fc59d047b5292c1abab1b
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/scsi/ibmvscsi/ibmvscsi.c: In function 'ibmvscsi_init_crq_queue':
>> drivers/scsi/ibmvscsi/ibmvscsi.c:334:21: error: 'struct crq_queue' has no member named 'msg'; did you mean 'msgs'?
334 | if (!queue->msg)
| ^~~
| msgs
drivers/scsi/ibmvscsi/ibmvscsi.c:388:60: error: 'struct crq_queue' has no member named 'msg'; did you mean 'msgs'?
388 | dma_free_coherent(hostdata->dev, PAGE_SIZE, queue->msg, queue->msg_token);
| ^~~
| msgs
vim +334 drivers/scsi/ibmvscsi/ibmvscsi.c
312
313 /**
314 * ibmvscsi_init_crq_queue() - Initializes and registers CRQ with hypervisor
315 * @queue: crq_queue to initialize and register
316 * @hostdata: ibmvscsi_host_data of host
317 * @max_requests: maximum requests (unused)
318 *
319 * Allocates a page for messages, maps it for dma, and registers
320 * the crq with the hypervisor.
321 * Returns zero on success.
322 */
323 static int ibmvscsi_init_crq_queue(struct crq_queue *queue,
324 struct ibmvscsi_host_data *hostdata,
325 int max_requests)
326 {
327 int rc;
328 int retrc;
329 struct vio_dev *vdev = to_vio_dev(hostdata->dev);
330
331 queue->size = PAGE_SIZE / sizeof(*queue->msgs);
332 queue->msgs = dma_alloc_coherent(hostdata->dev, PAGE_SIZE,
333 &queue->msg_token, GFP_KERNEL);
> 334 if (!queue->msg)
335 goto malloc_failed;
336
337 gather_partition_info();
338 set_adapter_info(hostdata);
339
340 retrc = rc = plpar_hcall_norets(H_REG_CRQ,
341 vdev->unit_address,
342 queue->msg_token, PAGE_SIZE);
343 if (rc == H_RESOURCE)
344 /* maybe kexecing and resource is busy. try a reset */
345 rc = ibmvscsi_reset_crq_queue(queue,
346 hostdata);
347
348 if (rc == H_CLOSED) {
349 /* Adapter is good, but other end is not ready */
350 dev_warn(hostdata->dev, "Partner adapter not ready\n");
351 retrc = 0;
352 } else if (rc != 0) {
353 dev_warn(hostdata->dev, "Error %d opening adapter\n", rc);
354 goto reg_crq_failed;
355 }
356
357 queue->cur = 0;
358 spin_lock_init(&queue->lock);
359
360 tasklet_init(&hostdata->srp_task, (void *)ibmvscsi_task,
361 (unsigned long)hostdata);
362
363 if (request_irq(vdev->irq,
364 ibmvscsi_handle_event,
365 0, "ibmvscsi", (void *)hostdata) != 0) {
366 dev_err(hostdata->dev, "couldn't register irq 0x%x\n",
367 vdev->irq);
368 goto req_irq_failed;
369 }
370
371 rc = vio_enable_interrupts(vdev);
372 if (rc != 0) {
373 dev_err(hostdata->dev, "Error %d enabling interrupts!!!\n", rc);
374 goto req_irq_failed;
375 }
376
377 return retrc;
378
379 req_irq_failed:
380 tasklet_kill(&hostdata->srp_task);
381 rc = 0;
382 do {
383 if (rc)
384 msleep(100);
385 rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
386 } while ((rc == H_BUSY) || (H_IS_LONG_BUSY(rc)));
387 reg_crq_failed:
388 dma_free_coherent(hostdata->dev, PAGE_SIZE, queue->msg, queue->msg_token);
389 malloc_failed:
390 return -1;
391 }
392
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74089 bytes --]
^ permalink raw reply
* Re: [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
From: Christophe Leroy @ 2021-10-12 6:11 UTC (permalink / raw)
To: Helge Deller, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Andrew Morton, James E.J. Bottomley,
Arnd Bergmann, Kees Cook, Greg Kroah-Hartman
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <91b38fce-8a5c-ccc7-fba5-b75b9769d4fc@gmx.de>
Le 12/10/2021 à 08:02, Helge Deller a écrit :
> On 10/11/21 17:25, Christophe Leroy wrote:
>> Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor'
>> to know whether arch has function descriptors.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/ia64/include/asm/sections.h | 4 ++--
>> arch/parisc/include/asm/sections.h | 6 ++++--
>> arch/powerpc/include/asm/sections.h | 6 ++++--
>> include/asm-generic/sections.h | 3 ++-
>> 4 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
>> index 35f24e52149a..80f5868afb06 100644
>> --- a/arch/ia64/include/asm/sections.h
>> +++ b/arch/ia64/include/asm/sections.h
>> @@ -7,6 +7,8 @@
>> * David Mosberger-Tang <davidm@hpl.hp.com>
>> */
>>
>> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> +
>> #include <linux/elf.h>
>> #include <linux/uaccess.h>
>> #include <asm-generic/sections.h>
>> @@ -27,8 +29,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
>> extern char __start_unwind[], __end_unwind[];
>> extern char __start_ivt_text[], __end_ivt_text[];
>>
>> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> -
>> #undef dereference_function_descriptor
>> static inline void *dereference_function_descriptor(void *ptr)
>> {
>> diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
>> index bb52aea0cb21..2e781ee19b66 100644
>> --- a/arch/parisc/include/asm/sections.h
>> +++ b/arch/parisc/include/asm/sections.h
>> @@ -2,6 +2,10 @@
>> #ifndef _PARISC_SECTIONS_H
>> #define _PARISC_SECTIONS_H
>>
>> +#ifdef CONFIG_64BIT
>> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> +#endif
>> +
>> /* nothing to see, move along */
>> #include <asm-generic/sections.h>
>>
>> @@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
>>
>> #ifdef CONFIG_64BIT
>>
>> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> -
>> #undef dereference_function_descriptor
>> void *dereference_function_descriptor(void *);
>>
>> diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
>> index 32e7035863ac..b7f1ba04e756 100644
>> --- a/arch/powerpc/include/asm/sections.h
>> +++ b/arch/powerpc/include/asm/sections.h
>> @@ -8,6 +8,10 @@
>>
>> #define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
>>
>> +#ifdef PPC64_ELF_ABI_v1
>> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> +#endif
>> +
>> #include <asm-generic/sections.h>
>>
>> extern bool init_mem_is_free;
>> @@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
>>
>> #ifdef PPC64_ELF_ABI_v1
>>
>> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
>> -
>> #undef dereference_function_descriptor
>> static inline void *dereference_function_descriptor(void *ptr)
>> {
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index d16302d3eb59..1db5cfd69817 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
>> extern __visible const void __nosave_begin, __nosave_end;
>>
>> /* Function descriptor handling (if any). Override in asm/sections.h */
>> -#ifndef dereference_function_descriptor
>> +#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
>> +#else
>
> why not
> #ifndef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
> instead of #if/#else ?
To avoid changing it again in patch 6, or getting an ugly #ifndef/#else
at the end.
>
>> #define dereference_function_descriptor(p) ((void *)(p))
>> #define dereference_kernel_function_descriptor(p) ((void *)(p))
>> #endif
>>
>
^ permalink raw reply
* Re: [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
From: Helge Deller @ 2021-10-12 6:02 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Andrew Morton, James E.J. Bottomley,
Arnd Bergmann, Kees Cook, Greg Kroah-Hartman
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <8db2a3ca2b26a8325c671baa3e0492914597f079.1633964380.git.christophe.leroy@csgroup.eu>
On 10/11/21 17:25, Christophe Leroy wrote:
> Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor'
> to know whether arch has function descriptors.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/ia64/include/asm/sections.h | 4 ++--
> arch/parisc/include/asm/sections.h | 6 ++++--
> arch/powerpc/include/asm/sections.h | 6 ++++--
> include/asm-generic/sections.h | 3 ++-
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
> index 35f24e52149a..80f5868afb06 100644
> --- a/arch/ia64/include/asm/sections.h
> +++ b/arch/ia64/include/asm/sections.h
> @@ -7,6 +7,8 @@
> * David Mosberger-Tang <davidm@hpl.hp.com>
> */
>
> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> +
> #include <linux/elf.h>
> #include <linux/uaccess.h>
> #include <asm-generic/sections.h>
> @@ -27,8 +29,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
> extern char __start_unwind[], __end_unwind[];
> extern char __start_ivt_text[], __end_ivt_text[];
>
> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> -
> #undef dereference_function_descriptor
> static inline void *dereference_function_descriptor(void *ptr)
> {
> diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
> index bb52aea0cb21..2e781ee19b66 100644
> --- a/arch/parisc/include/asm/sections.h
> +++ b/arch/parisc/include/asm/sections.h
> @@ -2,6 +2,10 @@
> #ifndef _PARISC_SECTIONS_H
> #define _PARISC_SECTIONS_H
>
> +#ifdef CONFIG_64BIT
> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> +#endif
> +
> /* nothing to see, move along */
> #include <asm-generic/sections.h>
>
> @@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
>
> #ifdef CONFIG_64BIT
>
> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> -
> #undef dereference_function_descriptor
> void *dereference_function_descriptor(void *);
>
> diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
> index 32e7035863ac..b7f1ba04e756 100644
> --- a/arch/powerpc/include/asm/sections.h
> +++ b/arch/powerpc/include/asm/sections.h
> @@ -8,6 +8,10 @@
>
> #define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
>
> +#ifdef PPC64_ELF_ABI_v1
> +#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> +#endif
> +
> #include <asm-generic/sections.h>
>
> extern bool init_mem_is_free;
> @@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
>
> #ifdef PPC64_ELF_ABI_v1
>
> -#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
> -
> #undef dereference_function_descriptor
> static inline void *dereference_function_descriptor(void *ptr)
> {
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index d16302d3eb59..1db5cfd69817 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
> extern __visible const void __nosave_begin, __nosave_end;
>
> /* Function descriptor handling (if any). Override in asm/sections.h */
> -#ifndef dereference_function_descriptor
> +#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
> +#else
why not
#ifndef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
instead of #if/#else ?
> #define dereference_function_descriptor(p) ((void *)(p))
> #define dereference_kernel_function_descriptor(p) ((void *)(p))
> #endif
>
^ permalink raw reply
* [PATCH 0/2] ftrace: make sure preemption disabled on recursion testing
From: 王贇 @ 2021-10-12 5:39 UTC (permalink / raw)
To: Guo Ren, Steven Rostedt, Ingo Molnar, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Colin Ian King, Masami Hiramatsu,
Peter Zijlstra (Intel), Nicholas Piggin, Jisheng Zhang,
linux-csky, linux-kernel, linux-parisc, linuxppc-dev, linux-riscv,
live-patching
The testing show that perf_ftrace_function_call() are using
smp_processor_id() with preemption enabled, all the checking
on CPU could be wrong after preemption, PATCH 1/2 will fix
that.
Besides, as Peter point out, the testing of recursion within
the section between ftrace_test_recursion_trylock()/_unlock()
pair also need the preemption disabled as the documentation
explained, PATCH 2/2 will make sure on that.
Michael Wang (2):
ftrace: disable preemption on the testing of recursion
ftrace: prevent preemption in perf_ftrace_function_call()
arch/csky/kernel/probes/ftrace.c | 2 --
arch/parisc/kernel/ftrace.c | 2 --
arch/powerpc/kernel/kprobes-ftrace.c | 2 --
arch/riscv/kernel/probes/ftrace.c | 2 --
arch/x86/kernel/kprobes/ftrace.c | 2 --
include/linux/trace_recursion.h | 10 +++++++++-
kernel/livepatch/patch.c | 6 ------
kernel/trace/trace_event_perf.c | 17 +++++++++++++----
kernel/trace/trace_functions.c | 5 -----
9 files changed, 22 insertions(+), 26 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH 0/2] ftrace: make sure preemption disabled on recursion testing
From: 王贇 @ 2021-10-12 5:41 UTC (permalink / raw)
To: Guo Ren, Steven Rostedt, Ingo Molnar, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Colin Ian King, Masami Hiramatsu,
Peter Zijlstra (Intel), Nicholas Piggin, Jisheng Zhang,
linux-csky, linux-kernel, linux-parisc, linuxppc-dev, linux-riscv,
live-patching
In-Reply-To: <8c7de46d-9869-aa5e-2bb9-5dbc2eda395e@linux.alibaba.com>
On 2021/10/12 下午1:39, 王贇 wrote:
> The testing show that perf_ftrace_function_call() are using
> smp_processor_id() with preemption enabled, all the checking
> on CPU could be wrong after preemption, PATCH 1/2 will fix
> that.
2/2 actually.
>
> Besides, as Peter point out, the testing of recursion within
> the section between ftrace_test_recursion_trylock()/_unlock()
> pair also need the preemption disabled as the documentation
> explained, PATCH 2/2 will make sure on that.
1/2 actually...
Regards,
Michael Wang
>
> Michael Wang (2):
> ftrace: disable preemption on the testing of recursion
> ftrace: prevent preemption in perf_ftrace_function_call()
>
> arch/csky/kernel/probes/ftrace.c | 2 --
> arch/parisc/kernel/ftrace.c | 2 --
> arch/powerpc/kernel/kprobes-ftrace.c | 2 --
> arch/riscv/kernel/probes/ftrace.c | 2 --
> arch/x86/kernel/kprobes/ftrace.c | 2 --
> include/linux/trace_recursion.h | 10 +++++++++-
> kernel/livepatch/patch.c | 6 ------
> kernel/trace/trace_event_perf.c | 17 +++++++++++++----
> kernel/trace/trace_functions.c | 5 -----
> 9 files changed, 22 insertions(+), 26 deletions(-)
>
^ permalink raw reply
* [PATCH 2/2] ftrace: prevent preemption in perf_ftrace_function_call()
From: 王贇 @ 2021-10-12 5:40 UTC (permalink / raw)
To: Guo Ren, Steven Rostedt, Ingo Molnar, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Colin Ian King, Masami Hiramatsu,
Peter Zijlstra (Intel), Nicholas Piggin, Jisheng Zhang,
linux-csky, linux-kernel, linux-parisc, linuxppc-dev, linux-riscv,
live-patching
In-Reply-To: <8c7de46d-9869-aa5e-2bb9-5dbc2eda395e@linux.alibaba.com>
With CONFIG_DEBUG_PREEMPT we observed reports like:
BUG: using smp_processor_id() in preemptible
caller is perf_ftrace_function_call+0x6f/0x2e0
CPU: 1 PID: 680 Comm: a.out Not tainted
Call Trace:
<TASK>
dump_stack_lvl+0x8d/0xcf
check_preemption_disabled+0x104/0x110
? optimize_nops.isra.7+0x230/0x230
? text_poke_bp_batch+0x9f/0x310
perf_ftrace_function_call+0x6f/0x2e0
...
__text_poke+0x5/0x620
text_poke_bp_batch+0x9f/0x310
This telling us the CPU could be changed after task is preempted, and
the checking on CPU before preemption will be invalid.
This patch just turn off preemption in perf_ftrace_function_call()
to prevent CPU changing.
CC: Steven Rostedt <rostedt@goodmis.org>
Reported-by: Abaci <abaci@linux.alibaba.com>
Signed-off-by: Michael Wang <yun.wang@linux.alibaba.com>
---
kernel/trace/trace_event_perf.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index 6aed10e..33c2f76 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -441,12 +441,19 @@ void perf_trace_buf_update(void *record, u16 type)
if (!rcu_is_watching())
return;
+ /*
+ * Prevent CPU changing from now on. rcu must
+ * be in watching if the task was migrated and
+ * scheduled.
+ */
+ preempt_disable_notrace();
+
if ((unsigned long)ops->private != smp_processor_id())
- return;
+ goto out;
bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (bit < 0)
- return;
+ goto out;
event = container_of(ops, struct perf_event, ftrace_ops);
@@ -468,16 +475,18 @@ void perf_trace_buf_update(void *record, u16 type)
entry = perf_trace_buf_alloc(ENTRY_SIZE, NULL, &rctx);
if (!entry)
- goto out;
+ goto unlock;
entry->ip = ip;
entry->parent_ip = parent_ip;
perf_trace_buf_submit(entry, ENTRY_SIZE, rctx, TRACE_FN,
1, ®s, &head, NULL);
-out:
+unlock:
ftrace_test_recursion_unlock(bit);
#undef ENTRY_SIZE
+out:
+ preempt_enable_notrace();
}
static int perf_ftrace_function_register(struct perf_event *event)
--
1.8.3.1
^ permalink raw reply related
* [PATCH 1/2] ftrace: disable preemption on the testing of recursion
From: 王贇 @ 2021-10-12 5:40 UTC (permalink / raw)
To: Guo Ren, Steven Rostedt, Ingo Molnar, James E.J. Bottomley,
Helge Deller, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek,
Joe Lawrence, Colin Ian King, Masami Hiramatsu,
Peter Zijlstra (Intel), Nicholas Piggin, Jisheng Zhang,
linux-csky, linux-kernel, linux-parisc, linuxppc-dev, linux-riscv,
live-patching
In-Reply-To: <8c7de46d-9869-aa5e-2bb9-5dbc2eda395e@linux.alibaba.com>
As the documentation explained, ftrace_test_recursion_trylock()
and ftrace_test_recursion_unlock() were supposed to disable and
enable preemption properly, however currently this work is done
outside of the function, which could be missing by mistake.
This path will make sure the preemption will be disabled when
trylock() succeed, and the unlock() will enable preemption when
the the testing of recursion are finished.
Reported-by: Abaci <abaci@linux.alibaba.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Michael Wang <yun.wang@linux.alibaba.com>
---
arch/csky/kernel/probes/ftrace.c | 2 --
arch/parisc/kernel/ftrace.c | 2 --
arch/powerpc/kernel/kprobes-ftrace.c | 2 --
arch/riscv/kernel/probes/ftrace.c | 2 --
arch/x86/kernel/kprobes/ftrace.c | 2 --
include/linux/trace_recursion.h | 10 +++++++++-
kernel/livepatch/patch.c | 6 ------
kernel/trace/trace_functions.c | 5 -----
8 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/arch/csky/kernel/probes/ftrace.c b/arch/csky/kernel/probes/ftrace.c
index ef2bb9b..dff7921 100644
--- a/arch/csky/kernel/probes/ftrace.c
+++ b/arch/csky/kernel/probes/ftrace.c
@@ -24,7 +24,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
return;
regs = ftrace_get_regs(fregs);
- preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)ip);
if (!p) {
p = get_kprobe((kprobe_opcode_t *)(ip - MCOUNT_INSN_SIZE));
@@ -64,7 +63,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
__this_cpu_write(current_kprobe, NULL);
}
out:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index 0a1e75a..3543496 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -216,7 +216,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
return;
regs = ftrace_get_regs(fregs);
- preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)ip);
if (unlikely(!p) || kprobe_disabled(p))
goto out;
@@ -245,7 +244,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
}
__this_cpu_write(current_kprobe, NULL);
out:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/powerpc/kernel/kprobes-ftrace.c b/arch/powerpc/kernel/kprobes-ftrace.c
index 7154d58..072ebe7 100644
--- a/arch/powerpc/kernel/kprobes-ftrace.c
+++ b/arch/powerpc/kernel/kprobes-ftrace.c
@@ -26,7 +26,6 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
return;
regs = ftrace_get_regs(fregs);
- preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)nip);
if (unlikely(!p) || kprobe_disabled(p))
goto out;
@@ -61,7 +60,6 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
__this_cpu_write(current_kprobe, NULL);
}
out:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/riscv/kernel/probes/ftrace.c b/arch/riscv/kernel/probes/ftrace.c
index aab85a8..7142ec4 100644
--- a/arch/riscv/kernel/probes/ftrace.c
+++ b/arch/riscv/kernel/probes/ftrace.c
@@ -15,7 +15,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
if (bit < 0)
return;
- preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)ip);
if (unlikely(!p) || kprobe_disabled(p))
goto out;
@@ -52,7 +51,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
__this_cpu_write(current_kprobe, NULL);
}
out:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/arch/x86/kernel/kprobes/ftrace.c b/arch/x86/kernel/kprobes/ftrace.c
index 596de2f..dd2ec14 100644
--- a/arch/x86/kernel/kprobes/ftrace.c
+++ b/arch/x86/kernel/kprobes/ftrace.c
@@ -25,7 +25,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
if (bit < 0)
return;
- preempt_disable_notrace();
p = get_kprobe((kprobe_opcode_t *)ip);
if (unlikely(!p) || kprobe_disabled(p))
goto out;
@@ -59,7 +58,6 @@ void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
__this_cpu_write(current_kprobe, NULL);
}
out:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
NOKPROBE_SYMBOL(kprobe_ftrace_handler);
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index a9f9c57..805f9c4 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -214,7 +214,14 @@ static __always_inline void trace_clear_recursion(int bit)
static __always_inline int ftrace_test_recursion_trylock(unsigned long ip,
unsigned long parent_ip)
{
- return trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START, TRACE_FTRACE_MAX);
+ int bit;
+
+ preempt_disable_notrace();
+ bit = trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START, TRACE_FTRACE_MAX);
+ if (bit < 0)
+ preempt_enable_notrace();
+
+ return bit;
}
/**
@@ -226,6 +233,7 @@ static __always_inline int ftrace_test_recursion_trylock(unsigned long ip,
static __always_inline void ftrace_test_recursion_unlock(int bit)
{
trace_clear_recursion(bit);
+ preempt_enable_notrace();
}
#endif /* CONFIG_TRACING */
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index e8029ae..6e66ccd 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -52,11 +52,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
bit = ftrace_test_recursion_trylock(ip, parent_ip);
if (WARN_ON_ONCE(bit < 0))
return;
- /*
- * A variant of synchronize_rcu() is used to allow patching functions
- * where RCU is not watching, see klp_synchronize_transition().
- */
- preempt_disable_notrace();
func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
stack_node);
@@ -120,7 +115,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
klp_arch_set_pc(fregs, (unsigned long)func->new_func);
unlock:
- preempt_enable_notrace();
ftrace_test_recursion_unlock(bit);
}
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 1f0e63f..9f1bfbe 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -186,7 +186,6 @@ static void function_trace_start(struct trace_array *tr)
return;
trace_ctx = tracing_gen_ctx();
- preempt_disable_notrace();
cpu = smp_processor_id();
data = per_cpu_ptr(tr->array_buffer.data, cpu);
@@ -194,7 +193,6 @@ static void function_trace_start(struct trace_array *tr)
trace_function(tr, ip, parent_ip, trace_ctx);
ftrace_test_recursion_unlock(bit);
- preempt_enable_notrace();
}
#ifdef CONFIG_UNWINDER_ORC
@@ -298,8 +296,6 @@ static inline void process_repeats(struct trace_array *tr,
if (bit < 0)
return;
- preempt_disable_notrace();
-
cpu = smp_processor_id();
data = per_cpu_ptr(tr->array_buffer.data, cpu);
if (atomic_read(&data->disabled))
@@ -324,7 +320,6 @@ static inline void process_repeats(struct trace_array *tr,
out:
ftrace_test_recursion_unlock(bit);
- preempt_enable_notrace();
}
static void
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC
From: Christophe Leroy @ 2021-10-12 6:02 UTC (permalink / raw)
To: Joel Stanley, Jordan Niethe; +Cc: linuxppc-dev
In-Reply-To: <20211012011350.395767-1-joel@jms.id.au>
Le 12/10/2021 à 03:13, Joel Stanley a écrit :
> The page_alloc.c code will call into __kernel_map_pages when
> DEBUG_PAGEALLOC is configured and enabled.
>
> As the implementation assumes hash, this should crash spectacularly if
> not for a bit of luck in __kernel_map_pages. In this function
> linear_map_hash_count is always zero, the for loop exits without doing
> any damage.
>
> There are no other platforms that determine if they support
> debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
> do that, this change turns the map/unmap into a noop when in radix
> mode and prints a warning once.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> I noticed this when I was looking at adding kfence support a while back.
> I've put that work aside and jpn has since gotten further than me, but I
> think this is a fix worth considering.
>
> arch/powerpc/include/asm/book3s/64/hash.h | 2 ++
> arch/powerpc/mm/book3s64/hash_utils.c | 2 +-
> arch/powerpc/mm/book3s64/pgtable.c | 12 ++++++++++++
> 3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
> index d959b0195ad9..674fe0e890dc 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash.h
> @@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start, unsigned long end,
> int nid, pgprot_t prot);
> int hash__remove_section_mapping(unsigned long start, unsigned long end);
>
> +void hash__kernel_map_pages(struct page *page, int numpages, int enable);
> +
> #endif /* !__ASSEMBLY__ */
> #endif /* __KERNEL__ */
> #endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index c145776d3ae5..cfd45245d009 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
> mmu_kernel_ssize, 0);
> }
>
> -void __kernel_map_pages(struct page *page, int numpages, int enable)
> +void hash__kernel_map_pages(struct page *page, int numpages, int enable)
> {
> unsigned long flags, vaddr, lmi;
> int i;
> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 9e16c7b1a6c5..0aefc272cd03 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -526,3 +526,15 @@ static int __init pgtable_debugfs_setup(void)
> return 0;
> }
> arch_initcall(pgtable_debugfs_setup);
> +
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +void __kernel_map_pages(struct page *page, int numpages, int enable)
> +{
> + if (radix_enabled()) {
> + pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
> + return;
> + }
> +
> + hash__kernel_map_pages(page, numpages, enable);
> +}
I think it would be better to do similar to most other functions (like
map_kernel_page() for instance):
In arch/powerpc/include/asm/book3s/64/pgtable.h do:
static inline void __kernel_map_pages(struct page *page, int numpages,
int enable)
{
if (radix_enabled())
radix__kernel_map_pages(...);
else
hash__kernel_map_pages(...);
}
Then in arch/powerpc/include/asm/book3s/64/radix.h do (or in
/arch/powerpc/mm/book3s64/radix_pgtable.c in you prefer ?):
static inline void radix__kernel_map_pages(struct page *page, int
numpages, int enable)
{
pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
}
> +#endif
>
^ permalink raw reply
* Re: [PATCH] powerpc: don't select KFENCE on platform PPC_FSL_BOOK3E
From: LEROY Christophe @ 2021-10-12 5:41 UTC (permalink / raw)
To: Liu Shixin, Marco Elver, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <77ce95e4-1af1-6536-5f0c-a573c648801a@huawei.com>
Le 12/10/2021 à 03:43, Liu Shixin a écrit :
> kindly ping.
Hi
Based on the discussion we had, this patch is not enough. It should at
least also de-activate DEBUG_PAGEALLOC,
However I'm looking at fixing it the other way round. Give me one week
or two.
Christophe
>
>
> On 2021/9/24 14:39, Liu Shixin wrote:
>> On platform PPC_FSL_BOOK3E, all lowmem is managed by tlbcam. That means
>> we didn't really map the kfence pool with page granularity. Therefore,
>> if KFENCE is enabled, the system will hit the following panic:
>>
>> BUG: Kernel NULL pointer dereference on read at 0x00000000
>> Faulting instruction address: 0xc01de598
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> BE PAGE_SIZE=4K SMP NR_CPUS=4 MPC8544 DS
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc3+ #298
>> NIP: c01de598 LR: c08ae9c4 CTR: 00000000
>> REGS: c0b4bea0 TRAP: 0300 Not tainted (5.12.0-rc3+)
>> MSR: 00021000 <CE,ME> CR: 24000228 XER: 20000000
>> DEAR: 00000000 ESR: 00000000
>> GPR00: c08ae9c4 c0b4bf60 c0ad64e0 ef720000 00021000 00000000 00000000 00000200
>> GPR08: c0ad5000 00000000 00000000 00000004 00000000 008fbb30 00000000 00000000
>> GPR16: 00000000 00000000 00000000 00000000 c0000000 00000000 00000000 00000000
>> GPR24: c08ca004 c08ca004 c0b6a0e0 c0b60000 c0b58f00 c0850000 c08ca000 ef720000
>> NIP [c01de598] kfence_protect+0x44/0x6c
>> LR [c08ae9c4] kfence_init+0xfc/0x2a4
>> Call Trace:
>> [c0b4bf60] [efffe160] 0xefffe160 (unreliable)
>> [c0b4bf70] [c08ae9c4] kfence_init+0xfc/0x2a4
>> [c0b4bfb0] [c0894d3c] start_kernel+0x3bc/0x574
>> [c0b4bff0] [c0000470] set_ivor+0x14c/0x188
>> Instruction dump:
>> 7c0802a6 8109d594 546a653a 90010014 54630026 39200000 7d48502e 2c0a0000
>> 41820010 554a0026 5469b53a 7d295214 <81490000> 38831000 554a003c 91490000
>> random: get_random_bytes called from print_oops_end_marker+0x40/0x78 with crng_init=0
>> ---[ end trace 0000000000000000 ]---
>>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> ---
>> arch/powerpc/Kconfig | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index d46db0bfb998..cffd57bcb5e4 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -185,7 +185,7 @@ config PPC
>> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
>> select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
>> select HAVE_ARCH_KGDB
>> - select HAVE_ARCH_KFENCE if PPC32
>> + select HAVE_ARCH_KFENCE if PPC32 && !PPC_FSL_BOOK3E
>> select HAVE_ARCH_MMAP_RND_BITS
>> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
>> select HAVE_ARCH_NVRAM_OPS
>
^ permalink raw reply
* [PATCH v2] tpm: ibmvtpm: Make use of dma_alloc_noncoherent()
From: Cai Huoqing @ 2021-10-12 3:24 UTC (permalink / raw)
To: caihuoqing
Cc: linux-kernel, Jason Gunthorpe, Jarkko Sakkinen, Paul Mackerras,
Peter Huewe, linuxppc-dev, linux-integrity
Replacing kmalloc/kfree/get_zeroed_page/free_page/dma_map_single/
dma_unmap_single() with dma_alloc_noncoherent/dma_free_noncoherent()
helps to reduce code size, and simplify the code, and the hardware
can keep DMA coherent itself.
Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
v1->v2:
*Change to dma_alloc/free_noncoherent from dma_alloc/free_coherent.
*Update changelog.
drivers/char/tpm/tpm_ibmvtpm.c | 63 +++++++++++-----------------------
1 file changed, 20 insertions(+), 43 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 3af4c07a9342..b4552f8400b8 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -356,15 +356,13 @@ static void tpm_ibmvtpm_remove(struct vio_dev *vdev)
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
- dma_unmap_single(ibmvtpm->dev, ibmvtpm->crq_dma_handle,
- CRQ_RES_BUF_SIZE, DMA_BIDIRECTIONAL);
- free_page((unsigned long)ibmvtpm->crq_queue.crq_addr);
-
- if (ibmvtpm->rtce_buf) {
- dma_unmap_single(ibmvtpm->dev, ibmvtpm->rtce_dma_handle,
- ibmvtpm->rtce_size, DMA_BIDIRECTIONAL);
- kfree(ibmvtpm->rtce_buf);
- }
+ dma_free_noncoherent(ibmvtpm->dev, CRQ_RES_BUF_SIZE, crq_q->crq_addr,
+ crq_q->crq_dma_handle, DMA_BIDIRECTIONAL);
+
+ if (ibmvtpm->rtce_buf)
+ dma_free_noncoherent(ibmvtpm->dev,
+ ibmvtpm->rtce_size, ibmvtpm->rtce_buf,
+ ibmvtpm->rtce_dma_handle, DMA_BIDIRECTIONAL);
kfree(ibmvtpm);
/* For tpm_ibmvtpm_get_desired_dma */
@@ -522,23 +520,12 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
return;
}
ibmvtpm->rtce_size = be16_to_cpu(crq->len);
- ibmvtpm->rtce_buf = kmalloc(ibmvtpm->rtce_size,
- GFP_ATOMIC);
- if (!ibmvtpm->rtce_buf) {
- dev_err(ibmvtpm->dev, "Failed to allocate memory for rtce buffer\n");
- return;
- }
-
- ibmvtpm->rtce_dma_handle = dma_map_single(ibmvtpm->dev,
- ibmvtpm->rtce_buf, ibmvtpm->rtce_size,
- DMA_BIDIRECTIONAL);
-
- if (dma_mapping_error(ibmvtpm->dev,
- ibmvtpm->rtce_dma_handle)) {
- kfree(ibmvtpm->rtce_buf);
- ibmvtpm->rtce_buf = NULL;
- dev_err(ibmvtpm->dev, "Failed to dma map rtce buffer\n");
- }
+ ibmvtpm->rtce_buf = dma_alloc_noncoherent(ibmvtpm->dev,
+ ibmvtpm->rtce_size,
+ &ibmvtpm->rtce_dma_handle,
+ DMA_BIDIRECTIONAL, GFP_ATOMIC);
+ if (!ibmvtpm->rtce_buf)
+ dev_err(ibmvtpm->dev, "Failed to dma allocate rtce buffer\n");
return;
case VTPM_GET_VERSION_RES:
@@ -618,22 +605,14 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
ibmvtpm->vdev = vio_dev;
crq_q = &ibmvtpm->crq_queue;
- crq_q->crq_addr = (struct ibmvtpm_crq *)get_zeroed_page(GFP_KERNEL);
- if (!crq_q->crq_addr) {
- dev_err(dev, "Unable to allocate memory for crq_addr\n");
- goto cleanup;
- }
crq_q->num_entry = CRQ_RES_BUF_SIZE / sizeof(*crq_q->crq_addr);
init_waitqueue_head(&crq_q->wq);
- ibmvtpm->crq_dma_handle = dma_map_single(dev, crq_q->crq_addr,
- CRQ_RES_BUF_SIZE,
- DMA_BIDIRECTIONAL);
-
- if (dma_mapping_error(dev, ibmvtpm->crq_dma_handle)) {
- dev_err(dev, "dma mapping failed\n");
+ crq_q->crq_addr = dma_alloc_noncoherent(dev, CRQ_RES_BUF_SIZE,
+ &ibmvtpm->crq_dma_handle,
+ DMA_BIDIRECTIONAL, GFP_KERNEL);
+ if (!crq_q->crq_addr)
goto cleanup;
- }
rc = plpar_hcall_norets(H_REG_CRQ, vio_dev->unit_address,
ibmvtpm->crq_dma_handle, CRQ_RES_BUF_SIZE);
@@ -642,7 +621,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
if (rc) {
dev_err(dev, "Unable to register CRQ rc=%d\n", rc);
- goto reg_crq_cleanup;
+ goto cleanup;
}
rc = request_irq(vio_dev->irq, ibmvtpm_interrupt, 0,
@@ -704,13 +683,11 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
do {
rc1 = plpar_hcall_norets(H_FREE_CRQ, vio_dev->unit_address);
} while (rc1 == H_BUSY || H_IS_LONG_BUSY(rc1));
-reg_crq_cleanup:
- dma_unmap_single(dev, ibmvtpm->crq_dma_handle, CRQ_RES_BUF_SIZE,
- DMA_BIDIRECTIONAL);
cleanup:
if (ibmvtpm) {
if (crq_q->crq_addr)
- free_page((unsigned long)crq_q->crq_addr);
+ dma_free_noncoherent(dev, CRQ_RES_BUF_SIZE, crq_q->crq_addr,
+ crq_q->crq_dma_handle, DMA_BIDIRECTIONAL);
kfree(ibmvtpm);
}
--
2.25.1
^ permalink raw reply related
* [PATCH v2] scsi: ibmvscsi: Use dma_alloc_noncoherent() instead of get_zeroed_page/dma_map_single()
From: Cai Huoqing @ 2021-10-12 3:23 UTC (permalink / raw)
To: caihuoqing
Cc: Tyrel Datwyler, linux-scsi, Martin K. Petersen,
James E.J. Bottomley, linux-kernel, Paul Mackerras, linuxppc-dev
Replacing get_zeroed_page/free_page/dma_map_single/dma_unmap_single()
with dma_alloc_noncoherent/dma_free_noncoherent() helps to reduce
code size, and simplify the code, and the hardware can keeep DMA
coherent itself.
Signed-off-by: Cai Huoqing <caihuoqing@baidu.com>
---
v1->v2:
*Change to dma_alloc/free_noncoherent from dma_alloc/free_coherent.
*Update changelog.
drivers/scsi/ibmvscsi/ibmvfc.c | 16 ++++------------
drivers/scsi/ibmvscsi/ibmvscsi.c | 29 +++++++++--------------------
2 files changed, 13 insertions(+), 32 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 1f1586ad48fe..6e95fd02fd25 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -869,8 +869,8 @@ static void ibmvfc_free_queue(struct ibmvfc_host *vhost,
{
struct device *dev = vhost->dev;
- dma_unmap_single(dev, queue->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
- free_page((unsigned long)queue->msgs.handle);
+ dma_free_noncoherent(dev, PAGE_SIZE, queue->msgs.handle,
+ queue->msg_token, DMA_BIDIRECTIONAL);
queue->msgs.handle = NULL;
ibmvfc_free_event_pool(vhost, queue);
@@ -5663,19 +5663,11 @@ static int ibmvfc_alloc_queue(struct ibmvfc_host *vhost,
return -ENOMEM;
}
- queue->msgs.handle = (void *)get_zeroed_page(GFP_KERNEL);
+ queue->msgs.handle = dma_alloc_noncoherent(dev, PAGE_SIZE, &queue->msg_token,
+ DMA_BIDIRECTIONAL, GFP_KERNEL);
if (!queue->msgs.handle)
return -ENOMEM;
- queue->msg_token = dma_map_single(dev, queue->msgs.handle, PAGE_SIZE,
- DMA_BIDIRECTIONAL);
-
- if (dma_mapping_error(dev, queue->msg_token)) {
- free_page((unsigned long)queue->msgs.handle);
- queue->msgs.handle = NULL;
- return -ENOMEM;
- }
-
queue->cur = 0;
queue->fmt = fmt;
queue->size = PAGE_SIZE / fmt_size;
diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index ea8e01f49cba..68409c298c74 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -151,10 +151,8 @@ static void ibmvscsi_release_crq_queue(struct crq_queue *queue,
msleep(100);
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while ((rc == H_BUSY) || (H_IS_LONG_BUSY(rc)));
- dma_unmap_single(hostdata->dev,
- queue->msg_token,
- queue->size * sizeof(*queue->msgs), DMA_BIDIRECTIONAL);
- free_page((unsigned long)queue->msgs);
+ dma_free_noncoherent(hostdata->dev, PAGE_SIZE,
+ queue->msgs, queue->msg_token, DMA_BIDIRECTIONAL);
}
/**
@@ -331,18 +329,12 @@ static int ibmvscsi_init_crq_queue(struct crq_queue *queue,
int retrc;
struct vio_dev *vdev = to_vio_dev(hostdata->dev);
- queue->msgs = (struct viosrp_crq *)get_zeroed_page(GFP_KERNEL);
-
- if (!queue->msgs)
- goto malloc_failed;
queue->size = PAGE_SIZE / sizeof(*queue->msgs);
-
- queue->msg_token = dma_map_single(hostdata->dev, queue->msgs,
- queue->size * sizeof(*queue->msgs),
- DMA_BIDIRECTIONAL);
-
- if (dma_mapping_error(hostdata->dev, queue->msg_token))
- goto map_failed;
+ queue->msgs = dma_alloc_noncoherent(hostdata->dev,
+ PAGE_SIZE, &queue->msg_token,
+ DMA_BIDIRECTIONAL, GFP_KERNEL);
+ if (!queue->msg)
+ goto malloc_failed;
gather_partition_info();
set_adapter_info(hostdata);
@@ -395,11 +387,8 @@ static int ibmvscsi_init_crq_queue(struct crq_queue *queue,
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while ((rc == H_BUSY) || (H_IS_LONG_BUSY(rc)));
reg_crq_failed:
- dma_unmap_single(hostdata->dev,
- queue->msg_token,
- queue->size * sizeof(*queue->msgs), DMA_BIDIRECTIONAL);
- map_failed:
- free_page((unsigned long)queue->msgs);
+ dma_free_noncoherent(hostdata->dev, PAGE_SIZE, queue->msg,
+ queue->msg_token, DMA_BIDIRECTIONAL);
malloc_failed:
return -1;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 02/52] powerpc/64s: guard optional TIDR SPR with CPU ftr test
From: Michael Ellerman @ 2021-10-12 2:08 UTC (permalink / raw)
To: Fabiano Rosas, Nicholas Piggin, kvm-ppc, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <87v9235rl2.fsf@linux.ibm.com>
Fabiano Rosas <farosas@linux.ibm.com> writes:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> The TIDR SPR only exists on POWER9. Avoid accessing it when the
>> feature bit for it is not set.
>
> Not related to this patch, but how does this work with compat mode? A P9
> compat mode guest would get an invalid instruction when trying to access
> this SPR?
Good question.
I assume you're talking about P9 compat mode on P10.
In general compat mode only applies to userspace, because it's
implemented by setting the PCR which only (mostly?) applies to PR=1.
I don't think there's any special casing in the ISA for the TIDR, so I
think it just falls into the unimplemented SPR case for mt/fspr.
That's documented in Book III section 5.4.4, in particular on page 1171
it says:
Execution of this instruction specifying an SPR number
that is undefined for the implementation causes one of
the following.
• if spr[0]=0:
- if MSR[PR]=1: Hypervisor Emulation Assistance interrupt
- if MSR[PR]=0: Hypervisor Emulation Assistance interrupt for SPR
0,4,5, and 6, and no operation (i.e., the instruction is treated
as a no-op) when LPCR[EVIRT]=0 and Hypervisor Emulation Assistance
interrupt when LPCR[EVIRT]=1 for all other SPRs
Linux doesn't set EVIRT, and I assume neither does phyp, so it behaves
like a nop.
We actually use that behaviour in xmon to detect that an SPR is not
implemented, by noticing that the mfspr has no effect on the target
register, see dump_one_spr().
We should really write some docs on compat mode in the linuxppc wiki
and/or Documentation ;)
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: don't select KFENCE on platform PPC_FSL_BOOK3E
From: Liu Shixin @ 2021-10-12 1:43 UTC (permalink / raw)
To: Marco Elver, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20210924063927.1341241-1-liushixin2@huawei.com>
kindly ping.
On 2021/9/24 14:39, Liu Shixin wrote:
> On platform PPC_FSL_BOOK3E, all lowmem is managed by tlbcam. That means
> we didn't really map the kfence pool with page granularity. Therefore,
> if KFENCE is enabled, the system will hit the following panic:
>
> BUG: Kernel NULL pointer dereference on read at 0x00000000
> Faulting instruction address: 0xc01de598
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K SMP NR_CPUS=4 MPC8544 DS
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.12.0-rc3+ #298
> NIP: c01de598 LR: c08ae9c4 CTR: 00000000
> REGS: c0b4bea0 TRAP: 0300 Not tainted (5.12.0-rc3+)
> MSR: 00021000 <CE,ME> CR: 24000228 XER: 20000000
> DEAR: 00000000 ESR: 00000000
> GPR00: c08ae9c4 c0b4bf60 c0ad64e0 ef720000 00021000 00000000 00000000 00000200
> GPR08: c0ad5000 00000000 00000000 00000004 00000000 008fbb30 00000000 00000000
> GPR16: 00000000 00000000 00000000 00000000 c0000000 00000000 00000000 00000000
> GPR24: c08ca004 c08ca004 c0b6a0e0 c0b60000 c0b58f00 c0850000 c08ca000 ef720000
> NIP [c01de598] kfence_protect+0x44/0x6c
> LR [c08ae9c4] kfence_init+0xfc/0x2a4
> Call Trace:
> [c0b4bf60] [efffe160] 0xefffe160 (unreliable)
> [c0b4bf70] [c08ae9c4] kfence_init+0xfc/0x2a4
> [c0b4bfb0] [c0894d3c] start_kernel+0x3bc/0x574
> [c0b4bff0] [c0000470] set_ivor+0x14c/0x188
> Instruction dump:
> 7c0802a6 8109d594 546a653a 90010014 54630026 39200000 7d48502e 2c0a0000
> 41820010 554a0026 5469b53a 7d295214 <81490000> 38831000 554a003c 91490000
> random: get_random_bytes called from print_oops_end_marker+0x40/0x78 with crng_init=0
> ---[ end trace 0000000000000000 ]---
>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> ---
> arch/powerpc/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index d46db0bfb998..cffd57bcb5e4 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -185,7 +185,7 @@ config PPC
> select HAVE_ARCH_KASAN if PPC32 && PPC_PAGE_SHIFT <= 14
> select HAVE_ARCH_KASAN_VMALLOC if PPC32 && PPC_PAGE_SHIFT <= 14
> select HAVE_ARCH_KGDB
> - select HAVE_ARCH_KFENCE if PPC32
> + select HAVE_ARCH_KFENCE if PPC32 && !PPC_FSL_BOOK3E
> select HAVE_ARCH_MMAP_RND_BITS
> select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
> select HAVE_ARCH_NVRAM_OPS
^ permalink raw reply
* [PATCH] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC
From: Joel Stanley @ 2021-10-12 1:13 UTC (permalink / raw)
To: Jordan Niethe; +Cc: linuxppc-dev
The page_alloc.c code will call into __kernel_map_pages when
DEBUG_PAGEALLOC is configured and enabled.
As the implementation assumes hash, this should crash spectacularly if
not for a bit of luck in __kernel_map_pages. In this function
linear_map_hash_count is always zero, the for loop exits without doing
any damage.
There are no other platforms that determine if they support
debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
do that, this change turns the map/unmap into a noop when in radix
mode and prints a warning once.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
I noticed this when I was looking at adding kfence support a while back.
I've put that work aside and jpn has since gotten further than me, but I
think this is a fix worth considering.
arch/powerpc/include/asm/book3s/64/hash.h | 2 ++
arch/powerpc/mm/book3s64/hash_utils.c | 2 +-
arch/powerpc/mm/book3s64/pgtable.c | 12 ++++++++++++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..674fe0e890dc 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start, unsigned long end,
int nid, pgprot_t prot);
int hash__remove_section_mapping(unsigned long start, unsigned long end);
+void hash__kernel_map_pages(struct page *page, int numpages, int enable);
+
#endif /* !__ASSEMBLY__ */
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
index c145776d3ae5..cfd45245d009 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
mmu_kernel_ssize, 0);
}
-void __kernel_map_pages(struct page *page, int numpages, int enable)
+void hash__kernel_map_pages(struct page *page, int numpages, int enable)
{
unsigned long flags, vaddr, lmi;
int i;
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 9e16c7b1a6c5..0aefc272cd03 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -526,3 +526,15 @@ static int __init pgtable_debugfs_setup(void)
return 0;
}
arch_initcall(pgtable_debugfs_setup);
+
+#ifdef CONFIG_DEBUG_PAGEALLOC
+void __kernel_map_pages(struct page *page, int numpages, int enable)
+{
+ if (radix_enabled()) {
+ pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
+ return;
+ }
+
+ hash__kernel_map_pages(page, numpages, enable);
+}
+#endif
--
2.33.0
^ permalink raw reply related
* Re: [PATCH] powerpc/boot: Use CONFIG_PPC_POWERNV to compile OPAL support
From: Joel Stanley @ 2021-10-11 23:21 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: linuxppc-dev
In-Reply-To: <20211011070356.99952-1-clg@kaod.org>
On Mon, 11 Oct 2021 at 07:42, Cédric Le Goater <clg@kaod.org> wrote:
>
> CONFIG_PPC64_BOOT_WRAPPER is selected by CPU_LITTLE_ENDIAN which is
> used to compile support for other platforms such as Microwatt. There
> is no need for OPAL calls on these.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> arch/powerpc/boot/serial.c | 2 +-
> arch/powerpc/boot/Makefile | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
> index 9a19e5905485..54d2522be485 100644
> --- a/arch/powerpc/boot/serial.c
> +++ b/arch/powerpc/boot/serial.c
> @@ -132,7 +132,7 @@ int serial_console_init(void)
> else if (dt_is_compatible(devp, "fsl,mpc5200-psc-uart"))
> rc = mpc5200_psc_console_init(devp, &serial_cd);
> #endif
> -#ifdef CONFIG_PPC64_BOOT_WRAPPER
> +#ifdef CONFIG_PPC_POWERNV
> else if (dt_is_compatible(devp, "ibm,opal-console-raw"))
> rc = opal_console_init(devp, &serial_cd);
> #endif
> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> index 089ee3ea55c8..9993c6256ad2 100644
> --- a/arch/powerpc/boot/Makefile
> +++ b/arch/powerpc/boot/Makefile
> @@ -123,7 +123,7 @@ src-wlib-y := string.S crt0.S stdio.c decompress.c main.c \
> oflib.c ofconsole.c cuboot.c
>
> src-wlib-$(CONFIG_PPC_MPC52xx) += mpc52xx-psc.c
> -src-wlib-$(CONFIG_PPC64_BOOT_WRAPPER) += opal-calls.S opal.c
> +src-wlib-$(CONFIG_PPC_POWERNV) += opal-calls.S opal.c
> ifndef CONFIG_PPC64_BOOT_WRAPPER
> src-wlib-y += crtsavres.S
> endif
> --
> 2.31.1
>
^ permalink raw reply
* Re: linux-next: build warnings in Linus' tree
From: Rob Herring @ 2021-10-11 20:42 UTC (permalink / raw)
To: Stephen Rothwell, Arnd Bergmann, Michael Ellerman
Cc: Linux Next Mailing List, PowerPC, Linux Kernel Mailing List
In-Reply-To: <20211011082704.3cff4568@canb.auug.org.au>
+Arnd in regards to removing platforms.
On Sun, Oct 10, 2021 at 4:27 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> [Cc'ing Rob]
>
> Rob: these warnings have been there for a long time ...
If anyone cares about these platforms, then the warnings should be
fixed by folks that care. If not, then perhaps the DT files should
just get removed.
FYI, u-boot removed mpc5xxx support in 2017, so maybe there's
similarly not a need to keep them in the kernel? It does appear NXP
will still sell you the parts though the last BSP was 2009.
Rob
^ permalink raw reply
* Re: [PATCH v3 02/52] powerpc/64s: guard optional TIDR SPR with CPU ftr test
From: Fabiano Rosas @ 2021-10-11 18:44 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20211004160049.1338837-3-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> The TIDR SPR only exists on POWER9. Avoid accessing it when the
> feature bit for it is not set.
Not related to this patch, but how does this work with compat mode? A P9
compat mode guest would get an invalid instruction when trying to access
this SPR?
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 12 ++++++++----
> arch/powerpc/xmon/xmon.c | 10 ++++++++--
> 2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 2acb1c96cfaf..f4a779fffd18 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -3767,7 +3767,8 @@ static void load_spr_state(struct kvm_vcpu *vcpu)
> mtspr(SPRN_EBBHR, vcpu->arch.ebbhr);
> mtspr(SPRN_EBBRR, vcpu->arch.ebbrr);
> mtspr(SPRN_BESCR, vcpu->arch.bescr);
> - mtspr(SPRN_TIDR, vcpu->arch.tid);
> + if (cpu_has_feature(CPU_FTR_P9_TIDR))
> + mtspr(SPRN_TIDR, vcpu->arch.tid);
> mtspr(SPRN_AMR, vcpu->arch.amr);
> mtspr(SPRN_UAMOR, vcpu->arch.uamor);
>
> @@ -3793,7 +3794,8 @@ static void store_spr_state(struct kvm_vcpu *vcpu)
> vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
> vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
> vcpu->arch.bescr = mfspr(SPRN_BESCR);
> - vcpu->arch.tid = mfspr(SPRN_TIDR);
> + if (cpu_has_feature(CPU_FTR_P9_TIDR))
> + vcpu->arch.tid = mfspr(SPRN_TIDR);
> vcpu->arch.amr = mfspr(SPRN_AMR);
> vcpu->arch.uamor = mfspr(SPRN_UAMOR);
> vcpu->arch.dscr = mfspr(SPRN_DSCR);
> @@ -3813,7 +3815,8 @@ struct p9_host_os_sprs {
> static void save_p9_host_os_sprs(struct p9_host_os_sprs *host_os_sprs)
> {
> host_os_sprs->dscr = mfspr(SPRN_DSCR);
> - host_os_sprs->tidr = mfspr(SPRN_TIDR);
> + if (cpu_has_feature(CPU_FTR_P9_TIDR))
> + host_os_sprs->tidr = mfspr(SPRN_TIDR);
> host_os_sprs->iamr = mfspr(SPRN_IAMR);
> host_os_sprs->amr = mfspr(SPRN_AMR);
> host_os_sprs->fscr = mfspr(SPRN_FSCR);
> @@ -3827,7 +3830,8 @@ static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
> mtspr(SPRN_UAMOR, 0);
>
> mtspr(SPRN_DSCR, host_os_sprs->dscr);
> - mtspr(SPRN_TIDR, host_os_sprs->tidr);
> + if (cpu_has_feature(CPU_FTR_P9_TIDR))
> + mtspr(SPRN_TIDR, host_os_sprs->tidr);
> mtspr(SPRN_IAMR, host_os_sprs->iamr);
>
> if (host_os_sprs->amr != vcpu->arch.amr)
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index dd8241c009e5..7958e5aae844 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -2107,8 +2107,14 @@ static void dump_300_sprs(void)
> if (!cpu_has_feature(CPU_FTR_ARCH_300))
> return;
>
> - printf("pidr = %.16lx tidr = %.16lx\n",
> - mfspr(SPRN_PID), mfspr(SPRN_TIDR));
> + if (cpu_has_feature(CPU_FTR_P9_TIDR)) {
> + printf("pidr = %.16lx tidr = %.16lx\n",
> + mfspr(SPRN_PID), mfspr(SPRN_TIDR));
> + } else {
> + printf("pidr = %.16lx\n",
> + mfspr(SPRN_PID));
> + }
> +
> printf("psscr = %.16lx\n",
> hv ? mfspr(SPRN_PSSCR) : mfspr(SPRN_PSSCR_PR));
^ permalink raw reply
* [PATCH 17/22] PCI/DPC: Use RESPONSE_IS_PCI_ERROR() to check read from hardware
From: Naveen Naidu @ 2021-10-11 18:08 UTC (permalink / raw)
To: bhelgaas
Cc: linux-pci, open list:PCI ENHANCED ERROR HANDLING EEH FOR POWERPC,
linux-kernel, Naveen Naidu, Oliver O'Halloran,
linux-kernel-mentees
In-Reply-To: <cover.1633972263.git.naveennaidu479@gmail.com>
An MMIO read from a PCI device that doesn't exist or doesn't respond
causes a PCI error. There's no real data to return to satisfy the
CPU read, so most hardware fabricates ~0 data.
Use RESPONSE_IS_PCI_ERROR() to check the response we get when we read
data from hardware.
This helps unify PCI error response checking and make error checks
consistent and easier to find.
Compile tested only.
Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
---
drivers/pci/pcie/dpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index c556e7beafe3..561c44d9429c 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -79,7 +79,7 @@ static bool dpc_completed(struct pci_dev *pdev)
u16 status;
pci_read_config_word(pdev, pdev->dpc_cap + PCI_EXP_DPC_STATUS, &status);
- if ((status != 0xffff) && (status & PCI_EXP_DPC_STATUS_TRIGGER))
+ if ((!RESPONSE_IS_PCI_ERROR(&status)) && (status & PCI_EXP_DPC_STATUS_TRIGGER))
return false;
if (test_bit(PCI_DPC_RECOVERING, &pdev->priv_flags))
@@ -312,7 +312,7 @@ static irqreturn_t dpc_irq(int irq, void *context)
pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, &status);
- if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT) || status == (u16)(~0))
+ if (!(status & PCI_EXP_DPC_STATUS_INTERRUPT) || RESPONSE_IS_PCI_ERROR(&status))
return IRQ_NONE;
pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS,
--
2.25.1
^ permalink raw reply related
* [PATCH 01/22] PCI: Add PCI_ERROR_RESPONSE and it's related defintions
From: Naveen Naidu @ 2021-10-11 17:37 UTC (permalink / raw)
To: bhelgaas
Cc: linux-samsung-soc, linux-rockchip, linux-pci, linuxppc-dev,
linux-kernel, linux-renesas-soc, Naveen Naidu,
bcm-kernel-feedback-list, linux-mediatek, linux-kernel-mentees,
linux-arm-kernel
In-Reply-To: <cover.1633972263.git.naveennaidu479@gmail.com>
An MMIO read from a PCI device that doesn't exist or doesn't respond
causes a PCI error. There's no real data to return to satisfy the
CPU read, so most hardware fabricates ~0 data.
Add a PCI_ERROR_RESPONSE definition for that and use it where
appropriate to make these checks consistent and easier to find.
Also add helper definitions SET_PCI_ERROR_RESPONSE and
RESPONSE_IS_PCI_ERROR to make the code more readable.
Signed-off-by: Naveen Naidu <naveennaidu479@gmail.com>
---
include/linux/pci.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index cd8aa6fce204..928c589bb5c4 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -154,6 +154,15 @@ enum pci_interrupt_pin {
/* The number of legacy PCI INTx interrupts */
#define PCI_NUM_INTX 4
+/*
+ * Reading from a device that doesn't respond typically returns ~0. A
+ * successful read from a device may also return ~0, so you need additional
+ * information to reliably identify errors.
+ */
+#define PCI_ERROR_RESPONSE (~0ULL)
+#define SET_PCI_ERROR_RESPONSE(val) (*val = ((typeof(*val)) PCI_ERROR_RESPONSE))
+#define RESPONSE_IS_PCI_ERROR(val) (*val == ((typeof(*val)) PCI_ERROR_RESPONSE))
+
/*
* pci_power_t values must match the bits in the Capabilities PME_Support
* and Control/Status PowerState fields in the Power Management capability.
--
2.25.1
^ permalink raw reply related
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