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

[Tomasz Jeznach <tomasz.jeznach@linux.dev>]

On 7/13/26 7:02 PM, Guo Ren wrote:
> LGTM!
>
> Reviewed-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> Co-developed-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
> Signed-off-by: Guo Ren (Alibaba DAMO Academy) <guoren@kernel.org>
>
> On Mon, Jul 13, 2026 at 8:29 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. For DDTP, poll BUSY in the low half,
>> then read the high half and compose the register value from the polled low
>> half. 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 v3:
>> - Use the DDTP access sequence from [1]: retain the low half returned by
>>    BUSY polling, read only the high half, and compose the DDTP value from
>>    those two 32-bit reads.
>>
>> 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.
>>
>> 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 | 9 ++++++---
>>   drivers/iommu/riscv/iommu.h | 9 +++------
>>   2 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/iommu/riscv/iommu.c b/drivers/iommu/riscv/iommu.c
>> index cec3ddd7ab1..d647b71ebec 100644
>> --- a/drivers/iommu/riscv/iommu.c
>> +++ b/drivers/iommu/riscv/iommu.c
>> @@ -670,9 +670,12 @@ void riscv_iommu_disable(struct riscv_iommu_device *iommu)
>>
>>   #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 +1504,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)

Thank you for this change. Sorry for being late on the discussion. LGTM

Reviewed-by: Tomasz Jeznach <tomasz.jeznach@linux.dev>

Best,
- Tomasz



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv