Re: [PATCH] arm64: Enable PCI write-combine resources under sysfs

Lorenzo Pieralisi
2020-09-03T11:21:49Z

On Thu, Sep 03, 2020 at 09:07:00AM +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2020-09-02 at 17:47 +0100, Lorenzo Pieralisi wrote:
> > Yes I do and I expressed them.
> >
> > The first concern is the WC ambiguity on non-x86 systems, it looks
> > like write combinining means everything and nothing at the same time
> > on != x86 arches.
> >
> > On x86 prefetchable BAR == WC mapping (still conditional on arch
> > features ie PAT, not a blanket enable). On ARM64 WC mapping currently
> > corresponds to normal NC memory and the PCIe specs allow read
> > side-effects BAR to be marked as prefetchable, I need to force PCI
> > sig
> > to remove the section I mentioned from the specifications because
> > there
> > is NO way it can be detected if a prefetchable BAR maps to read
> > side-effects memory.
>
> Im not sure I understand your sentence. It's been a long accepted rule
> in PCI land that "prefetchable" BARs means "no side effects" and in
> fact allows much more than just prefetching :-)

I am referring to the nefarious:

"Additional Guidance on the Prefetchable Bit in Memory Space BARs"

I read it 100 times and I still have no idea how it can be implemented,
it sorts of acknowledges that read side-effects memory can be marked
as a prefetchable BAR *if* the system meets some criteria.

As if endpoint designers knew the system where their endpoint is
plugged into (+ bit (3) in a BAR is read-only).

I think that that implementation note must be removed from the
specifications - if anyone dares to follow it this whole
WC resource mapping can trigger trouble.

Good news is that it would be trouble for all arches :)

> > A kernel device driver would (hopefully) know, sysfs code that just
> > checks the prefetchable attribute and exports resource_WC does not.
> >
> > As I mentioned, if the mapping is done in a device specific driver it
> > can be vetted and there are not many drivers mapping BARs as
> > ioremap_wc().
>
> It's been what other architectures have been doing for mroe than a
> decade without significant issues... I don't think you should worry too
> much about this.

Minus what I wrote above, I agree with you. I'd still be able to
understand what this patch changes in the mellanox driver HW
handling though - not sure what they expect from arch_can_pci_mmap_wc()
returning 1.

Thanks,
Lorenzo