Re: [PATCH v12 3/6] lib/arm: Add I/O memory copy helpers

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

Hi Jan,

Thank you for quick response. please see below.

On 11/09/2026 10:50, Jan Beulich wrote:
> On 11.09.2026 09:26, Oleksii Moisieiev wrote:
>> Introduce memcpy_fromio() and memcpy_toio() helpers to copy between
>> regular memory and MMIO space on Arm. The generic prototypes live in
>> io.h so other architectures can provide their own implementations.
>>
>> These helpers handle alignment safely by using ordered byte accesses for
>> any leading/trailing unaligned bytes and ordered 32-bit accesses for the
>> aligned bulk transfer.
> That's over-simplifying things (just like code comments do). If source
> and destination are equally misaligned modulo 4, what is said is true. If
> they are differently misaligned, the entire copy will be done byte-wise.
> Which can easily be a problem when 4-byte accesses are required for
> particular MMIO locations (which may e.g. actually represent device
> registers).
>
> As said on earlier versions: I think you either want to get misalignment
> handling right for all possible cases, or you want to demand aligned
> incoming pointers.
>
> (Ftaod, using byte accesses for two or three leading / trailing misaligned
> bytes can be equally wrong, when the MMIO location accessed wants to be
> accessed with a 16-bit load/store, for being e.g. a 16-bit device register.
> Similarly using 32-bit loads/stores can be wrong in the general case. IOW
> while some of that is said to a certain degree, I think there are
> unmentioned further constraints on when these functions may safely be
> used. For example "devices that tolerate 8-bit and 32-bit accesses" is
> still ambiguous as to what exactly it means. Not the least because
> "tolerate" doesn't mean "work correctly with".)
>
>> Using the ordered `readb/readl` and
>> `writeb/writel` accessors avoids unintended endianness conversion while
>> respecting device ordering requirements on ARM32/ARM64 hardware that may
>> not support 64-bit MMIO atomically.
> I'm having trouble making sense of this part. Why's endianness of concern
> here? The accessors used don't care about endianness at all, and what may
> have (wrongly) been used in earlier versions shouldn't matter here (or it
> would need calling out which other accessors would be wrong to use).
This is the leftover from v7 where __raw_write/__raw_read were used. 
readl()/writel() include little endian conversion through their relaxed 
variants.
https://patchew.org/Xen/cover.1768415200.git.oleksii._5Fmoisieiev@xxxxxxxx/d166348530b9229673e1a6e3b29ff4ee9123ab2f.1768415200.git.oleksii._5Fmoisieiev@xxxxxxxx/

I will remove the claim that ordered accessors avoid endianness conversion.

>> --- a/xen/include/xen/io.h
>> +++ b/xen/include/xen/io.h
>> @@ -67,4 +67,14 @@ static inline bool write_mmio(volatile void __iomem *mem, 
>> unsigned long data,
>>       return true;
>>   }
>>   
>> +/*
>> + * Copy between regular memory and MMIO space.  Implementations are
>> + * architecture-specific and must use appropriate MMIO accessors for
>> + * their memory and I/O models.
>> + */
>> +void memcpy_fromio(void *to, const volatile void __iomem *from,
>> +                   size_t count);
>> +void memcpy_toio(volatile void __iomem *to, const void *from,
>> +                 size_t count);
> For somebody wanting to use these functions and merely looking here, how
> would they know of all the constraints? That is implementations are not
> merely arch-specific, they may also impose arch-specific constraints.
>
> Jan
You are right about the alignment handling. Advancing both pointers by
the same amount preserves their relative alignment, so they never become
aligned together if their offsets modulo four differ. In that case the
current implementation copies the whole range using byte accesses.

I propose to handle arbitrary RAM alignment by aligning only the I/O
pointer, then using put_unaligned_le32()/get_unaligned_le32() on the
Normal-memory side of the word transfers. All I/O accesses would still
use readb/readl or writeb/writel. This would make the I/O access widths
independent of the RAM pointer's alignment. In particular, an aligned
I/O address and a length divisible by four would produce only 32-bit
I/O accesses, even with an unaligned RAM buffer.

So the changes for memcpy_toio will look like:

void memcpy_toio(volatile void __iomem *to, const void *from,
                  size_t count)
  {
-    while ( count && (!IS_ALIGNED((unsigned long)to, 4) ||
-                      !IS_ALIGNED((unsigned long)from, 4)) )
+    while ( count && !IS_ALIGNED((unsigned long)to, 4) )
      {
          writeb(*(const uint8_t *)from, to);
          from++;
@@ -29,7 +22,7 @@

      while ( count >= 4 )
      {
-        writel(*(const uint32_t *)from, to);
+        writel(get_unaligned_le32(from), to);
          from += 4;
          to += 4;
          count -= 4;

This does not make the helpers suitable for arbitrary device registers.
I will describe them as copying a byte sequence to/from a memory-like
I/O region. The region must support byte accesses at every byte address
and aligned 32-bit accesses with equivalent byte-storage semantics.
The implementation uses byte accesses to align the I/O pointer.

And also I will document the Arm requirements next to the declarations
in xen/io.h as you suggested.


What do you think about this approach?

Oleksii.