Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Bjorn Helgaas]

On Mon, Aug 12, 2013 at 1:42 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
>> On Thu, Aug 08, 2013 at 12:47:10PM -0400, Chris Metcalf wrote:
>>> On 8/6/2013 1:48 PM, Bjorn Helgaas wrote:
>>>> [+cc Myron, Adam]
>>>>
>>>> On Fri, Aug 2, 2013 at 10:24 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>>>>> According to LSI,
>>>>> the firmware is not fully functional yet. This change implements a
>>>>> kind of hybrid dma_ops to support this.
>>>>>
>>>>> Note that on most other platforms, the 64-bit DMA addressing space is the
>>>>> same as the 32-bit DMA space and they overlap the physical memory space.
>>>>> No special arrangement is needed to support this kind of mixed DMA
>>>>> capability.  On TILE-Gx, the 64-bit DMA space is completely separate
>>>>> from the 32-bit DMA space.
>>>> Help me understand what's going on here.  My understanding is that on
>>>> typical systems, the 32-bit DMA space is a subset of the 64-bit DMA
>>>> space.  In conventional PCI, "a master that supports 64-bit addressing
>>>> must generate a SAC, instead of a DAC, when the upper 32 bits of the
>>>> address are zero" (PCI spec r3.0, sec 3.9).  PCIe doesn't have
>>>> SAC/DAC, but it has both 32-bit and 64-bit address headers and has a
>>>> similar requirement: "For Addresses below 4GB, Requesters must use the
>>>> 32-bit format" (PCIe spec r3.0, sec 2.2.4.1).
>>>>
>>>> Those imply to me that the 0-4GB region of the 64-bit DMA space must
>>>> be identical to the 0-4GB 32-bit DMA space, and in fact, the receiver
>>>> of a transaction shouldn't be able to distinguish them.
>>>>
>>>> But it sounds like something's different on TILE-Gx?  Does it
>>>> translate bus addresses to physical memory addresses based on the type
>>>> of the transaction (SAC vs DAC, or 32-bit vs 64-bit header) in
>>>> addition to the address?  Even if it does, the spec doesn't allow a
>>>> DAC cycle or a 64-bit header where the 32 high-order bits are zero, so
>>>> it shouldn't matter.
>>> No, we don't translate based on the type of the transaction. Using
>>> "DMA space" in the commit message was probably misleading. What's
>>> really going on is different DMA windows. 32-bit DMA has the
>>> obvious naive implementation where [0,4GB] in DMA space maps to
>>> [0,4GB] in PA space.  However, for 64-bit DMA, we use DMA
>>> addresses with a non-zero high 32 bits, in the [1TB,2TB] range,
>>> but map the results down to PA [0,1TB] using our IOMMU.
>> I guess this means devices can DMA to physical addresses [0,3GB]
>> using either 32-bit bus addresses in the [0,3GB] range or 64-bit bus
>> addresses in the [1TB,1TB+3GB] range, right?
>
> True in general, but not true for any specific individual device.
>
> 64-bit capable devices won’t generate 32-bit bus addresses, because the dma_ops makes sure that only bus/DMA addresses in [1TB,1TB+3GB] are handed out to the devices.
>
> 32-bit only devices use bus addresses in [0,3GB] to access the PA [0,3GB].  PA in [3GB, 4GB] is not accessed by the 32-bit only devices because the bounce buffers are allocated under 3GB limit.
>
>>> We did consider having the 64-bit DMA window be [0,1TB] and map
>>> directly to PA space, like the 32-bit window. But this design
>>> suffers from the “PCI hole” problem. Basically, the BAR space is
>>> usually under 4GB (it occupies the range [3GB, 4GB] on tilegx) and
>>> the host bridge uses negative decoding in passing DMA traffic
>>> upstream. That is, DMA traffic with target address in [3GB, 4GB]
>>> are not passed to the host memory. This means that the same amount
>>> of physical memory as the BAR space cannot be used for DMA
>>> purpose. And because it is not easy avoid this region in
>>> allocating DMA memory, the kernel is simply told to not use this
>>> chunk of PA at all, so it is wasted.
>> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
>> as you describe.  And even if DMA *could* reach it, the CPU couldn't
>> see it because CPU accesses to that range would go to PCI for the
>> memory-mapped BAR space, not to memory.
>
> Right.  Unreachability is only a problem if the DMA window overlaps [3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA space can be reached by 64-bit capable devices.

So the [0-1TB] memory range (including [3GB-4GB]) is reachable by
64-bit DMA to bus addresses [1TB-2TB].  But if the CPU can't see
physical memory from [3GB-4GB], how is it useful to DMA there?

>> But I can't figure out why Tile needs to do something special.  I
>> think other arches handle the PCI hole for MMIO space the same way.
>>
>> I don't know if other arches alias the [0,3GB] physical address
>> range in both 32-bit and 64-bit DMA space like you do, but if that's
>> part of the problem, it seems like you could easily avoid the
>> aliasing by making the 64-bit DMA space [1TB+4GB,2TB] instead of
>> [1TB,2TB].
>
> Perhaps, but since 64-bit capable devices can't actually see the aliasing (since they aren't offered the [0,4GB] address range) they only see an un-aliased space.
>
>>> For the LSI device, the way we manage it is to ensure that the
>>> device’s streaming buffers and the consistent buffers come from
>>> different pools, with the latter using the under-4GB bounce
>>> buffers. Obviously, normal devices use the same buffer pool for
>>> both streaming and consistent, either under 4GB or the whole PA.
>> It seems like you could make your DMA space be the union of [0,3GB]
>> and [1TB+4GB,2TB], then use pci_set_dma_mask(dev, DMA_BIT_MASK(64))
>> and pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)) (I assume the
>> driver already sets those masks correctly if it works on other
>> arches).
>
> Unfortunately, the Megaraid driver doesn’t even call pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).

If the Megaraid driver needs that call, but it's missing, why wouldn't
we just add it?

> More generally, your proposed DMA space suggestion isn't optimal because then the PA in [3GB, 4GB] can’t be reached by 64-bit capable devices.

True.  I assumed it wasn't useful to DMA there because the CPU
couldn't see that memory anyway.  But apparently that assumption was
wrong?

>>> Given all of that, does this change make sense? I can certainly
>>> amend the commit description to include more commentary.
>> Obviously, I'm missing something.  I guess it really doesn't matter
>> because this is all arch code and I don't need to understand it, but
>> it does niggle at me somehow.
>
> I will add the following comment to <asm/pci.h> in hopes of making it a bit clearer:
>
>  /*
>  [...]
> + * This design lets us avoid the "PCI hole" problem where the host bridge
> + * won't pass DMA traffic with target addresses that happen to fall within the
> + * BAR space. This enables us to use all the physical memory for DMA, instead
> + * of wasting the same amount of physical memory as the BAR window size.

By "target addresses", I guess you mean the bus address, not the CPU
address, right?

The whole reason I'm interested in this is to figure out whether this
change is really specific to Tile, or whether other architectures need
similar changes.  I think host bridges on other arches behave the same
way (they don't allow DMA to addresses in the PCI hole), so I still
haven't figured out what is truly Tile-specific.

I guess the ability for 64-bit DMA to reach the PCI hole (3GB-4GB)
might be unique, but it doesn't sound useful.

>   */
>  #define        TILE_PCI_MEM_MAP_BASE_OFFSET    (1ULL << CHIP_PA_WIDTH())
>
> --
> Chris Metcalf, Tilera Corp.
> http://www.tilera.com
>

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Chris Metcalf]

(Trimming the quoted material a little to try to keep this email under control.)

