Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <c6f3c9a70e054e9087f657bf4f142732fd43784c.camel@linux.ibm.com>
Subject: Re: [PATCH] asm-generic/io.h: Silence -Wnull-pointer-arithmetic
 warning on PCI_IOBASE
From:   Niklas Schnelle <schnelle@linux.ibm.com>
To:     David Laight <David.Laight@ACULAB.COM>,
        "'Arnd Bergmann'" <arnd@arndb.de>
Cc:     Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        clang-built-linux <clang-built-linux@googlegroups.com>,
        linux-arch <linux-arch@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        linux-s390 <linux-s390@vger.kernel.org>,
        Guo Ren <guoren@kernel.org>
Date:   Wed, 14 Apr 2021 14:34:36 +0200
In-Reply-To: <40d4114fa34043d0841b81d09457c415@AcuMS.aculab.com>
References: <20210413115439.1011560-1-schnelle@linux.ibm.com>
         <CAK8P3a1WTZOYpJ2TSjnbytQJWgtfwkQ8bXXdnqCnOn6ugJqN_w@mail.gmail.com>
         <84ab737edbe13d390373850bf317920b3a486b87.camel@linux.ibm.com>
         <CAK8P3a2NR2nhEffFQJdMq2Do_g2ji-7p3+iWyzw+aXD6gov05w@mail.gmail.com>
         <11ead5c2c73c42cbbeef32966bc7e5c2@AcuMS.aculab.com>
         <CAK8P3a3PK9zyeP4ymELtc2ZYnymECoACiigw9Za+pvSJpCk5=g@mail.gmail.com>
         <40d4114fa34043d0841b81d09457c415@AcuMS.aculab.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2021-04-13 at 14:12 +0000, David Laight wrote:
> From: Arnd Bergmann
> > Sent: 13 April 2021 14:40
> > 
> > On Tue, Apr 13, 2021 at 3:06 PM David Laight <David.Laight@aculab.com> wrote:
> > > From: Arnd Bergmann
> > > > Sent: 13 April 2021 13:58
> > > ...
> > > > The remaining ones (csky, m68k, sparc32) need to be inspected
> > > > manually to see if they currently support PCI I/O space but in
> > > > fact use address zero as the base (with large resources) or they
> > > > should also turn the operations into a NOP.
> > > 
> > > I'd expect sparc32 to use an ASI to access PCI IO space.
> > > I can't quite remember whether IO space was supported at all.
> > 
> > I see this bit in arch/sparc/kernel/leon_pci.c
> > 
> >  * PCI Memory and Prefetchable Memory is direct-mapped. However I/O Space is
> >  * accessed through a Window which is translated to low 64KB in PCI space, the
> >  * first 4KB is not used so 60KB is available.
> > ...
> >         pci_add_resource_offset(&resources, &info->io_space,
> >                                 info->io_space.start - 0x1000);
> > 
> > which means that there is I/O space, which gets accessed through whichever
> > method readb() uses. Having the offset equal to the resource means that
> > the '(void *)0' start is correct.
> 
> It must have been the VMEbus (and maybe sBus) sparc that used an ASI.
> 
> I do remember issues with Solaris of some PCI cards not liking
> being assigned a BAR address of zero.
> That may be why the low 4k IO space isn't assigned here.
> (I've never run Linux on sparc, just SVR4 and Solaris.)
> 
> I guess setting PCI_IOBASE to zero is safer when you can't trust
> drivers not to use inb() instead of readb().
> Or whatever io_read() ends up being.
> 
> 	David

So "I guess setting PCI_IOBASE to zero is safer when you can't trust
drivers not to use inb()…" in principle is true on other architectures
than sparc too, right? So do you think this means we shouldn't go with
Arnd's idea of making inb() just WARN_ONCE() if PCI_IOBASE is not
defined or just that for sparc defining it as 0 would be preferred?

As for s390 since we only support a limited number of drivers I think
for us such a WARN_ONCE() for inb() would be preferable.

I guess one option would be to let each architecture opt in to leaving
PCI_IOBASE undefined but in the first patch push PCI_IOBASE 0 into all
drivers that currently don't define it at all _and_ do not define their
own inb() etc.

> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)