Jesse Barnes wrote:
On Wednesday, August 6, 2008 7:32 am Prarit Bhargava wrote:
You can't kmalloc pci_dev or setup some trivial values. You need to
use a proper value. The pci code does for us.
Oops -- I meant struct device, not struct pci_dev.
Anwyay, Jesse -- is this true? I can no longer do something like:
static struct device junk_dev = {
.bus_id = "junk device",
.coherent_dma_mask = 0xffffffff,
.dma_mask = &junk_dev.coherent_dma_mask,
};
And then use that as the device target for dma_alloc_coherent? AFAIK,
that has always worked for me.
It gets dangerous since platforms are in control of some pci_dev and dev
fields, and if they don't get initialized you can get into trouble.
True, but dma_alloc_coherent also allows for a NULL dev pointer, and
uses a dummy struct dev (fallback_device). So it should be callable
without any dev struct pointer.
In that case, I hit the BUG() check warning in iommu_is_span_boundary()
because boundary_size was calculated as (unsigned long) 0xffffffff + 1 = 0.
That's why we must cast to "unsigned long long".
ie) it is possible to hit this BUG() right now.
Calgary IOMMU has the same code. New AMD IOMMU has the same code too.
Then they don't handle the above problem and are broken when
dma_get_seg_boundary() returns 0xffffffff and will require patches.
/me hasn't tried out Calgary of AMD IOMMU.
Would be good to find someone to do some testing on one of those platforms...
I've pinged someone at AMD to see if I can get my hands on a system (or
if to see if there is a system available locally).
As for Calgary, I'm looking into it ATM. I think I can get my hands on one.
If I find the problem on those platforms I'll ping the maintainers and
post separate patches. ATM I'm much more concerned about GART.
Maybe I'm missing something -- what implies size has to be a power of
two?
Yes, see iommu_area_alloc().
/me looks and still doesn't see where the size passed into
gart_map_simple() must be a power of two. ... and if that was the case,
shouldn't we be failing all the time? I mean, I've seen values passed
into pci_alloc_consistent like 0x3820 -- clearly not a multiple of 2.
iommu_area_alloc() deals with pages, not actual sizes as
gart_map_simple() does.
Tomonori-san, I think I understand where your confusion may lie. The
size argument in the iommu-helper.c code is NOT the same size in
dma_alloc_coherent() and gart_map_simple(). In iommu-helper.c the size
is the # of pages, and the in the exported function calls it is an
actual size. Is that what is confusing you?
If anything, I would make this simple fix:
dma_addr_t map = dma_map_area(dev, paddr, size, dir, size - 1);
should be
dma_addr_t map = dma_map_area(dev, paddr, size, dir, size);
because after my patch we round up the mask argument to get the correct
alignment to # of pages anyway.
Feel like respinning with a full changelog against my for-linus branch? Maybe
you can convince Tomonori-san this time. :)
I no longer think the above suggested change is necessary. AFAICT, the
code is doing exactly the right thing. "size-1" is correct.
P.
Jesse
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
