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

[Casey Leedom <leedom@chelsio.com>]

> On Jul 2, 2015, at 11:49 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> 
> On Sat, Jun 27, 2015 at 08:00:48AM +1000, Benjamin Herrenschmidt wrote:
>> On Fri, 2015-06-26 at 16:24 +0000, Casey Leedom wrote:
>>>  Thanks for looking into this Ben.  As it stands now, it seems as
>>> if Write Combined mappings simply aren't supported and/or all
>>> driver writers trying to utilize Write Combined mappings have to
>>> "hand roll" their own solutions which really isn't supportable.
>>> 
>>>  One thing that might be considered is simply to treat the desire
>>> to utilize the Write Combining hardware as a separate issue and
>>> develop writel_wc(), writeq_wc(), etc.
> 
> That seems rather sloppy and cumbersome, its best to just provide the
> infrastructure for initial mapping for an area and let the hardware do it for
> you. With the current design drivers would just use regular read/write on all
> areas and the only thing that will set them apart will be the mapping.  With
> what you propose we'd end up having to shift a whole bunch of reads/writes for
> just the purpose of adding WC to an area that didn't have wc before. That's
> a waste of code and time.
> 
>>> Those could be defined
>>> as legal only for Write Combined Mappings and would "do the
>>> right thing" for each architecture.  
> 
> Yuck.

  Yeah, probably.  But it sounds like from Ben’s response, we should really use write_relaxed() as an already existing API for this.  But most of the architectures just define this as writel() so some work would need to be done to get it to work.

>> The question then is what is "the right thing". In the powerpc case,
>> we'll have a non-garded mapping, which means we also get no ordering
>> between load and stores.
> 
> I don't follow, you *ordering* between load and stores for WC? We should
> not need that for WC, its why WC is used for only very specific things
> such as framebuffer and PIO (which BTw I still don't quite get all this
> use case for infiniband to be honest, and I will note I do see some
> proprietary hardware extensions like bursts but nothing covering all
> this in a general doc, I think I think it all just has to do that this
> is a hardware hack in reality, which we sell as a feature).

  I was talking about the work that our drivers (cxgb4, cxgb4vf, etc.) do to ensure correct ordering between writes to memory and I/O space.  For instance, issuing a wmb() between writes to a DMA buffer and the write to a register telling the hardware that the data is available.  This isn’t necessary on the Strongly Ordered Intel architectures but is necessary on other architectures.

Casey