[External] Re: [PATCH v4 1/3] riscv: io: avoid null-pointer arithmetic in PIO helpers

[Arnd Bergmann]

On Wed, Jul 1, 2026, at 05:11, yunhui cui wrote:
> On Tue, May 5, 2026 at 2:34 PM Arnd Bergmann <arnd at arndb.de> wrote:
>> 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.
>
> Thanks for the review.
>
> I agree that NULL-pointer arithmetic warnings can point to real missing
> HAS_IOPORT dependencies in drivers, and we should not hide those globally.
>
> You are right about the generic ioport_map() change: the helper is already
> guarded by CONFIG_HAS_IOPORT_MAP, so the extra CONFIG_HAS_IOPORT check is
> redundant. I will drop that change.

Sounds good, thanks!

> For the RISC-V ins*/outs* helpers, they use PIO-specific fences
> (__io_pbr()/__io_par() and __io_pbw()/__io_paw()), while the asm-generic
> helpers would go through readsb()/writesb() and use normal MMIO string
> ordering. So removing them would change ordering semantics.
>
> I will keep the RISC-V helpers for now and only guard the port-I/O
> variants with CONFIG_HAS_IOPORT.

For the non-string versions, I think the generic implementation
should be identical to what riscv has, it should be trivial to
unify these.

The string helpers are a little quirky here, as they don't really
have any barriers at all in the generic implementation but go
through the low-level __raw_readl()/__raw_writel() loops. This
is necessary to avoid endianess issues and barriers inside of
the loop.
I think conceptually, we'd want the string helpers to have the
same barriers as the relaxed single I/O accessors and only
serialize against other I/O operations but not against DMA.
On most architectures, this would mean no barrier at all,
which is why the generic version works for them.

On riscv, we currently have custom macros that also do nothing:

/* FIXME: These are now the same as asm-generic */
#define __io_rbr()             do {} while (0)
#define __io_rar()             do {} while (0)
#define __io_rbw()             do {} while (0)
#define __io_raw()             do {} while (0)

If it helps unify riscv with the rest, we can certainly add
those to the readl_relaxed()/readsl()/insl() helpers
in the asm-generic version, but I don't understand
exactly why they even exist, or if you'd want to have
a separate definition for readsl() and insl() here.

     Arnd