Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow swiotlbto compile for 32 bit

[FUJITA Tomonori]

On Mon, 17 Nov 2008 16:16:06 +0000
Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:

> > For example, the following code assumes that the mask needs to be
> > 64 bits.
> 
> The use of unsigned long for the mask is throughout the API and not
> simply limited to swiotlb.c. All the callers of dma_set_seg_boundary
> (PCI and SCSI subsys it seems) do not use a value >4G anywhere I can
> see.

32bit is large enough for dma segment boundary mask, I think.

The problem that I talked about in the previous mail:

> 	max_slots = mask + 1
> 		    ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT
> 		    : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT);

Since the popular value of the mask is 0xffffffff. So the above code
(mask + 1 ?) works wrongly if the size of mask is 32bit (well,
accidentally the result of max_slots is identical though).


> Presumably if something was we would see "warning: overflow in
> implicit constant conversion" somewhere along the line. If no value is
> set then the default is 0xffffffff which is safe on 32 bit.
> 
> I suspect that even with PAE addresses above 4G aren't seen very often
> due to pre-existing subsystem specific bounce buffers or other existing
> limitations (like network buffers being in lowmem).

I guess that you talk about the dma_mask (and coherent_dma_mask) in
struct device. The dma segment boundary mask represents the different
dma limitation of a device.


> Perhaps dma_addr_t should be used though?

I think that 'unsigned long' is better for the dma segment boundary
mask since it represents the hardware limitation. The size of the
value are not related with kernel configurations at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/