Re: [PATCH] Enable K8 GART as an IOMMU

From: "Jan Beulich" <jbeulich@novell.com>
To: Mark Langsdorf <mark.langsdorf@amd.com>
Cc: xen-devel@lists.xensource.com
Subject: Re: [PATCH] Enable K8 GART as an IOMMU
Date: Tue, 13 Feb 2007 08:47:06 +0000	[thread overview]
Message-ID: <45D1891A.76E4.0078.0@novell.com> (raw)
In-Reply-To: <1449F58C868D8D4E9C72945771150BDFD965D2@SAUSEXMB1.amd.com>

>--- a/linux-2.6-xen-sparse/arch/i386/mm/hypervisor.c	Fri Feb 09 10:48:41 2007 +0000
>+++ b/linux-2.6-xen-sparse/arch/i386/mm/hypervisor.c	Fri Feb 09 16:32:04 2007 -0600
>@@ -252,7 +252,7 @@ static void contiguous_bitmap_clear(
> }
> 
> /* Protected by balloon_lock. */
>-#define MAX_CONTIG_ORDER 9 /* 2MB */
>+#define MAX_CONTIG_ORDER 16 /* 256MB */
> static unsigned long discontig_frames[1<<MAX_CONTIG_ORDER];
> static multicall_entry_t cr_mcl[1<<MAX_CONTIG_ORDER];

This seems dangerous to me.

>--- a/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/dma-mapping.h	Fri Feb 09 10:48:41 2007 +0000
>+++ b/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/dma-mapping.h	Fri Feb 09 16:32:04 2007 -0600
>@@ -62,7 +62,12 @@ static inline int valid_dma_direction(in
> 		(dma_direction == DMA_FROM_DEVICE));
> }
> 
>-#if 0
>+#ifdef 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 dma_mapping_error(dma_addr_t dma_addr)
> {
> 	if (dma_ops->mapping_error)

HYPERVISOR_memory_op() if CONFIG_XEN is undefined?

>--- a/linux-2.6-xen-sparse/lib/Makefile	Fri Feb 09 10:48:41 2007 +0000
>+++ b/linux-2.6-xen-sparse/lib/Makefile	Fri Feb 09 16:32:04 2007 -0600
>@@ -51,8 +51,7 @@ obj-$(CONFIG_SMP) += percpu_counter.o
> obj-$(CONFIG_SMP) += percpu_counter.o
> obj-$(CONFIG_AUDIT_GENERIC) += audit.o
> 
>-obj-$(CONFIG_SWIOTLB) += swiotlb.o
>-swiotlb-$(CONFIG_XEN) := ../arch/i386/kernel/swiotlb.o
>+obj-$(CONFIG_SWIOTLB) += swiotlb-xen.o
> 
> hostprogs-y	:= gen_crc32table
> clean-files	:= crc32table.h

This seems very unlikely to have been tested in a native build. You should
use cherry-pick-xen in the file.
I generally welcome moving arch/i386/kernel/swiotlb.c to lib/swiotlb-xen.c,
but would appreciate if you then also removed the original file (and perhaps
this should be done as a separate patch, so that it'd be clear that the move
itself doesn't change the file in any way (and if you need changes to it for
the IOMMU patch, have only those changes in the patch here).

Jan