On 8/12/2013 4:42 PM, Bjorn Helgaas wrote:
> On Mon, Aug 12, 2013 at 1:42 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>> On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
>>> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
>>> as you describe.  And even if DMA *could* reach it, the CPU couldn't
>>> see it because CPU accesses to that range would go to PCI for the
>>> memory-mapped BAR space, not to memory.
>> Right.  Unreachability is only a problem if the DMA window overlaps [3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA space can be reached by 64-bit capable devices.
> So the [0-1TB] memory range (including [3GB-4GB]) is reachable by
> 64-bit DMA to bus addresses [1TB-2TB].  But if the CPU can't see
> physical memory from [3GB-4GB], how is it useful to DMA there?

Sorry, looking back I can see that the thread is a little confusing.
The CPU can see the whole PA space. The confusion comes from the BAR space
in [3GB, 4GB].

On Tile, we define the CPU memory space as follows:

[0, 1TB]: PA
[1TB + 3GB, 1TB + 4GB]: BAR space for RC port 0, in [3GB, 4GB]
[1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: BAR space for RC port N, in [3GB, 4GB]

The mapping from [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB] to [3GB, 4GB] is done by a
hardware PIO region, which generates PCI bus addresses in [3GB, 4GB] for MMIOs to
the BAR space.

>> Unfortunately, the Megaraid driver doesn’t even call pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).
> If the Megaraid driver needs that call, but it's missing, why wouldn't
> we just add it?

The Megaraid driver doesn’t strictly need that call on other platforms, because
by default the device coherent_dma_mask is DMA_BIT_MASK(32) and the consistent
memory pool doesn’t come from the bounce buffers on most other platforms.

Of course, for the sake of correctness, this call should be added across all platforms.

>> More generally, your proposed DMA space suggestion isn't optimal because then the PA in [3GB, 4GB] can’t be reached by 64-bit capable devices.
> True.  I assumed it wasn't useful to DMA there because the CPU
> couldn't see that memory anyway.  But apparently that assumption was
> wrong?

Correct.

>>>> Given all of that, does this change make sense? I can certainly
>>>> amend the commit description to include more commentary.
>>> Obviously, I'm missing something.  I guess it really doesn't matter
>>> because this is all arch code and I don't need to understand it, but
>>> it does niggle at me somehow.
>> I will add the following comment to <asm/pci.h> in hopes of making it a bit clearer:
>>
>>  /*
>>  [...]
>> + * This design lets us avoid the "PCI hole" problem where the host bridge
>> + * won't pass DMA traffic with target addresses that happen to fall within the
>> + * BAR space. This enables us to use all the physical memory for DMA, instead
>> + * of wasting the same amount of physical memory as the BAR window size.
> By "target addresses", I guess you mean the bus address, not the CPU
> address, right?

Correct.

> The whole reason I'm interested in this is to figure out whether this
> change is really specific to Tile, or whether other architectures need
> similar changes.  I think host bridges on other arches behave the same
> way (they don't allow DMA to addresses in the PCI hole), so I still
> haven't figured out what is truly Tile-specific.

What is unique about Tile is that the PCI drivers must explicitly declare
its DMA capability by calling pci_set_dma_mask() and pci_set_consistent_dma_mask().

This is why we must patch those drivers that don’t call pci_set_consistent_dma_mask(),
as is the case in the Megaraid driver.

> I guess the ability for 64-bit DMA to reach the PCI hole (3GB-4GB)
> might be unique, but it doesn't sound useful.

It seems like other architectures might benefit from the approach we've taken
with tile, but it's certainly disruptive enough that it might not be worth it.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Bjorn Helgaas]

[+cc James in case he has opinions on the DMA mask question]

On Tue, Aug 13, 2013 at 10:12 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> (Trimming the quoted material a little to try to keep this email under control.)
>
> On 8/12/2013 4:42 PM, Bjorn Helgaas wrote:
>> On Mon, Aug 12, 2013 at 1:42 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>>> On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
>>>> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
>>>> as you describe.  And even if DMA *could* reach it, the CPU couldn't
>>>> see it because CPU accesses to that range would go to PCI for the
>>>> memory-mapped BAR space, not to memory.
>>> Right.  Unreachability is only a problem if the DMA window overlaps [3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA space can be reached by 64-bit capable devices.
>> So the [0-1TB] memory range (including [3GB-4GB]) is reachable by
>> 64-bit DMA to bus addresses [1TB-2TB].  But if the CPU can't see
>> physical memory from [3GB-4GB], how is it useful to DMA there?
>
> Sorry, looking back I can see that the thread is a little confusing.
> The CPU can see the whole PA space. The confusion comes from the BAR space
> in [3GB, 4GB].
>
> On Tile, we define the CPU memory space as follows:
>
> [0, 1TB]: PA
> [1TB + 3GB, 1TB + 4GB]: BAR space for RC port 0, in [3GB, 4GB]
> [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: BAR space for RC port N, in [3GB, 4GB]
>
> The mapping from [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB] to [3GB, 4GB] is done by a
> hardware PIO region, which generates PCI bus addresses in [3GB, 4GB] for MMIOs to
> the BAR space.

OK, I think I get it now.  CPU address space:
  [0, 1TB]: physical memory
  [1TB + 3GB, 1TB + 4GB]: translated to bus address [3GB, 4GB] under RC port 0
  [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: translated to bus address
[3GB, 4GB] under RC port N

Bus address space:
  [0, 3GB]: 32-bit DMA reaches physical memory [0, 3GB]
  [3GB, 4GB]: 32-bit DMA (peer-to-peer DMA under local RC port, I guess?)
  [1TB, 2TB]: 64-bit DMA mapped via IOMMU to physical memory [0, 1TB]

I guess the problem is that 32-bit DMA can't reach physical memory
[3GB, 4GB], so you're using bounce buffers so the bus address is in
[0, 3GB].  That makes sense, and I don't see another possibility other
than just throwing away the [3GB, 4GB] range by leaving it out of the
kernel allocator altogether, or using  hardware (which tilegx probably
doesn't have) to remap it somewhere else.

So it seems like just a question of how you wrap this all up in
dma_ops, and *that* is all arch stuff that I don't have an opinion on.

>>> Unfortunately, the Megaraid driver doesn’t even call pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).
>> If the Megaraid driver needs that call, but it's missing, why wouldn't
>> we just add it?
>
> The Megaraid driver doesn’t strictly need that call on other platforms, because
> by default the device coherent_dma_mask is DMA_BIT_MASK(32) and the consistent
> memory pool doesn’t come from the bounce buffers on most other platforms.
>
> Of course, for the sake of correctness, this call should be added across all platforms.
> ...

> What is unique about Tile is that the PCI drivers must explicitly declare
> its DMA capability by calling pci_set_dma_mask() and pci_set_consistent_dma_mask().

It looks like the reason you need drivers to explicitly call
pci_set_dma_mask() and pci_set_consistent_dma_mask() is because you
have hooks in those functions to tweak the dma_ops, even though the
mask itself might not be changed.

That doesn't sound like a robust solution: we have well-known defaults
for the mask sizes, and I don't think it's reasonable to expect
drivers to explicitly set the mask even if they are happy with the
defaults (though Documentation/DMA-API-HOWTO.txt does say that being
explicit is good style).  I'm afraid you'll just keep tripping over
drivers that don't work on tilegx because they don't set the mask.

Bjorn

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[James Bottomley]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5531 bytes --]

On Tue, 2013-08-13 at 14:30 -0600, Bjorn Helgaas wrote:
> [+cc James in case he has opinions on the DMA mask question]
> 
> On Tue, Aug 13, 2013 at 10:12 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> > (Trimming the quoted material a little to try to keep this email under control.)
> >
> > On 8/12/2013 4:42 PM, Bjorn Helgaas wrote:
> >> On Mon, Aug 12, 2013 at 1:42 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> >>> On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
> >>>> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
> >>>> as you describe.  And even if DMA *could* reach it, the CPU couldn't
> >>>> see it because CPU accesses to that range would go to PCI for the
> >>>> memory-mapped BAR space, not to memory.
> >>> Right.  Unreachability is only a problem if the DMA window overlaps [3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA space can be reached by 64-bit capable devices.
> >> So the [0-1TB] memory range (including [3GB-4GB]) is reachable by
> >> 64-bit DMA to bus addresses [1TB-2TB].  But if the CPU can't see
> >> physical memory from [3GB-4GB], how is it useful to DMA there?
> >
> > Sorry, looking back I can see that the thread is a little confusing.
> > The CPU can see the whole PA space. The confusion comes from the BAR space
> > in [3GB, 4GB].
> >
> > On Tile, we define the CPU memory space as follows:
> >
> > [0, 1TB]: PA
> > [1TB + 3GB, 1TB + 4GB]: BAR space for RC port 0, in [3GB, 4GB]
> > [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: BAR space for RC port N, in [3GB, 4GB]
> >
> > The mapping from [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB] to [3GB, 4GB] is done by a
> > hardware PIO region, which generates PCI bus addresses in [3GB, 4GB] for MMIOs to
> > the BAR space.
> 
> OK, I think I get it now.  CPU address space:
>   [0, 1TB]: physical memory
>   [1TB + 3GB, 1TB + 4GB]: translated to bus address [3GB, 4GB] under RC port 0
>   [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: translated to bus address
> [3GB, 4GB] under RC port N
> 
> Bus address space:
>   [0, 3GB]: 32-bit DMA reaches physical memory [0, 3GB]
>   [3GB, 4GB]: 32-bit DMA (peer-to-peer DMA under local RC port, I guess?)
>   [1TB, 2TB]: 64-bit DMA mapped via IOMMU to physical memory [0, 1TB]
> 
> I guess the problem is that 32-bit DMA can't reach physical memory
> [3GB, 4GB], so you're using bounce buffers so the bus address is in
> [0, 3GB].  That makes sense, and I don't see another possibility other
> than just throwing away the [3GB, 4GB] range by leaving it out of the
> kernel allocator altogether, or using  hardware (which tilegx probably
> doesn't have) to remap it somewhere else.

This is remarkably familiar.  I think almost every system on earth has a
configuration similar to this.  On PARISC, the top 256MB of memory on a
32 bit system is reserved for I/O access and is designated as "F Space".

What is unusual is that you seem to have responding memory behind the F
Space which is accessible to some bus entities.  On PARISC 32 bit, the
memory is just lost (inaccessible) on 64 bit, it's remapped above 32GB
(and the low F Space window expanded to 1GB).

> So it seems like just a question of how you wrap this all up in
> dma_ops, and *that* is all arch stuff that I don't have an opinion on.
> 
> >>> Unfortunately, the Megaraid driver doesn’t even call pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).
> >> If the Megaraid driver needs that call, but it's missing, why wouldn't
> >> we just add it?
> >
> > The Megaraid driver doesn’t strictly need that call on other platforms, because
> > by default the device coherent_dma_mask is DMA_BIT_MASK(32) and the consistent
> > memory pool doesn’t come from the bounce buffers on most other platforms.
> >
> > Of course, for the sake of correctness, this call should be added across all platforms.
> > ...
> 
> > What is unique about Tile is that the PCI drivers must explicitly declare
> > its DMA capability by calling pci_set_dma_mask() and pci_set_consistent_dma_mask().
> 
> It looks like the reason you need drivers to explicitly call
> pci_set_dma_mask() and pci_set_consistent_dma_mask() is because you
> have hooks in those functions to tweak the dma_ops, even though the
> mask itself might not be changed.
> 
> That doesn't sound like a robust solution: we have well-known defaults
> for the mask sizes, and I don't think it's reasonable to expect
> drivers to explicitly set the mask even if they are happy with the
> defaults (though Documentation/DMA-API-HOWTO.txt does say that being
> explicit is good style).  I'm afraid you'll just keep tripping over
> drivers that don't work on tilegx because they don't set the mask.

Right, it's not a robust solution at all.  A DMA mask is just that: an
accessibility mask.  The founding assumption is that an address line is
either connected or not, which is why the mask works.  What you have is
two different classes of memory: 0-3GB which is usable for I/O and 3-4GB
which isn't.  Surely what you need to do is implement ZONE_DMA32? which
stretches from 0-3GB, which means all kmallocs in the driver will be in
the right range.  Then you need a bounce pfn at 3GB which means that
user space which gets the 3-4GB is bounced.  There's a magic pfn in
BLK_BOUNCE_HIGH that was designed for this, but I'm not sure the design
contemplated BLK_BOUNCE_HIGH being different for 64 and 32 bit.

James

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[James Bottomley]

T24gVHVlLCAyMDEzLTA4LTEzIGF0IDE0OjMwIC0wNjAwLCBCam9ybiBIZWxnYWFzIHdyb3RlOg0K
PiBbK2NjIEphbWVzIGluIGNhc2UgaGUgaGFzIG9waW5pb25zIG9uIHRoZSBETUEgbWFzayBxdWVz
dGlvbl0NCj4gDQo+IE9uIFR1ZSwgQXVnIDEzLCAyMDEzIGF0IDEwOjEyIEFNLCBDaHJpcyBNZXRj
YWxmIDxjbWV0Y2FsZkB0aWxlcmEuY29tPiB3cm90ZToNCj4gPiAoVHJpbW1pbmcgdGhlIHF1b3Rl
ZCBtYXRlcmlhbCBhIGxpdHRsZSB0byB0cnkgdG8ga2VlcCB0aGlzIGVtYWlsIHVuZGVyIGNvbnRy
b2wuKQ0KPiA+DQo+ID4gT24gOC8xMi8yMDEzIDQ6NDIgUE0sIEJqb3JuIEhlbGdhYXMgd3JvdGU6
DQo+ID4+IE9uIE1vbiwgQXVnIDEyLCAyMDEzIGF0IDE6NDIgUE0sIENocmlzIE1ldGNhbGYgPGNt
ZXRjYWxmQHRpbGVyYS5jb20+IHdyb3RlOg0KPiA+Pj4gT24gOC85LzIwMTMgNjo0MiBQTSwgQmpv
cm4gSGVsZ2FhcyB3cm90ZToNCj4gPj4+PiBPSywgc28gcGh5c2ljYWwgbWVtb3J5IGluIHRoZSBb
M0dCLDRHQl0gcmFuZ2UgaXMgdW5yZWFjaGFibGUgdmlhIERNQQ0KPiA+Pj4+IGFzIHlvdSBkZXNj
cmliZS4gIEFuZCBldmVuIGlmIERNQSAqY291bGQqIHJlYWNoIGl0LCB0aGUgQ1BVIGNvdWxkbid0
DQo+ID4+Pj4gc2VlIGl0IGJlY2F1c2UgQ1BVIGFjY2Vzc2VzIHRvIHRoYXQgcmFuZ2Ugd291bGQg
Z28gdG8gUENJIGZvciB0aGUNCj4gPj4+PiBtZW1vcnktbWFwcGVkIEJBUiBzcGFjZSwgbm90IHRv
IG1lbW9yeS4NCj4gPj4+IFJpZ2h0LiAgVW5yZWFjaGFiaWxpdHkgaXMgb25seSBhIHByb2JsZW0g
aWYgdGhlIERNQSB3aW5kb3cgb3ZlcmxhcHMgWzNHLCA0R10sIGFuZCBzaW5jZSB0aGUgNjQtYml0
IERNQSB3aW5kb3cgaXMgWzFUQiwyVEJdLCB0aGUgd2hvbGUgUEEgc3BhY2UgY2FuIGJlIHJlYWNo
ZWQgYnkgNjQtYml0IGNhcGFibGUgZGV2aWNlcy4NCj4gPj4gU28gdGhlIFswLTFUQl0gbWVtb3J5
IHJhbmdlIChpbmNsdWRpbmcgWzNHQi00R0JdKSBpcyByZWFjaGFibGUgYnkNCj4gPj4gNjQtYml0
IERNQSB0byBidXMgYWRkcmVzc2VzIFsxVEItMlRCXS4gIEJ1dCBpZiB0aGUgQ1BVIGNhbid0IHNl
ZQ0KPiA+PiBwaHlzaWNhbCBtZW1vcnkgZnJvbSBbM0dCLTRHQl0sIGhvdyBpcyBpdCB1c2VmdWwg
dG8gRE1BIHRoZXJlPw0KPiA+DQo+ID4gU29ycnksIGxvb2tpbmcgYmFjayBJIGNhbiBzZWUgdGhh
dCB0aGUgdGhyZWFkIGlzIGEgbGl0dGxlIGNvbmZ1c2luZy4NCj4gPiBUaGUgQ1BVIGNhbiBzZWUg
dGhlIHdob2xlIFBBIHNwYWNlLiBUaGUgY29uZnVzaW9uIGNvbWVzIGZyb20gdGhlIEJBUiBzcGFj
ZQ0KPiA+IGluIFszR0IsIDRHQl0uDQo+ID4NCj4gPiBPbiBUaWxlLCB3ZSBkZWZpbmUgdGhlIENQ
VSBtZW1vcnkgc3BhY2UgYXMgZm9sbG93czoNCj4gPg0KPiA+IFswLCAxVEJdOiBQQQ0KPiA+IFsx
VEIgKyAzR0IsIDFUQiArIDRHQl06IEJBUiBzcGFjZSBmb3IgUkMgcG9ydCAwLCBpbiBbM0dCLCA0
R0JdDQo+ID4gWzFUQiArIDNHQiArIE4qNEdCLCAxVEIgKyAoMSArIE4pKjRHQl06IEJBUiBzcGFj
ZSBmb3IgUkMgcG9ydCBOLCBpbiBbM0dCLCA0R0JdDQo+ID4NCj4gPiBUaGUgbWFwcGluZyBmcm9t
IFsxVEIgKyAzR0IgKyBOKjRHQiwgMVRCICsgKDEgKyBOKSo0R0JdIHRvIFszR0IsIDRHQl0gaXMg
ZG9uZSBieSBhDQo+ID4gaGFyZHdhcmUgUElPIHJlZ2lvbiwgd2hpY2ggZ2VuZXJhdGVzIFBDSSBi
dXMgYWRkcmVzc2VzIGluIFszR0IsIDRHQl0gZm9yIE1NSU9zIHRvDQo+ID4gdGhlIEJBUiBzcGFj
ZS4NCj4gDQo+IE9LLCBJIHRoaW5rIEkgZ2V0IGl0IG5vdy4gIENQVSBhZGRyZXNzIHNwYWNlOg0K
PiAgIFswLCAxVEJdOiBwaHlzaWNhbCBtZW1vcnkNCj4gICBbMVRCICsgM0dCLCAxVEIgKyA0R0Jd
OiB0cmFuc2xhdGVkIHRvIGJ1cyBhZGRyZXNzIFszR0IsIDRHQl0gdW5kZXIgUkMgcG9ydCAwDQo+
ICAgWzFUQiArIDNHQiArIE4qNEdCLCAxVEIgKyAoMSArIE4pKjRHQl06IHRyYW5zbGF0ZWQgdG8g
YnVzIGFkZHJlc3MNCj4gWzNHQiwgNEdCXSB1bmRlciBSQyBwb3J0IE4NCj4gDQo+IEJ1cyBhZGRy
ZXNzIHNwYWNlOg0KPiAgIFswLCAzR0JdOiAzMi1iaXQgRE1BIHJlYWNoZXMgcGh5c2ljYWwgbWVt
b3J5IFswLCAzR0JdDQo+ICAgWzNHQiwgNEdCXTogMzItYml0IERNQSAocGVlci10by1wZWVyIERN
QSB1bmRlciBsb2NhbCBSQyBwb3J0LCBJIGd1ZXNzPykNCj4gICBbMVRCLCAyVEJdOiA2NC1iaXQg
RE1BIG1hcHBlZCB2aWEgSU9NTVUgdG8gcGh5c2ljYWwgbWVtb3J5IFswLCAxVEJdDQo+IA0KPiBJ
IGd1ZXNzIHRoZSBwcm9ibGVtIGlzIHRoYXQgMzItYml0IERNQSBjYW4ndCByZWFjaCBwaHlzaWNh
bCBtZW1vcnkNCj4gWzNHQiwgNEdCXSwgc28geW91J3JlIHVzaW5nIGJvdW5jZSBidWZmZXJzIHNv
IHRoZSBidXMgYWRkcmVzcyBpcyBpbg0KPiBbMCwgM0dCXS4gIFRoYXQgbWFrZXMgc2Vuc2UsIGFu
ZCBJIGRvbid0IHNlZSBhbm90aGVyIHBvc3NpYmlsaXR5IG90aGVyDQo+IHRoYW4ganVzdCB0aHJv
d2luZyBhd2F5IHRoZSBbM0dCLCA0R0JdIHJhbmdlIGJ5IGxlYXZpbmcgaXQgb3V0IG9mIHRoZQ0K
PiBrZXJuZWwgYWxsb2NhdG9yIGFsdG9nZXRoZXIsIG9yIHVzaW5nICBoYXJkd2FyZSAod2hpY2gg
dGlsZWd4IHByb2JhYmx5DQo+IGRvZXNuJ3QgaGF2ZSkgdG8gcmVtYXAgaXQgc29tZXdoZXJlIGVs
c2UuDQoNClRoaXMgaXMgcmVtYXJrYWJseSBmYW1pbGlhci4gIEkgdGhpbmsgYWxtb3N0IGV2ZXJ5
IHN5c3RlbSBvbiBlYXJ0aCBoYXMgYQ0KY29uZmlndXJhdGlvbiBzaW1pbGFyIHRvIHRoaXMuICBP
biBQQVJJU0MsIHRoZSB0b3AgMjU2TUIgb2YgbWVtb3J5IG9uIGENCjMyIGJpdCBzeXN0ZW0gaXMg
cmVzZXJ2ZWQgZm9yIEkvTyBhY2Nlc3MgYW5kIGlzIGRlc2lnbmF0ZWQgYXMgIkYgU3BhY2UiLg0K
DQpXaGF0IGlzIHVudXN1YWwgaXMgdGhhdCB5b3Ugc2VlbSB0byBoYXZlIHJlc3BvbmRpbmcgbWVt
b3J5IGJlaGluZCB0aGUgRg0KU3BhY2Ugd2hpY2ggaXMgYWNjZXNzaWJsZSB0byBzb21lIGJ1cyBl
bnRpdGllcy4gIE9uIFBBUklTQyAzMiBiaXQsIHRoZQ0KbWVtb3J5IGlzIGp1c3QgbG9zdCAoaW5h
Y2Nlc3NpYmxlKSBvbiA2NCBiaXQsIGl0J3MgcmVtYXBwZWQgYWJvdmUgMzJHQg0KKGFuZCB0aGUg
bG93IEYgU3BhY2Ugd2luZG93IGV4cGFuZGVkIHRvIDFHQikuDQoNCj4gU28gaXQgc2VlbXMgbGlr
ZSBqdXN0IGEgcXVlc3Rpb24gb2YgaG93IHlvdSB3cmFwIHRoaXMgYWxsIHVwIGluDQo+IGRtYV9v
cHMsIGFuZCAqdGhhdCogaXMgYWxsIGFyY2ggc3R1ZmYgdGhhdCBJIGRvbid0IGhhdmUgYW4gb3Bp
bmlvbiBvbi4NCj4gDQo+ID4+PiBVbmZvcnR1bmF0ZWx5LCB0aGUgTWVnYXJhaWQgZHJpdmVyIGRv
ZXNu4oCZdCBldmVuIGNhbGwgcGNpX3NldF9jb25zaXN0ZW50X2RtYV9tYXNrKGRldiwgRE1BX0JJ
VF9NQVNLKDMyKSkuDQo+ID4+IElmIHRoZSBNZWdhcmFpZCBkcml2ZXIgbmVlZHMgdGhhdCBjYWxs
LCBidXQgaXQncyBtaXNzaW5nLCB3aHkgd291bGRuJ3QNCj4gPj4gd2UganVzdCBhZGQgaXQ/DQo+
ID4NCj4gPiBUaGUgTWVnYXJhaWQgZHJpdmVyIGRvZXNu4oCZdCBzdHJpY3RseSBuZWVkIHRoYXQg
Y2FsbCBvbiBvdGhlciBwbGF0Zm9ybXMsIGJlY2F1c2UNCj4gPiBieSBkZWZhdWx0IHRoZSBkZXZp
Y2UgY29oZXJlbnRfZG1hX21hc2sgaXMgRE1BX0JJVF9NQVNLKDMyKSBhbmQgdGhlIGNvbnNpc3Rl
bnQNCj4gPiBtZW1vcnkgcG9vbCBkb2VzbuKAmXQgY29tZSBmcm9tIHRoZSBib3VuY2UgYnVmZmVy
cyBvbiBtb3N0IG90aGVyIHBsYXRmb3Jtcy4NCj4gPg0KPiA+IE9mIGNvdXJzZSwgZm9yIHRoZSBz
YWtlIG9mIGNvcnJlY3RuZXNzLCB0aGlzIGNhbGwgc2hvdWxkIGJlIGFkZGVkIGFjcm9zcyBhbGwg
cGxhdGZvcm1zLg0KPiA+IC4uLg0KPiANCj4gPiBXaGF0IGlzIHVuaXF1ZSBhYm91dCBUaWxlIGlz
IHRoYXQgdGhlIFBDSSBkcml2ZXJzIG11c3QgZXhwbGljaXRseSBkZWNsYXJlDQo+ID4gaXRzIERN
QSBjYXBhYmlsaXR5IGJ5IGNhbGxpbmcgcGNpX3NldF9kbWFfbWFzaygpIGFuZCBwY2lfc2V0X2Nv
bnNpc3RlbnRfZG1hX21hc2soKS4NCj4gDQo+IEl0IGxvb2tzIGxpa2UgdGhlIHJlYXNvbiB5b3Ug
bmVlZCBkcml2ZXJzIHRvIGV4cGxpY2l0bHkgY2FsbA0KPiBwY2lfc2V0X2RtYV9tYXNrKCkgYW5k
IHBjaV9zZXRfY29uc2lzdGVudF9kbWFfbWFzaygpIGlzIGJlY2F1c2UgeW91DQo+IGhhdmUgaG9v
a3MgaW4gdGhvc2UgZnVuY3Rpb25zIHRvIHR3ZWFrIHRoZSBkbWFfb3BzLCBldmVuIHRob3VnaCB0
aGUNCj4gbWFzayBpdHNlbGYgbWlnaHQgbm90IGJlIGNoYW5nZWQuDQo+IA0KPiBUaGF0IGRvZXNu
J3Qgc291bmQgbGlrZSBhIHJvYnVzdCBzb2x1dGlvbjogd2UgaGF2ZSB3ZWxsLWtub3duIGRlZmF1
bHRzDQo+IGZvciB0aGUgbWFzayBzaXplcywgYW5kIEkgZG9uJ3QgdGhpbmsgaXQncyByZWFzb25h
YmxlIHRvIGV4cGVjdA0KPiBkcml2ZXJzIHRvIGV4cGxpY2l0bHkgc2V0IHRoZSBtYXNrIGV2ZW4g
aWYgdGhleSBhcmUgaGFwcHkgd2l0aCB0aGUNCj4gZGVmYXVsdHMgKHRob3VnaCBEb2N1bWVudGF0
aW9uL0RNQS1BUEktSE9XVE8udHh0IGRvZXMgc2F5IHRoYXQgYmVpbmcNCj4gZXhwbGljaXQgaXMg
Z29vZCBzdHlsZSkuICBJJ20gYWZyYWlkIHlvdSdsbCBqdXN0IGtlZXAgdHJpcHBpbmcgb3Zlcg0K
PiBkcml2ZXJzIHRoYXQgZG9uJ3Qgd29yayBvbiB0aWxlZ3ggYmVjYXVzZSB0aGV5IGRvbid0IHNl
dCB0aGUgbWFzay4NCg0KUmlnaHQsIGl0J3Mgbm90IGEgcm9idXN0IHNvbHV0aW9uIGF0IGFsbC4g
IEEgRE1BIG1hc2sgaXMganVzdCB0aGF0OiBhbg0KYWNjZXNzaWJpbGl0eSBtYXNrLiAgVGhlIGZv
dW5kaW5nIGFzc3VtcHRpb24gaXMgdGhhdCBhbiBhZGRyZXNzIGxpbmUgaXMNCmVpdGhlciBjb25u
ZWN0ZWQgb3Igbm90LCB3aGljaCBpcyB3aHkgdGhlIG1hc2sgd29ya3MuICBXaGF0IHlvdSBoYXZl
IGlzDQp0d28gZGlmZmVyZW50IGNsYXNzZXMgb2YgbWVtb3J5OiAwLTNHQiB3aGljaCBpcyB1c2Fi
bGUgZm9yIEkvTyBhbmQgMy00R0INCndoaWNoIGlzbid0LiAgU3VyZWx5IHdoYXQgeW91IG5lZWQg
dG8gZG8gaXMgaW1wbGVtZW50IFpPTkVfRE1BMzI/IHdoaWNoDQpzdHJldGNoZXMgZnJvbSAwLTNH
Qiwgd2hpY2ggbWVhbnMgYWxsIGttYWxsb2NzIGluIHRoZSBkcml2ZXIgd2lsbCBiZSBpbg0KdGhl
IHJpZ2h0IHJhbmdlLiAgVGhlbiB5b3UgbmVlZCBhIGJvdW5jZSBwZm4gYXQgM0dCIHdoaWNoIG1l
YW5zIHRoYXQNCnVzZXIgc3BhY2Ugd2hpY2ggZ2V0cyB0aGUgMy00R0IgaXMgYm91bmNlZC4gIFRo
ZXJlJ3MgYSBtYWdpYyBwZm4gaW4NCkJMS19CT1VOQ0VfSElHSCB0aGF0IHdhcyBkZXNpZ25lZCBm
b3IgdGhpcywgYnV0IEknbSBub3Qgc3VyZSB0aGUgZGVzaWduDQpjb250ZW1wbGF0ZWQgQkxLX0JP
VU5DRV9ISUdIIGJlaW5nIGRpZmZlcmVudCBmb3IgNjQgYW5kIDMyIGJpdC4NCg0KSmFtZXMNCg0K

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Chris Metcalf]

On 8/13/2013 4:30 PM, Bjorn Helgaas wrote:
> [+cc James in case he has opinions on the DMA mask question]
>
> On Tue, Aug 13, 2013 at 10:12 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>> (Trimming the quoted material a little to try to keep this email under control.)
>>
>> On 8/12/2013 4:42 PM, Bjorn Helgaas wrote:
>>> On Mon, Aug 12, 2013 at 1:42 PM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>>>> On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
>>>>> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
>>>>> as you describe.  And even if DMA *could* reach it, the CPU couldn't
>>>>> see it because CPU accesses to that range would go to PCI for the
>>>>> memory-mapped BAR space, not to memory.
>>>> Right.  Unreachability is only a problem if the DMA window overlaps [3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA space can be reached by 64-bit capable devices.
>>> So the [0-1TB] memory range (including [3GB-4GB]) is reachable by
>>> 64-bit DMA to bus addresses [1TB-2TB].  But if the CPU can't see
>>> physical memory from [3GB-4GB], how is it useful to DMA there?
>> Sorry, looking back I can see that the thread is a little confusing.
>> The CPU can see the whole PA space. The confusion comes from the BAR space
>> in [3GB, 4GB].
>>
>> On Tile, we define the CPU memory space as follows:
>>
>> [0, 1TB]: PA
>> [1TB + 3GB, 1TB + 4GB]: BAR space for RC port 0, in [3GB, 4GB]
>> [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: BAR space for RC port N, in [3GB, 4GB]
>>
>> The mapping from [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB] to [3GB, 4GB] is done by a
>> hardware PIO region, which generates PCI bus addresses in [3GB, 4GB] for MMIOs to
>> the BAR space.
> OK, I think I get it now.  CPU address space:
>   [0, 1TB]: physical memory
>   [1TB + 3GB, 1TB + 4GB]: translated to bus address [3GB, 4GB] under RC port 0
>   [1TB + 3GB + N*4GB, 1TB + (1 + N)*4GB]: translated to bus address
> [3GB, 4GB] under RC port N
>
> Bus address space:
>   [0, 3GB]: 32-bit DMA reaches physical memory [0, 3GB]
>   [3GB, 4GB]: 32-bit DMA (peer-to-peer DMA under local RC port, I guess?)

Correct.

>   [1TB, 2TB]: 64-bit DMA mapped via IOMMU to physical memory [0, 1TB]
>
> I guess the problem is that 32-bit DMA can't reach physical memory
> [3GB, 4GB], so you're using bounce buffers so the bus address is in
> [0, 3GB].  That makes sense, and I don't see another possibility other
> than just throwing away the [3GB, 4GB] range by leaving it out of the
> kernel allocator altogether, or using  hardware (which tilegx probably
> doesn't have) to remap it somewhere else.
>
> So it seems like just a question of how you wrap this all up in
> dma_ops, and *that* is all arch stuff that I don't have an opinion on.
>
>>>> Unfortunately, the Megaraid driver doesn’t even call pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).
>>> If the Megaraid driver needs that call, but it's missing, why wouldn't
>>> we just add it?
>> The Megaraid driver doesn’t strictly need that call on other platforms, because
>> by default the device coherent_dma_mask is DMA_BIT_MASK(32) and the consistent
>> memory pool doesn’t come from the bounce buffers on most other platforms.
>>
>> Of course, for the sake of correctness, this call should be added across all platforms.
>> ...
>> What is unique about Tile is that the PCI drivers must explicitly declare
>> its DMA capability by calling pci_set_dma_mask() and pci_set_consistent_dma_mask().
> It looks like the reason you need drivers to explicitly call
> pci_set_dma_mask() and pci_set_consistent_dma_mask() is because you
> have hooks in those functions to tweak the dma_ops, even though the
> mask itself might not be changed.
>
> That doesn't sound like a robust solution: we have well-known defaults
> for the mask sizes, and I don't think it's reasonable to expect
> drivers to explicitly set the mask even if they are happy with the
> defaults (though Documentation/DMA-API-HOWTO.txt does say that being
> explicit is good style).  I'm afraid you'll just keep tripping over
> drivers that don't work on tilegx because they don't set the mask.

Yes, I think you (and James) are right.  We will look at changing to a DMA_MASK(32) default and do some testing and see what that ends up looking like.

James also wrote:
> What you have is
> two different classes of memory: 0-3GB which is usable for I/O and 3-4GB
> which isn't.  Surely what you need to do is implement ZONE_DMA32? which
> stretches from 0-3GB, which means all kmallocs in the driver will be in
> the right range.  Then you need a bounce pfn at 3GB which means that
> user space which gets the 3-4GB is bounced.  There's a magic pfn in
> BLK_BOUNCE_HIGH that was designed for this, but I'm not sure the design
> contemplated BLK_BOUNCE_HIGH being different for 64 and 32 bit.

Well, we do have ZONE_DMA already, for PAs 0-4GB.  This is a sort of generally reasonable definition, and works well for 32-bit USB devices, for example.  But you're right that it does mean that sometimes something like swiotlb_alloc_coherent() might try to allocate memory for 32-bit PCI and get something that falls in the 3-4GB hole, and it then throws it away and uses map_single().  So arguably we should just shift our ZONE_DMA ceiling down to 3GB to make PCI more likely to get a usable page, though that penalizes 32-bit USB, and if we ever thought about expanding the size of the PCI BAR there, it would penalize it more.

Bottom line, though, it does seem like in general the 32-bit devices (both PCI and USB) that we see in practice are pretty low performance, so it's hard to worry too much about it, as long as they work.

I don't know that we've looked at the BLK_BOUNCE_xxx stuff much; I'm personally not very familiar with it, except that as you say, it seems to be hard-wired to -1ULL for 64-bit systems.

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

[PATCH] tile PCI RC: make default consistent DMA mask 32-bit

[Chris Metcalf]

This change sets the PCI devices' initial DMA capabilities
conservatively and promotes them at the request of the driver,
as opposed to assuming advanced DMA capabilities. The old design
runs the risk of breaking drivers that assume default capabilities.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 arch/tile/include/asm/device.h      |  5 ++++-
 arch/tile/include/asm/dma-mapping.h | 21 +++++++++++++--------
 arch/tile/kernel/pci-dma.c          | 21 ++++++++++++---------
 arch/tile/kernel/pci_gx.c           | 15 +++++++++++++--
 4 files changed, 42 insertions(+), 20 deletions(-)

diff --git a/arch/tile/include/asm/device.h b/arch/tile/include/asm/device.h
index 5182705..6ab8bf14 100644
--- a/arch/tile/include/asm/device.h
+++ b/arch/tile/include/asm/device.h
@@ -23,7 +23,10 @@ struct dev_archdata {
 	/* Offset of the DMA address from the PA. */
 	dma_addr_t		dma_offset;
 
-	/* Highest DMA address that can be generated by this device. */
+	/*
+	 * Highest DMA address that can be generated by devices that
+	 * have limited DMA capability, i.e. non 64-bit capable.
+	 */
 	dma_addr_t		max_direct_dma_addr;
 };
 
