On Wed, 2021-04-14 at 13:50 +0000, David Laight wrote: > 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 <David.Laight@xxxxxxxxxx> 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. Ok yeah I think that's also what I'd like best. > > Of course, there will be some obscure fallout - there always is. Let me come up with a patch, then if this decision is wrong it's at least one of us s390 people breaking someone else's architecture instead of the usual other way around ;-D > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
