Re: [PATCH 2/6] sh: remove duplicate ioread/iowrite helpers
Hello Geert,
On Sun, 2025-06-08 at 11:39 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
>
> On Sat, 7 Jun 2025 at 14:08, John Paul Adrian Glaubitz
> wrote:
> > On Sat, 2025-03-15 at 11:59 +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann
> > >
> > > The ioread/iowrite functions on sh only do memory mapped I/O like the
> > > generic verion, and never map onto non-MMIO inb/outb variants, so they
> > > just add complexity. In particular, the use of asm-generic/iomap.h
> > > ties the declaration to the x86 implementation.
> > >
> > > Remove the custom versions and use the architecture-independent fallback
> > > code instead. Some of the calling conventions on sh are different here,
> > > so fix that by adding 'volatile' keywords where required by the generic
> > > implementation and change the cpg clock driver to no longer depend on
> > > the interesting choice of return types for ioread8/ioread16/ioread32.
> > >
> > > Signed-off-by: Arnd Bergmann
>
> > Those are quite a number of changes that I would like to test on real
> > hardware
> > first before merging them into the kernel.
> >
> > @Geert: Could you test it on your SH-7751 LANDISK board as well?
>
> Already done for a while, as this patch is commit 2494fce26e434071 ("sh:
> remove duplicate ioread/iowrite helpers") in v6.15-rc1 ;-)
Well, there is no Tested-By from either of us, so this isn't optimal.
I wished Arnd could have at least pinged me back regarding this. He knows I'm
actively maintaining arch/sh and I would like to properly test and review
such changes.
But I'm not doing this professionally, so I cannot be always there with 100%
capacity. Just pushing such changes in without any input from me defeats the
purpose of a maintainer.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`-GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
Re: [PATCH 2/6] sh: remove duplicate ioread/iowrite helpers
Hi Adrian,
On Sat, 7 Jun 2025 at 14:08, John Paul Adrian Glaubitz
wrote:
> On Sat, 2025-03-15 at 11:59 +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann
> >
> > The ioread/iowrite functions on sh only do memory mapped I/O like the
> > generic verion, and never map onto non-MMIO inb/outb variants, so they
> > just add complexity. In particular, the use of asm-generic/iomap.h
> > ties the declaration to the x86 implementation.
> >
> > Remove the custom versions and use the architecture-independent fallback
> > code instead. Some of the calling conventions on sh are different here,
> > so fix that by adding 'volatile' keywords where required by the generic
> > implementation and change the cpg clock driver to no longer depend on
> > the interesting choice of return types for ioread8/ioread16/ioread32.
> >
> > Signed-off-by: Arnd Bergmann
> Those are quite a number of changes that I would like to test on real hardware
> first before merging them into the kernel.
>
> @Geert: Could you test it on your SH-7751 LANDISK board as well?
Already done for a while, as this patch is commit 2494fce26e434071 ("sh:
remove duplicate ioread/iowrite helpers") in v6.15-rc1 ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 00/13] lib/crc: improve how arch-optimized code is integrated
On Sun, Jun 01, 2025 at 03:44:28PM -0700, Eric Biggers wrote: > This series improves how lib/crc supports arch-optimized code. First, > instead of the arch-optimized CRC code being in arch/$(SRCARCH)/lib/, it > will now be in lib/crc/$(SRCARCH)/. Second, the API functions (e.g. > crc32c()), arch-optimized functions (e.g. crc32c_arch()), and generic > functions (e.g. crc32c_base()) will now be part of a single module for > each CRC type, allowing better inlining and dead code elimination. The > second change is made possible by the first. > > As an example, consider CONFIG_CRC32=m on x86. We'll now have just > crc32.ko instead of both crc32-x86.ko and crc32.ko. The two modules > were already coupled together and always both got loaded together via > direct symbol dependency, so the separation provided no benefit. > > Note: later I'd like to apply the same design to lib/crypto/ too, where > often the API functions are out-of-line so this will work even better. > In those cases, for each algorithm we currently have 3 modules all > coupled together, e.g. libsha256.ko, libsha256-generic.ko, and > sha256-x86.ko. We should have just one, inline things properly, and > rely on the compiler's dead code elimination to decide the inclusion of > the generic code instead of manually setting it via kconfig. > > Having arch-specific code outside arch/ was somewhat controversial when > Zinc proposed it back in 2018. But I don't think the concerns are > warranted. It's better from a technical perspective, as it enables the > improvements mentioned above. This model is already successfully used > in other places in the kernel such as lib/raid6/. The community of each > architecture still remains free to work on the code, even if it's not in > arch/. At the time there was also a desire to put the library code in > the same files as the old-school crypto API, but that was a mistake; now > that the library is separate, that's no longer a constraint either. > > Patches 1 and 2, which I previously sent out by themselves, are > prerequisites because they eliminate the need for the CRC32 library API > to expose the generic functions. > > Eric Biggers (13): > crypto/crc32: register only one shash_alg > crypto/crc32c: register only one shash_alg Applied the first 2 patches to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=crc-next The remaining patches have new versions in the v2 series. - Eric
Re: [PATCH v2 00/12] lib/crc: improve how arch-optimized code is integrated
On Sat, Jun 07, 2025 at 05:47:02PM -0600, Jason A. Donenfeld wrote: > On Sat, Jun 07, 2025 at 01:04:42PM -0700, Eric Biggers wrote: > > Having arch-specific code outside arch/ was somewhat controversial when > > Zinc proposed it back in 2018. But I don't think the concerns are > > warranted. It's better from a technical perspective, as it enables the > > improvements mentioned above. This model is already successfully used > > in other places in the kernel such as lib/raid6/. The community of each > > architecture still remains free to work on the code, even if it's not in > > arch/. At the time there was also a desire to put the library code in > > the same files as the old-school crypto API, but that was a mistake; now > > that the library is separate, that's no longer a constraint either. > > I can't express how happy I am to see this revived. It's clearly the > right way forward and makes it a lot simpler for us to dispatch to > various arch implementations and also is organizationally simpler. > > Jason Thanks! Can I turn that into an Acked-by? - Eric
Re: [PATCH] (powerpc/512) Fix possible `dma_unmap_single()` on uninitialized pointer
Thomas Fourier writes:
> If the device configuration fails (if `dma_dev->device_config()`),
> `sg_dma_address(&sg)` is not initialized and the jump to `err_dma_prep`
> leads to calling `dma_unmap_single()` on `sg_dma_address(&sg)`.
>
> Signed-off-by: Thomas Fourier
> ---
> arch/powerpc/platforms/512x/mpc512x_lpbfifo.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
> b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
> index 9668b052cd4b..ef3be438f914 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
> @@ -241,8 +241,7 @@ static int mpc512x_lpbfifo_kick(void)
>
> /* Make DMA channel work with LPB FIFO data register */
> if (dma_dev->device_config(lpbfifo.chan, &dma_conf)) {
> - ret = -EINVAL;
> - goto err_dma_prep;
> + return -EINVAL;
> }
>
> sg_init_table(&sg, 1);
Yep looks good. That's the first use of goto for error handling, and
it's clearly too early. All the previous error cases do a direct return.
Reviewed-by: Michael Ellerman
cheers
Re: [PATCH] powerpc: unify two CONFIG_POWERPC64_CPU entries in the same choice block
Masahiro Yamada writes:
> There are two CONFIG_POWERPC64_CPU entries in the "CPU selection"
> choice block.
>
> I guess the intent is to display a different prompt depending on
> CPU_LITTLE_ENDIAN: "Generic (POWER5 and PowerPC 970 and above)" for big
> endian, and "Generic (POWER8 and above)" for little endian.
Yeah.
> I stumbled on this tricky use case, and worked around it on Kconfig with
> commit 4d46b5b623e0 ("kconfig: fix infinite loop in sym_calc_choice()").
> However, I doubt that supporting multiple entries with the same symbol
> in a choice block is worth the complexity - this is the only such case
> in the kernel tree.
>
> This commit merges the two entries. Once this cleanup is accepted in
> the powerpc subsystem, I will proceed to refactor the Kconfig parser.
OK. Sorry for the trouble.
It could be split into two symbols to keep the separate prompts, but it's
probably not worth the trouble.
Acked-by: Michael Ellerman (powerpc)
cheers
> diff --git a/arch/powerpc/platforms/Kconfig.cputype
> b/arch/powerpc/platforms/Kconfig.cputype
> index 613b383ed8b3..7b527d18aa5e 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -122,16 +122,11 @@ choice
> If unsure, select Generic.
>
> config POWERPC64_CPU
> - bool "Generic (POWER5 and PowerPC 970 and above)"
> - depends on PPC_BOOK3S_64 && !CPU_LITTLE_ENDIAN
> + bool "Generic 64 bits powerpc"
> + depends on PPC_BOOK3S_64
> + select ARCH_HAS_FAST_MULTIPLIER if CPU_LITTLE_ENDIAN
> select PPC_64S_HASH_MMU
> -
> -config POWERPC64_CPU
> - bool "Generic (POWER8 and above)"
> - depends on PPC_BOOK3S_64 && CPU_LITTLE_ENDIAN
> - select ARCH_HAS_FAST_MULTIPLIER
> - select PPC_64S_HASH_MMU
> - select PPC_HAS_LBARX_LHARX
> + select PPC_HAS_LBARX_LHARX if CPU_LITTLE_ENDIAN
>
> config POWERPC_CPU
> bool "Generic 32 bits powerpc"
> --
> 2.43.0
Re: [PATCH] powerpc: use always-y instead of extra-y in Makefiles
Masahiro Yamada writes: > On Tue, Jun 3, 2025 at 3:50 PM Christophe Leroy > wrote: >> Le 02/06/2025 à 18:32, Masahiro Yamada a écrit : >> > The extra-y syntax is planned for deprecation because it is similar >> > to always-y. >> > >> > When building the boot wrapper, always-y and extra-y are equivalent. >> > Use always-y instead. >> > >> > In arch/powerpc/kernel/Makefile, I added ifdef KBUILD_BUILTIN to >> > keep the current behavior: prom_init_check is skipped when building >> > only modular objects. >> >> I don't understand what you mean. >> >> CONFIG_PPC_OF_BOOT_TRAMPOLINE is a bool, it cannot be a module. >> >> prom_init_check is only to check the content of prom_init.o which is >> never a module. >> >> Is always-y to run _after_ prom_init.o is built ? > > The intent of "make ARCH=powerpc modules" > is to compile objects that are necessary for modules, > that is, all built-in objects are skipped. > > However, > always-$(CONFIG_PPC_OF_BOOT_TRAMPOLINE) += prom_init_check > would generate prom_init_check regardless, > and its prerequisite, prom_init.o as well. > > With CONFIG_MODULES=y and > CONFIG_MODVERSIONS=n, > and without ifdef KBUILD_BUILTIN, > > $ make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- modules > > would result in this: > > > CC [M] arch/powerpc/kvm/book3s_xive_native.o > CC [M] arch/powerpc/kvm/book3s_64_vio.o > LD [M] arch/powerpc/kvm/kvm.o > CC [M] arch/powerpc/kvm/book3s_hv.o > AS [M] arch/powerpc/kvm/book3s_hv_interrupts.o > CC [M] arch/powerpc/kvm/book3s_64_mmu_hv.o > CC [M] arch/powerpc/kvm/book3s_64_mmu_radix.o > CC [M] arch/powerpc/kvm/book3s_hv_nested.o > CC [M] arch/powerpc/kvm/book3s_hv_tm.o > LD [M] arch/powerpc/kvm/kvm-hv.o > CC [M] arch/powerpc/kernel/rtas_flash.o > CC arch/powerpc/kernel/prom_init.o > PROMCHK arch/powerpc/kernel/prom_init_check > CC [M] kernel/locking/locktorture.o > CC [M] kernel/time/test_udelay.o > CC [M] kernel/time/time_test.o > CC [M] kernel/backtracetest.o > CC [M] kernel/torture.o > CC [M] kernel/resource_kunit.o > CC [M] kernel/sysctl-test.o > CC [M] fs/ext4/inode-test.o > LD [M] fs/ext4/ext4-inode-test.o > CC [M] fs/fat/namei_vfat.o > LD [M] fs/fat/vfat.o > CC [M] fs/fat/fat_test.o > CC [M] fs/nls/nls_ucs2_utils.o > CC [M] fs/netfs/buffered_read.o > CC [M] fs/netfs/buffered_write.o > ... > > > > You can see these two lines: > > CC arch/powerpc/kernel/prom_init.o > PROMCHK arch/powerpc/kernel/prom_init_check > > are supposed to be skipped when "make modules", > but actually compiled without ifdef. > > So, I added ifdef KBUILD_BUILTIN to preserve > the current behavior. OK, that makes sense. I don't really ever build just modules, so I wouldn't notice, but some folks probably do. Acked-by: Michael Ellerman (powerpc) cheers
[mainline]Kernel OOPs at migrate_swap_task
Hello, IBM CI has reported a kernel oops, while running cpu monitor test [1], on the mainline kernel. [1]: https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/cpu/cpupower_monitor.py I am still in porcess of doing git bisect, meanwhile, wondering if below traces gives any clue. Traces: [ 2181.567879] Kernel attempted to read user page (5b0) - exploit attempt? (uid: 0) [ 2181.567913] BUG: Kernel NULL pointer dereference on read at 0x05b0 [ 2181.567918] Faulting instruction address: 0xc01d4a48 [ 2181.567924] Oops: Kernel access of bad area, sig: 11 [#1] [ 2181.567929] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=8192 NUMA pSeries [ 2181.567936] Modules linked in: dm_mod ext4 crc16 mbcache jbd2 loop nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 bonding tls rfkill ip_set nf_tables nfnetlink pseries_rng vmx_crypto fuse xfs sr_mod cdrom sd_mod sg ibmvscsi scsi_transport_srp ibmveth [last unloaded: scsi_debug] [ 2181.567990] CPU: 0 UID: 0 PID: 19 Comm: migration/0 Kdump: loaded Not tainted 6.15.0-ge271ed52b344 #1 VOLUNTARY [ 2181.568003] Stopper: multi_cpu_stop+0x0/0x22c <- migrate_swap+0xe8/0x214 [ 2181.568016] NIP: c01d4a48 LR: c01d4a3c CTR: c01d6864 [ 2181.568021] REGS: c87ffaf0 TRAP: 0300 Not tainted (6.15.0-ge271ed52b344) [ 2181.568026] MSR: 82803033 CR: 48004202 XER: [ 2181.568038] CFAR: c003a360 DAR: 05b0 DSISR: 4000 IRQMASK: 3 [ 2181.568038] GPR00: c01d4df4 c87ffd90 c1648100 0003 [ 2181.568038] GPR04: 0010 0941d4c72888 [ 2181.568038] GPR08: 0004fdad 013a 4000 [ 2181.568038] GPR12: c004ffd3c780 c2ff c01ae2e0 c40533c0 [ 2181.568038] GPR16: [ 2181.568038] GPR20: 0001 [ 2181.568038] GPR24: c00529930f4c c00529930180 c00642c60100 [ 2181.568038] GPR28: c00642c60ecc c013fd68c700 0010 c00642c60100 [ 2181.568088] NIP [c01d4a48] __migrate_swap_task+0x6c/0x1d8 [ 2181.568094] LR [c01d4a3c] __migrate_swap_task+0x60/0x1d8 [ 2181.568100] Call Trace: [ 2181.568103] [c87ffd90] [c01d4b70] __migrate_swap_task+0x194/0x1d8 (unreliable) [ 2181.568112] [c87ffdc0] [c01d4df4] migrate_swap_stop+0x240/0x2b4 [ 2181.568118] [c87ffe20] [c02efc04] multi_cpu_stop+0xd8/0x22c [ 2181.568123] [c87ffe90] [c02ef8e8] cpu_stopper_thread+0x158/0x24c [ 2181.568129] [c87fff40] [c01b93b0] smpboot_thread_fn+0x200/0x2c0 [ 2181.568137] [c87fff90] [c01ae40c] kthread+0x134/0x164 [ 2181.568144] [c87fffe0] [c000df98] start_kernel_thread+0x14/0x18 [ 2181.568150] Code: 60690001 992d0932 e90d0030 3d2200c2 39290378 7d49402a 394a0001 7d49412a 4be65771 6000 6000 e93f0a70 48426c89 6000 2c23 [ 2181.568165] ---[ end trace ]--- [ 2181.568687] If you happen to fix this, please add below tag. Reported-by: Venkat Rao Bagalkote Regards, Venkat.
Re: [PATCH] powerpc: use always-y instead of extra-y in Makefiles
On Mon, Jun 9, 2025 at 10:02 AM Michael Ellerman wrote: > > Masahiro Yamada writes: > > On Tue, Jun 3, 2025 at 3:50 PM Christophe Leroy > > wrote: > >> Le 02/06/2025 à 18:32, Masahiro Yamada a écrit : > >> > The extra-y syntax is planned for deprecation because it is similar > >> > to always-y. > >> > > >> > When building the boot wrapper, always-y and extra-y are equivalent. > >> > Use always-y instead. > >> > > >> > In arch/powerpc/kernel/Makefile, I added ifdef KBUILD_BUILTIN to > >> > keep the current behavior: prom_init_check is skipped when building > >> > only modular objects. > >> > >> I don't understand what you mean. > >> > >> CONFIG_PPC_OF_BOOT_TRAMPOLINE is a bool, it cannot be a module. > >> > >> prom_init_check is only to check the content of prom_init.o which is > >> never a module. > >> > >> Is always-y to run _after_ prom_init.o is built ? > > > > The intent of "make ARCH=powerpc modules" > > is to compile objects that are necessary for modules, > > that is, all built-in objects are skipped. > > > > However, > > always-$(CONFIG_PPC_OF_BOOT_TRAMPOLINE) += prom_init_check > > would generate prom_init_check regardless, > > and its prerequisite, prom_init.o as well. > > > > With CONFIG_MODULES=y and > > CONFIG_MODVERSIONS=n, > > and without ifdef KBUILD_BUILTIN, > > > > $ make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- modules > > > > would result in this: > > > > > > CC [M] arch/powerpc/kvm/book3s_xive_native.o > > CC [M] arch/powerpc/kvm/book3s_64_vio.o > > LD [M] arch/powerpc/kvm/kvm.o > > CC [M] arch/powerpc/kvm/book3s_hv.o > > AS [M] arch/powerpc/kvm/book3s_hv_interrupts.o > > CC [M] arch/powerpc/kvm/book3s_64_mmu_hv.o > > CC [M] arch/powerpc/kvm/book3s_64_mmu_radix.o > > CC [M] arch/powerpc/kvm/book3s_hv_nested.o > > CC [M] arch/powerpc/kvm/book3s_hv_tm.o > > LD [M] arch/powerpc/kvm/kvm-hv.o > > CC [M] arch/powerpc/kernel/rtas_flash.o > > CC arch/powerpc/kernel/prom_init.o > > PROMCHK arch/powerpc/kernel/prom_init_check > > CC [M] kernel/locking/locktorture.o > > CC [M] kernel/time/test_udelay.o > > CC [M] kernel/time/time_test.o > > CC [M] kernel/backtracetest.o > > CC [M] kernel/torture.o > > CC [M] kernel/resource_kunit.o > > CC [M] kernel/sysctl-test.o > > CC [M] fs/ext4/inode-test.o > > LD [M] fs/ext4/ext4-inode-test.o > > CC [M] fs/fat/namei_vfat.o > > LD [M] fs/fat/vfat.o > > CC [M] fs/fat/fat_test.o > > CC [M] fs/nls/nls_ucs2_utils.o > > CC [M] fs/netfs/buffered_read.o > > CC [M] fs/netfs/buffered_write.o > > ... > > > > > > > > You can see these two lines: > > > > CC arch/powerpc/kernel/prom_init.o > > PROMCHK arch/powerpc/kernel/prom_init_check > > > > are supposed to be skipped when "make modules", > > but actually compiled without ifdef. > > > > So, I added ifdef KBUILD_BUILTIN to preserve > > the current behavior. > > OK, that makes sense. > > I don't really ever build just modules, so I wouldn't notice, but some > folks probably do. > > Acked-by: Michael Ellerman (powerpc) No rush for this patch. Please take it to your ppc tree. Thank you. -- Best Regards Masahiro Yamada
Re: [PATCH] powerpc: unify two CONFIG_POWERPC64_CPU entries in the same choice block
On Mon, Jun 9, 2025 at 9:59 AM Michael Ellerman wrote:
>
> Masahiro Yamada writes:
> > There are two CONFIG_POWERPC64_CPU entries in the "CPU selection"
> > choice block.
> >
> > I guess the intent is to display a different prompt depending on
> > CPU_LITTLE_ENDIAN: "Generic (POWER5 and PowerPC 970 and above)" for big
> > endian, and "Generic (POWER8 and above)" for little endian.
>
> Yeah.
>
> > I stumbled on this tricky use case, and worked around it on Kconfig with
> > commit 4d46b5b623e0 ("kconfig: fix infinite loop in sym_calc_choice()").
> > However, I doubt that supporting multiple entries with the same symbol
> > in a choice block is worth the complexity - this is the only such case
> > in the kernel tree.
> >
> > This commit merges the two entries. Once this cleanup is accepted in
> > the powerpc subsystem, I will proceed to refactor the Kconfig parser.
>
> OK. Sorry for the trouble.
>
> It could be split into two symbols to keep the separate prompts, but it's
> probably not worth the trouble.
>
> Acked-by: Michael Ellerman (powerpc)
No rush for this patch.
Please take it to your ppc tree.
Thank you.
Best Regards
Masahiro Yamada