diff --git a/arch/tile/include/asm/dma-mapping.h b/arch/tile/include/asm/dma-mapping.h
index 6f522d5..1eae359 100644
--- a/arch/tile/include/asm/dma-mapping.h
+++ b/arch/tile/include/asm/dma-mapping.h
@@ -92,14 +92,19 @@ dma_set_mask(struct device *dev, u64 mask)
 {
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);
 
-	/* Handle legacy PCI devices with limited memory addressability. */
-	if ((dma_ops == gx_pci_dma_map_ops ||
-	     dma_ops == gx_hybrid_pci_dma_map_ops ||
-	     dma_ops == gx_legacy_pci_dma_map_ops) &&
-	    (mask <= DMA_BIT_MASK(32))) {
-		set_dma_ops(dev, gx_legacy_pci_dma_map_ops);
-		set_dma_offset(dev, 0);
-		if (mask > dev->archdata.max_direct_dma_addr)
+	/*
+	 * For PCI devices with 64-bit DMA addressing capability, promote
+	 * the dma_ops to hybrid, with the consistent memory DMA space limited
+	 * to 32-bit. For 32-bit capable devices, limit the streaming DMA
+	 * address range to max_direct_dma_addr.
+	 */
+	if (dma_ops == gx_pci_dma_map_ops ||
+	    dma_ops == gx_hybrid_pci_dma_map_ops ||
+	    dma_ops == gx_legacy_pci_dma_map_ops) {
+		if (mask == DMA_BIT_MASK(64) &&
+		    dma_ops == gx_legacy_pci_dma_map_ops)
+			set_dma_ops(dev, gx_hybrid_pci_dma_map_ops);
+		else if (mask > dev->archdata.max_direct_dma_addr)
 			mask = dev->archdata.max_direct_dma_addr;
 	}
 
diff --git a/arch/tile/kernel/pci-dma.c b/arch/tile/kernel/pci-dma.c
index d94f487..09b5870 100644
--- a/arch/tile/kernel/pci-dma.c
+++ b/arch/tile/kernel/pci-dma.c
@@ -588,15 +588,18 @@ int dma_set_coherent_mask(struct device *dev, u64 mask)
 {
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);
 
-	/* Handle hybrid PCI devices with limited memory addressability. */
-	if ((dma_ops == gx_pci_dma_map_ops ||
-	     dma_ops == gx_hybrid_pci_dma_map_ops ||
-	     dma_ops == gx_legacy_pci_dma_map_ops) &&
-	    (mask <= DMA_BIT_MASK(32))) {
-		if (dma_ops == gx_pci_dma_map_ops)
-			set_dma_ops(dev, gx_hybrid_pci_dma_map_ops);
-
-		if (mask > dev->archdata.max_direct_dma_addr)
+	/*
+	 * For PCI devices with 64-bit DMA addressing capability, promote
+	 * the dma_ops to full capability for both streams and consistent
+	 * memory access. For 32-bit capable devices, limit the consistent 
+	 * memory DMA range to max_direct_dma_addr.
+	 */
+	if (dma_ops == gx_pci_dma_map_ops ||
+	    dma_ops == gx_hybrid_pci_dma_map_ops ||
+	    dma_ops == gx_legacy_pci_dma_map_ops) {
+		if (mask == DMA_BIT_MASK(64))
+			set_dma_ops(dev, gx_pci_dma_map_ops);
+		else if (mask > dev->archdata.max_direct_dma_addr)
 			mask = dev->archdata.max_direct_dma_addr;
 	}
 
diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index 66ef9db..29acac6 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -1081,13 +1081,24 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
 	return pci_enable_resources(dev, mask);
 }
 
-/* Called for each device after PCI setup is done. */
+/*
+ * Called for each device after PCI setup is done.
+ * We initialize the PCI device capabilities conservatively, assuming that
+ * all devices can only address the 32-bit DMA space. The exception here is
+ * that the device dma_offset is set to the value that matches the 64-bit
+ * capable devices. This is OK because dma_offset is not used by legacy
+ * dma_ops, nor by the hybrid dma_ops's streaming DMAs, which are 64-bit ops.
+ * This implementation matches the kernel design of setting PCI devices'
+ * coherent_dma_mask to 0xffffffffull by default, allowing the device drivers
+ * to skip calling pci_set_consistent_dma_mask(DMA_BIT_MASK(32)).
+ */
 static void pcibios_fixup_final(struct pci_dev *pdev)
 {
-	set_dma_ops(&pdev->dev, gx_pci_dma_map_ops);
+	set_dma_ops(&pdev->dev, gx_legacy_pci_dma_map_ops);
 	set_dma_offset(&pdev->dev, TILE_PCI_MEM_MAP_BASE_OFFSET);
 	pdev->dev.archdata.max_direct_dma_addr =
 		TILE_PCI_MAX_DIRECT_DMA_ADDRESS;
+	pdev->dev.coherent_dma_mask = TILE_PCI_MAX_DIRECT_DMA_ADDRESS;
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, pcibios_fixup_final);
 
-- 
1.8.3.1

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Chris Metcalf]

