1. Patchew
  2. linux
  3. View series

[PATCH v2] iommu/riscv: Use 32-bit MMIO accesses for 64-bit registers

Zhanpeng Zhang posted 1 patch 1 week, 5 days ago
There is a newer version of this series
drivers/iommu/riscv/iommu.c | 18 +++++++++++-------
drivers/iommu/riscv/iommu.h |  9 +++------
2 files changed, 14 insertions(+), 13 deletions(-)
[PATCH v2] iommu/riscv: Use 32-bit MMIO accesses for 64-bit registers
Posted by Zhanpeng Zhang 1 week, 5 days ago 
The RISC-V IOMMU specification [1] permits 64-bit registers to be accessed
using two 32-bit transactions, high half first, and leaves the single-copy
atomicity of 8-byte IOMMU register accesses unspecified.

Use the generic hi_lo_readq_relaxed() and hi_lo_writeq_relaxed() helpers
for ordinary 64-bit IOMMU registers. Poll DDTP.BUSY in the low half before
reading the full register from high to low. HPM counter reads require a
rollover-aware sequence and remain outside these accessors.

This follows the 32-bit access direction proposed by Guo Ren [2] and uses
the generic non-atomic MMIO helpers suggested by David Laight.

[1] https://docs.riscv.org/reference/iommu/
[2] https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org

Suggested-by: Guo Ren <guoren@kernel.org>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
---
Changes in v2:
- Rework the patch based on Guo Ren's earlier proposal [1].
- Drop the build-time option and use 32-bit accesses unconditionally.
- Drop the global lock and use the generic high-low MMIO helpers, as
  suggested by David Laight.
- Poll DDTP.BUSY through its low half before reading the full register.

Link to v1: [2]
Specification discussion: [3]

[1]: https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org
[2]: https://lore.kernel.org/r/20260615064855.90316-1-zhangzhanpeng.jasper@bytedance.com
[3]: https://github.com/riscv-non-isa/riscv-iommu/issues/765

 drivers/iommu/riscv/iommu.c | 18 +++++++++++-------
 drivers/iommu/riscv/iommu.h |  9 +++------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
index cec3ddd7ab1..14b572058ed 100644
--- a/drivers/iommu/riscv/iommu.c
+++ b/drivers/iommu/riscv/iommu.c
@@ -668,12 +668,16 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu)
 	riscv_iommu_writel(iommu, RISCV_IOMMU_REG_PQCSR, 0);
 }
 
