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