(Oops, resending without the helpful [SPAM] marker that our
mail system appears to have injected into the subject line.)

On 8/9/2013 6:42 PM, Bjorn Helgaas wrote:
> On Thu, Aug 08, 2013 at 12:47:10PM -0400, Chris Metcalf wrote:
>> On 8/6/2013 1:48 PM, Bjorn Helgaas wrote:
>>> [+cc Myron, Adam]
>>>
>>> On Fri, Aug 2, 2013 at 10:24 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
>>>> According to LSI,
>>>> the firmware is not fully functional yet. This change implements a
>>>> kind of hybrid dma_ops to support this.
>>>>
>>>> Note that on most other platforms, the 64-bit DMA addressing space is the
>>>> same as the 32-bit DMA space and they overlap the physical memory space.
>>>> No special arrangement is needed to support this kind of mixed DMA
>>>> capability.  On TILE-Gx, the 64-bit DMA space is completely separate
>>>> from the 32-bit DMA space.
>>> Help me understand what's going on here.  My understanding is that on
>>> typical systems, the 32-bit DMA space is a subset of the 64-bit DMA
>>> space.  In conventional PCI, "a master that supports 64-bit addressing
>>> must generate a SAC, instead of a DAC, when the upper 32 bits of the
>>> address are zero" (PCI spec r3.0, sec 3.9).  PCIe doesn't have
>>> SAC/DAC, but it has both 32-bit and 64-bit address headers and has a
>>> similar requirement: "For Addresses below 4GB, Requesters must use the
>>> 32-bit format" (PCIe spec r3.0, sec 2.2.4.1).
>>>
>>> Those imply to me that the 0-4GB region of the 64-bit DMA space must
>>> be identical to the 0-4GB 32-bit DMA space, and in fact, the receiver
>>> of a transaction shouldn't be able to distinguish them.
>>>
>>> But it sounds like something's different on TILE-Gx?  Does it
>>> translate bus addresses to physical memory addresses based on the type
>>> of the transaction (SAC vs DAC, or 32-bit vs 64-bit header) in
>>> addition to the address?  Even if it does, the spec doesn't allow a
>>> DAC cycle or a 64-bit header where the 32 high-order bits are zero, so
>>> it shouldn't matter.
>> No, we don't translate based on the type of the transaction. Using
>> "DMA space" in the commit message was probably misleading. What's
>> really going on is different DMA windows. 32-bit DMA has the
>> obvious naive implementation where [0,4GB] in DMA space maps to
>> [0,4GB] in PA space.  However, for 64-bit DMA, we use DMA
>> addresses with a non-zero high 32 bits, in the [1TB,2TB] range,
>> but map the results down to PA [0,1TB] using our IOMMU.
> I guess this means devices can DMA to physical addresses [0,3GB]
> using either 32-bit bus addresses in the [0,3GB] range or 64-bit bus
> addresses in the [1TB,1TB+3GB] range, right?

