2003-08-11 23:08:32

by Krzysztof Halasa

[permalink] [raw]
Subject: consistent_dma_mask is a ghost?

[Repost, not sure why it haven't reach the list]

Hi,

I've run grep over the linux-2.6.0-test3 tree and it seems the whole
"consistent_dma_mask" thing does not really exist.

The following files reference it, either as a variable struct pci_dev*
dev->consistent_dma_mask or function set_consistent_dma_mask():

drivers/net/tg3.c: sets both consistent_dma_mask and dma_mask to 2^64-1,
and if that fails to 2^32-1.

drivers/atm/lanai.c: sets both to 2^32-1 = the default

drivers/pci/pci.c: the function pci_set_consistent_dma_mask() itself

drivers/pci/probe.c: sets default 2^32-1 for a device;

arch/ia64/sn/io/machvec/pci_dma.c: sn_pci_alloc_consistent() actually uses
consistent_dma_mask

arch/x86_64/kernel/pci-gart.c: pci_alloc_consistent() actually uses
consistent_dma_mask


This means that only _two_ platforms, ia64 and x86_64, have means to use
that information, and other platforms use set_dma_mask() and dev->dma_mask
for consistent (coherent) allocations ignoring consistent_dma_mask at all
(and possibly allocating memory from invalid region, if the masks are
not equal).

No wonder even on those two platforms no code uses consistent_dma_mask
to do some real work - i.e. both tg3 and lanai drivers use the same value
for consistent and streaming mapping. The "DMA" API doesn't have anything
like this either.

Is the whole thing a work in progress, only partially merged, and will we
see this working as documented, or should we just remove all traces of
useless consistent_dma_mask and use a single dma_mask for both kinds of
mapping? Should I prepare the patch?



Another problem, common to DMA API and PCI API:

Unless I'm mistaken, both dma_map_* and pci_map_* claim to use dma_mask
to return dma_addr_t bus address for a device. At least on i386, there
is no such thing at all - the returned address is just a result of
virt_to_phys(), and is not limited by the mask. I understand doing that
in accordance to the docs would sometimes mean memcpy() (with devices
using smaller than 2^32-1 dma_mask). Should the code be corrected/added
or are the docs to be changed to reflect reality?
--
Krzysztof Halasa
Network Administrator


2003-08-13 00:11:26

by Pete Zaitcev

[permalink] [raw]
Subject: Re: consistent_dma_mask is a ghost?

> This means that only _two_ platforms, ia64 and x86_64, have means to use
> that information, and other platforms use set_dma_mask() and dev->dma_mask
> for consistent (coherent) allocations ignoring consistent_dma_mask at all
> (and possibly allocating memory from invalid region, if the masks are
> not equal).

Platforms which worked correctly before continue to work
correctly thereafter. IMHO, the whole thing is a kludge,
designed to support AIC7xxx on SGI SN-2, and that's about
all it does. There's a device which uses fewer DMA bits
when it accesses its mailbox than when it accesses data.
Since the mailbox is allocated in consistent memory, this
can be used as a clue to restrict the allocation. This is a
fragile, opaque construct and it's conceptually wrong (what if
the driver accessed device mailboxes through streaming mappings?),
but it works for its purpose. Just don't use it in your drivers
and you'll be fine.

-- Pete

2003-08-13 11:24:26

by Alan

[permalink] [raw]
Subject: Re: consistent_dma_mask is a ghost?

On Mer, 2003-08-13 at 01:11, Pete Zaitcev wrote:
> Platforms which worked correctly before continue to work
> correctly thereafter. IMHO, the whole thing is a kludge,
> designed to support AIC7xxx on SGI SN-2, and that's about
> all it does. There's a device which uses fewer DMA bits
> when it accesses its mailbox than when it accesses data.

Same is true for megaraid, aacraid and several other cards, but they
just keep changing the pci mask at runtime. Thats in some ways even
uglier but works for now

2003-08-13 18:36:18

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: consistent_dma_mask is a ghost?

Pete Zaitcev <[email protected]> writes:

> Platforms which worked correctly before continue to work
> correctly thereafter. IMHO, the whole thing is a kludge,
> designed to support AIC7xxx on SGI SN-2, and that's about
> all it does.

I'm a little confused. I assume the driver is not in the official tree
and this SN-2 uses either Itanium or Opteron CPU.

> There's a device which uses fewer DMA bits
> when it accesses its mailbox than when it accesses data.
> Since the mailbox is allocated in consistent memory, this
> can be used as a clue to restrict the allocation. This is a
> fragile, opaque construct and it's conceptually wrong (what if
> the driver accessed device mailboxes through streaming mappings?),
> but it works for its purpose. Just don't use it in your drivers
> and you'll be fine.

Right, but at least the documentation needs a fix. I also have a device
which needs different masks for streaming and consistent allocations
(4 GB for DMA streaming but only 256 MB for memory shared directly between
main CPU and on-board one) and I was hoping that setting the consistent
dma mask to 256MB (-1) would actually have some meaning (while it's NOP
on most platforms).

The question is: would it be better to only fix the docs to reflect
reality, or let the kludge die and fix AIC7xxx driver instead?
I understand it could be trivially done in the driver, i.e. by setting
dma_mask to correct value just before the consistent allocation is
requested. It should have little impact (if any) on performance, even
if the driver needs it for normal operation rather than for init only.


Or, should we do it the other way around? Instead of fixing docs,
we can fix the code to actually support setting the consistent dma mask
reliably (at least the easy part).
--
Krzysztof Halasa
Network Administrator