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

[Julien Grall <julien@xxxxxxx>]

Hi,

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?
Cheers,

--
Julien Grall