2023-08-03 05:47:01

by Andrea Parri

[permalink] [raw]
Subject: [PATCH v2] riscv,mmio: Fix readX()-to-delay() ordering

Section 2.1 of the Platform Specification [1] states:

Unless otherwise specified by a given I/O device, I/O devices are on
ordering channel 0 (i.e., they are point-to-point strongly ordered).

which is not sufficient to guarantee that a readX() by a hart completes
before a subsequent delay() on the same hart (cf. memory-barriers.txt,
"Kernel I/O barrier effects").

Set the I(nput) bit in __io_ar() to restore the ordering, align inline
comments.

[1] https://github.com/riscv/riscv-platform-specs

Signed-off-by: Andrea Parri <[email protected]>
---
Changes since v1:
https://lore.kernel.org/lkml/[email protected]/

- surrendered to the intricacies of NOMMU/RV32 builds: dropping #2
(takers are welcome! ;-) )


arch/riscv/include/asm/mmio.h | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/include/asm/mmio.h b/arch/riscv/include/asm/mmio.h
index aff6c33ab0c08..4c58ee7f95ecf 100644
--- a/arch/riscv/include/asm/mmio.h
+++ b/arch/riscv/include/asm/mmio.h
@@ -101,9 +101,9 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
* Relaxed I/O memory access primitives. These follow the Device memory
* ordering rules but do not guarantee any ordering relative to Normal memory
* accesses. These are defined to order the indicated access (either a read or
- * write) with all other I/O memory accesses. Since the platform specification
- * defines that all I/O regions are strongly ordered on channel 2, no explicit
- * fences are required to enforce this ordering.
+ * write) with all other I/O memory accesses to the same peripheral. Since the
+ * platform specification defines that all I/O regions are strongly ordered on
+ * channel 0, no explicit fences are required to enforce this ordering.
*/
/* FIXME: These are now the same as asm-generic */
#define __io_rbr() do {} while (0)
@@ -125,14 +125,14 @@ static inline u64 __raw_readq(const volatile void __iomem *addr)
#endif

/*
- * 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
+ * I/O memory access primitives. Reads are ordered relative to any following
+ * Normal memory read and delay() loop. Writes are ordered relative to any
+ * prior Normal memory write. 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(v) __asm__ __volatile__ ("fence i,r" : : : "memory")
-#define __io_bw() __asm__ __volatile__ ("fence w,o" : : : "memory")
+#define __io_ar(v) ({ __asm__ __volatile__ ("fence i,ir" : : : "memory"); })
+#define __io_bw() ({ __asm__ __volatile__ ("fence w,o" : : : "memory"); })
#define __io_aw() mmiowb_set_pending()

#define readb(c) ({ u8 __v; __io_br(); __v = readb_cpu(c); __io_ar(__v); __v; })
--
2.34.1



Subject: Re: [PATCH v2] riscv,mmio: Fix readX()-to-delay() ordering

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

On Thu, 3 Aug 2023 06:27:38 +0200 you wrote:
> Section 2.1 of the Platform Specification [1] states:
>
> Unless otherwise specified by a given I/O device, I/O devices are on
> ordering channel 0 (i.e., they are point-to-point strongly ordered).
>
> which is not sufficient to guarantee that a readX() by a hart completes
> before a subsequent delay() on the same hart (cf. memory-barriers.txt,
> "Kernel I/O barrier effects").
>
> [...]

Here is the summary with links:
- [v2] riscv,mmio: Fix readX()-to-delay() ordering
https://git.kernel.org/riscv/c/4eb2eb1b4c0e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2023-08-10 20:16:35

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v2] riscv,mmio: Fix readX()-to-delay() ordering


On Thu, 03 Aug 2023 06:27:38 +0200, Andrea Parri wrote:
> Section 2.1 of the Platform Specification [1] states:
>
> Unless otherwise specified by a given I/O device, I/O devices are on
> ordering channel 0 (i.e., they are point-to-point strongly ordered).
>
> which is not sufficient to guarantee that a readX() by a hart completes
> before a subsequent delay() on the same hart (cf. memory-barriers.txt,
> "Kernel I/O barrier effects").
>
> [...]

Applied, thanks!

[1/1] riscv,mmio: Fix readX()-to-delay() ordering
https://git.kernel.org/palmer/c/4eb2eb1b4c0e

Best regards,
--
Palmer Dabbelt <[email protected]>