-#define riscv_iommu_read_ddtp(iommu) ({ \
-	u64 ddtp; \
-	riscv_iommu_readq_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp, \
-				  !(ddtp & RISCV_IOMMU_DDTP_BUSY), 10, \
-				  RISCV_IOMMU_DDTP_TIMEOUT); \
-	ddtp; })
+static u64 riscv_iommu_read_ddtp(struct riscv_iommu_device *iommu)
+{
+	u32 ddtp_lo;
+
+	riscv_iommu_readl_timeout(iommu, RISCV_IOMMU_REG_DDTP, ddtp_lo,
+				  !(ddtp_lo & RISCV_IOMMU_DDTP_BUSY), 10,
+				  RISCV_IOMMU_DDTP_TIMEOUT);
+
+	return riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
+}
 
 static int riscv_iommu_iodir_alloc(struct riscv_iommu_device *iommu)
 {
@@ -1501,7 +1505,7 @@ static int riscv_iommu_init_check(struct riscv_iommu_device *iommu)
 	 * regular boot flow and disable translation when we boot into a kexec
 	 * kernel and the previous kernel left them enabled.
 	 */
-	ddtp = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
+	ddtp = riscv_iommu_read_ddtp(iommu);
 	if (ddtp & RISCV_IOMMU_DDTP_BUSY)
 		return -EBUSY;
 
diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
index 46df79dd549..1b03790fbe1 100644
--- a/drivers/iommu/riscv/iommu.h
+++ b/drivers/iommu/riscv/iommu.h
@@ -11,6 +11,7 @@
 #ifndef _RISCV_IOMMU_H_
 #define _RISCV_IOMMU_H_
 
+#include <linux/io-64-nonatomic-hi-lo.h>
 #include <linux/iommu.h>
 #include <linux/types.h>
 #include <linux/iopoll.h>
@@ -70,17 +71,13 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
 	readl_relaxed((iommu)->reg + (addr))
 
 #define riscv_iommu_readq(iommu, addr) \
-	readq_relaxed((iommu)->reg + (addr))
+	hi_lo_readq_relaxed((iommu)->reg + (addr))
 
 #define riscv_iommu_writel(iommu, addr, val) \
 	writel_relaxed((val), (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)
+	hi_lo_writeq_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, \
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v2] iommu/riscv: Use 32-bit MMIO accesses for 64-bit registers
Posted by Guo Ren 1 week, 5 days ago 
On Mon, Jul 13, 2026 at 2:09 PM Zhanpeng Zhang
<zhangzhanpeng.jasper@bytedance.com> wrote:
>
> The RISC-V IOMMU specification [1] permits 64-bit registers to be accessed
> using two 32-bit transactions, high half first, and leaves the single-copy
> atomicity of 8-byte IOMMU register accesses unspecified.
>
> Use the generic hi_lo_readq_relaxed() and hi_lo_writeq_relaxed() helpers
> for ordinary 64-bit IOMMU registers. Poll DDTP.BUSY in the low half before
> reading the full register from high to low. HPM counter reads require a
> rollover-aware sequence and remain outside these accessors.
>
> This follows the 32-bit access direction proposed by Guo Ren [2] and uses
> the generic non-atomic MMIO helpers suggested by David Laight.
>
> [1] https://docs.riscv.org/reference/iommu/
> [2] https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org
>
> Suggested-by: Guo Ren <guoren@kernel.org>
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
> ---
> Changes in v2:
> - Rework the patch based on Guo Ren's earlier proposal [1].
> - Drop the build-time option and use 32-bit accesses unconditionally.
> - Drop the global lock and use the generic high-low MMIO helpers, as
>   suggested by David Laight.
> - Poll DDTP.BUSY through its low half before reading the full register.
>
> Link to v1: [2]
> Specification discussion: [3]
>
> [1]: https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org
> [2]: https://lore.kernel.org/r/20260615064855.90316-1-zhangzhanpeng.jasper@bytedance.com
> [3]: https://github.com/riscv-non-isa/riscv-iommu/issues/765
>
>  drivers/iommu/riscv/iommu.c | 18 +++++++++++-------
>  drivers/iommu/riscv/iommu.h |  9 +++------
>  2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
> index cec3ddd7ab1..14b572058ed 100644
> --- a/drivers/iommu/riscv/iommu.c
> +++ b/drivers/iommu/riscv/iommu.c
> @@ -668,12 +668,16 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu)
>         riscv_iommu_writel(iommu, RISCV_IOMMU_REG_PQCSR, 0);
>  }
>
> -#define riscv_iommu_read_ddtp(iommu) ({ \
> -       u64 ddtp; \
> -       riscv_iommu_readq_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp, \
> -                                 !(ddtp & RISCV_IOMMU_DDTP_BUSY), 10, \
> -                                 RISCV_IOMMU_DDTP_TIMEOUT); \
> -       ddtp; })
> +static u64 riscv_iommu_read_ddtp(struct riscv_iommu_device *iommu)
> +{
> +       u32 ddtp_lo;
> +
> +       riscv_iommu_readl_timeout(iommu, RISCV_IOMMU_REG_DDTP, ddtp_lo,
> +                                 !(ddtp_lo & RISCV_IOMMU_DDTP_BUSY), 10,
> +                                 RISCV_IOMMU_DDTP_TIMEOUT);
> +
> +       return riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
readq? Please ref [1]:

#define riscv_iommu_read_ddtp(iommu) ({ \
  u64 ddtp; \
- riscv_iommu_readq_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp, \
-  !(ddtp & RISCV_IOMMU_DDTP_BUSY), 10, \
+ u32 ddtp_lo, ddtp_hi; \
+ riscv_iommu_readl_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp_lo, \
+  !(ddtp_lo & RISCV_IOMMU_DDTP_BUSY), 10, \
   RISCV_IOMMU_DDTP_TIMEOUT); \
