Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 11 Jul 2022 06:22:25 +0900
From:   Stafford Horne <shorne@gmail.com>
To:     Arnd Bergmann <arnd@arndb.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Openrisc <openrisc@lists.librecores.org>,
        Jonas Bonn <jonas@southpole.se>,
        Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>,
        Peter Zijlstra <peterz@infradead.org>,
        Palmer Dabbelt <palmer@rivosinc.com>
Subject: Re: [PATCH 1/2] openrisc: Add pci bus support
Message-ID: <YstDEargBYwjN2Yp@antec>
References: <20220709211549.1163327-1-shorne@gmail.com>
 <20220709211549.1163327-2-shorne@gmail.com>
 <CAK8P3a0Mw0Q9_W+xawMe=7UfiQiJT98wybygqg8E8gEuurZCuw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a0Mw0Q9_W+xawMe=7UfiQiJT98wybygqg8E8gEuurZCuw@mail.gmail.com>
Precedence: bulk

On Sun, Jul 10, 2022 at 05:54:22PM +0200, Arnd Bergmann wrote:
> On Sat, Jul 9, 2022 at 11:15 PM Stafford Horne <shorne@gmail.com> wrote:
> >
> > This patch adds required definitions to allow for PCI buses on OpenRISC.
> > This is being in the QEMU virt platform.
> >
> > OpenRISC does not have IO ports so this defines PCI IO to be allowed in
> > any range.  Keeping PIO_RESERVED defined as 0 allows OpenRISC to use
> > MMIO for all IO.
> 
> >
> >  /*
> > - * PCI: can we really do 0 here if we have no port IO?
> > + * PCI: All address space can be used for IO
> >   */
> > -#define IO_SPACE_LIMIT         0
> > +#define IO_SPACE_LIMIT ~(0UL)
> 
> I think '0' is the correct limit here if you don't support PCI controllers
> that can map their I/O ports into MMIO space. If you don't define
> PCI_IOBASE to a meaningful value and set IO_SPACE_LIMIT as you
> do here, every virtual address is treated as an I/O port, so accessing
> a low port through /dev/ioport or a PCI driver results in an access to
> a NULL pointer, which is either a userspace address or low kernel
> memory, both of which are bad.

OK, I see, but I think IO_SPACE_LIMIT needs to be defined as something other
than 0.  It is used to define kernel/resource.c's ioport_resource. For example
on risc-v they set it to 16MB.

I will setup a LIMIT smaller than 4GB and add a PCI_IOBASE.

> Most PCI controller are however able to map I/O ports into the
> physical address space of the CPU, and in that case you can just
> define an otherwise unused address as PCI_IOBASE and map the
> ports there from the PCI host bridge driver.

OK, understood, do you think this needs to be documented in a architecture
manual?  Maybe it's fine for it to be linux specific.

-Stafford