This patch addresses the PCI I/O port address handling in RISC-V's
port-mapped I/O emulation routines when the MMU is not enabled.
The changes ensure that:
1. For non-MMU systems, the PCI I/O port addresses are properly
   calculated in marcos inX and outX when PCI_IOBASE is not
   defined, this avoids the null pointer calculating warning
   from the compiler.
2. In asm-generic/io.h, function ioport_map(), casting PCI_IOPORT
   to type "long" firstly makes it could compute with variable addr
   directly, which avoids the null pointer calculating warning when
   PCI_IOPORT is a null pointer in some case.

The original implementation used `PCI_IOBASE + (addr)` for MMU-enabled
systems, but failed to handle non-MMU cases correctly. This change adds
conditional compilation guards (#ifdef CONFIG_MMU) to differentiate
between MMU and non-MMU environments, providing consistent behavior
for both scenarios.

Fixes: 9cc205e3c17d ("RISC-V: Make port I/O string accessors actually work")
Reported-by: WangYuli <wangyuli@uniontech.com>
Signed-off-by: Liu Runrun <liurunrun@uniontech.com>
---
 arch/riscv/include/asm/io.h | 22 +++++++++++++++++++++-
 include/asm-generic/io.h    |  3 ++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index a0e51840b9db..d5181bb02c98 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -101,9 +101,15 @@ __io_reads_ins(reads, u32, l, __io_br(), __io_ar(addr))
 __io_reads_ins(ins,  u8, b, __io_pbr(), __io_par(addr))
 __io_reads_ins(ins, u16, w, __io_pbr(), __io_par(addr))
 __io_reads_ins(ins, u32, l, __io_pbr(), __io_par(addr))
+#ifdef CONFIG_MMU
 #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, count)
 #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
 #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
+#else
+#define insb(addr, buffer, count) __insb((void __iomem *)(long)addr, buffer, count)
+#define insw(addr, buffer, count) __insw((void __iomem *)(long)addr, buffer, count)
+#define insl(addr, buffer, count) __insl((void __iomem *)(long)addr, buffer, count)
+#endif /* CONFIG_MMU */
 
 __io_writes_outs(writes,  u8, b, __io_bw(), __io_aw())
 __io_writes_outs(writes, u16, w, __io_bw(), __io_aw())
@@ -115,23 +121,37 @@ __io_writes_outs(writes, u32, l, __io_bw(), __io_aw())
 __io_writes_outs(outs,  u8, b, __io_pbw(), __io_paw())
 __io_writes_outs(outs, u16, w, __io_pbw(), __io_paw())
 __io_writes_outs(outs, u32, l, __io_pbw(), __io_paw())
+#ifdef CONFIG_MMU
 #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
 #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
 #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
+#else
+#define outsb(addr, buffer, count) __outsb((void __iomem *)(long)addr, buffer, count)
+#define outsw(addr, buffer, count) __outsw((void __iomem *)(long)addr, buffer, count)
+#define outsl(addr, buffer, count) __outsl((void __iomem *)(long)addr, buffer, count)
+#endif /* CONFIG_MMU */
 
 #ifdef CONFIG_64BIT
 __io_reads_ins(reads, u64, q, __io_br(), __io_ar(addr))
 #define readsq(addr, buffer, count) __readsq(addr, buffer, count)
 
 __io_reads_ins(ins, u64, q, __io_pbr(), __io_par(addr))
+#ifdef CONFIG_MMU
 #define insq(addr, buffer, count) __insq(PCI_IOBASE + (addr), buffer, count)
+#else
+#define insq(addr, buffer, count) __insq((void __iomem *)(long)addr, buffer, count)
+#endif /* CONFIG_MMU */
 
 __io_writes_outs(writes, u64, q, __io_bw(), __io_aw())
 #define writesq(addr, buffer, count) __writesq(addr, buffer, count)
 
 __io_writes_outs(outs, u64, q, __io_pbr(), __io_paw())
+#ifdef CONFIG_MMU
 #define outsq(addr, buffer, count) __outsq(PCI_IOBASE + (addr), buffer, count)
-#endif
+#else
+#define outsq(addr, buffer, count) __outsq((void __iomem *)(long)addr, buffer, count)
+#endif /* CONFIG_MMU */
+#endif /* CONFIG_64BIT */
 
 #include <asm-generic/io.h>
 
diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index 11abad6c87e1..c0409188bb5e 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -1172,7 +1172,8 @@ static inline void __iomem *ioremap_np(phys_addr_t offset, size_t size)
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
 	port &= IO_SPACE_LIMIT;
-	return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
+	return (port > MMIO_UPPER_LIMIT) ?
+				NULL : (void __iomem *)((unsigned long)PCI_IOBASE + port);
 }
 #define ARCH_HAS_GENERIC_IOPORT_MAP
 #endif
