Re: [PATCH 2/5] xen/io: provide helpers for multi size MMIO accesses

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.04.2025 09:49, Julien Grall wrote:
> On 14/04/2025 15:07, Jan Beulich wrote:
>> On 11.04.2025 12:54, Roger Pau Monne wrote:
>>> Several handlers have the same necessity of reading from an MMIO region
>>> using 1, 2, 4 or 8 bytes accesses.  So far this has been open-coded in the
>>> function itself.  Instead provide a new handler that encapsulates the
>>> accesses.
>>>
>>> Since the added helpers are not architecture specific, introduce a new
>>> generic io.h header.
>>
>> Except that ...
>>
>>> --- /dev/null
>>> +++ b/xen/include/xen/io.h
>>> @@ -0,0 +1,63 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Generic helpers for doing MMIO accesses.
>>> + *
>>> + * Copyright (c) 2025 Cloud Software Group
>>> + */
>>> +#ifndef XEN_IO_H
>>> +#define XEN_IO_H
>>> +
>>> +#include <xen/bug.h>
>>> +
>>> +#include <asm/io.h>
>>> +
>>> +static inline uint64_t read_mmio(const volatile void __iomem *mem,
>>> +                                 unsigned int size)
>>> +{
>>> +    switch ( size )
>>> +    {
>>> +    case 1:
>>> +        return readb(mem);
>>> +
>>> +    case 2:
>>> +        return readw(mem);
>>> +
>>> +    case 4:
>>> +        return readl(mem);
>>> +
>>> +    case 8:
>>> +        return readq(mem);
>>
>> ... this and ...
>>
>>> +    }
>>> +
>>> +    ASSERT_UNREACHABLE();
>>> +    return ~0UL;
>>> +}
>>> +
>>> +static inline void write_mmio(volatile void __iomem *mem, uint64_t data,
>>> +                              unsigned int size)
>>> +{
>>> +    switch ( size )
>>> +    {
>>> +    case 1:
>>> +        writeb(data, mem);
>>> +        break;
>>> +
>>> +    case 2:
>>> +        writew(data, mem);
>>> +        break;
>>> +
>>> +    case 4:
>>> +        writel(data, mem);
>>> +        break;
>>> +
>>> +    case 8:
>>> +        writeq(data, mem);
>>> +        break;
>>
>> ... this may (generally will) not work on 32-bit architectures. Add
>> CONFIG_64BIT conditionals? At which point return type / last parameter
>> type could move from uint64_t to unsigned long.
> 
> Technically arm32 bit supports 64-bit write because we mandate LPAE. I see 
> this is used by the vPCI code. Are we expecting to have any 64-bit access?

vPCI is, I think, supposed to not see 64-bit accesses (to config space).
However, vMSI-X already may see such.

Jan