| Message ID | 20240930132321.2785718-2-jvetter@kalrayinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Consolidate IO memcpy functions | expand |
Hi Julian,
kernel test robot noticed the following build errors:
[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on deller-parisc/for-next linus/master arm/for-next arm/fixes v6.12-rc1 next-20241002]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Julian-Vetter/Consolidate-IO-memcpy-memset-into-iomap_copy-c/20240930-213742
base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
patch link: https://lore.kernel.org/r/20240930132321.2785718-2-jvetter%40kalrayinc.com
patch subject: [PATCH v7 01/10] Consolidate IO memcpy/memset into iomap_copy.c
config: powerpc-allnoconfig (https://download.01.org/0day-ci/archive/20241003/202410031104.2bzZJyNF-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241003/202410031104.2bzZJyNF-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410031104.2bzZJyNF-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/io.h:14,
from include/linux/irq.h:20,
from arch/powerpc/include/asm/hardirq.h:6,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from include/linux/kernel_stat.h:8,
from include/linux/cgroup.h:25,
from include/linux/memcontrol.h:13,
from include/linux/swap.h:9,
from include/linux/suspend.h:5,
from arch/powerpc/kernel/asm-offsets.c:21:
>> arch/powerpc/include/asm/io.h:709:23: error: conflicting types for 'memcpy_fromio'; have 'void(void *, const volatile void *, size_t)' {aka 'void(void *, const volatile void *, unsigned int)'}
709 | #define memcpy_fromio memcpy_fromio
| ^~~~~~~~~~~~~
include/asm-generic/io.h:105:13: note: in expansion of macro 'memcpy_fromio'
105 | extern void memcpy_fromio(void *to, const volatile void __iomem *from,
| ^~~~~~~~~~~~~
arch/powerpc/include/asm/io-defs.h:58:18: note: previous definition of 'memcpy_fromio' with type 'void(void *, const volatile void *, long unsigned int)'
58 | DEF_PCI_AC_NORET(memcpy_fromio, (void *d, const PCI_IO_ADDR s, unsigned long n),
| ^~~~~~~~~~~~~
arch/powerpc/include/asm/io.h:664:20: note: in definition of macro 'DEF_PCI_AC_NORET'
664 | static inline void name at \
| ^~~~
>> arch/powerpc/include/asm/io.h:710:21: error: conflicting types for 'memcpy_toio'; have 'void(volatile void *, const void *, size_t)' {aka 'void(volatile void *, const void *, unsigned int)'}
710 | #define memcpy_toio memcpy_toio
| ^~~~~~~~~~~
include/asm-generic/io.h:107:13: note: in expansion of macro 'memcpy_toio'
107 | extern void memcpy_toio(volatile void __iomem *to, const void *from,
| ^~~~~~~~~~~
arch/powerpc/include/asm/io-defs.h:60:18: note: previous definition of 'memcpy_toio' with type 'void(volatile void *, const void *, long unsigned int)'
60 | DEF_PCI_AC_NORET(memcpy_toio, (PCI_IO_ADDR d, const void *s, unsigned long n),
| ^~~~~~~~~~~
arch/powerpc/include/asm/io.h:664:20: note: in definition of macro 'DEF_PCI_AC_NORET'
664 | static inline void name at \
| ^~~~
>> arch/powerpc/include/asm/io.h:708:19: error: conflicting types for 'memset_io'; have 'void(volatile void *, int, size_t)' {aka 'void(volatile void *, int, unsigned int)'}
708 | #define memset_io memset_io
| ^~~~~~~~~
include/asm-generic/io.h:109:13: note: in expansion of macro 'memset_io'
109 | extern void memset_io(volatile void __iomem *dst, int c, size_t count);
| ^~~~~~~~~
arch/powerpc/include/asm/io-defs.h:56:18: note: previous definition of 'memset_io' with type 'void(volatile void *, int, long unsigned int)'
56 | DEF_PCI_AC_NORET(memset_io, (PCI_IO_ADDR a, int c, unsigned long n),
| ^~~~~~~~~
arch/powerpc/include/asm/io.h:664:20: note: in definition of macro 'DEF_PCI_AC_NORET'
664 | static inline void name at \
| ^~~~
make[3]: *** [scripts/Makefile.build:117: arch/powerpc/kernel/asm-offsets.s] Error 1
make[3]: Target 'prepare' not remade because of errors.
make[2]: *** [Makefile:1193: prepare0] Error 2
make[2]: Target 'prepare' not remade because of errors.
make[1]: *** [Makefile:224: __sub-make] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:224: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
vim +709 arch/powerpc/include/asm/io.h
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 676
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 677 /* Some drivers check for the presence of readq & writeq with
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 678 * a #ifdef, so we make them happy here.
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 679 */
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 680 #define readb readb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 681 #define readw readw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 682 #define readl readl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 683 #define writeb writeb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 684 #define writew writew
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 685 #define writel writel
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 686 #define readsb readsb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 687 #define readsw readsw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 688 #define readsl readsl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 689 #define writesb writesb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 690 #define writesw writesw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 691 #define writesl writesl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 692 #define inb inb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 693 #define inw inw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 694 #define inl inl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 695 #define outb outb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 696 #define outw outw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 697 #define outl outl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 698 #define insb insb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 699 #define insw insw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 700 #define insl insl
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 701 #define outsb outsb
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 702 #define outsw outsw
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 703 #define outsl outsl
68a64357d15ae4 include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-13 704 #ifdef __powerpc64__
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 705 #define readq readq
4cb3cee03d558f include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-11 706 #define writeq writeq
68a64357d15ae4 include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-13 707 #endif
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 @708 #define memset_io memset_io
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 @709 #define memcpy_fromio memcpy_fromio
894fa235eb4ca0 arch/powerpc/include/asm/io.h Christophe Leroy 2020-11-21 @710 #define memcpy_toio memcpy_toio
68a64357d15ae4 include/asm-powerpc/io.h Benjamin Herrenschmidt 2006-11-13 711
On 9/30/24 06:23, Julian Vetter wrote: > +void memset_io(volatile void __iomem *dst, int c, size_t count) > +{ > + uintptr_t qc = (u8)c; Missed one change to 'long' > + > + qc |= qc << 8; > + qc |= qc << 16; > + > +#ifdef CONFIG_64BIT > + qc |= qc << 32; > +#endif Could be 'qc *= -1ul / 0xff;' r~
From: Richard Henderson > Sent: 03 October 2024 17:47 > > On 9/30/24 06:23, Julian Vetter wrote: > > +void memset_io(volatile void __iomem *dst, int c, size_t count) > > +{ > > + uintptr_t qc = (u8)c; > > Missed one change to 'long' > > > + > > + qc |= qc << 8; > > + qc |= qc << 16; > > + > > +#ifdef CONFIG_64BIT > > + qc |= qc << 32; > > +#endif > > Could be 'qc *= -1ul / 0xff;' qc *= ~0ul / 0xff; would be slightly better. David > > > r~ - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h index 80de699bf6af..f14655ed4d9d 100644 --- a/include/asm-generic/io.h +++ b/include/asm-generic/io.h @@ -102,6 +102,12 @@ static inline void log_post_read_mmio(u64 val, u8 width, const volatile void __i #endif /* CONFIG_TRACE_MMIO_ACCESS */ +extern void memcpy_fromio(void *to, const volatile void __iomem *from, + size_t count); +extern void memcpy_toio(volatile void __iomem *to, const void *from, + size_t count); +extern void memset_io(volatile void __iomem *dst, int c, size_t count); + /* * __raw_{read,write}{b,w,l,q}() access memory in native endianness. * @@ -1150,58 +1156,6 @@ static inline void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) } #endif -#ifndef memset_io -#define memset_io memset_io -/** - * memset_io Set a range of I/O memory to a constant value - * @addr: The beginning of the I/O-memory range to set - * @val: The value to set the memory to - * @count: The number of bytes to set - * - * Set a range of I/O memory to a given value. - */ -static inline void memset_io(volatile void __iomem *addr, int value, - size_t size) -{ - memset(__io_virt(addr), value, size); -} -#endif - -#ifndef memcpy_fromio -#define memcpy_fromio memcpy_fromio -/** - * memcpy_fromio Copy a block of data from I/O memory - * @dst: The (RAM) destination for the copy - * @src: The (I/O memory) source for the data - * @count: The number of bytes to copy - * - * Copy a block of data from I/O memory. - */ -static inline void memcpy_fromio(void *buffer, - const volatile void __iomem *addr, - size_t size) -{ - memcpy(buffer, __io_virt(addr), size); -} -#endif - -#ifndef memcpy_toio -#define memcpy_toio memcpy_toio -/** - * memcpy_toio Copy a block of data into I/O memory - * @dst: The (I/O memory) destination for the copy - * @src: The (RAM) source for the data - * @count: The number of bytes to copy - * - * Copy a block of data to I/O memory. - */ -static inline void memcpy_toio(volatile void __iomem *addr, const void *buffer, - size_t size) -{ - memcpy(__io_virt(addr), buffer, size); -} -#endif - extern int devmem_is_allowed(unsigned long pfn); #endif /* __KERNEL__ */ diff --git a/lib/iomap_copy.c b/lib/iomap_copy.c index 2fd5712fb7c0..5567bf8db8bc 100644 --- a/lib/iomap_copy.c +++ b/lib/iomap_copy.c @@ -3,7 +3,11 @@ * Copyright 2006 PathScale, Inc. All Rights Reserved. */ +#include <asm/unaligned.h> + +#include <linux/align.h> #include <linux/export.h> +#include <linux/types.h> #include <linux/io.h> /** @@ -76,3 +80,132 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count) } EXPORT_SYMBOL_GPL(__iowrite64_copy); #endif + +#ifndef memcpy_fromio +/** + * memcpy_fromio Copy a block of data from I/O memory + * @to: The (RAM) destination for the copy + * @from: The (I/O memory) source for the data + * @count: The number of bytes to copy + * + * Copy a block of data from I/O memory. + */ +void memcpy_fromio(void *to, const volatile void __iomem *from, size_t count) +{ + while (count && !IS_ALIGNED((long)from, sizeof(long))) { + *(u8 *)to = __raw_readb(from); + from++; + to++; + count--; + } + + while (count >= sizeof(long)) { +#ifdef CONFIG_64BIT + long val = __raw_readq(from); +#else + long val = __raw_readl(from); +#endif + put_unaligned(val, (long *)to); + + + from += sizeof(long); + to += sizeof(long); + count -= sizeof(long); + } + + while (count) { + *(u8 *)to = __raw_readb(from); + from++; + to++; + count--; + } +} +EXPORT_SYMBOL(memcpy_fromio); +#endif + +#ifndef memcpy_toio +/** + * memcpy_toio Copy a block of data into I/O memory + * @to: The (I/O memory) destination for the copy + * @from: The (RAM) source for the data + * @count: The number of bytes to copy + * + * Copy a block of data to I/O memory. + */ +void memcpy_toio(volatile void __iomem *to, const void *from, size_t count) +{ + while (count && !IS_ALIGNED((long)to, sizeof(long))) { + __raw_writeb(*(u8 *)from, to); + from++; + to++; + count--; + } + + while (count >= sizeof(long)) { + long val = get_unaligned((long *)from); +#ifdef CONFIG_64BIT + __raw_writeq(val, to); +#else + __raw_writel(val, to); +#endif + + from += sizeof(long); + to += sizeof(long); + count -= sizeof(long); + } + + while (count) { + __raw_writeb(*(u8 *)from, to); + from++; + to++; + count--; + } +} +EXPORT_SYMBOL(memcpy_toio); +#endif + +#ifndef memset_io +/** + * memset_io Set a range of I/O memory to a constant value + * @dst: The beginning of the I/O-memory range to set + * @c: The value to set the memory to + * @count: The number of bytes to set + * + * Set a range of I/O memory to a given value. + */ +void memset_io(volatile void __iomem *dst, int c, size_t count) +{ + uintptr_t qc = (u8)c; + + qc |= qc << 8; + qc |= qc << 16; + +#ifdef CONFIG_64BIT + qc |= qc << 32; +#endif + + while (count && !IS_ALIGNED((long)dst, sizeof(long))) { + __raw_writeb(c, dst); + dst++; + count--; + } + + while (count >= sizeof(long)) { +#ifdef CONFIG_64BIT + __raw_writeq(qc, dst); +#else + __raw_writel(qc, dst); +#endif + + dst += sizeof(long); + count -= sizeof(long); + } + + while (count) { + __raw_writeb(c, dst); + dst++; + count--; + } +} +EXPORT_SYMBOL(memset_io); +#endif
Various architectures have almost the same implementations for memcpy_{to,from}io and memset_io functions. So, consolidate them into the existing lib/iomap_copy.c. Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> --- Changes for v7: - Addressed reviewer comments from David: - Replaced NATIVE_STORE_TYPE and uintptr_t by long - Split the read/write and the {get,put}_unaligned into two different lines for readability - Addressed reviewer comments from Arnd: - Placed "extern" definitions in asm-generic/io.h - Renamed functions from __memcpy_{to,from}io and __memset_io to memcpy_{to,from}io and memset_io - Removed the guarding '#ifndef __memcpy_fromio', etc. --- include/asm-generic/io.h | 58 ++--------------- lib/iomap_copy.c | 133 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 52 deletions(-)