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.
As to the top comment of the file: io.h is, to me at least, wider than
just dealing with MMIO accesses. IOW I fear that sentence may go stale
at some point, without anyone paying attention.
Jan
