[PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Langsdorf, Mark]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

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.

[-- Attachment #2: enable-dma-ops.patch --]
[-- Type: application/octet-stream, Size: 4685 bytes --]

diff -r 1fe88c952c06 linux-2.6-xen-sparse/arch/x86_64/kernel/Makefile
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/Makefile	Tue Feb 27 18:38:17 2007 -0600
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/Makefile	Wed Feb 28 15:34:25 2007 -0600
@@ -59,7 +59,6 @@ alternative-y			+= ../../i386/kernel/alt
 
 ifdef CONFIG_XEN
 time-y				+= ../../i386/kernel/time-xen.o
-pci-dma-y			+= ../../i386/kernel/pci-dma-xen.o
 microcode-$(subst m,y,$(CONFIG_MICROCODE))  := ../../i386/kernel/microcode-xen.o
 intel_cacheinfo-y		:= ../../i386/kernel/cpu/intel_cacheinfo-xen.o
 quirks-y			:= ../../i386/kernel/quirks-xen.o
diff -r 1fe88c952c06 linux-2.6-xen-sparse/arch/x86_64/kernel/io_apic-xen.c
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/io_apic-xen.c	Tue Feb 27 18:38:17 2007 -0600
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/io_apic-xen.c	Wed Feb 28 15:34:25 2007 -0600
@@ -369,7 +369,7 @@ void __init check_ioapic(void)
 				switch (vendor) { 
 				case PCI_VENDOR_ID_VIA:
 #ifdef CONFIG_IOMMU
-					if ((end_pfn > MAX_DMA32_PFN ||
+					if (global_need_iommu() ||
 					     force_iommu) &&
 					    !iommu_aperture_allowed) {
 						printk(KERN_INFO
diff -r 1fe88c952c06 linux-2.6-xen-sparse/arch/x86_64/kernel/pci-swiotlb-xen.c
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-swiotlb-xen.c	Tue Feb 27 18:38:17 2007 -0600
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-swiotlb-xen.c	Wed Feb 28 15:34:25 2007 -0600
@@ -8,35 +8,26 @@
 #include <asm/swiotlb.h>
 #include <asm/dma.h>
 
-#if 0
 int swiotlb __read_mostly;
 EXPORT_SYMBOL(swiotlb);
-#endif
 
 struct dma_mapping_ops swiotlb_dma_ops = {
-#if 0
 	.mapping_error = swiotlb_dma_mapping_error,
-	.alloc_coherent = swiotlb_alloc_coherent,
-	.free_coherent = swiotlb_free_coherent,
 	.map_single = swiotlb_map_single,
 	.unmap_single = swiotlb_unmap_single,
 	.sync_single_for_cpu = swiotlb_sync_single_for_cpu,
 	.sync_single_for_device = swiotlb_sync_single_for_device,
-	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
-	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
 	.sync_sg_for_cpu = swiotlb_sync_sg_for_cpu,
 	.sync_sg_for_device = swiotlb_sync_sg_for_device,
 	.map_sg = swiotlb_map_sg,
 	.unmap_sg = swiotlb_unmap_sg,
 	.dma_supported = NULL,
-#endif
 };
 
 void pci_swiotlb_init(void)
 {
-#if 0
 	/* don't initialize swiotlb if iommu=off (no_iommu=1) */
-	if (!iommu_detected && !no_iommu && end_pfn > MAX_DMA32_PFN)
+	if (!iommu_detected && !no_iommu)
 	       swiotlb = 1;
 	if (swiotlb_force)
 		swiotlb = 1;
@@ -45,11 +36,4 @@ void pci_swiotlb_init(void)
 		swiotlb_init();
 		dma_ops = &swiotlb_dma_ops;
 	}
-#else
-	swiotlb_init();
-	if (swiotlb) {
-		printk(KERN_INFO "PCI-DMA: Using software bounce buffering for IO (SWIOTLB)\n");
-		dma_ops = &swiotlb_dma_ops;
-	}
-#endif
 }
diff -r 1fe88c952c06 linux-2.6-xen-sparse/arch/x86_64/kernel/swiotlb-xen.c
--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/swiotlb-xen.c	Tue Feb 27 18:38:17 2007 -0600
+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/swiotlb-xen.c	Wed Feb 28 15:34:25 2007 -0600
@@ -26,9 +26,6 @@
 #include <asm/dma.h>
 #include <asm/uaccess.h>
 #include <xen/interface/memory.h>
-
-int swiotlb;
-EXPORT_SYMBOL(swiotlb);
 
 #define OFFSET(val,align) ((unsigned long)((val) & ( (align) - 1)))
 
diff -r 1fe88c952c06 linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/dma-mapping.h
--- a/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/dma-mapping.h	Tue Feb 27 18:38:17 2007 -0600
+++ b/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/dma-mapping.h	Wed Feb 28 15:34:25 2007 -0600
@@ -62,7 +62,30 @@ static inline int valid_dma_direction(in
 		(dma_direction == DMA_FROM_DEVICE));
 }
 
-#if 0
+#ifndef CONFIG_XEN
+#define global_need_iommu() 1
+#else
+#define global_need_iommu() (HYPERVISOR_memory_op(XENMEM_maximum_ram_page, NULL) > MAX_DMA32_PFN)
+#endif
+
+static inline int
+address_needs_mapping(struct device *hwdev, dma_addr_t addr)
+{
+	dma_addr_t mask = 0xffffffff;
+	/* If the device has a mask, use it, otherwise default to 32 bits */
+	if (hwdev && hwdev->dma_mask)
+		mask = *hwdev->dma_mask;
+	return (addr & ~mask) != 0;
+}
+
+static inline int
+range_straddles_page_boundary(void *p, size_t size)
+{
+	extern unsigned long *contiguous_bitmap;
+	return (((((unsigned long)p & ~PAGE_MASK) + size) > PAGE_SIZE) &&
+		!test_bit(__pa(p) >> PAGE_SHIFT, contiguous_bitmap));
+}
+
 static inline int dma_mapping_error(dma_addr_t dma_addr)
 {
 	if (dma_ops->mapping_error)
@@ -200,8 +223,6 @@ dma_cache_sync(void *vaddr, size_t size,
 
 extern struct device fallback_dev;
 extern int panic_on_overflow;
-#endif
 
 #endif /* _X8664_DMA_MAPPING_H */
 
-#include <asm-i386/mach-xen/asm/dma-mapping.h>

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Keir Fraser]

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

RE: [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Langsdorf, Mark]

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

Re: [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Keir Fraser]

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

RE: [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Langsdorf, Mark]

 

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

RE: [PATCH] Make x86_64 swiotlb code to support dma_ops [2/2]

[Jan Beulich]

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