Message ID | 20250325144252.27403-1-luxu.kernel@bytedance.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | iommu: riscv: Split 8-byte accesses on 32 bit I/O bus platform | expand |
Context | Check | Description |
---|---|---|
bjorn/pre-ci_am | success | Success |
bjorn/build-rv32-defconfig | success | build-rv32-defconfig |
bjorn/build-rv64-clang-allmodconfig | success | build-rv64-clang-allmodconfig |
bjorn/build-rv64-gcc-allmodconfig | success | build-rv64-gcc-allmodconfig |
bjorn/build-rv64-nommu-k210-defconfig | success | build-rv64-nommu-k210-defconfig |
bjorn/build-rv64-nommu-k210-virt | success | build-rv64-nommu-k210-virt |
bjorn/checkpatch | warning | checkpatch |
bjorn/dtb-warn-rv64 | success | dtb-warn-rv64 |
bjorn/header-inline | success | header-inline |
bjorn/kdoc | success | kdoc |
bjorn/module-param | success | module-param |
bjorn/verify-fixes | success | verify-fixes |
bjorn/verify-signedoff | success | verify-signedoff |
On 25 Mar 2025, at 14:42, Xu Lu <luxu.kernel@bytedance.com> wrote: > > Introduce a new configuration CONFIG_RISCV_IOMMU_32BIT to enable > splitting 8-byte access into 4-byte transactions for hardware platform > whose I/O bus limits access to 4-byte transfers. > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> Is such a platform conformant to the specification? Either way, why is this a static build-time configuration choice rather than a dynamic run-time choice based on the FDT / ACPI tables / some other platform probing method? Jess > --- > drivers/iommu/riscv/Kconfig | 9 +++++++++ > drivers/iommu/riscv/iommu.h | 28 +++++++++++++++++++++++----- > 2 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig > index c071816f59a6..b7c9ea22d969 100644 > --- a/drivers/iommu/riscv/Kconfig > +++ b/drivers/iommu/riscv/Kconfig > @@ -18,3 +18,12 @@ 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 > + bool "Support 4-Byte Accesses on RISC-V IOMMU Registers" > + depends on RISCV_IOMMU > + default n > + help > + Support hardware platform whose I/O bus limits access to 4-byte > + transfers. When enabled, all accesses to IOMMU registers will be > + split into 4-byte accesses. > diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h > index 46df79dd5495..0e3552a8142d 100644 > --- a/drivers/iommu/riscv/iommu.h > +++ b/drivers/iommu/riscv/iommu.h > @@ -14,6 +14,10 @@ > #include <linux/iommu.h> > #include <linux/types.h> > #include <linux/iopoll.h> > +#ifdef CONFIG_RISCV_IOMMU_32BIT > +#include <linux/io-64-nonatomic-hi-lo.h> > +#include <linux/io-64-nonatomic-lo-hi.h> > +#endif > > #include "iommu-bits.h" > > @@ -69,21 +73,35 @@ 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 > +#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 */ > +#define riscv_iommu_readq(iommu, addr) \ > + hi_lo_readq_relaxed((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, \ > +#define riscv_iommu_writeq(iommu, addr, val) \ > + ((addr == RISCV_IOMMU_REG_IOHPMCYCLES) ? \ > + lo_hi_writeq_relaxed((val), (iommu)->reg + (addr)) : \ > + hi_lo_writeq_relaxed((val), (iommu)->reg + (addr))) > + > +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \ > + readx_poll_timeout(hi_lo_readq_relaxed, (iommu)->reg + (addr), val, cond, \ > delay_us, timeout_us) > +#endif /* CONFIG_RISCV_IOMMU_32BIT */ > > #endif > -- > 2.20.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Jessica, > Is such a platform conformant to the specification? We have talked about this before [1]. I think the IOMMU spec does not mandate the implementation of 8-byte access functionality. The related sentences are listed below: "The 8-byte IOMMU registers are defined in such a way that software can perform two individual 4-byte accesses, or hardware can perform two independent 4-byte transactions resulting from an 8-byte access, to the high and low halves of the register, in that order, as long as the register semantics, with regard to side-effects, are respected between the two software accesses, or two hardware transactions, respectively." "Registers that are 64-bit wide may be accessed using either a 32-bit or a 64-bit access." > Either way, why is this a static build-time configuration choice rather than a dynamic run-time choice based on the FDT / ACPI tables / some other platform probing method? I did not find available field in RIMT table to describe it. So maybe adding a new config is faster. [1] https://www.uwsg.indiana.edu/hypermail/linux/kernel/2408.3/03968.html Best Regards, Xu Lu On Wed, Mar 26, 2025 at 2:51 AM Jessica Clarke <jrtc27@jrtc27.com> wrote: > > On 25 Mar 2025, at 14:42, Xu Lu <luxu.kernel@bytedance.com> wrote: > > > > Introduce a new configuration CONFIG_RISCV_IOMMU_32BIT to enable > > splitting 8-byte access into 4-byte transactions for hardware platform > > whose I/O bus limits access to 4-byte transfers. > > > > Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> > > Is such a platform conformant to the specification? Either way, why is > this a static build-time configuration choice rather than a dynamic > run-time choice based on the FDT / ACPI tables / some other platform > probing method? > > Jess > > > --- > > drivers/iommu/riscv/Kconfig | 9 +++++++++ > > drivers/iommu/riscv/iommu.h | 28 +++++++++++++++++++++++----- > > 2 files changed, 32 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig > > index c071816f59a6..b7c9ea22d969 100644 > > --- a/drivers/iommu/riscv/Kconfig > > +++ b/drivers/iommu/riscv/Kconfig > > @@ -18,3 +18,12 @@ 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 > > + bool "Support 4-Byte Accesses on RISC-V IOMMU Registers" > > + depends on RISCV_IOMMU > > + default n > > + help > > + Support hardware platform whose I/O bus limits access to 4-byte > > + transfers. When enabled, all accesses to IOMMU registers will be > > + split into 4-byte accesses. > > diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h > > index 46df79dd5495..0e3552a8142d 100644 > > --- a/drivers/iommu/riscv/iommu.h > > +++ b/drivers/iommu/riscv/iommu.h > > @@ -14,6 +14,10 @@ > > #include <linux/iommu.h> > > #include <linux/types.h> > > #include <linux/iopoll.h> > > +#ifdef CONFIG_RISCV_IOMMU_32BIT > > +#include <linux/io-64-nonatomic-hi-lo.h> > > +#include <linux/io-64-nonatomic-lo-hi.h> > > +#endif > > > > #include "iommu-bits.h" > > > > @@ -69,21 +73,35 @@ 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 > > +#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 */ > > +#define riscv_iommu_readq(iommu, addr) \ > > + hi_lo_readq_relaxed((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, \ > > +#define riscv_iommu_writeq(iommu, addr, val) \ > > + ((addr == RISCV_IOMMU_REG_IOHPMCYCLES) ? \ > > + lo_hi_writeq_relaxed((val), (iommu)->reg + (addr)) : \ > > + hi_lo_writeq_relaxed((val), (iommu)->reg + (addr))) > > + > > +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \ > > + readx_poll_timeout(hi_lo_readq_relaxed, (iommu)->reg + (addr), val, cond, \ > > delay_us, timeout_us) > > +#endif /* CONFIG_RISCV_IOMMU_32BIT */ > > > > #endif > > -- > > 2.20.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv >
diff --git a/drivers/iommu/riscv/Kconfig b/drivers/iommu/riscv/Kconfig index c071816f59a6..b7c9ea22d969 100644 --- a/drivers/iommu/riscv/Kconfig +++ b/drivers/iommu/riscv/Kconfig @@ -18,3 +18,12 @@ 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 + bool "Support 4-Byte Accesses on RISC-V IOMMU Registers" + depends on RISCV_IOMMU + default n + help + Support hardware platform whose I/O bus limits access to 4-byte + transfers. When enabled, all accesses to IOMMU registers will be + split into 4-byte accesses. diff --git a/drivers/iommu/riscv/iommu.h b/drivers/iommu/riscv/iommu.h index 46df79dd5495..0e3552a8142d 100644 --- a/drivers/iommu/riscv/iommu.h +++ b/drivers/iommu/riscv/iommu.h @@ -14,6 +14,10 @@ #include <linux/iommu.h> #include <linux/types.h> #include <linux/iopoll.h> +#ifdef CONFIG_RISCV_IOMMU_32BIT +#include <linux/io-64-nonatomic-hi-lo.h> +#include <linux/io-64-nonatomic-lo-hi.h> +#endif #include "iommu-bits.h" @@ -69,21 +73,35 @@ 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 +#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 */ +#define riscv_iommu_readq(iommu, addr) \ + hi_lo_readq_relaxed((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, \ +#define riscv_iommu_writeq(iommu, addr, val) \ + ((addr == RISCV_IOMMU_REG_IOHPMCYCLES) ? \ + lo_hi_writeq_relaxed((val), (iommu)->reg + (addr)) : \ + hi_lo_writeq_relaxed((val), (iommu)->reg + (addr))) + +#define riscv_iommu_readq_timeout(iommu, addr, val, cond, delay_us, timeout_us) \ + readx_poll_timeout(hi_lo_readq_relaxed, (iommu)->reg + (addr), val, cond, \ delay_us, timeout_us) +#endif /* CONFIG_RISCV_IOMMU_32BIT */ #endif
Introduce a new configuration CONFIG_RISCV_IOMMU_32BIT to enable splitting 8-byte access into 4-byte transactions for hardware platform whose I/O bus limits access to 4-byte transfers. Signed-off-by: Xu Lu <luxu.kernel@bytedance.com> --- drivers/iommu/riscv/Kconfig | 9 +++++++++ drivers/iommu/riscv/iommu.h | 28 +++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-)