True in general, but not true for any specific individual device.

64-bit capable devices won’t generate 32-bit bus addresses, because
the dma_ops makes sure that only bus/DMA addresses in [1TB,1TB+3GB]
are handed out to the devices.

32-bit only devices use bus addresses in [0,3GB] to access the PA [0,3GB].
PA in [3GB, 4GB] is not accessed by the 32-bit only devices because the
bounce buffers are allocated under 3GB limit.

>> We did consider having the 64-bit DMA window be [0,1TB] and map
>> directly to PA space, like the 32-bit window. But this design
>> suffers from the “PCI hole” problem. Basically, the BAR space is
>> usually under 4GB (it occupies the range [3GB, 4GB] on tilegx) and
>> the host bridge uses negative decoding in passing DMA traffic
>> upstream. That is, DMA traffic with target address in [3GB, 4GB]
>> are not passed to the host memory. This means that the same amount
>> of physical memory as the BAR space cannot be used for DMA
>> purpose. And because it is not easy avoid this region in
>> allocating DMA memory, the kernel is simply told to not use this
>> chunk of PA at all, so it is wasted.
> OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
> as you describe.  And even if DMA *could* reach it, the CPU couldn't
> see it because CPU accesses to that range would go to PCI for the
> memory-mapped BAR space, not to memory.

Right.  Unreachability is only a problem if the DMA window overlaps
[3G, 4G], and since the 64-bit DMA window is [1TB,2TB], the whole PA
space can be reached by 64-bit capable devices.

> But I can't figure out why Tile needs to do something special.  I
> think other arches handle the PCI hole for MMIO space the same way.
>
> I don't know if other arches alias the [0,3GB] physical address
> range in both 32-bit and 64-bit DMA space like you do, but if that's
> part of the problem, it seems like you could easily avoid the
> aliasing by making the 64-bit DMA space [1TB+4GB,2TB] instead of
> [1TB,2TB].

