[PATCH v1] iommu/riscv: Support 32-bit register accesses

[PATCH v1] iommu/riscv: Support 32-bit register accesses

[Zhanpeng Zhang]

Some RISC-V IOMMU implementations cannot perform 64-bit MMIO accesses
to the IOMMU register file. The RISC-V IOMMU architecture allows 64-bit
registers to be accessed using 32-bit accesses, provided the accesses are
properly aligned and do not span multiple registers.

Add a config option for such implementations and access 64-bit IOMMU
registers as paired 32-bit MMIO operations when it is enabled. Serialize
the paired accesses so the high and low halves cannot interleave with
another CPU. Full 64-bit register programming writes the high half before
the low half.

This option describes the register access width. It is not an RV32 kernel
mode and does not describe a 32-bit IOMMU architecture.

Co-developed-by: Xu Lu <luxu.kernel@bytedance.com>
Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
---
This is needed for platforms whose RISC-V IOMMU register window does not
support naturally aligned 64-bit MMIO accesses.

 drivers/iommu/riscv/Kconfig | 11 ++++++++
 drivers/iommu/riscv/iommu.c |  4 +++
 drivers/iommu/riscv/iommu.h | 55 +++++++++++++++++++++++++++++++++----
 3 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
index b86e5ab94183..54d624b9b2ef 100644
--- a/drivers/iommu/riscv/Kconfig
+++ b/drivers/iommu/riscv/Kconfig
@@ -22,3 +22,14 @@ config RISCV_IOMMU_PCI
 	def_bool y if RISCV_IOMMU && PCI_MSI
 	help
 	  Support for the PCIe implementation of RISC-V IOMMU architecture.
+
+config RISCV_IOMMU_32BIT_ACCESS
+	bool "Use 32-bit accesses for RISC-V IOMMU registers"
+	depends on RISCV_IOMMU
+	help
+	  Say Y when the RISC-V IOMMU MMIO window cannot be accessed
+	  using naturally aligned 64-bit loads and stores.
+
+	  When enabled, 64-bit IOMMU registers are accessed as paired
+	  32-bit MMIO operations. This option does not describe an RV32
+	  kernel or a 32-bit IOMMU architecture.
diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
index a31f50bbad35..7fa1721b5728 100644
--- a/drivers/iommu/riscv/iommu.c
+++ b/drivers/iommu/riscv/iommu.c
@@ -53,6 +53,10 @@ struct riscv_iommu_devres {
 	void *addr;
 };
 
+#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
+DEFINE_RAW_SPINLOCK(riscv_iommu_32bit_access_lock);
+#endif
+
 static void riscv_iommu_devres_pages_release(struct device *dev, void *res)
 {
 	struct riscv_iommu_devres *devres = res;
diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
index 46df79dd5495..ba78ef1858c5 100644
--- a/drivers/iommu/riscv/iommu.h
+++ b/drivers/iommu/riscv/iommu.h
@@ -14,6 +14,9 @@
 #include <linux/iommu.h>
 #include <linux/types.h>
 #include <linux/iopoll.h>
+#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
+#include <linux/spinlock.h>
+#endif
 
 #include "iommu-bits.h"
 
@@ -69,21 +72,61 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
 #define riscv_iommu_readl(iommu, addr) \
 	readl_relaxed((iommu)->reg + (addr))
 
-#define riscv_iommu_readq(iommu, addr) \
-	readq_relaxed((iommu)->reg + (addr))
-
 #define riscv_iommu_writel(iommu, addr, val) \
 	writel_relaxed((val), (iommu)->reg + (addr))
 
+#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
+	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
+			   delay_us, timeout_us)
+
+#ifndef CONFIG_RISCV_IOMMU_32BIT_ACCESS
+#define riscv_iommu_readq(iommu, addr) \
+	readq_relaxed((iommu)->reg + (addr))
+
 #define riscv_iommu_writeq(iommu, addr, val) \
 	writeq_relaxed((val), (iommu)->reg + (addr))
 
 #define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
 	readx_poll_timeout(readq_relaxed, (iommu)->reg + (addr), val, cond, \
 			   delay_us, timeout_us)
+#else /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
 
-#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
-	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
-			   delay_us, timeout_us)
+extern raw_spinlock_t riscv_iommu_32bit_access_lock;
+
+static inline u64 __riscv_iommu_readq_relaxed(void __iomem *addr)
+{
+	u32 lo, hi;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
+	do {
+		hi = readl_relaxed(addr + sizeof(u32));
+		lo = readl_relaxed(addr);
+	} while (hi != readl_relaxed(addr + sizeof(u32)));
+	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
+
+	return ((u64)hi << 32) | (u64)lo;
+}
+
+static inline void __riscv_iommu_writeq_relaxed(u64 value, void __iomem *addr)
+{
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
+	writel_relaxed((u32)(value >> 32), addr + sizeof(u32));
+	writel_relaxed((u32)value, addr);
+	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
+}
+
+#define riscv_iommu_readq(iommu, addr) \
+	__riscv_iommu_readq_relaxed((iommu)->reg + (addr))
+
+#define riscv_iommu_writeq(iommu, addr, val) \
+	__riscv_iommu_writeq_relaxed((val), (iommu)->reg + (addr))
+
+#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
+	readx_poll_timeout(__riscv_iommu_readq_relaxed, (iommu)->reg + (addr), \
+			   val, cond, delay_us, timeout_us)
+#endif /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
 
 #endif
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[Andreas Schwab]

