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

[Oleksii Moisieiev <Oleksii_Moisieiev@xxxxxxxx>]

Hi Jan,

A short follow-up to my previous reply, to make the proposal concrete

On 11/09/2026 16:06, Oleksii Moisieiev wrote:
> 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.



- Potential usage

The only user in this series is the SCMI shared-memory transport
(patch 4). There the "I/O" side is the SCMI shmem area: SRAM or normal
RAM mapped by Xen with Device attributes, i.e. plain byte-addressable
storage without any register semantics. Both callers pass a 32-bit
aligned I/O pointer (msg_payload sits at offset 28 of the page-aligned
shmem, plus a 4-byte status word for the response path), while the RAM
side is a caller-provided struct of arbitrary size, so count is not
necessarily a multiple of 4.

- io.h

I'd replace the current comment with something along these lines (kept
arch-neutral, so other implementations remain possible):

/*
  * Copy a sequence of bytes between regular memory and a memory-like
  * I/O region, e.g. shared memory placed in RAM or SRAM mapped with
  * device attributes.
  * The I/O region must behave like byte-addressable storage: it must
  * accept 8-bit accesses at any byte address and naturally aligned
  * 32-bit accesses, with plain byte-storage semantics in both cases
  * (no side effects, no dependency on the access width). An
  * implementation may use any of these widths, but never wider ones.
  * The helpers are not suitable for device registers with access-width
  * requirements.
  *
  * Neither pointer needs to be aligned. The access widths issued on the
  * I/O side depend only on the alignment of the I/O pointer and on
  * count, never on the alignment of the regular memory pointer.
  *
  * Implementations are architecture-specific; see the respective
  * arch/*/lib/memcpy-{from,to}io.c for the exact access pattern.
  */

The Arm-specific note in memcpy-{from,to}io.c would then state the
concrete pattern: 8-bit accesses until the I/O pointer is 32-bit
aligned and for the trailing count % 4 bytes, 32-bit accesses for the
aligned bulk. I'd also drop the "tolerate" wording in favour of the
above.

- implementation

As proposed: only the I/O pointer is aligned up; the RAM side uses
get_unaligned_le32()/put_unaligned_le32() for the 32-bit part. The
_le32 variants are used deliberately: paired with readl()/writel(),
which are defined as little-endian accessors, this preserves the byte
sequence regardless of CPU endianness. That is the only endianness
aspect worth a mention, and I'll rewrite the commit message
accordingly (dropping the sentence you pointed out).

Would that address your concerns?


Thanks,
Oleksii