+ ddtp_hi = riscv_iommu_readl(iommu, RISCV_IOMMU_REG_DDTP + 4); \
+ ddtp = ((u64)ddtp_hi << 32) | ddtp_lo; \
  ddtp; })


> +}
>
>  static int riscv_iommu_iodir_alloc(struct riscv_iommu_device *iommu)
>  {
> @@ -1501,7 +1505,7 @@ static int riscv_iommu_init_check(struct riscv_iommu_device *iommu)
>          * regular boot flow and disable translation when we boot into a kexec
>          * kernel and the previous kernel left them enabled.
>          */
> -       ddtp = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
> +       ddtp = riscv_iommu_read_ddtp(iommu);
>         if (ddtp & RISCV_IOMMU_DDTP_BUSY)
>                 return -EBUSY;
>
> diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
> index 46df79dd549..1b03790fbe1 100644
> --- a/drivers/iommu/riscv/iommu.h
> +++ b/drivers/iommu/riscv/iommu.h
> @@ -11,6 +11,7 @@
>  #ifndef _RISCV_IOMMU_H_
>  #define _RISCV_IOMMU_H_
>
> +#include <linux/io-64-nonatomic-hi-lo.h>
>  #include <linux/iommu.h>
>  #include <linux/types.h>
>  #include <linux/iopoll.h>
> @@ -70,17 +71,13 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
>         readl_relaxed((iommu)->reg + (addr))
>
>  #define riscv_iommu_readq(iommu, addr) \
> -       readq_relaxed((iommu)->reg + (addr))
> +       hi_lo_readq_relaxed((iommu)->reg + (addr))
>
>  #define riscv_iommu_writel(iommu, addr, val) \
>         writel_relaxed((val), (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)
> +       hi_lo_writeq_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, \
> --
> 2.50.1 (Apple Git-155)



-- 
Best Regards
 Guo Ren
Re: Re: [PATCH v2] iommu/riscv: Use 32-bit MMIO accesses for 64-bit registers
Posted by Zhanpeng Zhang 1 week, 5 days ago 
Hi Guo,

On 7/13/26 3:00 PM, Guo Ren wrote:
> On Mon, Jul 13, 2026 at 2:09 PM Zhanpeng Zhang
> <zhangzhanpeng.jasper@bytedance.com> wrote:
>>
>> The RISC-V IOMMU specification [1] permits 64-bit registers to be accessed
>> using two 32-bit transactions, high half first, and leaves the single-copy
>> atomicity of 8-byte IOMMU register accesses unspecified.
>>
>> Use the generic hi_lo_readq_relaxed() and hi_lo_writeq_relaxed() helpers
>> for ordinary 64-bit IOMMU registers. Poll DDTP.BUSY in the low half before
>> reading the full register from high to low. HPM counter reads require a
>> rollover-aware sequence and remain outside these accessors.
>>
>> This follows the 32-bit access direction proposed by Guo Ren [2] and uses
>> the generic non-atomic MMIO helpers suggested by David Laight.
>>
>> [1] https://docs.riscv.org/reference/iommu/
>> [2] https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org
>>
>> Suggested-by: Guo Ren <guoren@kernel.org>
>> Suggested-by: David Laight <david.laight.linux@gmail.com>
>> Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper@bytedance.com>
>> ---
>> Changes in v2:
>> - Rework the patch based on Guo Ren's earlier proposal [1].
>> - Drop the build-time option and use 32-bit accesses unconditionally.
>> - Drop the global lock and use the generic high-low MMIO helpers, as
>>    suggested by David Laight.
>> - Poll DDTP.BUSY through its low half before reading the full register.
>>
>> Link to v1: [2]
>> Specification discussion: [3]
>>
>> [1]: https://lore.kernel.org/r/20250903144217.837448-1-guoren@kernel.org
>> [2]: https://lore.kernel.org/r/20260615064855.90316-1-zhangzhanpeng.jasper@bytedance.com
>> [3]: https://github.com/riscv-non-isa/riscv-iommu/issues/765
>>
>>   drivers/iommu/riscv/iommu.c | 18 +++++++++++-------
>>   drivers/iommu/riscv/iommu.h |  9 +++------
>>   2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index cec3ddd7ab1..14b572058ed 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -668,12 +668,16 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu)
>>          riscv_iommu_writel(iommu, RISCV_IOMMU_REG_PQCSR, 0);
>>   }
>>
>> -#define riscv_iommu_read_ddtp(iommu) ({ \
>> -       u64 ddtp; \
>> -       riscv_iommu_readq_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp, \
>> -                                 !(ddtp & RISCV_IOMMU_DDTP_BUSY), 10, \
>> -                                 RISCV_IOMMU_DDTP_TIMEOUT); \
>> -       ddtp; })
>> +static u64 riscv_iommu_read_ddtp(struct riscv_iommu_device *iommu)
>> +{
>> +       u32 ddtp_lo;
>> +
>> +       riscv_iommu_readl_timeout(iommu, RISCV_IOMMU_REG_DDTP, ddtp_lo,
>> +                                 !(ddtp_lo & RISCV_IOMMU_DDTP_BUSY), 10,
>> +                                 RISCV_IOMMU_DDTP_TIMEOUT);
>> +
>> +       return riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
> readq? Please ref [1]:
>
> #define riscv_iommu_read_ddtp(iommu) ({ \
>    u64 ddtp; \
> - riscv_iommu_readq_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp, \
> -  !(ddtp & RISCV_IOMMU_DDTP_BUSY), 10, \
> + u32 ddtp_lo, ddtp_hi; \
> + riscv_iommu_readl_timeout((iommu), RISCV_IOMMU_REG_DDTP, ddtp_lo, \
> +  !(ddtp_lo & RISCV_IOMMU_DDTP_BUSY), 10, \
>     RISCV_IOMMU_DDTP_TIMEOUT); \
> + ddtp_hi = riscv_iommu_readl(iommu, RISCV_IOMMU_REG_DDTP + 4); \
> + ddtp = ((u64)ddtp_hi << 32) | ddtp_lo; \
>    ddtp; })
>
Oops, thanks for catching that, it will re-read the low 32 bits. DDTP
must retain that polled low-half value, read only the high half, and
then compose the result.