On Jun 15 2026, Zhanpeng Zhang wrote:

> +config RISCV_IOMMU_32BIT_ACCESS
> +	bool "Use 32-bit accesses for RISC-V IOMMU registers"
> +	depends on RISCV_IOMMU
> +	help
> +	  Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> +	  using naturally aligned 64-bit loads and stores.
> +
> +	  When enabled, 64-bit IOMMU registers are accessed as paired
> +	  32-bit MMIO operations. This option does not describe an RV32
> +	  kernel or a 32-bit IOMMU architecture.

What is the expected setting in a generic kernel?

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[Zhanpeng Zhang]

Hi Andreas,

For a generic kernel, the expected setting is to leave
RISCV_IOMMU_32BIT_ACCESS disabled.

If supporting such platforms from a single generic kernel is preferred, I can
look into replacing this build-time option with a runtime indication.

Regards,
Zhanpeng

> From: "Andreas Schwab"<schwab@suse.de>
> Date:  Mon, Jun 15, 2026, 16:22
> Subject:  [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses
> To: "Zhanpeng Zhang"<zhangzhanpeng.jasper@bytedance.com>
> Cc: "Tomasz Jeznach"<tjeznach@rivosinc.com>, "Joerg Roedel"<joro@8bytes.org>, "Will Deacon"<will@kernel.org>, "Robin Murphy"<robin.murphy@arm.com>, "Paul Walmsley"<pjw@kernel.org>, "Palmer Dabbelt"<palmer@dabbelt.com>, "Albert Ou"<aou@eecs.berkeley.edu>, "Alexandre Ghiti"<alex@ghiti.fr>, <iommu@lists.linux.dev>, <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>, "Xu Lu"<luxu.kernel@bytedance.com>, <cuiyunhui@bytedance.com>, <yuanzhu@bytedance.com>
> On Jun 15 2026, Zhanpeng Zhang wrote:
>
> > +config RISCV_IOMMU_32BIT_ACCESS
> > +        bool "Use 32-bit accesses for RISC-V IOMMU registers"
> > +        depends on RISCV_IOMMU
> > +        help
> > +          Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> > +          using naturally aligned 64-bit loads and stores.
> > +
> > +          When enabled, 64-bit IOMMU registers are accessed as paired
> > +          32-bit MMIO operations. This option does not describe an RV32
> > +          kernel or a 32-bit IOMMU architecture.
>
> What is the expected setting in a generic kernel?
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
>

Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[David Laight]

On Mon, 15 Jun 2026 14:48:55 +0800
Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com> wrote:

> Some RISC-V IOMMU implementations cannot perform 64-bit MMIO accesses
> to the IOMMU register file. The RISC-V IOMMU architecture allows 64-bit
> registers to be accessed using 32-bit accesses, provided the accesses are
> properly aligned and do not span multiple registers.
> 
> Add a config option for such implementations and access 64-bit IOMMU
> registers as paired 32-bit MMIO operations when it is enabled. Serialize
> the paired accesses so the high and low halves cannot interleave with
> another CPU. Full 64-bit register programming writes the high half before
> the low half.
> 
> This option describes the register access width. It is not an RV32 kernel
> mode and does not describe a 32-bit IOMMU architecture.
> 
> Co-developed-by: Xu Lu <luxu.kernel@bytedance.com>
> Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
> ---
> This is needed for platforms whose RISC-V IOMMU register window does not
> support naturally aligned 64-bit MMIO accesses.
> 
>  drivers/iommu/riscv/Kconfig | 11 ++++++++
>  drivers/iommu/riscv/iommu.c |  4 +++
>  drivers/iommu/riscv/iommu.h | 55 +++++++++++++++++++++++++++++++++----
>  3 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
> index b86e5ab94183..54d624b9b2ef 100644
> --- a/drivers/iommu/riscv/Kconfig
> +++ b/drivers/iommu/riscv/Kconfig
> @@ -22,3 +22,14 @@ config RISCV_IOMMU_PCI
>  	def_bool y if RISCV_IOMMU && PCI_MSI
>  	help
>  	  Support for the PCIe implementation of RISC-V IOMMU architecture.
> +
> +config RISCV_IOMMU_32BIT_ACCESS
> +	bool "Use 32-bit accesses for RISC-V IOMMU registers"
> +	depends on RISCV_IOMMU
> +	help
> +	  Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> +	  using naturally aligned 64-bit loads and stores.
> +
> +	  When enabled, 64-bit IOMMU registers are accessed as paired
> +	  32-bit MMIO operations. This option does not describe an RV32
> +	  kernel or a 32-bit IOMMU architecture.
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index a31f50bbad35..7fa1721b5728 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -53,6 +53,10 @@ struct riscv_iommu_devres {
>  	void *addr;
>  };
>  
> +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +DEFINE_RAW_SPINLOCK(riscv_iommu_32bit_access_lock);
> +#endif
> +
>  static void riscv_iommu_devres_pages_release(struct device *dev, void *res)
>  {
>  	struct riscv_iommu_devres *devres = res;
> diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
> index 46df79dd5495..ba78ef1858c5 100644
> --- a/drivers/iommu/riscv/iommu.h
> +++ b/drivers/iommu/riscv/iommu.h
> @@ -14,6 +14,9 @@
>  #include <linux/iommu.h>
>  #include <linux/types.h>
>  #include <linux/iopoll.h>
> +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +#include <linux/spinlock.h>
> +#endif
>  
>  #include "iommu-bits.h"
>  
> @@ -69,21 +72,61 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
>  #define riscv_iommu_readl(iommu, addr) \
>  	readl_relaxed((iommu)->reg + (addr))
>  
> -#define riscv_iommu_readq(iommu, addr) \
> -	readq_relaxed((iommu)->reg + (addr))
> -
>  #define riscv_iommu_writel(iommu, addr, val) \
>  	writel_relaxed((val), (iommu)->reg + (addr))
>  
> +#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> +	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> +			   delay_us, timeout_us)
> +
> +#ifndef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +#define riscv_iommu_readq(iommu, addr) \
> +	readq_relaxed((iommu)->reg + (addr))
> +
>  #define riscv_iommu_writeq(iommu, addr, val) \
>  	writeq_relaxed((val), (iommu)->reg + (addr))
>  
>  #define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
>  	readx_poll_timeout(readq_relaxed, (iommu)->reg + (addr), val, cond, \
>  			   delay_us, timeout_us)
> +#else /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
>  
> -#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> -	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> -			   delay_us, timeout_us)
> +extern raw_spinlock_t riscv_iommu_32bit_access_lock;
> +
> +static inline u64 __riscv_iommu_readq_relaxed(void __iomem *addr)
> +{
> +	u32 lo, hi;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> +	do {
> +		hi = readl_relaxed(addr + sizeof(u32));
> +		lo = readl_relaxed(addr);
> +	} while (hi != readl_relaxed(addr + sizeof(u32)));
> +	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);

Why the lock and the loop?
If you need the loop (for places where the hardware might be changing
both words?) then there is no need to read 'hi' twice when you loop.

But does it make any sense to be reading the value either while the hardware
is updating it (does that happen?) or concurrently with a write.


> +
> +	return ((u64)hi << 32) | (u64)lo;
> +}
> +
> +static inline void __riscv_iommu_writeq_relaxed(u64 value, void __iomem *addr)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> +	writel_relaxed((u32)(value >> 32), addr + sizeof(u32));
> +	writel_relaxed((u32)value, addr);
> +	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);

