[PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[Palmer Dabbelt]

I recently started noticing warnings along the lines of

    include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
            insw(addr, buffer, count);
            ^~~~~~~~~~~~~~~~~~~~~~~~~
    arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
    #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)

which are triggered by having PCI_IOBASE be a "void __iomem *".  I'm not
quite sure what the right thing to do is here: having it as u8 to make
the pointer arithmetic work seems reasonable to me, but a bunch of other
ports still have it as "void __iomem *".

Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
---
Not sure if I just started noticing these, but a bunch show up when
merging Linus' master from this afternoon.  Having some sort of fix here
seems reasonable, as "void *" arithmetic is undefined.  I didn't check
if the other ports are suffering from this too, I figured I'd just send
a patch so I don't forget about it.
---
 arch/riscv/include/asm/io.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/io.h b/arch/riscv/include/asm/io.h
index 1c5c641075d2..a2d2d10c48cf 100644
--- a/arch/riscv/include/asm/io.h
+++ b/arch/riscv/include/asm/io.h
@@ -27,7 +27,7 @@
  */
 #ifdef CONFIG_MMU
 #define IO_SPACE_LIMIT		(PCI_IO_SIZE - 1)
-#define PCI_IOBASE		((void __iomem *)PCI_IO_START)
+#define PCI_IOBASE		((u8 __iomem *)PCI_IO_START)
 #endif /* CONFIG_MMU */
 
 /*
-- 
2.45.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[Andreas Schwab]

On Mai 26 2024, Palmer Dabbelt wrote:

> I recently started noticing warnings along the lines of
>
>     include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>             insw(addr, buffer, count);
>             ^~~~~~~~~~~~~~~~~~~~~~~~~
>     arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
>     #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
>
> which are triggered by having PCI_IOBASE be a "void __iomem *".  I'm not
> quite sure what the right thing to do is here: having it as u8 to make
> the pointer arithmetic work seems reasonable to me,

A u8 null pointer is still a null pointer.  Are you sure you are quoting
the right warning?  AFAICS, PCI_IOBASE is not a null pointer.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[Nam Cao]

On Mon, May 27, 2024 at 09:44:13AM +0200, Andreas Schwab wrote:
> On Mai 26 2024, Palmer Dabbelt wrote:
> 
> > I recently started noticing warnings along the lines of
> >
> >     include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> >             insw(addr, buffer, count);
> >             ^~~~~~~~~~~~~~~~~~~~~~~~~
> >     arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
> >     #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
> >
> > which are triggered by having PCI_IOBASE be a "void __iomem *".  I'm not
> > quite sure what the right thing to do is here: having it as u8 to make
> > the pointer arithmetic work seems reasonable to me,
> 
> A u8 null pointer is still a null pointer.  Are you sure you are quoting
> the right warning?  AFAICS, PCI_IOBASE is not a null pointer.

Null pointer has nothing to do with this. The warning is about arithmetic
operation on void*, which is undefined behavior.

Compilers usually do arithmetic on void* the same way as for u8*, but that
is not defined by C.

Best regards,
Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[Nam Cao]

On Mon, May 27, 2024 at 09:55:15AM +0200, Nam Cao wrote:
> On Mon, May 27, 2024 at 09:44:13AM +0200, Andreas Schwab wrote:
> > On Mai 26 2024, Palmer Dabbelt wrote:
> > 
> > > I recently started noticing warnings along the lines of
> > >
> > >     include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
> > >             insw(addr, buffer, count);
> > >             ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >     arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
> > >     #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
> > >
> > > which are triggered by having PCI_IOBASE be a "void __iomem *".  I'm not
> > > quite sure what the right thing to do is here: having it as u8 to make
> > > the pointer arithmetic work seems reasonable to me,
> > 
> > A u8 null pointer is still a null pointer.  Are you sure you are quoting
> > the right warning?  AFAICS, PCI_IOBASE is not a null pointer.
> 
> Null pointer has nothing to do with this. The warning is about arithmetic
> operation on void*, which is undefined behavior.

Wait, I am dumb. It is about null pointer.
Pretend you didn't see my email.

> 
> Compilers usually do arithmetic on void* the same way as for u8*, but that
> is not defined by C.
> 
> Best regards,
> Nam

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] RISC-V: io: Don't have a void* PCI_IOBASE

[Palmer Dabbelt]

On Mon, 27 May 2024 00:44:13 PDT (-0700), schwab@suse.de wrote:
> On Mai 26 2024, Palmer Dabbelt wrote:
>
>> I recently started noticing warnings along the lines of
>>
>>     include/asm-generic/io.h:752:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>>             insw(addr, buffer, count);
>>             ^~~~~~~~~~~~~~~~~~~~~~~~~
>>     arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
>>     #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
>>
>> which are triggered by having PCI_IOBASE be a "void __iomem *".  I'm not
>> quite sure what the right thing to do is here: having it as u8 to make
>> the pointer arithmetic work seems reasonable to me,
>
> A u8 null pointer is still a null pointer.  Are you sure you are quoting
> the right warning?  AFAICS, PCI_IOBASE is not a null pointer.

Ya, sorry, I just saw the void* UB and didn't even read to the "null"...

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv