Re: [PATCH v3 00/32] PCI: fix config and I/O Address space memory mappings

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Tue, Apr 18, 2017 at 08:36:48PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2017-04-18 at 09:57 +0100, Lorenzo Pieralisi wrote:
> > I can add a defined(pgprot_nonposted) to pci_remap_iospace() if that's
> > not too ugly (I suspect Bjorn is thrilled about it :)), that plus
> > the Kconfig option for ioremap_nopost() should complete this series.
> > =

> > int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr)
> > {
> > #if defined(PCI_IOBASE) && defined(CONFIG_MMU) && defined(pgprot_nonpos=
ted)
> > =A0=A0=A0=A0=A0=A0=A0=A0unsigned long vaddr =3D (unsigned long)PCI_IOBA=
SE + res->start;
> > =

> > =A0=A0=A0=A0=A0=A0=A0=A0if (!(res->flags & IORESOURCE_IO))
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -EINVAL;
> > =

> > =A0=A0=A0=A0=A0=A0=A0=A0if (res->end > IO_SPACE_LIMIT)
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return -EINVAL
> > =A0=A0=A0=A0=A0=A0=A0=A0return ioremap_page_range(vaddr, vaddr + resour=
ce_size(res), phys_addr,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 pgprot_nonposted(PAGE_KERNEL));
> > #else
> > =A0=A0=A0=A0=A0=A0=A0=A0/* this architecture does not have memory mappe=
d I/O space,
> > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 so this function should never be called =
*/
> > =A0=A0=A0=A0=A0=A0=A0=A0WARN_ONCE(1, "This architecture does not suppor=
t memory mapped I/O\n");
> > =A0=A0=A0=A0=A0=A0=A0=A0return -ENODEV;
> > #endif
> =

> The above would effectively disable mmap'ing of IO space for any
> architecture that doesn't have pgprot_nonposted... so everybody except
> ARM. Thus breaking a number of systems that have been working fine for
> years.

pci_remap_iospace() is used on ARM/ARM64 only AFAICT I do not understand
what I would actually break (and I am not sure at all how well PCI IO
space is tested on ARM/ARM64 machines anyway).

> I fail to see the point....
> =

> I think you are giving the whole non-posted stuff way more importance
> than it deserves. It's originally a kludge Intel did to PCI because it
> well with their synchronous IO space, which was itself a remnant of
> pre-history that should have long died.
> =

> In the specific case of PCI (again I'm not talking about the general
> case of pgprot/ioremap_nonposted), we have routinely been "violating"
> that rule, at least on the CPU -> PCI Bridge path (the PCI bridge
> itself tends to respect it though I've seen exceptions) for decades
> without any adverse effect.
> =

> I don't think there's much code (if any) out there which actually
> relies on the non-posted characteristics of IO space.
> =

> I don't care *that* much mind you, we dropped IO space on PCI with
> POWER8, but it would break stuff on existing older machines such as
> PowerMacs for no good reason.

Again, the change above should not break anything.

> respect the "non-posted" semantics of IO and be done with it.

I can do that too (or leave IO space alone, I do not care either).

> Is there any other practical use of non-posted mappings ? Config space
> I suppose, though here mostly PCI host bridges handle it by doing a
> read back in the config ops...

I started by adding a pci_remap_cfgspace() interface. Bjorn did not
like it to be PCI specific so I made it a generic one. I am not aware
of any other non-posted writes ioremap requirements apart from config
space.

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel