(This didn't appear on LKML or any of the mirrors ... trying again)
It is possible that alloc_iommu()'s boundary_size overflows as
dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
boundary_size triggers a BUG_ON() in the iommu code.
Signed-off-by: Prarit Bhargava <[email protected]>
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index faf3229..baecb47 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -91,7 +91,7 @@ static unsigned long alloc_iommu(struct device *dev, int size)
base_index = ALIGN(iommu_bus_base & dma_get_seg_boundary(dev),
PAGE_SIZE) >> PAGE_SHIFT;
- boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
+ boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,
PAGE_SIZE) >> PAGE_SHIFT;
spin_lock_irqsave(&iommu_bitmap_lock, flags);
pci_alloc_consistent/dma_alloc_coherent is supposed to return size aligned
addresses.
>From Documentation/DMA-mapping.txt:
"pci_alloc_consistent returns two values: the virtual address which you
can use to access it from the CPU and dma_handle which you pass to the
card.
The cpu return address and the DMA bus master address are both
guaranteed to be aligned to the smallest PAGE_SIZE order which
is greater than or equal to the requested size. This invariant
exists (for example) to guarantee that if you allocate a chunk
which is smaller than or equal to 64 kilobytes, the extent of the
buffer you receive will not cross a 64K boundary."
Fix the GART's alloc_iommu code to return a size aligned address. Also fix
an incorrect alignment calculation in the iommu-helper code.
Signed-off-by: Prarit Bhargava <[email protected]>
diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
index faf3229..329718e 100644
--- a/arch/x86/kernel/pci-gart_64.c
+++ b/arch/x86/kernel/pci-gart_64.c
@@ -96,11 +96,12 @@ static unsigned long alloc_iommu(struct device *dev, int size)
spin_lock_irqsave(&iommu_bitmap_lock, flags);
offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
- size, base_index, boundary_size, 0);
+ size, base_index, boundary_size, size - 1);
if (offset == -1) {
need_flush = 1;
offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, 0,
- size, base_index, boundary_size, 0);
+ size, base_index, boundary_size,
+ size - 1);
}
if (offset != -1) {
set_bit_string(iommu_gart_bitmap, offset, size);
diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
index a3b8d4c..b970f1b 100644
--- a/lib/iommu-helper.c
+++ b/lib/iommu-helper.c
@@ -16,7 +16,7 @@ again:
index = find_next_zero_bit(map, size, start);
/* Align allocation */
- index = (index + align_mask) & ~align_mask;
+ index = (index + align_mask + 1) & ~align_mask;
end = index + nr;
if (end >= size)
On Mon, 21 Jul 2008 10:15:27 -0400
Prarit Bhargava <[email protected]> wrote:
> pci_alloc_consistent/dma_alloc_coherent is supposed to return size aligned
> addresses.
>
> From Documentation/DMA-mapping.txt:
>
> "pci_alloc_consistent returns two values: the virtual address which you
> can use to access it from the CPU and dma_handle which you pass to the
> card.
>
> The cpu return address and the DMA bus master address are both
> guaranteed to be aligned to the smallest PAGE_SIZE order which
> is greater than or equal to the requested size. This invariant
> exists (for example) to guarantee that if you allocate a chunk
> which is smaller than or equal to 64 kilobytes, the extent of the
> buffer you receive will not cross a 64K boundary."
>
> Fix the GART's alloc_iommu code to return a size aligned address. Also fix
> an incorrect alignment calculation in the iommu-helper code.
>
> Signed-off-by: Prarit Bhargava <[email protected]>
>
> diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
> index faf3229..329718e 100644
> --- a/arch/x86/kernel/pci-gart_64.c
> +++ b/arch/x86/kernel/pci-gart_64.c
> @@ -96,11 +96,12 @@ static unsigned long alloc_iommu(struct device *dev, int size)
>
> spin_lock_irqsave(&iommu_bitmap_lock, flags);
> offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
> - size, base_index, boundary_size, 0);
> + size, base_index, boundary_size, size - 1);
> if (offset == -1) {
> need_flush = 1;
> offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, 0,
> - size, base_index, boundary_size, 0);
> + size, base_index, boundary_size,
> + size - 1);
> }
This affects map_sg and map_single unnecessarily.
> if (offset != -1) {
> set_bit_string(iommu_gart_bitmap, offset, size);
> diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
> index a3b8d4c..b970f1b 100644
> --- a/lib/iommu-helper.c
> +++ b/lib/iommu-helper.c
> @@ -16,7 +16,7 @@ again:
> index = find_next_zero_bit(map, size, start);
>
> /* Align allocation */
> - index = (index + align_mask) & ~align_mask;
> + index = (index + align_mask + 1) & ~align_mask;
Doesn't look right. What we do here is __ALIGN_MASK().
On Mon, 21 Jul 2008 10:15:22 -0400
Prarit Bhargava <[email protected]> wrote:
> (This didn't appear on LKML or any of the mirrors ... trying again)
>
> It is possible that alloc_iommu()'s boundary_size overflows as
> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
> boundary_size triggers a BUG_ON() in the iommu code.
Did you actually hit this? pci-gart_64.c is used only by X86_64.
FUJITA Tomonori wrote:
> On Mon, 21 Jul 2008 10:15:22 -0400
> Prarit Bhargava <[email protected]> wrote:
>
>
>> (This didn't appear on LKML or any of the mirrors ... trying again)
>>
>> It is possible that alloc_iommu()'s boundary_size overflows as
>> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
>> boundary_size triggers a BUG_ON() in the iommu code.
>>
>
> Did you actually hit this? pci-gart_64.c is used only by X86_64.
>
I hit this by declaring a device struct and not declaring a value for
dev->dma_parms->segment_boundary_mask.
I was attempting to alloc out of the IOMMU and the code then dies on the
bugcheck in iommu_is_span_boundary() because boundary_size = 0.
P.
On Mon, 21 Jul 2008 11:21:39 -0400
Prarit Bhargava <[email protected]> wrote:
>
>
> FUJITA Tomonori wrote:
> > On Mon, 21 Jul 2008 10:15:22 -0400
> > Prarit Bhargava <[email protected]> wrote:
> >
> >
> >> (This didn't appear on LKML or any of the mirrors ... trying again)
> >>
> >> It is possible that alloc_iommu()'s boundary_size overflows as
> >> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
> >> boundary_size triggers a BUG_ON() in the iommu code.
> >>
> >
> > Did you actually hit this? pci-gart_64.c is used only by X86_64.
> >
>
> I hit this by declaring a device struct and not declaring a value for
> dev->dma_parms->segment_boundary_mask.
What do you mean? You set dev->dma_params but does't set
dev->dma_parms->segment_boundary_mask? If so, you need to fix your
code. If you set dev->dma_parms, dma_parms needs to be initialized
properly.
FUJITA Tomonori wrote:
> On Mon, 21 Jul 2008 11:21:39 -0400
> Prarit Bhargava <[email protected]> wrote:
>
>
>> FUJITA Tomonori wrote:
>>
>>> On Mon, 21 Jul 2008 10:15:22 -0400
>>> Prarit Bhargava <[email protected]> wrote:
>>>
>>>
>>>
>>>> (This didn't appear on LKML or any of the mirrors ... trying again)
>>>>
>>>> It is possible that alloc_iommu()'s boundary_size overflows as
>>>> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
>>>> boundary_size triggers a BUG_ON() in the iommu code.
>>>>
>>>>
>>> Did you actually hit this? pci-gart_64.c is used only by X86_64.
>>>
>>>
>> I hit this by declaring a device struct and not declaring a value for
>> dev->dma_parms->segment_boundary_mask.
>>
>
> What do you mean? You set dev->dma_params but does't set
> dev->dma_parms->segment_boundary_mask? If so, you need to fix your
> code. If you set dev->dma_parms, dma_parms needs to be initialized
> properly.
>
No, I didn't set dev->dma_params.
P.