WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH 18 of 38] x86: unify pci iommu setup and allow sw

On Wed, 2008-11-19 at 11:19 +0900, FUJITA Tomonori wrote:
> 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).

Ah, I hadn't spotted this, you are right it probably works but just by
chance. Thanks for pointing it out.

> > 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.

I was talking about the segment_boundary_mask in struct
device_dma_parameters which is the source of the "mask" value in the
code you quoted.

> > 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.

Right, it's just that on occasion we have to cope with slightly larger
values while manipulating things.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>