-- 
2.49.0

On Fri, Apr 11, 2025, at 04:34, Liu Runrun wrote:
> This patch addresses the PCI I/O port address handling in RISC-V's
> port-mapped I/O emulation routines when the MMU is not enabled.
> The changes ensure that:

Do you have a system that requires this? I sent a patch the other
day to make PCI 'depends on MMU', based on how nothing today
uses it. Having a NOMMU system with PCI wounds rather silly,
so I hope we don't ever get that.

> 1. For non-MMU systems, the PCI I/O port addresses are properly
>    calculated in marcos inX and outX when PCI_IOBASE is not
>    defined, this avoids the null pointer calculating warning
>    from the compiler.

This is the wrong way around: the warning tells you that you
have failed to configure PCI_IOBASE for the particular hardware,
and that you actually get a NULL pointer dereference.

The solution is not to shut up the warning but making it
not a NULL pointer dereference!

Part of the issue is that historically the asm-generic/io.h
header includes an incorrect fallback of PCI_IOBASE when
the architecture does not provide the correct one. I think only
sparc still relies on that, so that fallback definition should
be moved into arch/sparc/include/asm/io_{32,64}.h instead.

> 2. In asm-generic/io.h, function ioport_map(), casting PCI_IOPORT
>    to type "long" firstly makes it could compute with variable addr
>    directly, which avoids the null pointer calculating warning when
>    PCI_IOPORT is a null pointer in some case.

I don't understand that sentence, please rephrase.

> The original implementation used `PCI_IOBASE + (addr)` for MMU-enabled
> systems, but failed to handle non-MMU cases correctly. This change adds
> conditional compilation guards (#ifdef CONFIG_MMU) to differentiate
> between MMU and non-MMU environments, providing consistent behavior
> for both scenarios.

This also looks wrong: what you are distinguishing here is systems
with (potentially) I/O port support and those that never have I/O
port support.

> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
> index a0e51840b9db..d5181bb02c98 100644
> --- a/arch/riscv/include/asm/io.h
> +++ b/arch/riscv/include/asm/io.h
> @@ -101,9 +101,15 @@ __io_reads_ins(reads, u32, l, __io_br(), 
> __io_ar(addr))
>  __io_reads_ins(ins,  u8, b, __io_pbr(), __io_par(addr))
>  __io_reads_ins(ins, u16, w, __io_pbr(), __io_par(addr))
>  __io_reads_ins(ins, u32, l, __io_pbr(), __io_par(addr))
> +#ifdef CONFIG_MMU
>  #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, 
> count)
>  #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, 
> count)
>  #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, 
> count)

I see that these are defined unconditionally here, which is probably
the real mistake. What I think we need instead is to enclose
them in "#ifdef CONFIG_HAS_IOPORT" with no "#else" block, and
ensure that HAS_IOPORT is only set when building for targets that
have a sensible definition of PCI_IOBASE and IO_SPACE_LIMIT.

The same approach is used in asm-generic/io.h, we just haven't
done it for all architectures yet, since it's not entirely
clear which ones can support ISA bridges and legacy PCI devices
with port I/O.

Ideally we'd go through the individual PCI host driver
implementations and only enable HAS_IOPORT for those that
are can handle the I/O space window correctly, but leave
it out if none of them are selected.

      Arnd