What are you actually locking against?
I'm pretty sure it doesn't make any sense for two cpu to be writing to the
same entry at the same time.

Can't the writel_relaxed() be re-ordered, so you aren't guaranteeing they'll
be done high-low.
Same with the reads.

	David

> +}
> +
> +#define riscv_iommu_readq(iommu, addr) \
> +	__riscv_iommu_readq_relaxed((iommu)->reg + (addr))
> +
> +#define riscv_iommu_writeq(iommu, addr, val) \
> +	__riscv_iommu_writeq_relaxed((val), (iommu)->reg + (addr))
> +
> +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> +	readx_poll_timeout(__riscv_iommu_readq_relaxed, (iommu)->reg + (addr), \
> +			   val, cond, delay_us, timeout_us)
> +#endif /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
>  
>  #endif

Re: [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[Zhanpeng Zhang]

Hi David,

Thanks, I agree with your point about the lock and the relaxed accessors.

Guo Ren also pointed me to the earlier discussion on this topic, and I think the
right next step is to align with that thread rather than continue this separate
Kconfig-based patch.

So I will drop this patch.

Thanks,
Zhanpeng

> From: "David Laight"<david.laight.linux@gmail.com>
> Date:  Mon, Jun 15, 2026, 18:00
> Subject:  [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses
> To: "Zhanpeng Zhang"<zhangzhanpeng.jasper@bytedance.com>
> Cc: "Tomasz Jeznach"<tjeznach@rivosinc.com>, "Joerg Roedel"<joro@8bytes.org>, "Will Deacon"<will@kernel.org>, "Robin Murphy"<robin.murphy@arm.com>, "Paul Walmsley"<pjw@kernel.org>, "Palmer Dabbelt"<palmer@dabbelt.com>, "Albert Ou"<aou@eecs.berkeley.edu>, "Alexandre Ghiti"<alex@ghiti.fr>, <iommu@lists.linux.dev>, <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>, "Xu Lu"<luxu.kernel@bytedance.com>, <cuiyunhui@bytedance.com>, <yuanzhu@bytedance.com>
> On Mon, 15 Jun 2026 14:48:55 +0800
> Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com> wrote:
>
> > Some RISC-V IOMMU implementations cannot perform 64-bit MMIO accesses
> > to the IOMMU register file. The RISC-V IOMMU architecture allows 64-bit
> > registers to be accessed using 32-bit accesses, provided the accesses are
> > properly aligned and do not span multiple registers.
> >
> > Add a config option for such implementations and access 64-bit IOMMU
> > registers as paired 32-bit MMIO operations when it is enabled. Serialize
> > the paired accesses so the high and low halves cannot interleave with
> > another CPU. Full 64-bit register programming writes the high half before
> > the low half.
> >
> > This option describes the register access width. It is not an RV32 kernel
> > mode and does not describe a 32-bit IOMMU architecture.
> >
> > Co-developed-by: Xu Lu <luxu.kernel@bytedance.com>
> > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> > Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
> > ---
> > This is needed for platforms whose RISC-V IOMMU register window does not
> > support naturally aligned 64-bit MMIO accesses.
> >
> >  drivers/iommu/riscv/Kconfig | 11 ++++++++
> >  drivers/iommu/riscv/iommu.c |  4 +++
> >  drivers/iommu/riscv/iommu.h | 55 +++++++++++++++++++++++++++++++++----
> >  3 files changed, 64 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
> > index b86e5ab94183..54d624b9b2ef 100644
> > --- a/drivers/iommu/riscv/Kconfig
> > +++ b/drivers/iommu/riscv/Kconfig
> > @@ -22,3 +22,14 @@ config RISCV_IOMMU_PCI
> >          def_bool y if RISCV_IOMMU && PCI_MSI
> >          help
> >            Support for the PCIe implementation of RISC-V IOMMU architecture.
> > +
> > +config RISCV_IOMMU_32BIT_ACCESS
> > +        bool "Use 32-bit accesses for RISC-V IOMMU registers"
> > +        depends on RISCV_IOMMU
> > +        help
> > +          Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> > +          using naturally aligned 64-bit loads and stores.
> > +
> > +          When enabled, 64-bit IOMMU registers are accessed as paired
> > +          32-bit MMIO operations. This option does not describe an RV32
> > +          kernel or a 32-bit IOMMU architecture.
> > diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> > index a31f50bbad35..7fa1721b5728 100644
> > --- a/drivers/iommu/riscv/iommu.c
> > +++ b/drivers/iommu/riscv/iommu.c
> > @@ -53,6 +53,10 @@ struct riscv_iommu_devres {
> >          void *addr;
> >  };
> >
> > +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +DEFINE_RAW_SPINLOCK(riscv_iommu_32bit_access_lock);
> > +#endif
> > +
> >  static void riscv_iommu_devres_pages_release(struct device *dev, void *res)
> >  {
> >          struct riscv_iommu_devres *devres = res;
> > diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
> > index 46df79dd5495..ba78ef1858c5 100644
> > --- a/drivers/iommu/riscv/iommu.h
> > +++ b/drivers/iommu/riscv/iommu.h
> > @@ -14,6 +14,9 @@
> >  #include <linux/iommu.h>
> >  #include <linux/types.h>
> >  #include <linux/iopoll.h>
> > +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +#include <linux/spinlock.h>
> > +#endif
> >
> >  #include "iommu-bits.h"
> >
> > @@ -69,21 +72,61 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
> >  #define riscv_iommu_readl(iommu, addr) \
> >          readl_relaxed((iommu)->reg + (addr))
> >
> > -#define riscv_iommu_readq(iommu, addr) \
> > -        readq_relaxed((iommu)->reg + (addr))
> > -
> >  #define riscv_iommu_writel(iommu, addr, val) \
> >          writel_relaxed((val), (iommu)->reg + (addr))
> >
> > +#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > +        readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> > +                           delay_us, timeout_us)
> > +
> > +#ifndef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +#define riscv_iommu_readq(iommu, addr) \
> > +        readq_relaxed((iommu)->reg + (addr))
> > +
> >  #define riscv_iommu_writeq(iommu, addr, val) \
> >          writeq_relaxed((val), (iommu)->reg + (addr))
> >
> >  #define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> >          readx_poll_timeout(readq_relaxed, (iommu)->reg + (addr), val, cond, \
> >                             delay_us, timeout_us)
> > +#else /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
> >
> > -#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > -        readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> > -                           delay_us, timeout_us)
> > +extern raw_spinlock_t riscv_iommu_32bit_access_lock;
> > +
> > +static inline u64 __riscv_iommu_readq_relaxed(void __iomem *addr)
> > +{
> > +        u32 lo, hi;
> > +        unsigned long flags;
> > +
> > +        raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> > +        do {
> > +                hi = readl_relaxed(addr + sizeof(u32));
> > +                lo = readl_relaxed(addr);
> > +        } while (hi != readl_relaxed(addr + sizeof(u32)));
> > +        raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
>
> Why the lock and the loop?
> If you need the loop (for places where the hardware might be changing
> both words?) then there is no need to read 'hi' twice when you loop.
>
> But does it make any sense to be reading the value either while the hardware
> is updating it (does that happen?) or concurrently with a write.
>
>
> > +
> > +        return ((u64)hi << 32) | (u64)lo;
> > +}
> > +
> > +static inline void __riscv_iommu_writeq_relaxed(u64 value, void __iomem *addr)
> > +{
> > +        unsigned long flags;
> > +
> > +        raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> > +        writel_relaxed((u32)(value >> 32), addr + sizeof(u32));
> > +        writel_relaxed((u32)value, addr);
> > +        raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
>
> What are you actually locking against?
> I'm pretty sure it doesn't make any sense for two cpu to be writing to the
> same entry at the same time.
>
> Can't the writel_relaxed() be re-ordered, so you aren't guaranteeing they'll
> be done high-low.
> Same with the reads.
>
>         David
>
> > +}
> > +
> > +#define riscv_iommu_readq(iommu, addr) \
> > +        __riscv_iommu_readq_relaxed((iommu)->reg + (addr))
> > +
> > +#define riscv_iommu_writeq(iommu, addr, val) \
> > +        __riscv_iommu_writeq_relaxed((val), (iommu)->reg + (addr))
> > +
> > +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > +        readx_poll_timeout(__riscv_iommu_readq_relaxed, (iommu)->reg + (addr), \
> > +                           val, cond, delay_us, timeout_us)
> > +#endif /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
> >
> >  #endif
>

Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[Guo Ren]

Hi Zhanpeng Zhang,

I noticed you posted a similar fix recently. However, I had already
submitted a similar solution back in September 2025 [1]. It would be
great if you could review it. A few concerns with the current patch:

1. Using spin_lock_irqsave in such a small semantic structure is
unnecessary. IRQ protection should be provided by the higher-level
IOMMU driver interfaces. Many existing call sites already handle IRQ
disabling, so adding it here feels redundant and adds avoidable
overhead.

2. More critically, the RISCV_IOMMU_32BIT_ACCESS Kconfig option is
problematic. According to the RISC-V IOMMU specification (Software
Guidelines, “Reading and writing IOMMU registers”):

> Registers that are 64-bit wide may be accessed using either a 32-bit
> or a 64-bit access.

This clearly requires that any hardware supporting 64-bit MMIO access
must also support 32-bit MMIO access. Therefore, the IOMMU driver should
be built on a 32-bit access base for maximum hardware compatibility.

3. Only performance-monitoring counters require 64-bit IO access or the
high-low-high do-while retry strategy. For ordinary status and control
MMIO registers, a single read is sufficient.

4. Introducing a compile-time Kconfig that forces 32-bit splitting (or
64-bit access) across the entire driver is unnecessary for the vast
majority of registers and would prevent a single kernel Image (or .ko)
from working across all compliant platforms.

I believe the correct approach is to keep the driver on a 32-bit access
foundation (as required by the spec) and apply special 64-bit or
high-low-high handling only to performance-monitoring counters when
justified by data. This preserves broad hardware support without
unnecessary complexity.

What do you think? I’m happy to discuss or collaborate on a cleaner
solution based on the earlier patch [1].

[1]: https://lore.kernel.org/linux-riscv/20250903144217.837448-1-guoren@kernel.org/

Best Regards
  GUO Ren

> Some RISC-V IOMMU implementations cannot perform 64-bit MMIO accesses
> to the IOMMU register file. The RISC-V IOMMU architecture allows 64-bit
> registers to be accessed using 32-bit accesses, provided the accesses are
> properly aligned and do not span multiple registers.
> 
> Add a config option for such implementations and access 64-bit IOMMU
> registers as paired 32-bit MMIO operations when it is enabled. Serialize
> the paired accesses so the high and low halves cannot interleave with
> another CPU. Full 64-bit register programming writes the high half before
> the low half.
> 
> This option describes the register access width. It is not an RV32 kernel
> mode and does not describe a 32-bit IOMMU architecture.
> 
> Co-developed-by: Xu Lu <luxu.kernel@bytedance.com>
> Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
> ---
> This is needed for platforms whose RISC-V IOMMU register window does not
> support naturally aligned 64-bit MMIO accesses.
> 
>  drivers/iommu/riscv/Kconfig | 11 ++++++++
>  drivers/iommu/riscv/iommu.c |  4 +++
>  drivers/iommu/riscv/iommu.h | 55 +++++++++++++++++++++++++++++++++----
>  3 files changed, 64 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
> index b86e5ab94183..54d624b9b2ef 100644
> --- a/drivers/iommu/riscv/Kconfig
> +++ b/drivers/iommu/riscv/Kconfig
> @@ -22,3 +22,14 @@ config RISCV_IOMMU_PCI
>  	def_bool y if RISCV_IOMMU && PCI_MSI
>  	help
>  	  Support for the PCIe implementation of RISC-V IOMMU architecture.
> +
> +config RISCV_IOMMU_32BIT_ACCESS
> +	bool "Use 32-bit accesses for RISC-V IOMMU registers"
> +	depends on RISCV_IOMMU
> +	help
> +	  Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> +	  using naturally aligned 64-bit loads and stores.
> +
> +	  When enabled, 64-bit IOMMU registers are accessed as paired
> +	  32-bit MMIO operations. This option does not describe an RV32
> +	  kernel or a 32-bit IOMMU architecture.
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index a31f50bbad35..7fa1721b5728 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -53,6 +53,10 @@ struct riscv_iommu_devres {
>  	void *addr;
>  };
>  
> +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +DEFINE_RAW_SPINLOCK(riscv_iommu_32bit_access_lock);
> +#endif
> +
>  static void riscv_iommu_devres_pages_release(struct device *dev, void *res)
>  {
>  	struct riscv_iommu_devres *devres = res;
> diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
> index 46df79dd5495..ba78ef1858c5 100644
> --- a/drivers/iommu/riscv/iommu.h
> +++ b/drivers/iommu/riscv/iommu.h
> @@ -14,6 +14,9 @@
>  #include <linux/iommu.h>
>  #include <linux/types.h>
>  #include <linux/iopoll.h>
> +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +#include <linux/spinlock.h>
> +#endif
>  
>  #include "iommu-bits.h"
>  
> @@ -69,21 +72,61 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
>  #define riscv_iommu_readl(iommu, addr) \
>  	readl_relaxed((iommu)->reg + (addr))
>  
> -#define riscv_iommu_readq(iommu, addr) \
> -	readq_relaxed((iommu)->reg + (addr))
> -
>  #define riscv_iommu_writel(iommu, addr, val) \
>  	writel_relaxed((val), (iommu)->reg + (addr))
>  
> +#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> +	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> +			   delay_us, timeout_us)
> +
> +#ifndef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> +#define riscv_iommu_readq(iommu, addr) \
> +	readq_relaxed((iommu)->reg + (addr))
> +
>  #define riscv_iommu_writeq(iommu, addr, val) \
>  	writeq_relaxed((val), (iommu)->reg + (addr))
>  
>  #define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
>  	readx_poll_timeout(readq_relaxed, (iommu)->reg + (addr), val, cond, \
>  			   delay_us, timeout_us)
> +#else /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
>  
> -#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> -	readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> -			   delay_us, timeout_us)
> +extern raw_spinlock_t riscv_iommu_32bit_access_lock;
> +
> +static inline u64 __riscv_iommu_readq_relaxed(void __iomem *addr)
> +{
> +	u32 lo, hi;
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> +	do {
> +		hi = readl_relaxed(addr + sizeof(u32));
> +		lo = readl_relaxed(addr);
> +	} while (hi != readl_relaxed(addr + sizeof(u32)));
> +	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
> +
> +	return ((u64)hi << 32) | (u64)lo;
> +}
> +
> +static inline void __riscv_iommu_writeq_relaxed(u64 value, void __iomem *addr)
> +{
> +	unsigned long flags;
> +
> +	raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> +	writel_relaxed((u32)(value >> 32), addr + sizeof(u32));
> +	writel_relaxed((u32)value, addr);
> +	raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
> +}
> +
> +#define riscv_iommu_readq(iommu, addr) \
> +	__riscv_iommu_readq_relaxed((iommu)->reg + (addr))
> +
> +#define riscv_iommu_writeq(iommu, addr, val) \
> +	__riscv_iommu_writeq_relaxed((val), (iommu)->reg + (addr))
> +
> +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> +	readx_poll_timeout(__riscv_iommu_readq_relaxed, (iommu)->reg + (addr), \
> +			   val, cond, delay_us, timeout_us)
> +#endif /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
>  
>  #endif
> -- 
> 2.50.1 (Apple Git-155)

Re: [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses

[Zhanpeng Zhang]

Hi Guo Ren,

Thanks for the pointer, and sorry that I missed the earlier discussion.

I agree with your concerns. The lock in my v1 is not the right approach, and
David also pointed out the same issue. More importantly, after reading your
comments, I agree that this should not be handled as a separate Kconfig-selected
path.

If the RISC-V IOMMU specification allows 64-bit registers to be accessed with
32-bit MMIO operations, then a generic kernel should be able to support that
without requiring a platform-specific build-time option. So I will drop my v1
and will not continue this separate patch.

I will take a closer look at your earlier proposal and the previous discussion.
If the preferred direction is to make 32-bit register accesses part of the
driver's common register access model, I am happy to help test, review, or
rework patches in that direction.

Best Regards,
Zhanpeng

> From: "Guo Ren"<guoren@kernel.org>
> Date:  Mon, Jun 15, 2026, 20:38
> Subject:  [External] Re: [PATCH v1] iommu/riscv: Support 32-bit register accesses
> To: <zhangzhanpeng.jasper@bytedance.com>
> Cc: <alex@ghiti.fr>, <aou@eecs.berkeley.edu>, <cuiyunhui@bytedance.com>, <iommu@lists.linux.dev>, <joro@8bytes.org>, <linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org>, <luxu.kernel@bytedance.com>, <palmer@dabbelt.com>, <pjw@kernel.org>, <robin.murphy@arm.com>, <tjeznach@rivosinc.com>, <will@kernel.org>, <yuanzhu@bytedance.com>
> Hi Zhanpeng Zhang,
>
> I noticed you posted a similar fix recently. However, I had already
> submitted a similar solution back in September 2025 [1]. It would be
> great if you could review it. A few concerns with the current patch:
>
> 1. Using spin_lock_irqsave in such a small semantic structure is
> unnecessary. IRQ protection should be provided by the higher-level
> IOMMU driver interfaces. Many existing call sites already handle IRQ
> disabling, so adding it here feels redundant and adds avoidable
> overhead.
>
> 2. More critically, the RISCV_IOMMU_32BIT_ACCESS Kconfig option is
> problematic. According to the RISC-V IOMMU specification (Software
> Guidelines, “Reading and writing IOMMU registers”):
>
> > Registers that are 64-bit wide may be accessed using either a 32-bit
> > or a 64-bit access.
>
> This clearly requires that any hardware supporting 64-bit MMIO access
> must also support 32-bit MMIO access. Therefore, the IOMMU driver should
> be built on a 32-bit access base for maximum hardware compatibility.
>
> 3. Only performance-monitoring counters require 64-bit IO access or the
> high-low-high do-while retry strategy. For ordinary status and control
> MMIO registers, a single read is sufficient.
>
> 4. Introducing a compile-time Kconfig that forces 32-bit splitting (or
> 64-bit access) across the entire driver is unnecessary for the vast
> majority of registers and would prevent a single kernel Image (or .ko)
> from working across all compliant platforms.
>
> I believe the correct approach is to keep the driver on a 32-bit access
> foundation (as required by the spec) and apply special 64-bit or
> high-low-high handling only to performance-monitoring counters when
> justified by data. This preserves broad hardware support without
> unnecessary complexity.
>
> What do you think? I’m happy to discuss or collaborate on a cleaner
> solution based on the earlier patch [1].
>
> [1]: https://lore.kernel.org/linux-riscv/20250903144217.837448-1-guoren@kernel.org/
>
> Best Regards
>   GUO Ren
>
> > Some RISC-V IOMMU implementations cannot perform 64-bit MMIO accesses
> > to the IOMMU register file. The RISC-V IOMMU architecture allows 64-bit
> > registers to be accessed using 32-bit accesses, provided the accesses are
> > properly aligned and do not span multiple registers.
> >
> > Add a config option for such implementations and access 64-bit IOMMU
> > registers as paired 32-bit MMIO operations when it is enabled. Serialize
> > the paired accesses so the high and low halves cannot interleave with
> > another CPU. Full 64-bit register programming writes the high half before
> > the low half.
> >
> > This option describes the register access width. It is not an RV32 kernel
> > mode and does not describe a 32-bit IOMMU architecture.
> >
> > Co-developed-by: Xu Lu <luxu.kernel@bytedance.com>
> > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com>
> > Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
> > ---
> > This is needed for platforms whose RISC-V IOMMU register window does not
> > support naturally aligned 64-bit MMIO accesses.
> >
> >  drivers/iommu/riscv/Kconfig | 11 ++++++++
> >  drivers/iommu/riscv/iommu.c |  4 +++
> >  drivers/iommu/riscv/iommu.h | 55 +++++++++++++++++++++++++++++++++----
> >  3 files changed, 64 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig
> > index b86e5ab94183..54d624b9b2ef 100644
> > --- a/drivers/iommu/riscv/Kconfig
> > +++ b/drivers/iommu/riscv/Kconfig
> > @@ -22,3 +22,14 @@ config RISCV_IOMMU_PCI
> >          def_bool y if RISCV_IOMMU && PCI_MSI
> >          help
> >            Support for the PCIe implementation of RISC-V IOMMU architecture.
> > +
> > +config RISCV_IOMMU_32BIT_ACCESS
> > +        bool "Use 32-bit accesses for RISC-V IOMMU registers"
> > +        depends on RISCV_IOMMU
> > +        help
> > +          Say Y when the RISC-V IOMMU MMIO window cannot be accessed
> > +          using naturally aligned 64-bit loads and stores.
> > +
> > +          When enabled, 64-bit IOMMU registers are accessed as paired
> > +          32-bit MMIO operations. This option does not describe an RV32
> > +          kernel or a 32-bit IOMMU architecture.
> > diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> > index a31f50bbad35..7fa1721b5728 100644
> > --- a/drivers/iommu/riscv/iommu.c
> > +++ b/drivers/iommu/riscv/iommu.c
> > @@ -53,6 +53,10 @@ struct riscv_iommu_devres {
> >          void *addr;
> >  };
> >
> > +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +DEFINE_RAW_SPINLOCK(riscv_iommu_32bit_access_lock);
> > +#endif
> > +
> >  static void riscv_iommu_devres_pages_release(struct device *dev, void *res)
> >  {
> >          struct riscv_iommu_devres *devres = res;
> > diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
> > index 46df79dd5495..ba78ef1858c5 100644
> > --- a/drivers/iommu/riscv/iommu.h
> > +++ b/drivers/iommu/riscv/iommu.h
> > @@ -14,6 +14,9 @@
> >  #include <linux/iommu.h>
> >  #include <linux/types.h>
> >  #include <linux/iopoll.h>
> > +#ifdef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +#include <linux/spinlock.h>
> > +#endif
> >
> >  #include "iommu-bits.h"
> >
> > @@ -69,21 +72,61 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
> >  #define riscv_iommu_readl(iommu, addr) \
> >          readl_relaxed((iommu)->reg + (addr))
> >
> > -#define riscv_iommu_readq(iommu, addr) \
> > -        readq_relaxed((iommu)->reg + (addr))
> > -
> >  #define riscv_iommu_writel(iommu, addr, val) \
> >          writel_relaxed((val), (iommu)->reg + (addr))
> >
> > +#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > +        readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> > +                           delay_us, timeout_us)
> > +
> > +#ifndef CONFIG_RISCV_IOMMU_32BIT_ACCESS
> > +#define riscv_iommu_readq(iommu, addr) \
> > +        readq_relaxed((iommu)->reg + (addr))
> > +
> >  #define riscv_iommu_writeq(iommu, addr, val) \
> >          writeq_relaxed((val), (iommu)->reg + (addr))
> >
> >  #define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> >          readx_poll_timeout(readq_relaxed, (iommu)->reg + (addr), val, cond, \
> >                             delay_us, timeout_us)
> > +#else /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
> >
> > -#define riscv_iommu_readl_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > -        readx_poll_timeout(readl_relaxed, (iommu)->reg + (addr), val, cond, \
> > -                           delay_us, timeout_us)
> > +extern raw_spinlock_t riscv_iommu_32bit_access_lock;
> > +
> > +static inline u64 __riscv_iommu_readq_relaxed(void __iomem *addr)
> > +{
> > +        u32 lo, hi;
> > +        unsigned long flags;
> > +
> > +        raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> > +        do {
> > +                hi = readl_relaxed(addr + sizeof(u32));
> > +                lo = readl_relaxed(addr);
> > +        } while (hi != readl_relaxed(addr + sizeof(u32)));
> > +        raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
> > +
> > +        return ((u64)hi << 32) | (u64)lo;
> > +}
> > +
> > +static inline void __riscv_iommu_writeq_relaxed(u64 value, void __iomem *addr)
> > +{
> > +        unsigned long flags;
> > +
> > +        raw_spin_lock_irqsave(&riscv_iommu_32bit_access_lock, flags);
> > +        writel_relaxed((u32)(value >> 32), addr + sizeof(u32));
> > +        writel_relaxed((u32)value, addr);
> > +        raw_spin_unlock_irqrestore(&riscv_iommu_32bit_access_lock, flags);
> > +}
> > +
> > +#define riscv_iommu_readq(iommu, addr) \
> > +        __riscv_iommu_readq_relaxed((iommu)->reg + (addr))
> > +
> > +#define riscv_iommu_writeq(iommu, addr, val) \
> > +        __riscv_iommu_writeq_relaxed((val), (iommu)->reg + (addr))
> > +
> > +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \
> > +        readx_poll_timeout(__riscv_iommu_readq_relaxed, (iommu)->reg + (addr), \
> > +                           val, cond, delay_us, timeout_us)
> > +#endif /* CONFIG_RISCV_IOMMU_32BIT_ACCESS */
> >
> >  #endif
> > --
> > 2.50.1 (Apple Git-155)
>