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

[Guo Ren]

On Tue, Jul 14, 2026 at 8:27 PM Robin Murphy <robin.murphy at arm.com> wrote:
>
> On 13/07/2026 1:29 pm, Zhanpeng Zhang 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 at kernel.org>
> > Suggested-by: David Laight <david.laight.linux at gmail.com>
> > Signed-off-by: Zhanpeng Zhang <zhangzhanpeng.jasper at 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); \
>
> It looks like whenever you read DDTP you're only really looking at the
> BUSY/MODE fields anyway, so does this actually need to read the upper
> bits of PPN at all? (The spec says they don't even need to be written on
> RV32 either)

The motivation is not RV32 compatibility. It addresses RV64 platforms
where the IOMMU control plane can only be accessed via 32-bit MMIO.

>
> > +     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))
>
> This seems unnecessary (similarly for writeq() below) - once you've
> included the header, then it automatically provides readq{_relaxed}()
> for RV32 via the non-atomic implementation, and wherever atomicity
> doesn't matter, then as written there seems to be no reason for RV64 to
> stop using the regular arch readq().
>
> Not that it makes any difference to me either way, but I don't see any
> real issue with the spec - if software *may* make a 64-bit access to any
> 64-bit register unconditionally, then that can only imply that hardware
> *must* be able to accommodate RV64 software choosing to do so, and
> therefore must support *both* 32b and 64b accesses in general, except
> perhaps on RV32-only systems if software could never make 64b accesses
> in the first place. If 64b single-copy atomicity is not
> required/specified then it should be valid to achieve that by just
> sticking a downsizer in the upstream interconnect to split 64b accesses
> into 32bx2 incremental bursts.
>
> If do actually you need this as an erratum workaround to support
> specific hardware which has misinterpreted the spec then I think you
> should be clear about that. Otherwise, adding speculative "workarounds"
> which (slightly) penalise hardware that got it right, while inviting
> future hardware to get it wrong, doesn't seem like the right way to go
> at all...

The only three 64-bit registers the driver ever touches are DDTP,
CAPABILITIES, and MSI_CFG_TBL_ADDR. None of them is on any
performance-critical or hot path. Even if native 64-bit MMIO accesses
were used, there would be no observable performance benefit.
Therefore, this change does not penalize any hardware.

Regarding the specification, there remains substantial disagreement
over whether 64-bit accesses are mandatory. The relevant sentence uses
the word “may” (“may be accessed using either a 32-bit or a 64-bit
access”). Per RFC 2119, “may” does not express a requirement. Because
every existing implementation already provides working 32-bit MMIO
access, we believe the safer and less controversial choice is to use
the portable 32-bit path via the generic hi-lo helpers.

-- 
Best Regards
 Guo Ren