Messages in this thread |  | | Subject | Re: [PATCH v4 1/3] riscv: io: avoid null-pointer arithmetic in PIO helpers | | From | bot+bpf-ci@kernel ... | | Date | Tue, 5 May 2026 07:20:07 +0000 (UTC) |
| |
> diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h > index 09bb5f57a9d3..6f5d70313c83 100644 > --- a/arch/riscv/include/asm/io.h > +++ b/arch/riscv/include/asm/io.h > @@ -56,6 +56,8 @@ > #define __io_pbw() RISCV_FENCE(iow, o) > #define __io_paw() RISCV_FENCE(o, io) > > +#define PCI_IO_ADDR(addr) ((void __iomem *)((unsigned long)PCI_IOBASE + (addr))) > + > /* > * Accesses from a single hart to a single I/O address must be ordered. This > * allows us to use the raw read macros, but we still need to fence before and
Arnd Bergmann raised concerns about this approach in his review of v4 (https://lore.kernel.org/all/3720c3a4-cc0d-4ba8-97ae-35def0189e2e@app.fastmail.com/).
He points out that the NULL pointer arithmetic warnings are actually useful because they indicate real bugs - NULL pointer dereferences that will crash the kernel if the driver is ever loaded. The proper fix is to ensure drivers have 'depends on HAS_IOPORT' in their Kconfig entries.
He suggests removing the custom RISC-V PIO accessors (ins*/outs*) entirely and using the asm-generic versions instead, which already have the correct #ifdef checks. This would be a cleaner approach than adding guards to suppress warnings.
[ ... ]
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h > index ca5a1ce6f0f8..d799e5ccc943 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 > } > #define ARCH_HAS_GENERIC_IOPORT_MAP > #endif
Is this change necessary? The function definition here is already guarded by the #ifdef CONFIG_HAS_IOPORT_MAP check, which should prevent it from being visible when CONFIG_HAS_IOPORT is disabled.
--- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25361792504 |  |