Perhaps, but since 64-bit capable devices can't actually see the
aliasing (since they aren't offered the [0,4GB] address range) they
only see an un-aliased space.

>> For the LSI device, the way we manage it is to ensure that the
>> device’s streaming buffers and the consistent buffers come from
>> different pools, with the latter using the under-4GB bounce
>> buffers. Obviously, normal devices use the same buffer pool for
>> both streaming and consistent, either under 4GB or the whole PA.
> It seems like you could make your DMA space be the union of [0,3GB]
> and [1TB+4GB,2TB], then use pci_set_dma_mask(dev, DMA_BIT_MASK(64))
> and pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)) (I assume the
> driver already sets those masks correctly if it works on other
> arches).

Unfortunately, the Megaraid driver doesn’t even call
pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)).

More generally, your proposed DMA space suggestion isn't optimal because
then the PA in [3GB, 4GB] can’t be reached by 64-bit capable devices.

>> Given all of that, does this change make sense? I can certainly
>> amend the commit description to include more commentary.
> Obviously, I'm missing something.  I guess it really doesn't matter
> because this is all arch code and I don't need to understand it, but
> it does niggle at me somehow.

I will add the following comment to <asm/pci.h> in hopes of making it a
bit clearer:

 /*
 [...]
+ * This design lets us avoid the "PCI hole" problem where the host bridge
+ * won't pass DMA traffic with target addresses that happen to fall within the
+ * BAR space. This enables us to use all the physical memory for DMA, instead
+ * of wasting the same amount of physical memory as the BAR window size.
  */
 #define        TILE_PCI_MEM_MAP_BASE_OFFSET    (1ULL << CHIP_PA_WIDTH())

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Bjorn Helgaas]

On Thu, Aug 08, 2013 at 12:47:10PM -0400, Chris Metcalf wrote:
> On 8/6/2013 1:48 PM, Bjorn Helgaas wrote:
> > [+cc Myron, Adam]
> >
> > On Fri, Aug 2, 2013 at 10:24 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> >> According to LSI,
> >> the firmware is not fully functional yet. This change implements a
> >> kind of hybrid dma_ops to support this.
> >>
> >> Note that on most other platforms, the 64-bit DMA addressing space is the
> >> same as the 32-bit DMA space and they overlap the physical memory space.
> >> No special arrangement is needed to support this kind of mixed DMA
> >> capability.  On TILE-Gx, the 64-bit DMA space is completely separate
> >> from the 32-bit DMA space.
> > Help me understand what's going on here.  My understanding is that on
> > typical systems, the 32-bit DMA space is a subset of the 64-bit DMA
> > space.  In conventional PCI, "a master that supports 64-bit addressing
> > must generate a SAC, instead of a DAC, when the upper 32 bits of the
> > address are zero" (PCI spec r3.0, sec 3.9).  PCIe doesn't have
> > SAC/DAC, but it has both 32-bit and 64-bit address headers and has a
> > similar requirement: "For Addresses below 4GB, Requesters must use the
> > 32-bit format" (PCIe spec r3.0, sec 2.2.4.1).
> >
> > Those imply to me that the 0-4GB region of the 64-bit DMA space must
> > be identical to the 0-4GB 32-bit DMA space, and in fact, the receiver
> > of a transaction shouldn't be able to distinguish them.
> >
> > But it sounds like something's different on TILE-Gx?  Does it
> > translate bus addresses to physical memory addresses based on the type
> > of the transaction (SAC vs DAC, or 32-bit vs 64-bit header) in
> > addition to the address?  Even if it does, the spec doesn't allow a
> > DAC cycle or a 64-bit header where the 32 high-order bits are zero, so
> > it shouldn't matter.
> 
> No, we don't translate based on the type of the transaction. Using
> "DMA space" in the commit message was probably misleading. What's
> really going on is different DMA windows. 32-bit DMA has the
> obvious naive implementation where [0,4GB] in DMA space maps to
> [0,4GB] in PA space.  However, for 64-bit DMA, we use DMA
> addresses with a non-zero high 32 bits, in the [1TB,2TB] range,
> but map the results down to PA [0,1TB] using our IOMMU.

I guess this means devices can DMA to physical addresses [0,3GB]
using either 32-bit bus addresses in the [0,3GB] range or 64-bit bus
addresses in the [1TB,1TB+3GB] range, right?

> We did consider having the 64-bit DMA window be [0,1TB] and map
> directly to PA space, like the 32-bit window. But this design
> suffers from the “PCI hole” problem. Basically, the BAR space is
> usually under 4GB (it occupies the range [3GB, 4GB] on tilegx) and
> the host bridge uses negative decoding in passing DMA traffic
> upstream. That is, DMA traffic with target address in [3GB, 4GB]
> are not passed to the host memory. This means that the same amount
> of physical memory as the BAR space cannot be used for DMA
> purpose. And because it is not easy avoid this region in
> allocating DMA memory, the kernel is simply told to not use this
> chunk of PA at all, so it is wasted.

OK, so physical memory in the [3GB,4GB] range is unreachable via DMA
as you describe.  And even if DMA *could* reach it, the CPU couldn't
see it because CPU accesses to that range would go to PCI for the
memory-mapped BAR space, not to memory.

But I can't figure out why Tile needs to do something special.  I
think other arches handle the PCI hole for MMIO space the same way.

I don't know if other arches alias the [0,3GB] physical address
range in both 32-bit and 64-bit DMA space like you do, but if that's
part of the problem, it seems like you could easily avoid the
aliasing by making the 64-bit DMA space [1TB+4GB,2TB] instead of
[1TB,2TB].

> For the LSI device, the way we manage it is to ensure that the
> device’s streaming buffers and the consistent buffers come from
> different pools, with the latter using the under-4GB bounce
> buffers. Obviously, normal devices use the same buffer pool for
> both streaming and consistent, either under 4GB or the whole PA.

It seems like you could make your DMA space be the union of [0,3GB]
and [1TB+4GB,2TB], then use pci_set_dma_mask(dev, DMA_BIT_MASK(64))
and pci_set_consistent_dma_mask(dev, DMA_BIT_MASK(32)) (I assume the
driver already sets those masks correctly if it works on other
arches).

> Given all of that, does this change make sense? I can certainly
> amend the commit description to include more commentary.

Obviously, I'm missing something.  I guess it really doesn't matter
because this is all arch code and I don't need to understand it, but
it does niggle at me somehow.

Bjorn

