Xen devel - Feb 2007 - Retry 2: Use i386 swiotlb code in lib/swiotlb-xen.c [2/2]

Move the arch/i386/kernel/swiotlb.c code to lib/swiotlb-xen.c
code in order to simplify maintenance of Xen in the future.

The first patch simplies moves the code to lib/swiotlb-xen.c;
the second patch adds the necessary changes to the Xen code
base to allow x86_64 systems to boot with SWIOTLB and the
dma_ops framework.

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>


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

>>> "Langsdorf, Mark" <mark.langsdorf@amd.com> 23.02.07
23:32 >>>
>Move the arch/i386/kernel/swiotlb.c code to lib/swiotlb-xen.c
>code in order to simplify maintenance of Xen in the future.
>
>The first patch simplies moves the code to lib/swiotlb-xen.c;
>the second patch adds the necessary changes to the Xen code
>base to allow x86_64 systems to boot with SWIOTLB and the
>dma_ops framework.
>
>Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
>
>--- a/linux-2.6-xen-sparse/lib/Makefile	Fri Feb 23 15:18:00 2007 -0600
>+++ b/linux-2.6-xen-sparse/lib/Makefile	Fri Feb 23 17:51:54 2007 -0600
The whole change to this file would really belong into patch 1 I think. Also,
patch
1 should remove arch/i386/kernel/swiotlb.c, not just add the new file.
>@@ -64,3 +63,12 @@ quiet_cmd_crc32 = GEN     $@
> 
> $(obj)/crc32table.h: $(obj)/gen_crc32table
> 	$(call cmd,crc32)
>+
>+ifdef CONFIG_XEN
>+include $(srctree)/scripts/Makefile.xen
>+
>+obj-y := $(call filterxen, $(obj-y), $(n-obj-xen))
>+obj-y := $(call cherrypickxen, $(obj-y))
>+extra-y := $(call cherrypickxen, $(extra-y))
>+endif
>+
I think the first obj-y := line and the extra-y := line are irrelevant here.
>--- a/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-swiotlb-xen.c	Fri Feb 23
15:18:00 2007 -0600
>+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-swiotlb-xen.c	Fri Feb 23
17:51:54 2007 -0600
>...
>-	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
>-	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
>+//	.sync_single_range_for_cpu = swiotlb_sync_single_range_for_cpu,
>+//	.sync_single_range_for_device = swiotlb_sync_single_range_for_device,
If this is really unnecessary, why do you keep it commented? Otherwise,
what''s
the problem with adding the swiotlb_sync_single_range_for_* functions (which the
merge with lib/swiotlb.c would let us have anyway)?
>--- a/linux-2.6-xen-sparse/lib/swiotlb-xen.c	Fri Feb 23 15:18:00 2007 -0600
>+++ b/linux-2.6-xen-sparse/lib/swiotlb-xen.c	Fri Feb 23 17:51:54 2007 -0600
>...
>@@ -50,6 +52,7 @@ int swiotlb_force;
> int swiotlb_force;
> 
> static char *iotlb_virt_start;
>+unsigned long iotlb_size;
> static unsigned long iotlb_nslabs;
> 
> /*
This new variable seems pointless to me, and moves us farther from native. The
only place it is needed in is
>+void
>+swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
>+			dma_addr_t dma_handle)
>+{
>+	if (!(vaddr >= (void *)iotlb_virt_start
>+		    && vaddr < (void *)(iotlb_virt_start + iotlb_size)))
>+		free_pages((unsigned long) vaddr, get_order(size));
>+	else
>+		/* DMA_TO_DEVICE to avoid memcpy in unmap_single */
>+		swiotlb_unmap_single (hwdev, dma_handle, size, DMA_TO_DEVICE);
>+}
which I would think should use in_swiotlb_aperture() just as other places do.
>--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>+++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/pci-dma-xen.c	Fri Feb 23
17:51:54 2007 -0600
>...
>+		/* Hardcode 31 address bits for now: aacraid limitation. */
>+		if (xen_create_contiguous_region((unsigned long)memory, get_order(size),
31) != 0) {
>+			free_pages((unsigned long)memory, get_order(size));
>+			return NULL;
>+		}
Please don''t re-introduce hardcoded numbers here. i386 does this purely
based
on device requirements, and so should x86-64.

Jan

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