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.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Apr 13, 2021 at 3:06 PM David Laight <[email protected]> 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.
As this leaves only two others, I checked those as well:
csky does not actually have a PCI host bridge driver at the moment, so
we don't care about breaking port access on it it, and I would suggest
leaving I/O port access disabled. (Added Guo Ren to Cc for confirmation).
m68k only supports PCI on coldfire M54xx, and this variant does set
a PCI_IOBASE after all. The normal MMU based m68k have no PCI
and do define their out inb/outb/..., so nothing changes for them.
To summarize: only sparc32 needs to set PCI_IOBASE to zero, everyone
else should just WARN_ONCE() or return 0xff/0xffff/0xffffffff.
Arnd
From: Arnd Bergmann
> Sent: 13 April 2021 14:40
>
> On Tue, Apr 13, 2021 at 3:06 PM David Laight <[email protected]> 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
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
On Tue, Apr 13, 2021 at 9:40 PM Arnd Bergmann <[email protected]> wrote:
>
> On Tue, Apr 13, 2021 at 3:06 PM David Laight <[email protected]> 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.
>
> As this leaves only two others, I checked those as well:
>
> csky does not actually have a PCI host bridge driver at the moment, so
> we don't care about breaking port access on it it, and I would suggest
> leaving I/O port access disabled. (Added Guo Ren to Cc for confirmation).
Yes, we haven't reserved the PCI_IO region in the VM layout.
>
> m68k only supports PCI on coldfire M54xx, and this variant does set
> a PCI_IOBASE after all. The normal MMU based m68k have no PCI
> and do define their out inb/outb/..., so nothing changes for them.
>
> To summarize: only sparc32 needs to set PCI_IOBASE to zero, everyone
> else should just WARN_ONCE() or return 0xff/0xffff/0xffffffff.
>
> Arnd
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
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 <[email protected]> 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)
From: Niklas Schnelle
> Sent: 14 April 2021 13:35
>
> 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 <[email protected]> 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.
How much code outside of legacy x86 drivers should be using inb() etc?
I'm not sure any other (modern) cpu have separate IO instructions.
Because some PCI(e) resources might be available on memory or IO BARs
(possible duplicate BAR on some cards) aren't there also ioreadb()
functions (with addresses as parameters)?
IIRC on x86 they treat small values as IO ports and large ones
as memory mapped addresses.
If PCI IO space is memory mapped then these would be directly equivalent
to readb() (etc).
So perhaps inb() should just not be defined at all except on x86?
(Perhaps except for COMPILE_TEST).
If it is defined, then maybe it should never be called?
So a WARN_ONCE() returning ~0 for reads might even be best.
Of course, there will be some obscure fallout - there always is.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)