> >> Due to the use of the IOMMU, the 64-bit DMA
> >> space doesn't overlap the physical memory space.  On the other hand,
> >> the 32-bit DMA space overlaps the physical memory space under 4GB.
> >> The separate address spaces make it necessary to have separate dma_ops.
> >>
> >> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> >> ---
> >>  arch/tile/include/asm/dma-mapping.h | 10 +++++++---
> >>  arch/tile/kernel/pci-dma.c          | 40 ++++++++++++++++++++++++++++---------
> >>  2 files changed, 38 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/tile/include/asm/dma-mapping.h b/arch/tile/include/asm/dma-mapping.h
> >> index f2ff191..4a60059 100644
> >> --- a/arch/tile/include/asm/dma-mapping.h
> >> +++ b/arch/tile/include/asm/dma-mapping.h
> >> @@ -23,6 +23,7 @@
> >>  extern struct dma_map_ops *tile_dma_map_ops;
> >>  extern struct dma_map_ops *gx_pci_dma_map_ops;
> >>  extern struct dma_map_ops *gx_legacy_pci_dma_map_ops;
> >> +extern struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
> >>
> >>  static inline struct dma_map_ops *get_dma_ops(struct device *dev)
> >>  {
> >> @@ -44,12 +45,12 @@ static inline void set_dma_offset(struct device *dev, dma_addr_t off)
> >>
> >>  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> >>  {
> >> -       return paddr + get_dma_offset(dev);
> >> +       return paddr;
> >>  }
> >>
> >>  static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
> >>  {
> >> -       return daddr - get_dma_offset(dev);
> >> +       return daddr;
> >>  }
> >>
> >>  static inline void dma_mark_clean(void *addr, size_t size) {}
> >> @@ -88,7 +89,10 @@ dma_set_mask(struct device *dev, u64 mask)
> >>         struct dma_map_ops *dma_ops = get_dma_ops(dev);
> >>
> >>         /* Handle legacy PCI devices with limited memory addressability. */
> >> -       if ((dma_ops == gx_pci_dma_map_ops) && (mask <= DMA_BIT_MASK(32))) {
> >> +       if ((dma_ops == gx_pci_dma_map_ops ||
> >> +            dma_ops == gx_hybrid_pci_dma_map_ops ||
> >> +            dma_ops == gx_legacy_pci_dma_map_ops) &&
> >> +           (mask <= DMA_BIT_MASK(32))) {
> >>                 set_dma_ops(dev, gx_legacy_pci_dma_map_ops);
> >>                 set_dma_offset(dev, 0);
> >>                 if (mask > dev->archdata.max_direct_dma_addr)
> >> diff --git a/arch/tile/kernel/pci-dma.c b/arch/tile/kernel/pci-dma.c
> >> index b9fe80e..7e22e73 100644
> >> --- a/arch/tile/kernel/pci-dma.c
> >> +++ b/arch/tile/kernel/pci-dma.c
> >> @@ -357,7 +357,7 @@ static void *tile_pci_dma_alloc_coherent(struct device *dev, size_t size,
> >>
> >>         addr = page_to_phys(pg);
> >>
> >> -       *dma_handle = phys_to_dma(dev, addr);
> >> +       *dma_handle = addr + get_dma_offset(dev);
> >>
> >>         return page_address(pg);
> >>  }
> >> @@ -387,7 +387,7 @@ static int tile_pci_dma_map_sg(struct device *dev, struct scatterlist *sglist,
> >>                 sg->dma_address = sg_phys(sg);
> >>                 __dma_prep_pa_range(sg->dma_address, sg->length, direction);
> >>
> >> -               sg->dma_address = phys_to_dma(dev, sg->dma_address);
> >> +               sg->dma_address = sg->dma_address + get_dma_offset(dev);
> >>  #ifdef CONFIG_NEED_SG_DMA_LENGTH
> >>                 sg->dma_length = sg->length;
> >>  #endif
> >> @@ -422,7 +422,7 @@ static dma_addr_t tile_pci_dma_map_page(struct device *dev, struct page *page,
> >>         BUG_ON(offset + size > PAGE_SIZE);
> >>         __dma_prep_page(page, offset, size, direction);
> >>
> >> -       return phys_to_dma(dev, page_to_pa(page) + offset);
> >> +       return page_to_pa(page) + offset + get_dma_offset(dev);
> >>  }
> >>
> >>  static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
> >> @@ -432,7 +432,7 @@ static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
> >>  {
> >>         BUG_ON(!valid_dma_direction(direction));
> >>
> >> -       dma_address = dma_to_phys(dev, dma_address);
> >> +       dma_address -= get_dma_offset(dev);
> >>
> >>         __dma_complete_page(pfn_to_page(PFN_DOWN(dma_address)),
> >>                             dma_address & PAGE_OFFSET, size, direction);
> >> @@ -445,7 +445,7 @@ static void tile_pci_dma_sync_single_for_cpu(struct device *dev,
> >>  {
> >>         BUG_ON(!valid_dma_direction(direction));
> >>
> >> -       dma_handle = dma_to_phys(dev, dma_handle);
> >> +       dma_handle -= get_dma_offset(dev);
> >>
> >>         __dma_complete_pa_range(dma_handle, size, direction);
> >>  }
> >> @@ -456,7 +456,7 @@ static void tile_pci_dma_sync_single_for_device(struct device *dev,
> >>                                                 enum dma_data_direction
> >>                                                 direction)
> >>  {
> >> -       dma_handle = dma_to_phys(dev, dma_handle);
> >> +       dma_handle -= get_dma_offset(dev);
> >>
> >>         __dma_prep_pa_range(dma_handle, size, direction);
> >>  }
> >> @@ -558,21 +558,43 @@ static struct dma_map_ops pci_swiotlb_dma_ops = {
> >>         .mapping_error = swiotlb_dma_mapping_error,
> >>  };
> >>
> >> +static struct dma_map_ops pci_hybrid_dma_ops = {
> >> +       .alloc = tile_swiotlb_alloc_coherent,
> >> +       .free = tile_swiotlb_free_coherent,
> >> +       .map_page = tile_pci_dma_map_page,
> >> +       .unmap_page = tile_pci_dma_unmap_page,
> >> +       .map_sg = tile_pci_dma_map_sg,
> >> +       .unmap_sg = tile_pci_dma_unmap_sg,
> >> +       .sync_single_for_cpu = tile_pci_dma_sync_single_for_cpu,
> >> +       .sync_single_for_device = tile_pci_dma_sync_single_for_device,
> >> +       .sync_sg_for_cpu = tile_pci_dma_sync_sg_for_cpu,
> >> +       .sync_sg_for_device = tile_pci_dma_sync_sg_for_device,
> >> +       .mapping_error = tile_pci_dma_mapping_error,
> >> +       .dma_supported = tile_pci_dma_supported
> >> +};
> >> +
> >>  struct dma_map_ops *gx_legacy_pci_dma_map_ops = &pci_swiotlb_dma_ops;
> >> +struct dma_map_ops *gx_hybrid_pci_dma_map_ops = &pci_hybrid_dma_ops;
> >>  #else
> >>  struct dma_map_ops *gx_legacy_pci_dma_map_ops;
> >> +struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
> >>  #endif
> >>  EXPORT_SYMBOL(gx_legacy_pci_dma_map_ops);
> >> +EXPORT_SYMBOL(gx_hybrid_pci_dma_map_ops);
> >>
> >>  #ifdef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK
> >>  int dma_set_coherent_mask(struct device *dev, u64 mask)
> >>  {
> >>         struct dma_map_ops *dma_ops = get_dma_ops(dev);
> >>
> >> -       /* Handle legacy PCI devices with limited memory addressability. */
> >> -       if (((dma_ops == gx_pci_dma_map_ops) ||
> >> -           (dma_ops == gx_legacy_pci_dma_map_ops)) &&
> >> +       /* Handle hybrid PCI devices with limited memory addressability. */
> >> +       if ((dma_ops == gx_pci_dma_map_ops ||
> >> +            dma_ops == gx_hybrid_pci_dma_map_ops ||
> >> +            dma_ops == gx_legacy_pci_dma_map_ops) &&
> >>             (mask <= DMA_BIT_MASK(32))) {
> >> +               if (dma_ops == gx_pci_dma_map_ops)
> >> +                       set_dma_ops(dev, gx_hybrid_pci_dma_map_ops);
> >> +
> >>                 if (mask > dev->archdata.max_direct_dma_addr)
> >>                         mask = dev->archdata.max_direct_dma_addr;
> >>         }
> >> --
> >> 1.8.3.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> Chris Metcalf, Tilera Corp.
> http://www.tilera.com
> 

Re: [PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Bjorn Helgaas]

[+cc Myron, Adam]

On Fri, Aug 2, 2013 at 10:24 AM, Chris Metcalf <cmetcalf@tilera.com> wrote:
> The LSI MEGARAID SAS HBA suffers from the problem where it can do
> 64-bit DMA to streaming buffers but not to consistent buffers.
> In other words, 64-bit DMA is used for disk data transfers and 32-bit
> DMA must be used for control message transfers.

Is this related at all to the make_local_pdev() hacks in megaraid.c?
The intent there seems to be to use a 32-bit DMA mask for certain
transactions and a 64-bit mask for others.  But I think it's actually
broken, because the implementation changes the mask to 32-bit for
*all* future transactions, not just the one using make_local_pdev().

> According to LSI,
> the firmware is not fully functional yet. This change implements a
> kind of hybrid dma_ops to support this.
>
> Note that on most other platforms, the 64-bit DMA addressing space is the
> same as the 32-bit DMA space and they overlap the physical memory space.
> No special arrangement is needed to support this kind of mixed DMA
> capability.  On TILE-Gx, the 64-bit DMA space is completely separate
> from the 32-bit DMA space.

Help me understand what's going on here.  My understanding is that on
typical systems, the 32-bit DMA space is a subset of the 64-bit DMA
space.  In conventional PCI, "a master that supports 64-bit addressing
must generate a SAC, instead of a DAC, when the upper 32 bits of the
address are zero" (PCI spec r3.0, sec 3.9).  PCIe doesn't have
SAC/DAC, but it has both 32-bit and 64-bit address headers and has a
similar requirement: "For Addresses below 4GB, Requesters must use the
32-bit format" (PCIe spec r3.0, sec 2.2.4.1).

Those imply to me that the 0-4GB region of the 64-bit DMA space must
be identical to the 0-4GB 32-bit DMA space, and in fact, the receiver
of a transaction shouldn't be able to distinguish them.

But it sounds like something's different on TILE-Gx?  Does it
translate bus addresses to physical memory addresses based on the type
of the transaction (SAC vs DAC, or 32-bit vs 64-bit header) in
addition to the address?  Even if it does, the spec doesn't allow a
DAC cycle or a 64-bit header where the 32 high-order bits are zero, so
it shouldn't matter.

Bjorn

>  Due to the use of the IOMMU, the 64-bit DMA
> space doesn't overlap the physical memory space.  On the other hand,
> the 32-bit DMA space overlaps the physical memory space under 4GB.
> The separate address spaces make it necessary to have separate dma_ops.
>
> Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
> ---
>  arch/tile/include/asm/dma-mapping.h | 10 +++++++---
>  arch/tile/kernel/pci-dma.c          | 40 ++++++++++++++++++++++++++++---------
>  2 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/arch/tile/include/asm/dma-mapping.h b/arch/tile/include/asm/dma-mapping.h
> index f2ff191..4a60059 100644
> --- a/arch/tile/include/asm/dma-mapping.h
> +++ b/arch/tile/include/asm/dma-mapping.h
> @@ -23,6 +23,7 @@
>  extern struct dma_map_ops *tile_dma_map_ops;
>  extern struct dma_map_ops *gx_pci_dma_map_ops;
>  extern struct dma_map_ops *gx_legacy_pci_dma_map_ops;
> +extern struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
>
>  static inline struct dma_map_ops *get_dma_ops(struct device *dev)
>  {
> @@ -44,12 +45,12 @@ static inline void set_dma_offset(struct device *dev, dma_addr_t off)
>
>  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
>  {
> -       return paddr + get_dma_offset(dev);
> +       return paddr;
>  }
>
>  static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
>  {
> -       return daddr - get_dma_offset(dev);
> +       return daddr;
>  }
>
>  static inline void dma_mark_clean(void *addr, size_t size) {}
> @@ -88,7 +89,10 @@ dma_set_mask(struct device *dev, u64 mask)
>         struct dma_map_ops *dma_ops = get_dma_ops(dev);
>
>         /* Handle legacy PCI devices with limited memory addressability. */
> -       if ((dma_ops == gx_pci_dma_map_ops) && (mask <= DMA_BIT_MASK(32))) {
> +       if ((dma_ops == gx_pci_dma_map_ops ||
> +            dma_ops == gx_hybrid_pci_dma_map_ops ||
> +            dma_ops == gx_legacy_pci_dma_map_ops) &&
> +           (mask <= DMA_BIT_MASK(32))) {
>                 set_dma_ops(dev, gx_legacy_pci_dma_map_ops);
>                 set_dma_offset(dev, 0);
>                 if (mask > dev->archdata.max_direct_dma_addr)
> diff --git a/arch/tile/kernel/pci-dma.c b/arch/tile/kernel/pci-dma.c
> index b9fe80e..7e22e73 100644
> --- a/arch/tile/kernel/pci-dma.c
> +++ b/arch/tile/kernel/pci-dma.c
> @@ -357,7 +357,7 @@ static void *tile_pci_dma_alloc_coherent(struct device *dev, size_t size,
>
>         addr = page_to_phys(pg);
>
> -       *dma_handle = phys_to_dma(dev, addr);
> +       *dma_handle = addr + get_dma_offset(dev);
>
>         return page_address(pg);
>  }
> @@ -387,7 +387,7 @@ static int tile_pci_dma_map_sg(struct device *dev, struct scatterlist *sglist,
>                 sg->dma_address = sg_phys(sg);
>                 __dma_prep_pa_range(sg->dma_address, sg->length, direction);
>
> -               sg->dma_address = phys_to_dma(dev, sg->dma_address);
> +               sg->dma_address = sg->dma_address + get_dma_offset(dev);
>  #ifdef CONFIG_NEED_SG_DMA_LENGTH
>                 sg->dma_length = sg->length;
>  #endif
> @@ -422,7 +422,7 @@ static dma_addr_t tile_pci_dma_map_page(struct device *dev, struct page *page,
>         BUG_ON(offset + size > PAGE_SIZE);
>         __dma_prep_page(page, offset, size, direction);
>
> -       return phys_to_dma(dev, page_to_pa(page) + offset);
> +       return page_to_pa(page) + offset + get_dma_offset(dev);
>  }
>
>  static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
> @@ -432,7 +432,7 @@ static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
>  {
>         BUG_ON(!valid_dma_direction(direction));
>
> -       dma_address = dma_to_phys(dev, dma_address);
> +       dma_address -= get_dma_offset(dev);
>
>         __dma_complete_page(pfn_to_page(PFN_DOWN(dma_address)),
>                             dma_address & PAGE_OFFSET, size, direction);
> @@ -445,7 +445,7 @@ static void tile_pci_dma_sync_single_for_cpu(struct device *dev,
>  {
>         BUG_ON(!valid_dma_direction(direction));
>
> -       dma_handle = dma_to_phys(dev, dma_handle);
> +       dma_handle -= get_dma_offset(dev);
>
>         __dma_complete_pa_range(dma_handle, size, direction);
>  }
> @@ -456,7 +456,7 @@ static void tile_pci_dma_sync_single_for_device(struct device *dev,
>                                                 enum dma_data_direction
>                                                 direction)
>  {
> -       dma_handle = dma_to_phys(dev, dma_handle);
> +       dma_handle -= get_dma_offset(dev);
>
>         __dma_prep_pa_range(dma_handle, size, direction);
>  }
> @@ -558,21 +558,43 @@ static struct dma_map_ops pci_swiotlb_dma_ops = {
>         .mapping_error = swiotlb_dma_mapping_error,
>  };
>
> +static struct dma_map_ops pci_hybrid_dma_ops = {
> +       .alloc = tile_swiotlb_alloc_coherent,
> +       .free = tile_swiotlb_free_coherent,
> +       .map_page = tile_pci_dma_map_page,
> +       .unmap_page = tile_pci_dma_unmap_page,
> +       .map_sg = tile_pci_dma_map_sg,
> +       .unmap_sg = tile_pci_dma_unmap_sg,
> +       .sync_single_for_cpu = tile_pci_dma_sync_single_for_cpu,
> +       .sync_single_for_device = tile_pci_dma_sync_single_for_device,
> +       .sync_sg_for_cpu = tile_pci_dma_sync_sg_for_cpu,
> +       .sync_sg_for_device = tile_pci_dma_sync_sg_for_device,
> +       .mapping_error = tile_pci_dma_mapping_error,
> +       .dma_supported = tile_pci_dma_supported
> +};
> +
>  struct dma_map_ops *gx_legacy_pci_dma_map_ops = &pci_swiotlb_dma_ops;
> +struct dma_map_ops *gx_hybrid_pci_dma_map_ops = &pci_hybrid_dma_ops;
>  #else
>  struct dma_map_ops *gx_legacy_pci_dma_map_ops;
> +struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
>  #endif
>  EXPORT_SYMBOL(gx_legacy_pci_dma_map_ops);
> +EXPORT_SYMBOL(gx_hybrid_pci_dma_map_ops);
>
>  #ifdef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK
>  int dma_set_coherent_mask(struct device *dev, u64 mask)
>  {
>         struct dma_map_ops *dma_ops = get_dma_ops(dev);
>
> -       /* Handle legacy PCI devices with limited memory addressability. */
> -       if (((dma_ops == gx_pci_dma_map_ops) ||
> -           (dma_ops == gx_legacy_pci_dma_map_ops)) &&
> +       /* Handle hybrid PCI devices with limited memory addressability. */
> +       if ((dma_ops == gx_pci_dma_map_ops ||
> +            dma_ops == gx_hybrid_pci_dma_map_ops ||
> +            dma_ops == gx_legacy_pci_dma_map_ops) &&
>             (mask <= DMA_BIT_MASK(32))) {
> +               if (dma_ops == gx_pci_dma_map_ops)
> +                       set_dma_ops(dev, gx_hybrid_pci_dma_map_ops);
> +
>                 if (mask > dev->archdata.max_direct_dma_addr)
>                         mask = dev->archdata.max_direct_dma_addr;
>         }
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] tile: support LSI MEGARAID SAS HBA hybrid dma_ops

[Chris Metcalf]

The LSI MEGARAID SAS HBA suffers from the problem where it can do
64-bit DMA to streaming buffers but not to consistent buffers.
In other words, 64-bit DMA is used for disk data transfers and 32-bit
DMA must be used for control message transfers. According to LSI,
the firmware is not fully functional yet. This change implements a
kind of hybrid dma_ops to support this.

Note that on most other platforms, the 64-bit DMA addressing space is the
same as the 32-bit DMA space and they overlap the physical memory space.
No special arrangement is needed to support this kind of mixed DMA
capability.  On TILE-Gx, the 64-bit DMA space is completely separate
from the 32-bit DMA space.  Due to the use of the IOMMU, the 64-bit DMA
space doesn't overlap the physical memory space.  On the other hand,
the 32-bit DMA space overlaps the physical memory space under 4GB.
The separate address spaces make it necessary to have separate dma_ops.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
 arch/tile/include/asm/dma-mapping.h | 10 +++++++---
 arch/tile/kernel/pci-dma.c          | 40 ++++++++++++++++++++++++++++---------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/arch/tile/include/asm/dma-mapping.h b/arch/tile/include/asm/dma-mapping.h
index f2ff191..4a60059 100644
--- a/arch/tile/include/asm/dma-mapping.h
+++ b/arch/tile/include/asm/dma-mapping.h
@@ -23,6 +23,7 @@
 extern struct dma_map_ops *tile_dma_map_ops;
 extern struct dma_map_ops *gx_pci_dma_map_ops;
 extern struct dma_map_ops *gx_legacy_pci_dma_map_ops;
+extern struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
 
 static inline struct dma_map_ops *get_dma_ops(struct device *dev)
 {
@@ -44,12 +45,12 @@ static inline void set_dma_offset(struct device *dev, dma_addr_t off)
 
 static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
-	return paddr + get_dma_offset(dev);
+	return paddr;
 }
 
 static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t daddr)
 {
-	return daddr - get_dma_offset(dev);
+	return daddr;
 }
 
 static inline void dma_mark_clean(void *addr, size_t size) {}
@@ -88,7 +89,10 @@ dma_set_mask(struct device *dev, u64 mask)
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);
 
 	/* Handle legacy PCI devices with limited memory addressability. */
-	if ((dma_ops == gx_pci_dma_map_ops) && (mask <= DMA_BIT_MASK(32))) {
+	if ((dma_ops == gx_pci_dma_map_ops ||
+	     dma_ops == gx_hybrid_pci_dma_map_ops ||
+	     dma_ops == gx_legacy_pci_dma_map_ops) &&
+	    (mask <= DMA_BIT_MASK(32))) {
 		set_dma_ops(dev, gx_legacy_pci_dma_map_ops);
 		set_dma_offset(dev, 0);
 		if (mask > dev->archdata.max_direct_dma_addr)
diff --git a/arch/tile/kernel/pci-dma.c b/arch/tile/kernel/pci-dma.c
index b9fe80e..7e22e73 100644
--- a/arch/tile/kernel/pci-dma.c
+++ b/arch/tile/kernel/pci-dma.c
@@ -357,7 +357,7 @@ static void *tile_pci_dma_alloc_coherent(struct device *dev, size_t size,
 
 	addr = page_to_phys(pg);
 
-	*dma_handle = phys_to_dma(dev, addr);
+	*dma_handle = addr + get_dma_offset(dev);
 
 	return page_address(pg);
 }
@@ -387,7 +387,7 @@ static int tile_pci_dma_map_sg(struct device *dev, struct scatterlist *sglist,
 		sg->dma_address = sg_phys(sg);
 		__dma_prep_pa_range(sg->dma_address, sg->length, direction);
 
-		sg->dma_address = phys_to_dma(dev, sg->dma_address);
+		sg->dma_address = sg->dma_address + get_dma_offset(dev);
 #ifdef CONFIG_NEED_SG_DMA_LENGTH
 		sg->dma_length = sg->length;
 #endif
@@ -422,7 +422,7 @@ static dma_addr_t tile_pci_dma_map_page(struct device *dev, struct page *page,
 	BUG_ON(offset + size > PAGE_SIZE);
 	__dma_prep_page(page, offset, size, direction);
 
-	return phys_to_dma(dev, page_to_pa(page) + offset);
+	return page_to_pa(page) + offset + get_dma_offset(dev);
 }
 
 static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
@@ -432,7 +432,7 @@ static void tile_pci_dma_unmap_page(struct device *dev, dma_addr_t dma_address,
 {
 	BUG_ON(!valid_dma_direction(direction));
 
-	dma_address = dma_to_phys(dev, dma_address);
+	dma_address -= get_dma_offset(dev);
 
 	__dma_complete_page(pfn_to_page(PFN_DOWN(dma_address)),
 			    dma_address & PAGE_OFFSET, size, direction);
@@ -445,7 +445,7 @@ static void tile_pci_dma_sync_single_for_cpu(struct device *dev,
 {
 	BUG_ON(!valid_dma_direction(direction));
 
-	dma_handle = dma_to_phys(dev, dma_handle);
+	dma_handle -= get_dma_offset(dev);
 
 	__dma_complete_pa_range(dma_handle, size, direction);
 }
@@ -456,7 +456,7 @@ static void tile_pci_dma_sync_single_for_device(struct device *dev,
 						enum dma_data_direction
 						direction)
 {
-	dma_handle = dma_to_phys(dev, dma_handle);
+	dma_handle -= get_dma_offset(dev);
 
 	__dma_prep_pa_range(dma_handle, size, direction);
 }
@@ -558,21 +558,43 @@ static struct dma_map_ops pci_swiotlb_dma_ops = {
 	.mapping_error = swiotlb_dma_mapping_error,
 };
 
+static struct dma_map_ops pci_hybrid_dma_ops = {
+	.alloc = tile_swiotlb_alloc_coherent,
+	.free = tile_swiotlb_free_coherent,
+	.map_page = tile_pci_dma_map_page,
+	.unmap_page = tile_pci_dma_unmap_page,
+	.map_sg = tile_pci_dma_map_sg,
+	.unmap_sg = tile_pci_dma_unmap_sg,
+	.sync_single_for_cpu = tile_pci_dma_sync_single_for_cpu,
+	.sync_single_for_device = tile_pci_dma_sync_single_for_device,
+	.sync_sg_for_cpu = tile_pci_dma_sync_sg_for_cpu,
+	.sync_sg_for_device = tile_pci_dma_sync_sg_for_device,
+	.mapping_error = tile_pci_dma_mapping_error,
+	.dma_supported = tile_pci_dma_supported
+};
+
 struct dma_map_ops *gx_legacy_pci_dma_map_ops = &pci_swiotlb_dma_ops;
+struct dma_map_ops *gx_hybrid_pci_dma_map_ops = &pci_hybrid_dma_ops;
 #else
 struct dma_map_ops *gx_legacy_pci_dma_map_ops;
+struct dma_map_ops *gx_hybrid_pci_dma_map_ops;
 #endif
 EXPORT_SYMBOL(gx_legacy_pci_dma_map_ops);
+EXPORT_SYMBOL(gx_hybrid_pci_dma_map_ops);
 
 #ifdef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK
 int dma_set_coherent_mask(struct device *dev, u64 mask)
 {
 	struct dma_map_ops *dma_ops = get_dma_ops(dev);
 
-	/* Handle legacy PCI devices with limited memory addressability. */
-	if (((dma_ops == gx_pci_dma_map_ops) ||
-	    (dma_ops == gx_legacy_pci_dma_map_ops)) &&
+	/* Handle hybrid PCI devices with limited memory addressability. */
+	if ((dma_ops == gx_pci_dma_map_ops ||
+	     dma_ops == gx_hybrid_pci_dma_map_ops ||
+	     dma_ops == gx_legacy_pci_dma_map_ops) &&
 	    (mask <= DMA_BIT_MASK(32))) {
+		if (dma_ops == gx_pci_dma_map_ops)
+			set_dma_ops(dev, gx_hybrid_pci_dma_map_ops);
+
 		if (mask > dev->archdata.max_direct_dma_addr)
 			mask = dev->archdata.max_direct_dma_addr;
 	}
-- 
1.8.3.1