| Message ID | 20240906114151.519028-2-jvetter@kalrayinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Consolidate IO memcpy functions | expand |
On Fri, Sep 6, 2024, at 11:41, Julian Vetter wrote: > Various architectures have almost the same implementations for > __memcpy_{to,from}io and __memset_io functions. So, consolidate them and > introduce a CONFIG_GENERIC_IO flag to build the given lib/io.c. > > Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com> > Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> Thanks for working on this. The implementation looks correct to me and we should be able to use this on most architectures, but since this is a shared file, it would be good to make it as polished as possible. > lib/Kconfig | 3 ++ > lib/Makefile | 2 + > lib/io.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 106 insertions(+) > create mode 100644 lib/io.c I feel the name is a little too geneal, maybe use io_copy.c and CONFIG_GENERIC_IO_COPY for the name? Alternatively, this could be part of lib/iomap_copy.c, which already has some related helper functions. In that case, I think we would rely on the per-architecture "#define __memcpy_fromio __memcpy_fromio" etc macros instead of a Kconfig symbol. > +void __memcpy_fromio(void *to, const volatile void __iomem *from, size_t count) > +{ > + while (count && !IS_ALIGNED((unsigned long)from, NATIVE_STORE_SIZE)) { > + *(u8 *)to = __raw_readb(from); > + from++; > + to++; > + count--; > + } There is a corner case that this bit actually misses (same in the existing implementation): If the __iomem buffer is aligned, but the 'to' buffer is not, this may cause an alignment fault on architectures without native unaligned stores. At the moment I think both csky and some configurations of loongarch64 can run into this. I think the best way to deal with this is to use get_unaligned()/put_unaligned() in place of the pointer dereference. This has no effect on architectures that have native unaligned access but otherwise uses byte access on the kernel buffer, which I think is a good tradeoff. > + while (count >= NATIVE_STORE_SIZE) { > + *(uintptr_t *)to = __raw_read_native(from); > + from += NATIVE_STORE_SIZE; > + to += NATIVE_STORE_SIZE; > + count -= NATIVE_STORE_SIZE; > + } The use of __raw_read_native() somehow bothers me, it seems a little more complicated than open-coding the two possibles paths, or even using the aligned-access helpers like if (IS_ENABLED(CONFIG_64BIT)) __iowrite64_copy(to, from, count & WORD_MASK) else __iowrite32_copy(to, from, count & WORD_MASK) Those helpers do require the kernel buffer to be naturally aligned though. > +void __memset_io(volatile void __iomem *dst, int c, size_t count) > +{ > + uintptr_t qc = (u8)c; > + > + qc |= qc << 8; > + qc |= qc << 16; > +#if IS_ENABLED(CONFIG_64BIT) > + qc |= qc << 32; > +#endif If you use IS_ENABLED() here, please do it like if (IS_ENABLED(CONFIG_64BIT) qc |= qc << 32; Arnd
diff --git a/lib/Kconfig b/lib/Kconfig index b38849af6f13..2ebdc41cf0a1 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -765,6 +765,9 @@ config GENERIC_LIB_UCMPDI2 config GENERIC_LIB_DEVMEM_IS_ALLOWED bool +config GENERIC_IO + bool + config PLDMFW bool default n diff --git a/lib/Makefile b/lib/Makefile index 322bb127b4dc..d231a1c49715 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -135,6 +135,8 @@ obj-$(CONFIG_HAS_IOMEM) += iomap_copy.o devres.o obj-$(CONFIG_CHECK_SIGNATURE) += check_signature.o obj-$(CONFIG_DEBUG_LOCKING_API_SELFTESTS) += locking-selftest.o +obj-$(CONFIG_GENERIC_IO) += io.o + lib-y += logic_pio.o lib-$(CONFIG_INDIRECT_IOMEM) += logic_iomem.o diff --git a/lib/io.c b/lib/io.c new file mode 100644 index 000000000000..c3385566a3e1 --- /dev/null +++ b/lib/io.c @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * derived from arch/arm/kernel/io.c + * + * Copyright (C) 2024 Kalray Inc. + * Author(s): Julian Vetter + */ + +#include <linux/export.h> +#include <linux/types.h> +#include <linux/io.h> + +#define NATIVE_STORE_SIZE (BITS_PER_LONG/8) + +#if IS_ENABLED(CONFIG_64BIT) +#define __raw_write_native(val, dst) __raw_writeq(val, dst) +#define __raw_read_native(src) __raw_readq((src)) +#else +#define __raw_write_native(val, dst) __raw_writel(val, dst) +#define __raw_read_native(src) __raw_readl(src) +#endif + +void __memcpy_fromio(void *to, const volatile void __iomem *from, size_t count) +{ + while (count && !IS_ALIGNED((unsigned long)from, NATIVE_STORE_SIZE)) { + *(u8 *)to = __raw_readb(from); + from++; + to++; + count--; + } + + while (count >= NATIVE_STORE_SIZE) { + *(uintptr_t *)to = __raw_read_native(from); + from += NATIVE_STORE_SIZE; + to += NATIVE_STORE_SIZE; + count -= NATIVE_STORE_SIZE; + } + + while (count) { + *(u8 *)to = __raw_readb(from); + from++; + to++; + count--; + } +} +EXPORT_SYMBOL(__memcpy_fromio); + +void __memcpy_toio(volatile void __iomem *to, const void *from, size_t count) +{ + while (count && !IS_ALIGNED((unsigned long)to, NATIVE_STORE_SIZE)) { + __raw_writeb(*(u8 *)from, to); + from++; + to++; + count--; + } + + while (count >= NATIVE_STORE_SIZE) { + __raw_write_native(*(uintptr_t *)from, to); + from += NATIVE_STORE_SIZE; + to += NATIVE_STORE_SIZE; + count -= NATIVE_STORE_SIZE; + } + + while (count) { + __raw_writeb(*(u8 *)from, to); + from++; + to++; + count--; + } +} +EXPORT_SYMBOL(__memcpy_toio); + +void __memset_io(volatile void __iomem *dst, int c, size_t count) +{ + uintptr_t qc = (u8)c; + + qc |= qc << 8; + qc |= qc << 16; +#if IS_ENABLED(CONFIG_64BIT) + qc |= qc << 32; +#endif + + while (count && !IS_ALIGNED((unsigned long)dst, NATIVE_STORE_SIZE)) { + __raw_writeb(c, dst); + dst++; + count--; + } + + while (count >= NATIVE_STORE_SIZE) { + __raw_write_native(qc, dst); + dst += NATIVE_STORE_SIZE; + count -= NATIVE_STORE_SIZE; + } + + while (count) { + __raw_writeb(c, dst); + dst++; + count--; + } +} +EXPORT_SYMBOL(__memset_io);
Various architectures have almost the same implementations for __memcpy_{to,from}io and __memset_io functions. So, consolidate them and introduce a CONFIG_GENERIC_IO flag to build the given lib/io.c. Reviewed-by: Yann Sionneau <ysionneau@kalrayinc.com> Signed-off-by: Julian Vetter <jvetter@kalrayinc.com> --- lib/Kconfig | 3 ++ lib/Makefile | 2 + lib/io.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) create mode 100644 lib/io.c