The macro one [1] is correct, I've fixed it in v3.

>
>> +}
>>
>>   static int riscv_iommu_iodir_alloc(struct riscv_iommu_device *iommu)
>>   {
>> @@ -1501,7 +1505,7 @@ static int riscv_iommu_init_check(struct riscv_iommu_device *iommu)
>>           * regular boot flow and disable translation when we boot into a kexec
>>           * kernel and the previous kernel left them enabled.
>>           */
>> -       ddtp = riscv_iommu_readq(iommu, RISCV_IOMMU_REG_DDTP);
>> +       ddtp = riscv_iommu_read_ddtp(iommu);
>>          if (ddtp & RISCV_IOMMU_DDTP_BUSY)
>>                  return -EBUSY;
>>
>> diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h
>> index 46df79dd549..1b03790fbe1 100644
>> --- a/drivers/iommu/riscv/iommu.h
>> +++ b/drivers/iommu/riscv/iommu.h
>> @@ -11,6 +11,7 @@
>>   #ifndef _RISCV_IOMMU_H_
>>   #define _RISCV_IOMMU_H_
>>
>> +#include <linux/io-64-nonatomic-hi-lo.h>
>>   #include <linux/iommu.h>
>>   #include <linux/types.h>
>>   #include <linux/iopoll.h>
>> @@ -70,17 +71,13 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu);
>>          readl_relaxed((iommu)->reg + (addr))
>>
>>   #define riscv_iommu_readq(iommu, addr) \
>> -       readq_relaxed((iommu)->reg + (addr))
>> +       hi_lo_readq_relaxed((iommu)->reg + (addr))
>>
>>   #define riscv_iommu_writel(iommu, addr, val) \
>>          writel_relaxed((val), (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)
>> +       hi_lo_writeq_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, \
>> --
>> 2.50.1 (Apple Git-155)
>
>
>
Thanks,
Zhanpeng

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.