>>> "Langsdorf, Mark" <mark.langsdorf@xxxxxxx> 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@xxxxxxx>
>
>--- 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home