LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/pseries: remove unneeded semicolon
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Yang Li, mpe; +Cc: linuxppc-dev, paulus, linux-kernel
In-Reply-To: <1617672785-81372-1-git-send-email-yang.lee@linux.alibaba.com>

On Tue, 6 Apr 2021 09:33:05 +0800, Yang Li wrote:
> Eliminate the following coccicheck warning:
> ./arch/powerpc/platforms/pseries/lpar.c:1633:2-3: Unneeded semicolon

Applied to powerpc/next.

[1/1] powerpc/pseries: remove unneeded semicolon
      https://git.kernel.org/powerpc/c/01ed0510941ae1350c501977132bdb54630614e2

cheers

^ permalink raw reply

* Re: [PATCH v2] powerpc: iommu: fix build when neither PCI or IBMVIO is set
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: linux-kernel, Randy Dunlap
  Cc: linuxppc-dev, kernel test robot, Anton Blanchard
In-Reply-To: <20210404192623.10697-1-rdunlap@infradead.org>

On Sun, 4 Apr 2021 12:26:23 -0700, Randy Dunlap wrote:
> When neither CONFIG_PCI nor CONFIG_IBMVIO is set/enabled, iommu.c has a
> build error. The fault injection code is not useful in that kernel config,
> so make the FAIL_IOMMU option depend on PCI || IBMVIO.
> 
> Prevents this build error (warning escalated to error):
> ../arch/powerpc/kernel/iommu.c:178:30: error: 'fail_iommu_bus_notifier' defined but not used [-Werror=unused-variable]
>   178 | static struct notifier_block fail_iommu_bus_notifier = {

Applied to powerpc/next.

[1/1] powerpc: iommu: fix build when neither PCI or IBMVIO is set
      https://git.kernel.org/powerpc/c/b27dadecdf9102838331b9a0b41ffc1cfe288154

cheers

^ permalink raw reply

* Re: [PATCH v2] powerpc/64s: power4 nap fixup in C
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: linuxppc-dev, Nicholas Piggin; +Cc: Andreas Schwab
In-Reply-To: <20210406025508.821718-1-npiggin@gmail.com>

On Tue, 6 Apr 2021 12:55:08 +1000, Nicholas Piggin wrote:
> There is no need for this to be in asm, use the new intrrupt entry wrapper.

Applied to powerpc/next.

[1/1] powerpc/64s: power4 nap fixup in C
      https://git.kernel.org/powerpc/c/98db179a78dd8379e9d2cbfc3f00224168a9344c

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/64s: power4 nap fixup in C
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210312012044.3660743-1-npiggin@gmail.com>

On Fri, 12 Mar 2021 11:20:44 +1000, Nicholas Piggin wrote:
> There is no need for this to be in asm, use the new intrrupt entry wrapper.

Applied to powerpc/next.

[1/1] powerpc/64s: power4 nap fixup in C
      https://git.kernel.org/powerpc/c/98db179a78dd8379e9d2cbfc3f00224168a9344c

cheers

^ permalink raw reply

* Re: [PATCH v2 1/6] powerpc/mm/64s: Add _PAGE_KERNEL_ROX
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20210331003845.216246-1-mpe@ellerman.id.au>

On Wed, 31 Mar 2021 11:38:40 +1100, Michael Ellerman wrote:
> In the past we had a fallback definition for _PAGE_KERNEL_ROX, but we
> removed that in commit d82fd29c5a8c ("powerpc/mm: Distribute platform
> specific PAGE and PMD flags and definitions") and added definitions
> for each MMU family.
> 
> However we missed adding a definition for 64s, which was not really a
> bug because it's currently not used.
> 
> [...]

Applied to powerpc/next.

[1/6] powerpc/mm/64s: Add _PAGE_KERNEL_ROX
      https://git.kernel.org/powerpc/c/56bec2f9d4d05675cada96772a8a93010f4d82bf
[2/6] powerpc/pseries: Add key to flags in pSeries_lpar_hpte_updateboltedpp()
      https://git.kernel.org/powerpc/c/b56d55a5aa4aa9fc166595a7feb57f153ef7b555
[3/6] powerpc/64s: Use htab_convert_pte_flags() in hash__mark_rodata_ro()
      https://git.kernel.org/powerpc/c/2c02e656a29d5f64193eb93da92781bcf0517146
[4/6] powerpc/mm/64s/hash: Factor out change_memory_range()
      https://git.kernel.org/powerpc/c/6f223ebe9c3f3ed315a06cec156086f1f7f7ded1
[5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
      https://git.kernel.org/powerpc/c/87e65ad7bd3a84a992723753fcc23d31c2d063c2
[6/6] powerpc/mm/64s: Allow STRICT_KERNEL_RWX again
      https://git.kernel.org/powerpc/c/bd573a81312fd9d6520b1cc81a88fd29e670e1ff

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/mm: Revert "powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc"
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: linuxppc-dev, Aneesh Kumar K.V, mpe
In-Reply-To: <20210318034855.74513-1-aneesh.kumar@linux.ibm.com>

On Thu, 18 Mar 2021 09:18:55 +0530, Aneesh Kumar K.V wrote:
> This reverts commit 675bceb097e6 ("powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc")
> 
> All the related issues are fixed by the series
> https://lore.kernel.org/linux-mm/20200902114222.181353-1-aneesh.kumar@linux.ibm.com
> 
> Hence enable it back

Applied to powerpc/next.

[1/1] powerpc/mm: Revert "powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc"
      https://git.kernel.org/powerpc/c/937c49d10b4dc8e81ed1a24ffab8d70bba138af1

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/pseries: Only register vio drivers if vio bus exists
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: pmenzel
In-Reply-To: <20210316010938.525657-1-mpe@ellerman.id.au>

On Tue, 16 Mar 2021 12:09:38 +1100, Michael Ellerman wrote:
> The vio bus is a fake bus, which we use on pseries LPARs (guests) to
> discover devices provided by the hypervisor. There's no need or sense
> in creating the vio bus on bare metal systems.
> 
> Which is why commit 4336b9337824 ("powerpc/pseries: Make vio and
> ibmebus initcalls pseries specific") made the initialisation of the
> vio bus only happen in LPARs.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/pseries: Only register vio drivers if vio bus exists
      https://git.kernel.org/powerpc/c/11d92156f7a862091009d7655d19c1e7de37fc7a

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: dja
In-Reply-To: <20210315034159.315675-1-mpe@ellerman.id.au>

On Mon, 15 Mar 2021 14:41:59 +1100, Michael Ellerman wrote:
> The ".machine" directive allows changing the machine for which code is
> being generated. It's equivalent to passing an -mcpu option on the
> command line.
> 
> Although it can be useful, it's generally a bad idea because it adds
> another way to influence code generation separate from the flags
> passed via the build system. ie. if we need to build different pieces
> of code with different flags we should do that via our Makefiles, not
> using ".machine".
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/kexec: Don't use .machine ppc64 in trampoline_64.S
      https://git.kernel.org/powerpc/c/acd4dfeb49c8ec1071b1e67683c5779e97fdc5b9

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/64: Move security code into security.c
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20210326101201.1973552-1-mpe@ellerman.id.au>

On Fri, 26 Mar 2021 21:12:01 +1100, Michael Ellerman wrote:
> When the original spectre/meltdown mitigations were merged we put them
> in setup_64.c for lack of a better place.
> 
> Since then we created security.c for some of the other mitigation
> related code. But it should all be in there.
> 
> This sort of code movement can cause trouble for backports, but
> hopefully this code is relatively stable these days (famous last words).

Applied to powerpc/next.

[1/1] powerpc/64: Move security code into security.c
      https://git.kernel.org/powerpc/c/c6b4c9147f8b85d159f670d7bce71a93d16062a2

cheers

^ permalink raw reply

* Re: [PATCH 1/6] powerpc/mm/64s: Add _PAGE_KERNEL_ROX
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>

On Fri, 12 Feb 2021 00:51:25 +1100, Michael Ellerman wrote:
> In the past we had a fallback definition for _PAGE_KERNEL_ROX, but we
> removed that in commit d82fd29c5a8c ("powerpc/mm: Distribute platform
> specific PAGE and PMD flags and definitions") and added definitions
> for each MMU family.
> 
> However we missed adding a definition for 64s, which was not really a
> bug because it's currently not used.
> 
> [...]

Applied to powerpc/next.

[1/6] powerpc/mm/64s: Add _PAGE_KERNEL_ROX
      https://git.kernel.org/powerpc/c/56bec2f9d4d05675cada96772a8a93010f4d82bf
[2/6] powerpc/pseries: Add key to flags in pSeries_lpar_hpte_updateboltedpp()
      https://git.kernel.org/powerpc/c/b56d55a5aa4aa9fc166595a7feb57f153ef7b555
[3/6] powerpc/64s: Use htab_convert_pte_flags() in hash__mark_rodata_ro()
      https://git.kernel.org/powerpc/c/2c02e656a29d5f64193eb93da92781bcf0517146
[4/6] powerpc/mm/64s/hash: Factor out change_memory_range()
      https://git.kernel.org/powerpc/c/6f223ebe9c3f3ed315a06cec156086f1f7f7ded1
[5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
      https://git.kernel.org/powerpc/c/87e65ad7bd3a84a992723753fcc23d31c2d063c2
[6/6] powerpc/mm/64s: Allow STRICT_KERNEL_RWX again
      https://git.kernel.org/powerpc/c/bd573a81312fd9d6520b1cc81a88fd29e670e1ff

cheers

^ permalink raw reply

* Re: [PATCH v3 03/15] powerpc/align: Convert emulate_spe() to user_access_begin
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Christophe Leroy,
	Daniel Axtens, Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <c95a648fdf75992c9d88f3c73cc23e7537fcf2ad.1615555354.git.christophe.leroy@csgroup.eu>

On Fri, 12 Mar 2021 13:25:11 +0000 (UTC), Christophe Leroy wrote:
> This patch converts emulate_spe() to using user_access_begin
> logic.
> 
> Since commit 662bbcb2747c ("mm, sched: Allow uaccess in atomic with
> pagefault_disable()"), might_fault() doesn't fire when called from
> sections where pagefaults are disabled, which must be the case
> when using _inatomic variants of __get_user and __put_user. So
> the might_fault() in user_access_begin() is not a problem.
> 
> [...]

Applied to powerpc/next.

[03/15] powerpc/align: Convert emulate_spe() to user_access_begin
        https://git.kernel.org/powerpc/c/3fa3db32956d74c0784171ae0334685502bb169a

cheers

^ permalink raw reply

* Re: [PATCH 00/10] Convert signal32 to user read access by block
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Michael Ellerman, Christophe Leroy, cmr,
	Paul Mackerras
  Cc: linux-arch, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616151715.git.christophe.leroy@csgroup.eu>

On Fri, 19 Mar 2021 11:06:49 +0000 (UTC), Christophe Leroy wrote:
> Similarly to the work done earlier with writes, this series
> converts signal32 to using user_read_access_begin/end and
> unsafe_get_user() and friends.
> 
> Applies on to of the signal64 series, ie on merge-test (ca6e327fefb2)
> 
> Christophe Leroy (10):
>   signal: Add unsafe_get_compat_sigset()
>   powerpc/uaccess: Also perform 64 bits copies in
>     unsafe_copy_from_user() on ppc32
>   powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
>   powerpc/signal32: Rename save_user_regs_unsafe() and
>     save_general_regs_unsafe()
>   powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
>   powerpc/signal32: Perform access_ok() inside restore_user_regs()
>   powerpc/signal32: Reorder user reads in restore_tm_user_regs()
>   powerpc/signal32: Convert restore_[tm]_user_regs() to user access
>     block
>   powerpc/signal32: Convert do_setcontext[_tm]() to user access block
>   powerpc/signal32: Simplify logging in sigreturn()
> 
> [...]

Applied to powerpc/next.

[01/10] signal: Add unsafe_get_compat_sigset()
        https://git.kernel.org/powerpc/c/fb05121fd6a20f0830ff2a4420c51af6ca4ac6e7
[02/10] powerpc/uaccess: Also perform 64 bits copies in unsafe_copy_from_user() on ppc32
        https://git.kernel.org/powerpc/c/c1cc1570bc8d94f288060f262f11be8f7672578c
[03/10] powerpc/signal: Add unsafe_copy_ck{fpr/vsx}_from_user
        https://git.kernel.org/powerpc/c/7c11f8893a76ac4e86c07f4b57371d5fa593627f
[04/10] powerpc/signal32: Rename save_user_regs_unsafe() and save_general_regs_unsafe()
        https://git.kernel.org/powerpc/c/f918a81e209f24acb45cd935bcfb78d2c024f6a1
[05/10] powerpc/signal32: Remove ifdefery in middle of if/else in sigreturn()
        https://git.kernel.org/powerpc/c/ca9e1605cdd9473a0eb4d6da238d2524be12591a
[06/10] powerpc/signal32: Perform access_ok() inside restore_user_regs()
        https://git.kernel.org/powerpc/c/362471b3192e4184fff5fedee1ea20bdf637a0c8
[07/10] powerpc/signal32: Reorder user reads in restore_tm_user_regs()
        https://git.kernel.org/powerpc/c/036fc2cb1dc2245c2ea7d2f03c7af80417b6310c
[08/10] powerpc/signal32: Convert restore_[tm]_user_regs() to user access block
        https://git.kernel.org/powerpc/c/627b72bee84d6652e0af26617e71ce2b3c18fcd5
[09/10] powerpc/signal32: Convert do_setcontext[_tm]() to user access block
        https://git.kernel.org/powerpc/c/887f3ceb51cd34109ac17bfc98695162e299e657
[10/10] powerpc/signal32: Simplify logging in sigreturn()
        https://git.kernel.org/powerpc/c/c7393a71eb1abdda7e3a3ef798bae60de11540ec

cheers

^ permalink raw reply

* Re: [PATCH v3 1/2] powerpc/64s: Fix pte update for kernel memory on radix
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Jordan Niethe, linuxppc-dev; +Cc: naveen.n.rao, npiggin, cmr
In-Reply-To: <20210208032957.1232102-1-jniethe5@gmail.com>

On Mon, 8 Feb 2021 14:29:56 +1100, Jordan Niethe wrote:
> When adding a pte a ptesync is needed to order the update of the pte
> with subsequent accesses otherwise a spurious fault may be raised.
> 
> radix__set_pte_at() does not do this for performance gains. For
> non-kernel memory this is not an issue as any faults of this kind are
> corrected by the page fault handler.  For kernel memory these faults are
> not handled.  The current solution is that there is a ptesync in
> flush_cache_vmap() which should be called when mapping from the vmalloc
> region.
> 
> [...]

Applied to powerpc/next.

[1/2] powerpc/64s: Fix pte update for kernel memory on radix
      https://git.kernel.org/powerpc/c/b8b2f37cf632434456182e9002d63cbc4cccc50c
[2/2] selftests/powerpc: Test for spurious kernel memory faults on radix
      https://git.kernel.org/powerpc/c/29e3ea8cbd2958cf237b84652ec236803f2c6202

cheers

^ permalink raw reply

* Re: [PATCH 1/3] powernv/memtrace: Allow mmaping trace buffers
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Jordan Niethe, linuxppc-dev
In-Reply-To: <20210225032108.1458352-1-jniethe5@gmail.com>

On Thu, 25 Feb 2021 14:21:06 +1100, Jordan Niethe wrote:
> Let the memory removed from the linear mapping to be used for the trace
> buffers be mmaped. This is a useful way of providing cache-inhibited
> memory for the alignment_handler selftest.

Patches 1 & 2 applied to powerpc/next.

[1/3] powernv/memtrace: Allow mmaping trace buffers
      https://git.kernel.org/powerpc/c/08a022ad3dfafc7e33d4529015e14bb75179cacc
[2/3] selftests/powerpc: Suggest memtrace instead of /dev/mem for ci memory
      https://git.kernel.org/powerpc/c/812aa68ef7d4d71bed996468ead665092a3f8de9

cheers

^ permalink raw reply

* Re: [PATCH v2 0/8] Implement EBPF on powerpc32
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, songliubraving, Michael Ellerman, kafai,
	naveen.n.rao, Christophe Leroy, daniel, john.fastabend, sandipan,
	yhs, ast, kpsingh, andrii, Paul Mackerras
  Cc: netdev, bpf, linuxppc-dev, linux-kernel
In-Reply-To: <cover.1616430991.git.christophe.leroy@csgroup.eu>

On Mon, 22 Mar 2021 16:37:45 +0000 (UTC), Christophe Leroy wrote:
> This series implements extended BPF on powerpc32. For the implementation
> details, see the patch before the last.
> 
> The following operations are not implemented:
> 
> 		case BPF_ALU64 | BPF_DIV | BPF_X: /* dst /= src */
> 		case BPF_ALU64 | BPF_MOD | BPF_X: /* dst %= src */
> 		case BPF_STX | BPF_XADD | BPF_DW: /* *(u64 *)(dst + off) += src */
> 
> [...]

Applied to powerpc/next.

[1/8] powerpc/bpf: Remove classical BPF support for PPC32
      https://git.kernel.org/powerpc/c/6944caad78fc4de4ecd0364bbc9715b62b020965
[2/8] powerpc/bpf: Change register numbering for bpf_set/is_seen_register()
      https://git.kernel.org/powerpc/c/ed573b57e77a7860fe4026e1700faa2f6938caf1
[3/8] powerpc/bpf: Move common helpers into bpf_jit.h
      https://git.kernel.org/powerpc/c/f1b1583d5faa86cb3dcb7b740594868debad7c30
[4/8] powerpc/bpf: Move common functions into bpf_jit_comp.c
      https://git.kernel.org/powerpc/c/4ea76e90a97d22f86adbb10044d29d919e620f2e
[5/8] powerpc/bpf: Change values of SEEN_ flags
      https://git.kernel.org/powerpc/c/c426810fcf9f96e3b43d16039e41ecb959f6dc29
[6/8] powerpc/asm: Add some opcodes in asm/ppc-opcode.h for PPC32 eBPF
      https://git.kernel.org/powerpc/c/355a8d26cd0416e7e764e4db766cf91e773a03e7
[7/8] powerpc/bpf: Implement extended BPF on PPC32
      https://git.kernel.org/powerpc/c/51c66ad849a703d9bbfd7704c941827aed0fd9fd
[8/8] powerpc/bpf: Reallocate BPF registers to volatile registers when possible on PPC32
      https://git.kernel.org/powerpc/c/40272035e1d0edcd515ad45be297c4cce044536d

cheers

^ permalink raw reply

* Re: [PATCH] powerpc: Switch to relative jump labels
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Christophe Leroy,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <828348da7868eda953ce023994404dfc49603b64.1616514473.git.christophe.leroy@csgroup.eu>

On Tue, 23 Mar 2021 15:47:59 +0000 (UTC), Christophe Leroy wrote:
> Convert powerpc to relative jump labels.
> 
> Before the patch, pseries_defconfig vmlinux.o has:
> 9074 __jump_table  0003f2a0  0000000000000000  0000000000000000  01321fa8  2**0
> 
> With the patch, the same config gets:
> 9074 __jump_table  0002a0e0  0000000000000000  0000000000000000  01321fb4  2**0
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: Switch to relative jump labels
      https://git.kernel.org/powerpc/c/b0b3b2c78ec075cec4721986a95abbbac8c3da4f

cheers

^ permalink raw reply

* Re: [PATCH v2 00/15] powerpc: Cleanup of uaccess.h and adding asm goto for get_user()
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Christophe Leroy,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1615398265.git.christophe.leroy@csgroup.eu>

On Wed, 10 Mar 2021 17:46:39 +0000 (UTC), Christophe Leroy wrote:
> This series cleans up uaccess.h and adds asm goto for get_user()
> 
> v2:
> - Further clean ups
> - asm goto for get_user()
> - Move a few patches unrelated to put_user/get_user into another misc series.
> 
> [...]

Applied to powerpc/next.

[01/15] powerpc/uaccess: Remove __get_user_allowed() and unsafe_op_wrap()
        https://git.kernel.org/powerpc/c/8cdf748d557f15ae6f9e0d4108cc3ea6e1ee4419
[02/15] powerpc/uaccess: Define ___get_user_instr() for ppc32
        https://git.kernel.org/powerpc/c/9bd68dc5d7463cb959bff9ac4b6c7e578171de35
[03/15] powerpc/align: Convert emulate_spe() to user_access_begin
        https://git.kernel.org/powerpc/c/3fa3db32956d74c0784171ae0334685502bb169a
[04/15] powerpc/uaccess: Remove __get/put_user_inatomic()
        https://git.kernel.org/powerpc/c/bad956b8fe1a8b3b634d596ed2023ec30726cdf1
[05/15] powerpc/uaccess: Move get_user_instr helpers in asm/inst.h
        https://git.kernel.org/powerpc/c/35506a3e2d7c4d93cb564e23471a448cbd98f085
[06/15] powerpc/align: Don't use __get_user_instr() on kernel addresses
        https://git.kernel.org/powerpc/c/111631b5e9dae764754657aad00bd6cd1a805d0d
[07/15] powerpc/uaccess: Call might_fault() inconditionaly
        https://git.kernel.org/powerpc/c/ed0d9c66f97c6865e87fa6e3631bbc3919a31ad6
[08/15] powerpc/uaccess: Remove __unsafe_put_user_goto()
        https://git.kernel.org/powerpc/c/be15a165796598cd3929ca9aac56ba5ec69e41c1
[09/15] powerpc/uaccess: Remove __chk_user_ptr() in __get/put_user
        https://git.kernel.org/powerpc/c/028e15616857add3ba4951f989027675370b0e82
[10/15] powerpc/uaccess: Remove calls to __get_user_bad() and __put_user_bad()
        https://git.kernel.org/powerpc/c/9975f852ce1bf041a1a81bf882e29ee7a3b78ca6
[11/15] powerpc/uaccess: Split out __get_user_nocheck()
        https://git.kernel.org/powerpc/c/f904c22f2a9fb09fe705efdedbe4af9a30bdf633
[12/15] powerpc/uaccess: Rename __get/put_user_check/nocheck
        https://git.kernel.org/powerpc/c/17f8c0bc21bbb7d1fe729c7f656924a6ea72079b
[13/15] powerpc/uaccess: Refactor get/put_user() and __get/put_user()
        https://git.kernel.org/powerpc/c/e72fcdb26cde72985c418b39f72ecaa222e1f4d5
[14/15] powerpc/uaccess: Introduce __get_user_size_goto()
        https://git.kernel.org/powerpc/c/035785ab2826beb43cfa65a2df37d60074915a4d
[15/15] powerpc/uaccess: Use asm goto for get_user when compiler supports it
        https://git.kernel.org/powerpc/c/5cd29b1fd3e8f2b45fe6d011588d832417defe31

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/32: Remove powerpc specific definition of 'ptrdiff_t'
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Christophe Leroy,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e43d133bf52fa19e577f64f3a3a38cedc570377d.1617616601.git.christophe.leroy@csgroup.eu>

On Mon, 5 Apr 2021 09:57:27 +0000 (UTC), Christophe Leroy wrote:
> For unknown reason, old commit d27dfd388715 ("Import pre2.0.8")
> changed 'ptrdiff_t' from 'int' to 'long'.
> 
> GCC expects it as 'int' really, and this leads to the following
> warning when building KFENCE:
> 
>   CC      mm/kfence/report.o
> In file included from ./include/linux/printk.h:7,
>                  from ./include/linux/kernel.h:16,
>                  from mm/kfence/report.c:10:
> mm/kfence/report.c: In function 'kfence_report_error':
> ./include/linux/kern_levels.h:5:18: warning: format '%td' expects argument of type 'ptrdiff_t', but argument 6 has type 'long int' [-Wformat=]
>     5 | #define KERN_SOH "\001"  /* ASCII Start Of Header */
>       |                  ^~~~~~
> ./include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
>    11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
>       |                  ^~~~~~~~
> ./include/linux/printk.h:343:9: note: in expansion of macro 'KERN_ERR'
>   343 |  printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
>       |         ^~~~~~~~
> mm/kfence/report.c:213:3: note: in expansion of macro 'pr_err'
>   213 |   pr_err("Out-of-bounds %s at 0x%p (%luB %s of kfence-#%td):\n",
>       |   ^~~~~~
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/32: Remove powerpc specific definition of 'ptrdiff_t'
      https://git.kernel.org/powerpc/c/c46bbf5d2defae50d61ddf31502017ee8952af83

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/perf: Fix PMU constraint check for EBB events
From: Michael Ellerman @ 2021-04-10 14:28 UTC (permalink / raw)
  To: Athira Rajeev, mpe; +Cc: cascardo, maddy, linuxppc-dev
In-Reply-To: <1617725761-1464-1-git-send-email-atrajeev@linux.vnet.ibm.com>

On Tue, 6 Apr 2021 12:16:01 -0400, Athira Rajeev wrote:
> The power PMU group constraints includes check for EBB events
> to make sure all events in a group must agree on EBB. This
> will prevent scheduling EBB and non-EBB events together.
> But in the existing check, settings for constraint mask and
> value is interchanged. Patch fixes the same.
> 
> Before the patch, PMU selftest "cpu_event_pinned_vs_ebb_test"
> fails with below in dmesg logs. This happens because EBB event
> gets enabled along with a non-EBB cpu event.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/perf: Fix PMU constraint check for EBB events
      https://git.kernel.org/powerpc/c/10f8f96179ecc7f69c927f6d231f6d02736cea83

cheers

^ permalink raw reply

* RE: Bogus struct page layout on 32-bit
From: David Laight @ 2021-04-10 14:17 UTC (permalink / raw)
  To: 'Matthew Wilcox', kernel test robot
  Cc: kbuild-all@lists.01.org, clang-built-linux@googlegroups.com,
	Jesper Dangaard Brouer, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, Paul Mackerras, linux-fsdevel@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, David S. Miller,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20210410024313.GX2531743@casper.infradead.org>

From: Matthew Wilcox
> Sent: 10 April 2021 03:43
> On Sat, Apr 10, 2021 at 06:45:35AM +0800, kernel test robot wrote:
> > >> include/linux/mm_types.h:274:1: error: static_assert failed due to requirement
> '__builtin_offsetof(struct page, lru) == __builtin_offsetof(struct folio, lru)' "offsetof(struct page,
> lru) == offsetof(struct folio, lru)"
> >    FOLIO_MATCH(lru, lru);
> >    include/linux/mm_types.h:272:2: note: expanded from macro 'FOLIO_MATCH'
> >            static_assert(offsetof(struct page, pg) == offsetof(struct folio, fl))
> 
> Well, this is interesting.  pahole reports:
> 
> struct page {
>         long unsigned int          flags;                /*     0     4 */
>         /* XXX 4 bytes hole, try to pack */
>         union {
>                 struct {
>                         struct list_head lru;            /*     8     8 */
> ...
> struct folio {
>         union {
>                 struct {
>                         long unsigned int flags;         /*     0     4 */
>                         struct list_head lru;            /*     4     8 */
> 
> so this assert has absolutely done its job.
> 
> But why has this assert triggered?  Why is struct page layout not what
> we thought it was?  Turns out it's the dma_addr added in 2019 by commit
> c25fff7171be ("mm: add dma_addr_t to struct page").  On this particular
> config, it's 64-bit, and ppc32 requires alignment to 64-bit.  So
> the whole union gets moved out by 4 bytes.
> 
> Unfortunately, we can't just fix this by putting an 'unsigned long pad'
> in front of it.  It still aligns the entire union to 8 bytes, and then
> it skips another 4 bytes after the pad.
> 
> We can fix it like this ...
> 
> +++ b/include/linux/mm_types.h
> @@ -96,11 +96,12 @@ struct page {
>                         unsigned long private;
>                 };
>                 struct {        /* page_pool used by netstack */
> +                       unsigned long _page_pool_pad;
>                         /**
>                          * @dma_addr: might require a 64-bit value even on
>                          * 32-bit architectures.
>                          */
> -                       dma_addr_t dma_addr;
> +                       dma_addr_t dma_addr __packed;
>                 };
>                 struct {        /* slab, slob and slub */
>                         union {
> 
> but I don't know if GCC is smart enough to realise that dma_addr is now
> on an 8 byte boundary and it can use a normal instruction to access it,
> or whether it'll do something daft like use byte loads to access it.

I'm betting it will use byte accesses.
Checked - it does seem to use 4-byte accesses.
(godbolt is rather short of 32 bit compilers...)

> 
> We could also do:
> 
> +                       dma_addr_t dma_addr __packed __aligned(sizeof(void *));

I wonder if __aligned(n) should be defined as
	__attribute__((packed,aligned(n))
I don't think you ever want the 'unpacked' variant.

But explicitly reducing the alignment of single member is much
better than the habit of marking the structure 'packed'.

(Never mind the habit of adding __packed 'because we don't want
the compiler to add random padding.)

> 
> and I see pahole, at least sees this correctly:
> 
>                 struct {
>                         long unsigned int _page_pool_pad; /*     4     4 */
>                         dma_addr_t dma_addr __attribute__((__aligned__(4))); /*     8     8 */
>                 } __attribute__((__packed__)) __attribute__((__aligned__(4)));

Is the attribute on the struct an artifact of pahole?
It should just have an alignment of 4 without anything special.

> 
> This presumably affects any 32-bit architecture with a 64-bit phys_addr_t
> / dma_addr_t.  Advice, please?

Only those where a 64-bit value is 64-bit aligned.
So that excludes x86 (which can have 64-bit dma) but includes sparc32
(which probably doesn't).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: Bogus struct page layout on 32-bit
From: Matthew Wilcox @ 2021-04-10 14:06 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: Grygorii Strashko, kbuild-all, kernel test robot, Ivan Khoronzhuk,
	clang-built-linux, open list, Linux-MM, netdev@vger.kernel.org,
	Jesper Dangaard Brouer, linux-fsdevel, Paul Mackerras,
	Matteo Croce, linuxppc-dev, David S. Miller, Linux ARM
In-Reply-To: <CAC_iWjLXZ6-hhvmvee6r4R_N64u-hrnLqE_CSS1nQk+YaMQQnA@mail.gmail.com>

How about moving the flags into the union?  A bit messy, but we don't
have to play games with __packed__.

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 1210a8e41fad..f374d2f06255 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -68,16 +68,22 @@ struct mem_cgroup;
 #endif
 
 struct page {
-	unsigned long flags;		/* Atomic flags, some possibly
-					 * updated asynchronously */
 	/*
-	 * Five words (20/40 bytes) are available in this union.
-	 * WARNING: bit 0 of the first word is used for PageTail(). That
-	 * means the other users of this union MUST NOT use the bit to
+	 * This union is six words (24 / 48 bytes) in size.
+	 * The first word is reserved for atomic flags, often updated
+	 * asynchronously.  Use the PageFoo() macros to access it.  Some
+	 * of the flags can be reused for your own purposes, but the
+	 * word as a whole often contains other information and overwriting
+	 * it will cause functions like page_zone() and page_node() to stop
+	 * working correctly.
+	 *
+	 * Bit 0 of the second word is used for PageTail(). That
+	 * means the other users of this union MUST leave the bit zero to
 	 * avoid collision and false-positive PageTail().
 	 */
 	union {
 		struct {	/* Page cache and anonymous pages */
+			unsigned long flags;
 			/**
 			 * @lru: Pageout list, eg. active_list protected by
 			 * lruvec->lru_lock.  Sometimes used as a generic list
@@ -96,6 +102,8 @@ struct page {
 			unsigned long private;
 		};
 		struct {	/* page_pool used by netstack */
+			unsigned long _pp_flags;
+			unsigned long _pp_pad;
 			/**
 			 * @dma_addr: might require a 64-bit value even on
 			 * 32-bit architectures.
@@ -103,6 +111,7 @@ struct page {
 			dma_addr_t dma_addr;
 		};
 		struct {	/* slab, slob and slub */
+			unsigned long _slab_flags;
 			union {
 				struct list_head slab_list;
 				struct {	/* Partial pages */
@@ -130,6 +139,7 @@ struct page {
 			};
 		};
 		struct {	/* Tail pages of compound page */
+			unsigned long _tail1_flags;
 			unsigned long compound_head;	/* Bit zero is set */
 
 			/* First tail page only */
@@ -139,12 +149,14 @@ struct page {
 			unsigned int compound_nr; /* 1 << compound_order */
 		};
 		struct {	/* Second tail page of compound page */
+			unsigned long _tail2_flags;
 			unsigned long _compound_pad_1;	/* compound_head */
 			atomic_t hpage_pinned_refcount;
 			/* For both global and memcg */
 			struct list_head deferred_list;
 		};
 		struct {	/* Page table pages */
+			unsigned long _pt_flags;
 			unsigned long _pt_pad_1;	/* compound_head */
 			pgtable_t pmd_huge_pte; /* protected by page->ptl */
 			unsigned long _pt_pad_2;	/* mapping */
@@ -159,6 +171,7 @@ struct page {
 #endif
 		};
 		struct {	/* ZONE_DEVICE pages */
+			unsigned long _zd_flags;
 			/** @pgmap: Points to the hosting device page map. */
 			struct dev_pagemap *pgmap;
 			void *zone_device_data;
@@ -174,8 +187,11 @@ struct page {
 			 */
 		};
 
-		/** @rcu_head: You can use this to free a page by RCU. */
-		struct rcu_head rcu_head;
+		struct {
+			unsigned long _rcu_flags;
+			/** @rcu_head: You can use this to free a page by RCU. */
+			struct rcu_head rcu_head;
+		};
 	};
 
 	union {		/* This union is 4 bytes in size. */

^ permalink raw reply related

* Re: Bogus struct page layout on 32-bit
From: Ilias Apalodimas @ 2021-04-10  8:52 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Grygorii Strashko, kbuild-all, kernel test robot, Ivan Khoronzhuk,
	clang-built-linux, open list, Matthew Wilcox, Linux-MM,
	netdev@vger.kernel.org, Paul Mackerras, linux-fsdevel,
	Matteo Croce, linuxppc-dev, David S. Miller, Linux ARM
In-Reply-To: <20210410082158.79ad09a6@carbon>

+CC Grygorii for the cpsw part as Ivan's email is not valid anymore

Thanks for catching this. Interesting indeed...

On Sat, 10 Apr 2021 at 09:22, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
> On Sat, 10 Apr 2021 03:43:13 +0100
> Matthew Wilcox <willy@infradead.org> wrote:
>
> > On Sat, Apr 10, 2021 at 06:45:35AM +0800, kernel test robot wrote:
> > > >> include/linux/mm_types.h:274:1: error: static_assert failed due to requirement '__builtin_offsetof(struct page, lru) == __builtin_offsetof(struct folio, lru)' "offsetof(struct page, lru) == offsetof(struct folio, lru)"
> > >    FOLIO_MATCH(lru, lru);
> > >    include/linux/mm_types.h:272:2: note: expanded from macro 'FOLIO_MATCH'
> > >            static_assert(offsetof(struct page, pg) == offsetof(struct folio, fl))
> >
> > Well, this is interesting.  pahole reports:
> >
> > struct page {
> >         long unsigned int          flags;                /*     0     4 */
> >         /* XXX 4 bytes hole, try to pack */
> >         union {
> >                 struct {
> >                         struct list_head lru;            /*     8     8 */
> > ...
> > struct folio {
> >         union {
> >                 struct {
> >                         long unsigned int flags;         /*     0     4 */
> >                         struct list_head lru;            /*     4     8 */
> >
> > so this assert has absolutely done its job.
> >
> > But why has this assert triggered?  Why is struct page layout not what
> > we thought it was?  Turns out it's the dma_addr added in 2019 by commit
> > c25fff7171be ("mm: add dma_addr_t to struct page").  On this particular
> > config, it's 64-bit, and ppc32 requires alignment to 64-bit.  So
> > the whole union gets moved out by 4 bytes.
>
> Argh, good that you are catching this!
>
> > Unfortunately, we can't just fix this by putting an 'unsigned long pad'
> > in front of it.  It still aligns the entire union to 8 bytes, and then
> > it skips another 4 bytes after the pad.
> >
> > We can fix it like this ...
> >
> > +++ b/include/linux/mm_types.h
> > @@ -96,11 +96,12 @@ struct page {
> >                         unsigned long private;
> >                 };
> >                 struct {        /* page_pool used by netstack */
> > +                       unsigned long _page_pool_pad;
>
> I'm fine with this pad.  Matteo is currently proposing[1] to add a 32-bit
> value after @dma_addr, and he could use this area instead.
>
> [1] https://lore.kernel.org/netdev/20210409223801.104657-3-mcroce@linux.microsoft.com/
>
> When adding/changing this, we need to make sure that it doesn't overlap
> member @index, because network stack use/check page_is_pfmemalloc().
> As far as my calculations this is safe to add.  I always try to keep an
> eye out for this, but I wonder if we could have a build check like yours.
>
>
> >                         /**
> >                          * @dma_addr: might require a 64-bit value even on
> >                          * 32-bit architectures.
> >                          */
> > -                       dma_addr_t dma_addr;
> > +                       dma_addr_t dma_addr __packed;
> >                 };
> >                 struct {        /* slab, slob and slub */
> >                         union {
> >
> > but I don't know if GCC is smart enough to realise that dma_addr is now
> > on an 8 byte boundary and it can use a normal instruction to access it,
> > or whether it'll do something daft like use byte loads to access it.
> >
> > We could also do:
> >
> > +                       dma_addr_t dma_addr __packed __aligned(sizeof(void *));
> >
> > and I see pahole, at least sees this correctly:
> >
> >                 struct {
> >                         long unsigned int _page_pool_pad; /*     4     4 */
> >                         dma_addr_t dma_addr __attribute__((__aligned__(4))); /*     8     8 */
> >                 } __attribute__((__packed__)) __attribute__((__aligned__(4)));
> >
> > This presumably affects any 32-bit architecture with a 64-bit phys_addr_t
> > / dma_addr_t.  Advice, please?
>
> I'm not sure that the 32-bit behavior is with 64-bit (dma) addrs.
>
> I don't have any 32-bit boards with 64-bit DMA.  Cc. Ivan, wasn't your
> board (572x ?) 32-bit with driver 'cpsw' this case (where Ivan added
> XDP+page_pool) ?
>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
>

^ permalink raw reply

* [PATCH] powerpc/eeh: Fix EEH handling for hugepages in ioremap space.
From: Mahesh Salgaonkar @ 2021-04-10 10:53 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Aneesh Kumar K.V, Oliver O'Halloran, stable

During the EEH MMIO error checking, the current implementation fails to map
the (virtual) MMIO address back to the pci device on radix with hugepage
mappings for I/O. This results into failure to dispatch EEH event with no
recovery even when EEH capability has been enabled on the device.

eeh_check_failure(token)		# token = virtual MMIO address
  addr = eeh_token_to_phys(token);
  edev = eeh_addr_cache_get_dev(addr);
  if (!edev)
	return 0;
  eeh_dev_check_failure(edev);	<= Dispatch the EEH event

In case of hugepage mappings, eeh_token_to_phys() has a bug in virt -> phys
translation that results in wrong physical address, which is then passed to
eeh_addr_cache_get_dev() to match it against cached pci I/O address ranges
to get to a PCI device. Hence, it fails to find a match and the EEH event
never gets dispatched leaving the device in failed state.

The commit 33439620680be ("powerpc/eeh: Handle hugepages in ioremap space")
introduced following logic to translate virt to phys for hugepage mappings:

eeh_token_to_phys():
+	pa = pte_pfn(*ptep);
+
+	/* On radix we can do hugepage mappings for io, so handle that */
+       if (hugepage_shift) {
+               pa <<= hugepage_shift;			<= This is wrong
+               pa |= token & ((1ul << hugepage_shift) - 1);
+       }

This patch fixes the virt -> phys translation in eeh_token_to_phys()
function.

# cat /sys/kernel/debug/powerpc/eeh_address_cache
mem addr range [0x0000040080000000-0x00000400807fffff]: 0030:01:00.1
mem addr range [0x0000040080800000-0x0000040080ffffff]: 0030:01:00.1
mem addr range [0x0000040081000000-0x00000400817fffff]: 0030:01:00.0
mem addr range [0x0000040081800000-0x0000040081ffffff]: 0030:01:00.0
mem addr range [0x0000040082000000-0x000004008207ffff]: 0030:01:00.1
mem addr range [0x0000040082080000-0x00000400820fffff]: 0030:01:00.0
mem addr range [0x0000040082100000-0x000004008210ffff]: 0030:01:00.1
mem addr range [0x0000040082110000-0x000004008211ffff]: 0030:01:00.0

Above is the list of cached io address ranges of pci 0030:01:00.<fn>.

Before this patch:

Tracing 'arg1' of function eeh_addr_cache_get_dev() during error injection
clearly shows that 'addr=' contains wrong physical address:

   kworker/u16:0-7       [001] ....   108.883775: eeh_addr_cache_get_dev:
	   (eeh_addr_cache_get_dev+0xc/0xf0) addr=0x80103000a510

dmesg shows no EEH recovery messages:

[  108.563768] bnx2x: [bnx2x_timer:5801(eth2)]MFW seems hanged: drv_pulse (0x9ae) != mcp_pulse (0x7fff)
[  108.563788] bnx2x: [bnx2x_hw_stats_update:870(eth2)]NIG timer max (4294967295)
[  108.883788] bnx2x: [bnx2x_acquire_hw_lock:2013(eth1)]lock_status 0xffffffff  resource_bit 0x1
[  108.884407] bnx2x 0030:01:00.0 eth1: MDC/MDIO access timeout
[  108.884976] bnx2x 0030:01:00.0 eth1: MDC/MDIO access timeout
<..>

After this patch:

eeh_addr_cache_get_dev() trace shows correct physical address:

  <idle>-0       [001] ..s.  1043.123828: eeh_addr_cache_get_dev:
	  (eeh_addr_cache_get_dev+0xc/0xf0) addr=0x40080bc7cd8

dmesg logs shows EEH recovery getting triggerred:

[  964.323980] bnx2x: [bnx2x_timer:5801(eth2)]MFW seems hanged: drv_pulse (0x746f) != mcp_pulse (0x7fff)
[  964.323991] EEH: Recovering PHB#30-PE#10000
[  964.324002] EEH: PE location: N/A, PHB location: N/A
[  964.324006] EEH: Frozen PHB#30-PE#10000 detected
<..>

Cc: stable@vger.kernel.org
Fixes: 33439620680be ("powerpc/eeh: Handle hugepages in ioremap space")
Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reported-by: Dominic DeMarco <ddemarc@us.ibm.com>
---
 arch/powerpc/kernel/eeh.c |   11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index cd60bc1c87011..c8a8020ef7768 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -362,14 +362,11 @@ static inline unsigned long eeh_token_to_phys(unsigned long token)
 	pa = pte_pfn(*ptep);
 
 	/* On radix we can do hugepage mappings for io, so handle that */
-	if (hugepage_shift) {
-		pa <<= hugepage_shift;
-		pa |= token & ((1ul << hugepage_shift) - 1);
-	} else {
-		pa <<= PAGE_SHIFT;
-		pa |= token & (PAGE_SIZE - 1);
-	}
+	if (!hugepage_shift)
+		hugepage_shift = PAGE_SHIFT;
 
+	pa <<= PAGE_SHIFT;
+	pa |= token & ((1ul << hugepage_shift) -1);
 	return pa;
 }
 



^ permalink raw reply related

* Re: [PATCH v3] powerpc/traps: Enhance readability for trap types
From: Christophe Leroy @ 2021-04-10  9:42 UTC (permalink / raw)
  To: Michael Ellerman, Xiongwei Song, benh, paulus, oleg, npiggin,
	aneesh.kumar, ravi.bangoria, mikey, haren, akpm, rppt, jniethe5,
	atrajeev, maddy, peterz, kjain, kan.liang, aik, alistair, pmladek,
	john.ogness
  Cc: Xiongwei Song, linuxppc-dev, linux-kernel
In-Reply-To: <874kgfdmxq.fsf@mpe.ellerman.id.au>



Le 10/04/2021 à 02:04, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 08/04/2021 à 16:07, Xiongwei Song a écrit :
>>> From: Xiongwei Song <sxwjean@gmail.com>
>>>
>>> Create a new header named traps.h, define macros to list ppc interrupt
>>> types in traps.h, replace the reference of the trap hex values with these
>>> macros.
> ...
>>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>>> index 7c633896d758..5ce9898bc9a6 100644
>>> --- a/arch/powerpc/include/asm/interrupt.h
>>> +++ b/arch/powerpc/include/asm/interrupt.h
>>> @@ -8,6 +8,7 @@
>>>    #include <asm/ftrace.h>
>>>    #include <asm/kprobes.h>
>>>    #include <asm/runlatch.h>
>>> +#include <asm/traps.h>
>>>    
>>>    struct interrupt_state {
>>>    #ifdef CONFIG_PPC_BOOK3E_64
>>> @@ -59,7 +60,7 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>>>    		 * CT_WARN_ON comes here via program_check_exception,
>>>    		 * so avoid recursion.
>>>    		 */
>>> -		if (TRAP(regs) != 0x700)
>>> +		if (TRAP(regs) != INTERRUPT_PROGRAM)
>>>    			CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
>>>    	}
>>>    #endif
>>> @@ -156,7 +157,8 @@ static inline void interrupt_nmi_enter_prepare(struct pt_regs *regs, struct inte
>>>    	/* Don't do any per-CPU operations until interrupt state is fixed */
>>>    #endif
>>>    	/* Allow DEC and PMI to be traced when they are soft-NMI */
>>> -	if (TRAP(regs) != 0x900 && TRAP(regs) != 0xf00 && TRAP(regs) != 0x260) {
>>> +	if (TRAP(regs) != INTERRUPT_DECREMENTER &&
>>> +	    TRAP(regs) != INTERRUPT_PERFMON) {
>>
>> I think too long names hinder readability, see later for suggestions.
> 
> I asked for the longer names :)
> 
> I think they make it easier for people who are less familiar with the
> architecture than us to make sense of the names.

Ok

> 
> And there's only a couple of cases where it requires splitting a line,
> and they could be converted to use switch if we think it's a problem.

> 
>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>> index 0c0b1c2cfb49..641b3feef7ee 100644
>>> --- a/arch/powerpc/mm/fault.c
>>> +++ b/arch/powerpc/mm/fault.c
>>> @@ -588,20 +589,24 @@ void __bad_page_fault(struct pt_regs *regs, int sig)
>>>    	/* kernel has accessed a bad area */
>>>    
>>>    	switch (TRAP(regs)) {
>>> -	case 0x300:
>>> -	case 0x380:
>>> -	case 0xe00:
>>> +	case INTERRUPT_DATA_STORAGE:
>>> +#ifdef CONFIG_PPC_BOOK3S
>>> +	case INTERRUPT_DATA_SEGMENT:
>>> +	case INTERRUPT_H_DATA_STORAGE:
>>> +#endif
>>
>> It would be better to avoid #ifdefs when none where necessary before.
> 
> Yes I agree.
> 
> I think these can all be avoided by defining most of the values
> regardless of what platform we're building for. Only the values that
> overlap need to be kept behind an ifdef.

Even if values overlap we can keep multiple definitions for the same value.

It is only when the same name has different values that we need to keep them behind ifdef. Is there 
any ?

Christophe

^ permalink raw reply

* Re: [PATCH v4] powerpc/traps: Enhance readability for trap types
From: Xiongwei Song @ 2021-04-10  9:32 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: ravi.bangoria, aik, linux-kernel, paulus, kan.liang, mikey, maddy,
	haren, peterz, kjain, pmladek, john.ogness, alistair, jniethe5,
	atrajeev, Xiongwei Song, oleg, aneesh.kumar, akpm, linuxppc-dev,
	rppt
In-Reply-To: <1618014353.jyyolglj7u.astroid@bobo.none>


> On Apr 10, 2021, at 8:35 AM, Nicholas Piggin <npiggin@gmail.com> wrote:
> 
> Thanks for working on this, I think it's a nice cleanup and helps
> non-powerpc people understand the code a bit better.
> 

My pleasure.

> Excerpts from Xiongwei Song's message of April 10, 2021 12:28 am:
>> From: Xiongwei Song <sxwjean@gmail.com>
>> 
>> Create a new header named traps.h, define macros to list ppc interrupt
>> types in traps.h, replace the references of the trap hex values with these
>> macros.
>> 
>> Referred the hex numbers in arch/powerpc/kernel/exceptions-64e.S,
>> arch/powerpc/kernel/exceptions-64s.S and
>> arch/powerpc/include/asm/kvm_asm.h.
>> 
>> Reported-by: kernel test robot <lkp@intel.com>
> 
> It now looks like lkp asked for this whole cleanup patch. I would
> put [kernel test robot <lkp@intel.com>] in your v3->4 changelog
> item.
> 

Agree. I just forgot to delete this line in the patch.

>> Signed-off-by: Xiongwei Song <sxwjean@gmail.com>
>> ---
>> 
>> v3-v4:
>> Fix compile issue:
>> arch/powerpc/kernel/process.c:1473:14: error: 'INTERRUPT_MACHINE_CHECK' undeclared (first use in this function); did you mean 'TAINT_MACHINE_CHECK'?
>> I didn't add "Reported-by: kernel test robot <lkp@intel.com>" here,
>> because it's improper for this patch.
> 
> [...]
> 
>> diff --git a/arch/powerpc/include/asm/traps.h b/arch/powerpc/include/asm/traps.h
>> new file mode 100644
>> index 000000000000..2e64e10afcef
>> --- /dev/null
>> +++ b/arch/powerpc/include/asm/traps.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef _ASM_PPC_TRAPS_H
>> +#define _ASM_PPC_TRAPS_H
> 
> These could go in interrupt.h.
> 
>> +#if defined(CONFIG_BOOKE) || defined(CONFIG_4xx)
>> +#define INTERRUPT_MACHINE_CHECK   0x000
>> +#define INTERRUPT_CRITICAL_INPUT  0x100
>> +#define INTERRUPT_ALTIVEC_UNAVAIL 0x200
>> +#define INTERRUPT_PERFMON         0x260
>> +#define INTERRUPT_DOORBELL        0x280
>> +#define INTERRUPT_DEBUG           0xd00
>> +#else
>> +#define INTERRUPT_SYSTEM_RESET    0x100
>> +#define INTERRUPT_MACHINE_CHECK   0x200
> 
> [...]
> 
>> @@ -1469,7 +1470,9 @@ static void __show_regs(struct pt_regs *regs)
>> 	trap = TRAP(regs);
>> 	if (!trap_is_syscall(regs) && cpu_has_feature(CPU_FTR_CFAR))
>> 		pr_cont("CFAR: "REG" ", regs->orig_gpr3);
>> -	if (trap == 0x200 || trap == 0x300 || trap == 0x600) {
>> +	if (trap == INTERRUPT_MACHINE_CHECK ||
>> +	    trap == INTERRUPT_DATA_STORAGE ||
>> +	    trap == INTERRUPT_ALIGNMENT) {
>> 		if (IS_ENABLED(CONFIG_4xx) || IS_ENABLED(CONFIG_BOOKE))
>> 			pr_cont("DEAR: "REG" ESR: "REG" ", regs->dar, regs->dsisr);
>> 		else
> 
> This is now a change in behaviour because previously BOOKE/4xx tested
> 0x200, but now it tests 0.

Yes. Previously BOOKE/4xx tested 0x200, but checked this line history, please see
the commit below:

commit c54006491dde7d1b8050c5542716b751be92ed80
Author: Anton Blanchard <anton@samba.org>
Date:   Fri Nov 15 15:41:19 2013 +1100

    powerpc: Print DAR and DSISR on machine check oopses
    
    Machine check exceptions set DAR and DSISR, so print them in our
    oops output.
    
    Signed-off-by: Anton Blanchard <anton@samba.org>
    Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 75c2d1009985..37c4103a8cff 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -864,7 +864,7 @@ void show_regs(struct pt_regs * regs)
        trap = TRAP(regs);
        if ((regs->trap != 0xc00) && cpu_has_feature(CPU_FTR_CFAR))
                printk("CFAR: "REG"\n", regs->orig_gpr3);
-       if (trap == 0x300 || trap == 0x600)
+       if (trap == 0x200 || trap == 0x300 || trap == 0x600)
 #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
                printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
 #else

0x200 aims to test Machine check, but for 64e, the regs->trap should be 0x000 here,
under the commit comments, I changed the code behavior. Sorry I didn’t add 
comments. 

> 
> That looks wrong for 4xx. 64e does put 0x000 there but I wonder if it 
> should use 0x200 instead.

Ok. Thanks for pointing this out, let me learn about 4xx. 

> Bit difficult to test this stuff, I do have
> some MCE injection patches for QEMU for 64s, might be able to look at
> porting them to 64e although I have no idea about booke machine checks.
> 

Yes, would appreciate your sharing.

> Anyway I don't think this patch should change generated code at all.
> Either change the code first with smaller patches, or make sure you
> keep the tests the same.

Agree.

Regards,
Xiongwei

> 
> Thanks,
> Nick


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox