On Wed, 2022-05-04 at 23:31 +0200, Arnd Bergmann wrote: > On Wed, May 4, 2022 at 11:08 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Fri, Apr 29, 2022 at 03:49:59PM +0200, Niklas Schnelle wrote: > > > We introduce a new HAS_IOPORT Kconfig option to indicate support for > > > I/O Port access. In a future patch HAS_IOPORT=n will disable compilation > > > of the I/O accessor functions inb()/outb() and friends on architectures > > > which can not meaningfully support legacy I/O spaces such as s390 or > > > where such support is optional. > > > > So you plan to drop inb()/outb() on architectures where I/O port space > > is optional? So even platforms that have I/O port space may not be > > able to use it? > > > > This feels like a lot of work where the main benefit is to keep > > Kconfig from offering drivers that aren't of interest on s390. > > > > Granted, there may be issues where inb()/outb() does the wrong thing > > such as dereferencing null pointers when I/O port space isn't > > implemented. I think that's a defect in inb()/outb() and could be > > fixed there. > > The current implementation in asm-generic/io.h implements inb()/outb() > using readb()/writeb() with a fixed architecture specific offset. > > There are three possible things that can happen here: > > a) there is a host bridge driver that maps its I/O ports to this window, > and everything works > b) the address range is reserved and accessible but no host bridge > driver has mapped its registers there, so an access causes a > page fault > c) the architecture does not define an offset, and accessing low I/O > ports ends up as a NULL pointer dereference > > The main goal is to avoid c), which is what happens on s390, but > can also happen elsewhere. Catching b) would be nice as well, > but is much harder to do from generic code as you'd need an > architecture specific inline asm statement to insert a ex_table > fixup, or a runtime conditional on each access. > > Arnd Yes and to add to this, we did try a local solution in inb()/outb() before. This added a warning when they are used and we know at compile time that we're dealing with case c). This approach was nacked by Linus though as we were turning a compile time known broken case into a runtime one: https://lore.kernel.org/lkml/CAHk-=wg80je=K7madF4e7WrRNp37e3qh6y10Svhdc7O8SZ_-8g@xxxxxxxxxxxxxx/ I do agree with this assesment and think this is the right™ approach but it is more churn as can be seen by the size of this series. I think longer term it could be valuable though especially if more platforms phase out I/O port support like POWER9 for which this also allows filtering what drivers will never work.
