Xen devel - Feb 2007 - [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

As per Keir''s suggestion, I''m simplifying my approach
to getting GART into the kernel.  This patch set
makes the x86_64 swiotlb code work with dma_ops.

The first patch creates the arch/x86_64/kernel/pci-dma-xen.c
file based on the standard pci-dma.c, and creates
arch/x86_64/kernel/swiotlb-xen.c based on 
arch/i386/kernel/swiotlb.c.  

The second patch makes the necessary changes for
swiotlb to work with Xen on AMD64.

-Mark Langsdorf
AMD, Inc.


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

On 28/2/07 20:17, "Langsdorf, Mark" <mark.langsdorf@amd.com>
wrote:
> The first patch creates the arch/x86_64/kernel/pci-dma-xen.c
> file based on the standard pci-dma.c, and creates
> arch/x86_64/kernel/swiotlb-xen.c based on
> arch/i386/kernel/swiotlb.c.
Do we really need to duplicate the swiotlb code? Did you need to make big
changes?

Other points that I can see from a quick browse include the fact that
alloc_coherent() still looks broken afaics and the one-liner in
io_apic-xen.c is a bit random (since none of the other use-iommu-or-not
decision points are changed, and the GART stuff which is presumably what is
being kludged around is not even included in Xen builds yet).

 -- Keir


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

> On 28/2/07 20:17, "Langsdorf, Mark"
<mark.langsdorf@amd.com> wrote:
> 
> > The first patch creates the arch/x86_64/kernel/pci-dma-xen.c
> > file based on the standard pci-dma.c, and creates
> > arch/x86_64/kernel/swiotlb-xen.c based on
> > arch/i386/kernel/swiotlb.c.
> 
> Do we really need to duplicate the swiotlb code? Did you need 
> to make big changes?
No, the only change I made was to remove the swiotlb declaration
and export, which could have been done in pci-swiotlb-xen instead.

I made the move in case there needed to be any x86_64 specific
changes in the future, it would be easier to do so.  I understand
the need to balance that against keeping the code bases in sync
for common fixes.  I don''t have a preference one way or the other.
> Other points that I can see from a quick browse include the fact that
> alloc_coherent() still looks broken afaics
The i386 swiotlb implementation doesn''t seem to have an
alloc_coherent().  Since all this patch is intended to do
is move swiotlb into the x86_64 directory, I''m not sure
how to resolve the broken implementation.
> and the one-liner in io_apic-xen.c is a bit random
> (since none of the other use-iommu-or-not
> decision points are changed, and the GART stuff which is 
> presumably what is being kludged around is not even
> included in Xen builds yet).
Thanks for catching that.  I''ll fix it in the next
revision.

-Mark Langsdorf
AMD, Inc.



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

On 28/2/07 21:35, "Langsdorf, Mark" <mark.langsdorf@amd.com>
wrote:
>> Other points that I can see from a quick browse include the fact that
>> alloc_coherent() still looks broken afaics
> 
> The i386 swiotlb implementation doesn''t seem to have an
> alloc_coherent().  Since all this patch is intended to do
> is move swiotlb into the x86_64 directory, I''m not sure
> how to resolve the broken implementation.
By pulling in a new arch/x86_64/pci-dma-xen.c you are replacing the
implementations of things like dma_alloc_coherent(). These replacements,
being pretty much unmodified from the x86/64 native originals, simply
aren''t
going to work on Xen. I''m not sure how far we''ll need to
deviate -- I
suspect we''ll end up with a file that look much like the
i386/pci-dma-xen.c
but with calls to swiotlb_xxx() replaced with dma_ops->xxx. Maybe we could
even stick with just i386/pci-dma-xen.c and macro up the uses of
swiotlb_xxx() (so that they can be replaced with uses of dma_ops->xxx for
x86/64 by cpp).

 -- Keir


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

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com 
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
> Keir Fraser
> Sent: Wednesday, February 28, 2007 3:43 PM
> To: Langsdorf, Mark; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] Make x86_64 swiotlb code to 
> support dma_ops [2/2]
> 
> On 28/2/07 21:35, "Langsdorf, Mark"
<mark.langsdorf@amd.com> wrote:
> 
> >> Other points that I can see from a quick browse include 
> the fact that
> >> alloc_coherent() still looks broken afaics
> > 
> > The i386 swiotlb implementation doesn''t seem to have an
> > alloc_coherent().  Since all this patch is intended to do
> > is move swiotlb into the x86_64 directory, I''m not sure
> > how to resolve the broken implementation.
> 
> By pulling in a new arch/x86_64/pci-dma-xen.c you are replacing the
> implementations of things like dma_alloc_coherent().
> These replacements, being pretty much unmodified from the x86/64
> native originals, simply aren''t going to work on Xen.
Oh!  Sorry for being dense.
> I''m not sure how far we''ll need to deviate -- I
> suspect we''ll end up with a file that look much like the 
> i386/pci-dma-xen.c
> but with calls to swiotlb_xxx() replaced with dma_ops->xxx. 
> Maybe we could
> even stick with just i386/pci-dma-xen.c and macro up the uses of
> swiotlb_xxx() (so that they can be replaced with uses of 
> dma_ops->xxx for x86/64 by cpp).
Given that pci-dma-xen for x86_64 is always going to have 
most of the functions replaced by either SWIOTLB or an
IOMMU, would it be sufficient to copy the (known good)
implementations of dma_alloc_coherent() from the i386
version?

I really don''t want to get into the maintenance of the
pci-dma-xen infrastructure; I just want to be able to use
dma_ops to simplify the interaction between the 4 different
dma solutions available for AMD64 (SWIOTLB, GART, Calgary,
and the forthcoming chipset IOMMU).

-Mark Langsdorf
AMD, Inc.



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

>> I''m not sure how far we''ll need to deviate -- I
>> suspect we''ll end up with a file that look much like the 
>> i386/pci-dma-xen.c
>> but with calls to swiotlb_xxx() replaced with dma_ops->xxx. 
>> Maybe we could
>> even stick with just i386/pci-dma-xen.c and macro up the uses of
>> swiotlb_xxx() (so that they can be replaced with uses of 
>> dma_ops->xxx for x86/64 by cpp).
>
>Given that pci-dma-xen for x86_64 is always going to have 
>most of the functions replaced by either SWIOTLB or an
>IOMMU, would it be sufficient to copy the (known good)
>implementations of dma_alloc_coherent() from the i386
>version?
That is what I would have preferred from the beginning. I just didn''t
complain because native lib/swiotlb.c also has these, and I think
they''re not very difficult to fix (and also because I was afraid I
already complained about too many other things).

Jan

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