Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Wed, 2015-06-24 at 15:29 -0700, Luis R. Rodriguez wrote:

> Nope but at least what made me squint at this being a possible
> "feature" was that in practice when reviewing all of the kernels
> pending device drivers using MTRR (potential write-combine candidates)
> I encountered a slew of them which had the architectural unfortunate
> practice of combining PCI bars for MMIO and their respective
> write-combined desirable area (framebuffer for video, PIO buffers for
> infiniband, etc). Now, to me that read more as a practice for old
> school devices when such things were likely still being evaluated,
> more modern devices seem to adhere to sticking a full PCI bar with
> write-combining or not. Did you not encounter such mismatch splits on
> powerpc ? Was such possibility addressed?

Yes, I remember we dealt with some networking (or maybe IB) stuff back
in the day. We dealt with it by using a WC mapping and explicit barriers
to prevent combine when not wanted.

It is to be noted that on powerpc at least, writel() and co will never
combine due to the memory barriers in them. Only "normal" stores (or
__raw_writel) will.

On Intel things I different I assume...

The problem I see is that architectures can provide widely different
mechanisms and semantics in those areas and it's hard to define a
generic driver interface.

> If what you are implying here is applicable to the x86 world I'm all
> for enabling this as we'd have less code to maintain but I'll note
> that getting a clarification alone on that prefetchable !=
> write-combining was in and of itself hard, I'd be surprised if we
> could get full architectural buy-in to this as an immediate automatic
> feature.

I'm happy not to make it automatic for kernel space. As for user
mappings, maybe the right thing to do is to let us do what we do by
default with a quirk that can set a flag in pci_dev to disable that
behaviour (maybe on a per BAR basis ?). I think the common case is
that WC works.

>  Because of this and because PAT did have some errata as well,
> I would not be surprised if some PCI bridges / devices would end up
> finding corner cases, as such if we can really do what you're saying
> and unless we can get some super sane certainty over it across the
> board, I'd be inclined to leave such things as a part of a new API.
> Maybe have some folks test using the new API for all calls and after
> some sanity of testing / releases consider a full switch.
> 
> That is, unless of course you're sure all this is sane and would wager
> all-in on it from the get-go.

Cheers,
Ben.