Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180816204506.GA21144@agluck-desk> <CAK8P3a2vAMzL22ds=kM1WRZgATzOrAz_y8Yk6edqJTVRJJFydw@mail.gmail.com>
 <20180816232639.GA25889@agluck-desk>
In-Reply-To: <20180816232639.GA25889@agluck-desk>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 17 Aug 2018 10:47:34 +0200
Message-ID: <CAK8P3a0hFDv2kvVN=QobTVaZxAX-M+YZzkBU_b=vDGLbCLziNg@mail.gmail.com>
Subject: Re: how to fix acpi_pci_root_remap_iospace?
To:     Tony Luck <tony.luck@intel.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        jchandra@broadcom.com, Sinan Kaya <okaya@codeaurora.org>,
        Tomasz Nowicki <tn@semihalf.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Boris Brezillon <boris.brezillon@bootlin.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Aug 17, 2018 at 1:27 AM Luck, Tony <tony.luck@intel.com> wrote:
>
> On Thu, Aug 16, 2018 at 11:10:33PM +0200, Arnd Bergmann wrote:
> > Another way would be to add
> >
> >  #include <asm-generic/io.h>
> > +#undef PCI_IOBASE
> >
> > in your asm/io.h. This is about as ugly as the your version, but
> > it would be local to ia64 ;-)
>
> Third way ...
>
>
> Is "0" actually the right value for PCI_IOBASE for some platform?
>
> #ifndef PCI_IOBASE
> #define PCI_IOBASE ((void __iomem *)0)
> #endif
>
> Or is this just here to make sure that:
>
> static inline u8 inb(unsigned long addr)
> {
>         u8 val;
>
>         __io_pbr();
>         val = __raw_readb(PCI_IOBASE + addr);
>         __io_par();
>         return val;
> }
>
> etc. Do not throw errors?

Defining it to zero is the traditional approach on some systems, and it's used
for at least two different reasons, both of which I don't particularly like:

- Some (particularly older) targets that have its I/O space mapped
into its linear
  virtual memory define inb() to be effectively an alias for readb() with the
  same numeric arguments. This kind of works in most cases but breaks in
  many corner cases such as
  * user space using /dev/ioport, which now grants access to all of
kernel memory
  * ISA device drivers using fixed 16-bit addresses on inb/outb, which
now points
    into user space memory
  * drivers that get the correct address from a resource but then truncate it by
    storing it in a 16-bit or 32-bit (on 64-bit machines) local variable.

- Some targets don't have any support for I/O space on their PCI bus and just
   want to get things to compile by setting PCI_IOBASE to zero, this still opens
    up some of the same problems as above, but doesn't really help otherwise.

> Should we really just enclose all of inb, inw, inl, ...
> inside of:
>
> #ifdef PCI_IOBASE
>
> ... all those static functions that use PCI_IOBASE ...

This breaks compilation of a couple of important drivers such as serial-8250
which support either I/O or memory space, so it requires some cleanup
first, or the definition of an alternative nop inb/outb family that does not
try to access the bus.

       Arnd