Messages in this thread |  | | Date | Tue, 05 May 2026 08:33:04 +0200 | | From | "Arnd Bergmann" <> | | Subject | Re: [PATCH v4 1/3] riscv: io: avoid null-pointer arithmetic in PIO helpers |
| |
On Tue, May 5, 2026, at 08:20, Yunhui Cui wrote: > The RISC-V PIO helpers derive I/O addresses from PCI_IOBASE in ins*(), > outs*(), and ioport_map(). > > Under configurations where I/O port support is not available, these > expressions can still be formed during compilation and trigger > -Wnull-pointer-arithmetic warnings from clang.
If a driver attempts to use ISA port operations in a configuration without CONFIG_HAS_IOPORT, there is a NULL pointer warning because this is actually a NULL pointer access that will crash the kernel if the driver is ever loaded. You should not attempt to shut up the useful warning here but instead make sure every such code has a proper 'depends on HAS_IOPORT' dependency.
> > +#ifdef CONFIG_HAS_IOPORT > __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)) > -#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) > +#define insb(addr, buffer, count) __insb(PCI_IO_ADDR(addr), buffer, count) > +#define insw(addr, buffer, count) __insw(PCI_IO_ADDR(addr), buffer, count) > +#define insl(addr, buffer, count) __insl(PCI_IO_ADDR(addr), buffer, count) > +#endif
Can you try to remove the custom accessors altogether and use the ones from asm-generic instead? These should already have the correct behavior all the time, in particular they already come with the correct #ifdef check that is apparently missing in the riscv version.
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h > index ca5a1ce6f0f89..d799e5ccc9437 100644 > --- a/include/asm-generic/io.h > +++ b/include/asm-generic/io.h > @@ -1205,8 +1205,12 @@ static inline void __iomem > *ioremap_np(phys_addr_t offset, size_t size) > #define ioport_map ioport_map > static inline void __iomem *ioport_map(unsigned long port, unsigned > int nr) > { > +#ifdef CONFIG_HAS_IOPORT > port &= IO_SPACE_LIMIT; > return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port; > +#else > + return NULL; > +#endif > }
The function definition here is guarded by the #ifdef CONFIG_HAS_IOPORT_MAP check, which should already prevent it from being visible when CONFIG_HAS_IOPORT is disabled.
Arnd
|  |