Re: [PATCH v7 09/19] xen/riscv: introduce io.h

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.04.2024 12:20, Oleksii Kurochko wrote:
> The header taken form Linux 6.4.0-rc1 and is based on
> arch/riscv/include/asm/mmio.h with the following changes:
> - drop forcing of endianess for read*(), write*() functions as
>   no matter what CPU endianness, what endianness a particular device
>   (and hence its MMIO region(s)) is using is entirely independent.
>   Hence conversion, where necessary, needs to occur at a layer up.
>   Another one reason to drop endianess conversion here is:
>   
> https://patchwork.kernel.org/project/linux-riscv/patch/20190411115623.5749-3-hch@xxxxxx/
>   One of the answers of the author of the commit:
>     And we don't know if Linux will be around if that ever changes.
>     The point is:
>      a) the current RISC-V spec is LE only
>      b) the current linux port is LE only except for this little bit
>     There is no point in leaving just this bitrotting code around.  It
>     just confuses developers, (very very slightly) slows down compiles
>     and will bitrot.  It also won't be any significant help to a future
>     developer down the road doing a hypothetical BE RISC-V Linux port.
> - drop unused argument of __io_ar() macros.
> - drop "#define _raw_{read,write}{b,w,l,d,q} _raw_{read,write}{b,w,l,d,q}"
>   as they are unnecessary.
> - Adopt the Xen code style for this header, considering that significant 
> changes
>   are not anticipated in the future.
>   In the event of any issues, adapting them to Xen style should be easily
>   manageable.
> - drop unnecessary  __r variables in macros read*_cpu()
> - update inline assembler constraints for addr argument for
>   __raw_read{b,w,l,q} and __raw_write{b,w,l,q} to tell a compiler that
>  *addr will be accessed.
> - add stubs for __raw_readq() and __raw_writeq() for RISCV_32
> 
> Addionally, to the header was added definions of ioremap_*().
> 
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
despite ...

> --- /dev/null
> +++ b/xen/arch/riscv/include/asm/io.h
> @@ -0,0 +1,168 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + *  The header taken form Linux 6.4.0-rc1 and is based on
> + *  arch/riscv/include/asm/mmio.h with the following changes:
> + *   - drop forcing of endianess for read*(), write*() functions as
> + *     no matter what CPU endianness, what endianness a particular device
> + *     (and hence its MMIO region(s)) is using is entirely independent.
> + *     Hence conversion, where necessary, needs to occur at a layer up.
> + *     Another one reason to drop endianess conversion is:
> + *     
> https://patchwork.kernel.org/project/linux-riscv/patch/20190411115623.5749-3-hch@xxxxxx/
> + *     One of the answers of the author of the commit:
> + *       And we don't know if Linux will be around if that ever changes.
> + *       The point is:
> + *        a) the current RISC-V spec is LE only
> + *        b) the current linux port is LE only except for this little bit
> + *       There is no point in leaving just this bitrotting code around.  It
> + *       just confuses developers, (very very slightly) slows down compiles
> + *      and will bitrot.  It also won't be any significant help to a future
> + *       developer down the road doing a hypothetical BE RISC-V Linux port.
> + *   - drop unused argument of __io_ar() macros.
> + *   - drop "#define _raw_{read,write}{b,w,l,q} _raw_{read,write}{b,w,l,q}"
> + *     as they are unnecessary.
> + *   - Adopt the Xen code style for this header, considering that significant
> + *     changes are not anticipated in the future.
> + *     In the event of any issues, adapting them to Xen style should be 
> easily
> + *     manageable.
> + *   - drop unnecessary __r variables in macros read*_cpu()
> + *   - update inline assembler constraints for addr argument for
> + *     __raw_read{b,w,l,q} and __raw_write{b,w,l,q} to tell a compiler that
> + *     *addr will be accessed.
> + *
> + * Copyright (C) 1996-2000 Russell King
> + * Copyright (C) 2012 ARM Ltd.
> + * Copyright (C) 2014 Regents of the University of California
> + * Copyright (C) 2024 Vates
> + */
> +
> +#ifndef _ASM_RISCV_IO_H
> +#define _ASM_RISCV_IO_H
> +
> +#include <asm/byteorder.h>
> +
> +/*
> + * The RISC-V ISA doesn't yet specify how to query or modify PMAs, so we 
> can't
> + * change the properties of memory regions.  This should be fixed by the
> + * upcoming platform spec.
> + */
> +#define ioremap_nocache(addr, size) ioremap(addr, size)
> +#define ioremap_wc(addr, size) ioremap(addr, size)
> +#define ioremap_wt(addr, size) ioremap(addr, size)
> +
> +/* Generic IO read/write.  These perform native-endian accesses. */
> +static inline void __raw_writeb(uint8_t val, volatile void __iomem *addr)
> +{
> +    asm volatile ( "sb %1, %0"
> +                   : "=m" (*(volatile uint8_t __force *)addr) : "r" (val) );
> +}
> +
> +static inline void __raw_writew(uint16_t val, volatile void __iomem *addr)
> +{
> +    asm volatile ( "sh %1, %0"
> +                   : "=m" (*(volatile uint16_t __force *)addr) : "r" (val) );
> +}
> +
> +static inline void __raw_writel(uint32_t val, volatile void __iomem *addr)
> +{
> +    asm volatile ( "sw %1, %0"
> +                   : "=m" (*(volatile uint32_t __force *)addr) : "r" (val) );
> +}
> +
> +static inline void __raw_writeq(uint64_t val, volatile void __iomem *addr)
> +{
> +#ifdef CONFIG_RISCV_32
> +    BUILD_BUG_ON("unimplemented");
> +#else
> +    asm volatile ( "sd %1, %0"
> +                   : "=m" (*(volatile uint64_t __force *)addr) : "r" (val) );
> +#endif
> +}

... this and its read counterpart likely being in need of re-doing by anyone
wanting to enable RV32 support.

Jan