On 22.07.2025 13:41, Oleksii Moisieiev wrote:
> This commit introduces two helper functions, `__memcpy_fromio` and
> `__memcpy_toio`, to provide a robust mechanism for copying data between
> standard memory and memory-mapped I/O (MMIO) space for the ARM
> architecture.
>
> These functions are designed to handle memory transfers safely,
> accounting for potential address alignment issues to ensure correctness
> and improve performance where possible. The implementation is specific
> to ARM and uses relaxed I/O accessors.
The implementation could be reused by any arch providing
{read,write}*_relaxed(), couldn't it?
> __memcpy_fromio:
> Copies a block of data from an I/O memory source to a destination in
> standard ("real") memory. The implementation first handles any unaligned
> bytes at the beginning of the source buffer individually using byte-wise
> reads. It then copies the bulk of the data using 32-bit reads for
> efficiency, and finally processes any remaining bytes at the end of the
> buffer.
>
> __memcpy_toio:
> Copies a block of data from standard memory to a destination in I/O
> memory space. It follows a similar strategy, handling any initial
> unaligned portion of the destination buffer byte-by-byte before using
> more efficient 32-bit writes for the main, aligned part of the transfer.
> Any trailing bytes are also handled individually.
> xen/include/xen/lib/arm/io.h
Why exactly do the functions need two leading underscores in their names?
> --- a/xen/lib/Makefile
> +++ b/xen/lib/Makefile
> @@ -1,4 +1,5 @@
> obj-$(CONFIG_X86) += x86/
> +obj-$(CONFIG_ARM) += arm/
Nit: Alphabetically sorted please.
> --- /dev/null
> +++ b/xen/lib/arm/Makefile
> @@ -0,0 +1 @@
> +obj-y += io.o
> \ No newline at end of file
Please make sure all files properly end in a newline.
> --- /dev/null
> +++ b/xen/lib/arm/io.c
> @@ -0,0 +1,80 @@
> +#include <asm/io.h>
> +#include <xen/lib/arm/io.h>
> +
> +/*
> + * memcpy_fromio - Copy data from IO memory space to "real" memory space.
> + * @to: Where to copy to
> + * @from: Where to copy from
> + * @count: The size of the area.
> + */
> +void __memcpy_fromio(void *to, const volatile void __iomem *from,
> + size_t count)
> +{
> + while ( count && !IS_ALIGNED((unsigned long)from, 4) )
> + {
> + *(u8 *)to = readb_relaxed(from);
No u<N> anymore in new code please; use uint<N>_t instead.
Further, what tells you that accessing a 16-bit register residing in MMIO
can legitimately be accessed using two 8-bit accesses?
> + from++;
> + to++;
> + count--;
> + }
> +
> + while ( count >= 4 )
> + {
> + *(u32 *)to = readl_relaxed(from);
> + from += 4;
> + to += 4;
> + count -= 4;
> + }
Not attempting 64-bit accesses on 64-bit arches will want an explanatory
comment, I think.
> + while ( count )
> + {
> + *(u8 *)to = readb_relaxed(from);
> + from++;
> + to++;
> + count--;
> + }
> +}
> +
> +/*
> + * memcpy_toio - Copy data from "real" memory space to IO memory space.
> + * @to: Where to copy to
> + * @from: Where to copy from
> + * @count: The size of the area.
> + */
> +void __memcpy_toio(volatile void __iomem *to, const void *from,
> + size_t count)
> +{
> + while ( count && !IS_ALIGNED((unsigned long)to, 4) )
> + {
> + writeb_relaxed(*(u8 *)from, to);
Please never cast away const-ness. This is a violation of some Misra rule,
iirc.
Jan
