Message ID | dae9f595e5afd1e6a46149919e6689afe263e1ce.1710517542.git.oleksii.kurochko@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Enable build of full Xen for RISC-V | expand |
On 15.03.2024 19:06, Oleksii Kurochko wrote: > --- /dev/null > +++ b/xen/arch/riscv/include/asm/io.h > @@ -0,0 +1,167 @@ > +/* 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@lst.de/ > + * 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}" In the commit message I'm not worried as much, but at least here the odd mention of d as suffixes would better be purged. > + * 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) ); Please respect line length restrictions. > +} > + > +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) ); > +} > + > +#ifdef CONFIG_RISCV_32 > +static inline void __raw_writeq(uint64_t val, volatile void __iomem *addr) > +{ > + BUILD_BUG_ON("unimplemented\n"); > +} > +#else > +static inline void __raw_writeq(uint64_t val, volatile void __iomem *addr) > +{ > + asm volatile ( "sd %1, %0" : "=m" (*(volatile uint64_t __force *)addr) : "r" (val) ); > +} > +#endif Please avoid code duplication if it easily can be avoided: 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 } (Note also the dropped \n.) > +static inline uint8_t __raw_readb(const volatile void __iomem *addr) > +{ > + uint8_t val; > + > + asm volatile ( "lb %0, %1" : "=r" (val) : "m" (*(volatile uint8_t __force *)addr) ); Please don't cast away const. > + return val; > +} > + > +static inline uint16_t __raw_readw(const volatile void __iomem *addr) > +{ > + uint16_t val; > + > + asm volatile ( "lh %0, %1" : "=r" (val) : "m" (*(volatile uint16_t __force *)addr) ); > + return val; > +} > + > +static inline uint32_t __raw_readl(const volatile void __iomem *addr) > +{ > + uint32_t val; > + > + asm volatile ( "lw %0, %1" : "=r" (val) : "m" (*(volatile uint32_t __force *)addr) ); > + return val; > +} > + > +#ifdef CONFIG_RISCV_32 > +static inline uint64_t __raw_readq(const volatile void __iomem *addr) > +{ > + BUILD_BUG_ON("unimplemented\n"); > + return 0; > +} > +#else > +static inline uint64_t __raw_readq(const volatile void __iomem *addr) > +{ > + uint64_t val; > + > + asm volatile ( "ld %0, %1" : "=r" (val) : "m" (*(volatile uint64_t __force *)addr) ); > + return val; > +} > +#endif > + > +/* > + * Unordered I/O memory access primitives. These are even more relaxed than > + * the relaxed versions, as they don't even order accesses between successive > + * operations to the I/O regions. > + */ > +#define readb_cpu(c) __raw_readb(c) > +#define readw_cpu(c) __raw_readw(c) > +#define readl_cpu(c) __raw_readl(c) > + > +#define writeb_cpu(v, c) __raw_writeb(v, c) > +#define writew_cpu(v, c) __raw_writew(v, c) > +#define writel_cpu(v, c) __raw_writel(v, c) > + > +#define readq_cpu(c) __raw_readq(c) > +#define writeq_cpu(v, c) __raw_writeq(v, c) Since (now?) outside of any #ifdef (which is okay with me), why not each grouped with their respective 3 siblings? > +/* > + * I/O memory access primitives. Reads are ordered relative to any > + * following Normal memory access. Writes are ordered relative to any prior > + * Normal memory access. The memory barriers here are necessary as RISC-V > + * doesn't define any ordering between the memory space and the I/O space. > + */ > +#define __io_br() do { } while (0) > +#define __io_ar() asm volatile ( "fence i,r" : : : "memory" ); > +#define __io_bw() asm volatile ( "fence w,o" : : : "memory" ); > +#define __io_aw() do { } while (0) > + > +#define readb(c) ({ uint8_t v; __io_br(); v = readb_cpu(c); __io_ar(); v; }) I'm wary of "v" as a macro local: What if at a use site there's x = readb(v); ? That'll fail to compile, provided at the use site v is actually a pointer, but the reason for the error won't necessarily be obvious. (Hence why in the past people did prefix underscores on such local variable names, while now we're advocating for suffixes.) > +#define readw(c) ({ uint16_t v; __io_br(); v = readw_cpu(c); __io_ar(); v; }) > +#define readl(c) ({ uint32_t v; __io_br(); v = readl_cpu(c); __io_ar(); v; }) > + > +#define writeb(v, c) ({ __io_bw(); writeb_cpu(v, c); __io_aw(); }) > +#define writew(v, c) ({ __io_bw(); writew_cpu(v, c); __io_aw(); }) > +#define writel(v, c) ({ __io_bw(); writel_cpu(v, c); __io_aw(); }) > + > +#define readq(c) ({ uint64_t v; __io_br(); v = readq_cpu(c); __io_ar(); v; }) > +#define writeq(v, c) ({ __io_bw(); writeq_cpu(v, c); __io_aw(); }) Same here then. Jan
On Thu, 2024-03-21 at 13:27 +0100, Jan Beulich wrote: > On 15.03.2024 19:06, Oleksii Kurochko wrote: > > --- /dev/null > > +++ b/xen/arch/riscv/include/asm/io.h > > @@ -0,0 +1,167 @@ > > +/* 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@lst.de/ > > + * 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}" > > In the commit message I'm not worried as much, but at least here the > odd mention > of d as suffixes would better be purged. Probably, I use incorrect words, but what I meant that it was dropped: #define _raw_{read,write}{b,w,l,d,q} _raw_{read,write}{b,w,l,d,q} before declaration/definition of inline functions ( __raw_{read,write}{b,w,l,d,q} ). ~ Oleksii
On 22.03.2024 12:02, Oleksii wrote: > On Thu, 2024-03-21 at 13:27 +0100, Jan Beulich wrote: >> On 15.03.2024 19:06, Oleksii Kurochko wrote: >>> --- /dev/null >>> +++ b/xen/arch/riscv/include/asm/io.h >>> @@ -0,0 +1,167 @@ >>> +/* 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@lst.de/ >>> + * 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}" >> >> In the commit message I'm not worried as much, but at least here the >> odd mention >> of d as suffixes would better be purged. > Probably, I use incorrect words, but what I meant that it was dropped: > #define _raw_{read,write}{b,w,l,d,q} _raw_{read,write}{b,w,l,d,q} > before declaration/definition of inline functions ( > __raw_{read,write}{b,w,l,d,q} ). But where did you find a raw_readd() or raw_writed() (with no matter how many leading underscores)? Jan
On Fri, 2024-03-22 at 12:36 +0100, Jan Beulich wrote: > On 22.03.2024 12:02, Oleksii wrote: > > On Thu, 2024-03-21 at 13:27 +0100, Jan Beulich wrote: > > > On 15.03.2024 19:06, Oleksii Kurochko wrote: > > > > --- /dev/null > > > > +++ b/xen/arch/riscv/include/asm/io.h > > > > @@ -0,0 +1,167 @@ > > > > +/* 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@lst.de/ > > > > + * 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}" > > > > > > In the commit message I'm not worried as much, but at least here > > > the > > > odd mention > > > of d as suffixes would better be purged. > > Probably, I use incorrect words, but what I meant that it was > > dropped: > > #define _raw_{read,write}{b,w,l,d,q} > > _raw_{read,write}{b,w,l,d,q} > > before declaration/definition of inline functions ( > > __raw_{read,write}{b,w,l,d,q} ). > > But where did you find a raw_readd() or raw_writed() (with no matter > how > many leading underscores)? Oh, {d} options didn't exist. Missed that, wrote it automatically. Thanks I'll update the commit message and the header comment. ~ Oleksii
diff --git a/xen/arch/riscv/include/asm/io.h b/xen/arch/riscv/include/asm/io.h new file mode 100644 index 0000000000..4eb4cd4b49 --- /dev/null +++ b/xen/arch/riscv/include/asm/io.h @@ -0,0 +1,167 @@ +/* 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@lst.de/ + * 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. + * + * 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) ); +} + +#ifdef CONFIG_RISCV_32 +static inline void __raw_writeq(uint64_t val, volatile void __iomem *addr) +{ + BUILD_BUG_ON("unimplemented\n"); +} +#else +static inline void __raw_writeq(uint64_t val, volatile void __iomem *addr) +{ + asm volatile ( "sd %1, %0" : "=m" (*(volatile uint64_t __force *)addr) : "r" (val) ); +} +#endif + +static inline uint8_t __raw_readb(const volatile void __iomem *addr) +{ + uint8_t val; + + asm volatile ( "lb %0, %1" : "=r" (val) : "m" (*(volatile uint8_t __force *)addr) ); + return val; +} + +static inline uint16_t __raw_readw(const volatile void __iomem *addr) +{ + uint16_t val; + + asm volatile ( "lh %0, %1" : "=r" (val) : "m" (*(volatile uint16_t __force *)addr) ); + return val; +} + +static inline uint32_t __raw_readl(const volatile void __iomem *addr) +{ + uint32_t val; + + asm volatile ( "lw %0, %1" : "=r" (val) : "m" (*(volatile uint32_t __force *)addr) ); + return val; +} + +#ifdef CONFIG_RISCV_32 +static inline uint64_t __raw_readq(const volatile void __iomem *addr) +{ + BUILD_BUG_ON("unimplemented\n"); + return 0; +} +#else +static inline uint64_t __raw_readq(const volatile void __iomem *addr) +{ + uint64_t val; + + asm volatile ( "ld %0, %1" : "=r" (val) : "m" (*(volatile uint64_t __force *)addr) ); + return val; +} +#endif + +/* + * Unordered I/O memory access primitives. These are even more relaxed than + * the relaxed versions, as they don't even order accesses between successive + * operations to the I/O regions. + */ +#define readb_cpu(c) __raw_readb(c) +#define readw_cpu(c) __raw_readw(c) +#define readl_cpu(c) __raw_readl(c) + +#define writeb_cpu(v, c) __raw_writeb(v, c) +#define writew_cpu(v, c) __raw_writew(v, c) +#define writel_cpu(v, c) __raw_writel(v, c) + +#define readq_cpu(c) __raw_readq(c) +#define writeq_cpu(v, c) __raw_writeq(v, c) + +/* + * I/O memory access primitives. Reads are ordered relative to any + * following Normal memory access. Writes are ordered relative to any prior + * Normal memory access. The memory barriers here are necessary as RISC-V + * doesn't define any ordering between the memory space and the I/O space. + */ +#define __io_br() do { } while (0) +#define __io_ar() asm volatile ( "fence i,r" : : : "memory" ); +#define __io_bw() asm volatile ( "fence w,o" : : : "memory" ); +#define __io_aw() do { } while (0) + +#define readb(c) ({ uint8_t v; __io_br(); v = readb_cpu(c); __io_ar(); v; }) +#define readw(c) ({ uint16_t v; __io_br(); v = readw_cpu(c); __io_ar(); v; }) +#define readl(c) ({ uint32_t v; __io_br(); v = readl_cpu(c); __io_ar(); v; }) + +#define writeb(v, c) ({ __io_bw(); writeb_cpu(v, c); __io_aw(); }) +#define writew(v, c) ({ __io_bw(); writew_cpu(v, c); __io_aw(); }) +#define writel(v, c) ({ __io_bw(); writel_cpu(v, c); __io_aw(); }) + +#define readq(c) ({ uint64_t v; __io_br(); v = readq_cpu(c); __io_ar(); v; }) +#define writeq(v, c) ({ __io_bw(); writeq_cpu(v, c); __io_aw(); }) + +#endif /* _ASM_RISCV_IO_H */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * indent-tabs-mode: nil + * End: + */
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@lst.de/ 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@gmail.com> --- Changes in V6: - drop unnecessary spaces and fix typos in the file comment. - s/CONFIG_64BIT/CONFIG_RISCV_32 as .d suffix for instruction doesn't exist for RV32. - add stubs for __raw_readq() and __raw_writeq() for RISCV_32 - update inline assembler constraints for addr argument for __raw_read{b,w,l,q} and __raw_write{b,w,l,q} to tell compiler that *addr will be accessed. - s/u8/uint8_t - update the commit message --- Changes in V5: - Xen code style related fixes - drop #define _raw_{read,write}{b,w,l,d,q} _raw_{read,write}{b,w,l,d,q} - drop cpu_to_le16() - remove unuused argument in _io_ar() - update the commit message - drop unnessary __r variables in macros read*_cpu() - update the comments at the top of the header. --- Changes in V4: - delete inner parentheses in macros. - s/u<N>/uint<N>. --- Changes in V3: - re-sync with linux kernel - update the commit message --- Changes in V2: - Nothing changed. Only rebase. --- xen/arch/riscv/include/asm/io.h | 167 ++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 xen/arch/riscv/include/asm/io.h