Xen devel - Jan 2006 - Support for AGP aperture as IOMMU in AMD64 mode [2/2]

These are the diffs against the pristine versions of
arch/x86_64/kernel/[aperture.c,pci-gart.c] to better
show the changes necessary to adapt those files to
Xen.

They were included with the patch and should not be
applied again.

-Mark Langsdorf
AMD, Inc.

--- pristine-linux-2.6.12/arch/x86_64/kernel/aperture.c	2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6-xen-sparse/arch/xen/x86_64/kernel/aperture.c
2006-01-12 10:53:17.000000000 -0800
@@ -83,7 +83,7 @@
 		printk("Aperture from %s beyond 4GB. Ignoring.\n",name);
 		return 0; 
 	}
-	if (e820_mapped(aper_base, aper_base + aper_size, E820_RAM)) {  
+	if (0 && e820_mapped(aper_base, aper_base + aper_size,
E820_RAM)) {  
 		printk("Aperture from %s pointing to e820 RAM.
Ignoring.\n",name);
 		return 0; 
 	} 
--- pristine-linux-2.6.12/arch/x86_64/kernel/pci-gart.c	2005-06-17
12:48:29.000000000 -0700
+++ linux-2.6-xen-sparse/arch/xen/x86_64/kernel/pci-gart.c
2006-01-16 14:32:15.000000000 -0800
@@ -30,6 +30,7 @@
 #include <asm/proto.h>
 #include <asm/cacheflush.h>
 #include <asm/kdebug.h>
+#include <asm-xen/xen-public/memory.h>
 
 dma_addr_t bad_dma_address;
 
@@ -39,6 +40,11 @@
 
 u32 *iommu_gatt_base; 		/* Remapping table */
 
+/* gart remapping */
+#define virt_to_gart(x) (phys_to_gart(virt_to_phys(x)))
+#define gart_to_virt(x) (phys_to_virt(gart_to_phys(x)))
+#include <asm-xen/asm-i386/agp.h>
+
 int no_iommu; 
 static int no_agp; 
 #ifdef CONFIG_IOMMU_DEBUG
@@ -416,7 +422,7 @@
 	if (!dev)
 		dev = &fallback_dev;
 
-	phys_mem = virt_to_phys(addr); 
+	phys_mem = virt_to_bus(addr);
 	if (!need_iommu(dev, phys_mem, size))
 		return phys_mem; 
 
@@ -721,6 +727,7 @@
 	unsigned aper_size, gatt_size, new_aper_size;
 	
 	printk(KERN_INFO "PCI-DMA: Disabling AGP.\n");
+
 	aper_size = aper_base = info->aper_size = 0;
 	for_all_nb(dev) { 
 		new_aper_base = read_aperture(dev, &new_aper_size); 
@@ -740,17 +747,20 @@
 	info->aper_size = aper_size>>20; 
 
 	gatt_size = (aper_size >> PAGE_SHIFT) * sizeof(u32); 
-	gatt = (void *)__get_free_pages(GFP_KERNEL,
get_order(gatt_size)); 
+	gatt = (void *) alloc_gatt_pages(get_order(gatt_size));
 	if (!gatt) 
 		panic("Cannot allocate GATT table"); 
 	memset(gatt, 0, gatt_size); 
-	agp_gatt_table = gatt;
+	if (!agp_gatt_table)
+		agp_gatt_table = gatt;
+	else
+		goto nommu;
 	
 	for_all_nb(dev) { 
 		u32 ctl; 
 		u32 gatt_reg; 
 
-		gatt_reg = __pa(gatt) >> 12; 
+		gatt_reg = (0xffffffff & virt_to_gart(gatt)) >> 12;
 		gatt_reg <<= 4; 
 		pci_write_config_dword(dev, 0x98, gatt_reg);
 		pci_read_config_dword(dev, 0x90, &ctl); 
@@ -782,6 +792,7 @@
 	struct pci_dev *dev;
 	unsigned long scratch;
 	long i;
+	long ram_end = HYPERVISOR_memory_op(XENMEM_maximum_ram_page,
NULL);
 
 #ifndef CONFIG_AGP_AMD64
 	no_agp = 1; 
@@ -800,7 +811,7 @@
 	} 
 	
 	if (no_iommu ||
-	    (!force_iommu && end_pfn < 0xffffffff>>PAGE_SHIFT) ||
+	    (!force_iommu && ram_end < 0xfffff) ||
 	    !iommu_aperture ||
 	    (no_agp && init_k8_gatt(&info) < 0)) {
 		printk(KERN_INFO "PCI-DMA: Disabling IOMMU.\n"); 
@@ -976,5 +987,7 @@
 	    if (*p == '','')
 		    ++p;
     }
+    if (force_iommu || fallback_aper_force)
+	swiotlb = -1;
     return 1;
 } 


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

On 16 Jan 2006, at 23:50, Langsdorf, Mark wrote:
> These are the diffs against the pristine versions of
> arch/x86_64/kernel/[aperture.c,pci-gart.c] to better
> show the changes necessary to adapt those files to
> Xen.
>
> They were included with the patch and should not be
> applied again.
Changes to these files will have to be merged upstream into the native 
x86/64 files. Hence they need cleaning up and posting to linux-kernel 
and Andi Kleen. At the moment they don''t quite pass muster.

A few things I can see are: why disable call to e820_mapped()? I see 
you added an implementation for that in the main patch you sent out. If 
it''s not right to call it at that point in aperture.c then we need to 
come up with a cleaner abstraction. virt_to_gart/gart_to_virt should be 
moved to our agp.h if we want to keep them. Alternatively you only use 
them a couple of times so expanding them at the call site would be 
okay. You unconditionally allocate a table to the ''gatt''
variable, but
only set the agp_gatt_table variable if it is NULL. Should you free the 
table if agp_gatt_table!=NULL? Can that ever happen, and if so why not 
on native?

The big patch you sent out we also need to go through in some detail. 
It''s rather bigger than I would have expected. Hopefully there is some 
possibility of cleaning up and keeping things closer to the native 
original source files.

  Cheers,
  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: Thursday, January 19, 2006 1:32 PM
> To: Langsdorf, Mark
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] Support for AGP aperture as IOMMU in 
> AMD64 mode [2/2]
> 
> 
> 
> On 16 Jan 2006, at 23:50, Langsdorf, Mark wrote:
> 
> > These are the diffs against the pristine versions of 
> > arch/x86_64/kernel/[aperture.c,pci-gart.c] to better show 
> the changes 
> > necessary to adapt those files to Xen.
> >
> > They were included with the patch and should not be
> > applied again.
> 
> Changes to these files will have to be merged upstream into 
> the native x86/64 files. Hence they need cleaning up and
> posting to linux-kernel and Andi Kleen. At the moment they
> don''t quite pass muster.
Some of those change are definitely Xen specific, such as the
switch from virt_to_phys() to virt_to_bus around line 416,
and the switch from __get_free_pages to alloc_gatt_pages near
line 740.  Similarly, I had to introduce some hypervisor calls
to get the true size of memory so that the GART is enabled even
if dom0 has less than 4 GB of memory.
 
> A few things I can see are: why disable call to e820_mapped()? 
Couldn''t get the implementation of e820_mapped() to work right, 
and missed I had the debug statement in there still.  Any ideas
on what I''m doing wrong in e820_mapped?
> virt_to_gart/gart_to_virt should be moved to our agp.h if we
> want to keep them. Alternatively you only use them a couple
> of times so expanding them at the call site would be okay.
Done.
> You unconditionally allocate a table to the ''gatt''
variable, but
> only set the agp_gatt_table variable if it is NULL. Should 
> you free the table if agp_gatt_table!=NULL? Can that ever happen,
> and if so why not on native?
agp_gatt_table is set in the AGP code on native.  I can''t figure
out why Xen isn''t setting it right, hence the work-around.

Looking at that code again, the else statement makes no sense and
should probably be removed.
> The big patch you sent out we also need to go through in some detail. 
> It''s rather bigger than I would have expected. Hopefully 
> there is some possibility of cleaning up and keeping things closer
> to the native original source files.
Most of the big patch is adding 3 mostly unmodified files
from arch/x86_64/kernel to arch/xen/x86_64/kernel.  The
rest of the code changes are pretty minor.

If you want, I can restructure the patch to reflect -
submit 1 patch to add pci-dma.c, pci-gart.c, and 
aperture.c, and another set of patches to reflect the
incremental changes to those files.  Would that help?

-Mark Langsdorf
AMD, Inc.


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

On Thu, Jan 19, 2006 at 06:10:11PM -0600, Langsdorf, Mark wrote:
> Some of those change are definitely Xen specific, such as the
> switch from virt_to_phys() to virt_to_bus around line 416,
mainline has 

#define virt_to_bus virt_to_phys,

so that should just work. However I''m pretty sure someone was pushing
for deprecating virt_to_bus?
> and the switch from __get_free_pages to alloc_gatt_pages near
> line 740.  Similarly, I had to introduce some hypervisor calls
> to get the true size of memory so that the GART is enabled even
> if dom0 has less than 4 GB of memory.
That could be hidden in an arch specific is_iommu_needed() call that
does the obvious things on bare metal and Xen.
> > The big patch you sent out we also need to go through in some detail. 
> > It''s rather bigger than I would have expected. Hopefully 
> > there is some possibility of cleaning up and keeping things closer
> > to the native original source files.
> 
> Most of the big patch is adding 3 mostly unmodified files
> from arch/x86_64/kernel to arch/xen/x86_64/kernel.  The
> rest of the code changes are pretty minor.
Wouldn''t making Xen a subarch of x86-64 alleviate the need for copying
these files over? you could then just use the unmodified files. Also,
the dma-ops in 2.6.16-rc1 should make it cleaner as well. I believe
the statement at the summit was that the subarch changes will go in
momentarily.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


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

On 20 Jan 2006, at 02:03, Muli Ben-Yehuda wrote:
>> Some of those change are definitely Xen specific, such as the
>> switch from virt_to_phys() to virt_to_bus around line 416,
>
> mainline has
>
> #define virt_to_bus virt_to_phys,
>
> so that should just work. However I''m pretty sure someone was
pushing
> for deprecating virt_to_bus?
Yeah, I fell foul of that and it''s the reason why phys_to_gart() was 
originally introduced. I suspect that those uses of virt_to_bus() 
should actually be phys_to_gart(virt_to_phys()).

  -- Keir


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

> >> Some of those change are definitely Xen specific, such as 
> the switch 
> >> from virt_to_phys() to virt_to_bus around line 416,
> >
> > mainline has
> >
> > #define virt_to_bus virt_to_phys,
> >
> > so that should just work. However I''m pretty sure someone 
> was pushing 
> > for deprecating virt_to_bus?
> 
> Yeah, I fell foul of that and it''s the reason why phys_to_gart()
was
> originally introduced. I suspect that those uses of virt_to_bus() 
> should actually be phys_to_gart(virt_to_phys()).
Preliminary testing shows that the code works with that
change.

I''m a bit confused about what my next step with this patch should
be.  Should I stop working out of the xen-unstable repository and
make my changes against linux-2.6.xen repository?

-Mark Langsdorf
AMD, Inc.


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

On 20 Jan 2006, at 17:06, Langsdorf, Mark wrote:
> Preliminary testing shows that the code works with that
> change.
>
> I''m a bit confused about what my next step with this patch should
> be.  Should I stop working out of the xen-unstable repository and
> make my changes against linux-2.6.xen repository?
We plan to move to the linux-2.6-xen tree in xen-unstable in the very 
near future, and most probably before the gart patch will be ready to 
be checked in. So yes, I think it would be a good idea to sync with our 
2.6.15-based linux-2.6-xen tree.

  -- Keir


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

> On 20 Jan 2006, at 17:06, Langsdorf, Mark wrote:
> 
> > Preliminary testing shows that the code works with that change.
> >
> > I''m a bit confused about what my next step with this patch 
> should be.  
> > Should I stop working out of the xen-unstable repository 
> and make my 
> > changes against linux-2.6.xen repository?
> 
> We plan to move to the linux-2.6-xen tree in xen-unstable in the very 
> near future, and most probably before the gart patch will be ready to 
> be checked in. So yes, I think it would be a good idea to 
> sync with our 2.6.15-based linux-2.6-xen tree.
I''m working on that now.

Is there any documentation on linux-2.6-xen anywhere?  I
checked the mailing list archives since October but nothing
jumped out in a "how do you set this up for the first time"
kind of way.

Thanks for any answers.

-Mark Langsdorf
AMD, Inc.


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

On Fri, Jan 20, 2006 at 12:26:35PM -0600, Langsdorf, Mark wrote:
> Is there any documentation on linux-2.6-xen anywhere?  I
> checked the mailing list archives since October but nothing
> jumped out in a "how do you set this up for the first time"
> kind of way.
AFAICR, what I did was just clone tht tree, build it (making sure ot
build for a Xen subarch) and drop the kernel in the right place for
the bootloader to pick it up. I don''t think it has been integrated
with the Xen build system yet, although it obviously will be once it''s
merged.

The tree is available at http://xenbits.xensource.com/linux-2.6-xen.hg.

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


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

> AFAICR, what I did was just clone tht tree, build it (making 
> sure ot build for a Xen subarch) and drop the kernel in the 
> right place for the bootloader to pick it up. I don''t think 
> it has been integrated with the Xen build system yet, 
> although it obviously will be once it''s merged.
> 
> The tree is available at 
> http://xenbits.xensource.com/linux-2.6-xen.hg.
The AMD64 tree isn''t building a bzImage in arch/x86_64/boot/ ,
and the top level vmlinuz doesn''t seem to work with GRUB 
(or I''m passing the wrong arguments, I''m not sure).

Here''s my grub boot line:
	kernel (hd0,0)/vmlinuz-2.6.15-xen-dom0 root=/dev/hda5 
	initrd (hd0,0)/initrd-2.6.15-xen-dom0

I get a GRUB error "Error 13: Invalid or unsupported executable
format" when I try to boot, though.

-Mark Langsdorf
AMD, Inc.


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

> The AMD64 tree isn''t building a bzImage in arch/x86_64/boot/ ,
> and the top level vmlinuz doesn''t seem to work with GRUB
> (or I''m passing the wrong arguments, I''m not sure).
>
> Here''s my grub boot line:
> 	kernel (hd0,0)/vmlinuz-2.6.15-xen-dom0 root=/dev/hda5
> 	initrd (hd0,0)/initrd-2.6.15-xen-dom0
>
> I get a GRUB error "Error 13: Invalid or unsupported executable
> format" when I try to boot, though.
You need the kernel line to point to Xen.  Include the vmlinuz as a
"module"
line, and include the initrd as a second "module".

Grub can''t boot the Xenified vmlinuz directly, but it can boot Xen and
pass
the Linux kernel to Xen as a module.

Cheers,
Mark


-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

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

On 23 Jan 2006, at 15:13, Langsdorf, Mark wrote:
> The AMD64 tree isn''t building a bzImage in arch/x86_64/boot/ ,
> and the top level vmlinuz doesn''t seem to work with GRUB
> (or I''m passing the wrong arguments, I''m not sure).
>
> Here''s my grub boot line:
> 	kernel (hd0,0)/vmlinuz-2.6.15-xen-dom0 root=/dev/hda5
> 	initrd (hd0,0)/initrd-2.6.15-xen-dom0
>
> I get a GRUB error "Error 13: Invalid or unsupported executable
> format" when I try to boot, though.
kernel line should reference the Xen image, then a module line to 
reference the dom0 kernel image, then a second module line (not initrd 
line) to reference the initrd image.

Your current grub boot lines are trying to load a xenlinux image 
directly as if it were a native linux boot image.

  -- Keir


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

> > The AMD64 tree isn''t building a bzImage in 
> arch/x86_64/boot/ , and the 
> > top level vmlinuz doesn''t seem to work with GRUB (or
I''m
> passing the 
> > wrong arguments, I''m not sure).
> >
> > Here''s my grub boot line:
> > 	kernel (hd0,0)/vmlinuz-2.6.15-xen-dom0 root=/dev/hda5
> > 	initrd (hd0,0)/initrd-2.6.15-xen-dom0
> >
> > I get a GRUB error "Error 13: Invalid or unsupported executable 
> > format" when I try to boot, though.
> 
> kernel line should reference the Xen image, then a module line to 
> reference the dom0 kernel image, then a second module line 
> (not initrd line) to reference the initrd image.
> 
> Your current grub boot lines are trying to load a xenlinux image 
> directly as if it were a native linux boot image.
Which Xen image?  arch/i386 has a boot-xen directory, but
arch/x86_64 does not, and I don''t see anything that looks like
a xen build image in the directory structure.  

-Mark Langsdorf
AMD, Inc.


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

You still have to build Xen itself from the xen-unstable source tree, as 
usual.  Use that, with the kernel you just built, and it should work fine.

Cheers,
Mark

On Monday 23 January 2006 15:44, Langsdorf, Mark wrote:
> > > The AMD64 tree isn''t building a bzImage in
> >
> > arch/x86_64/boot/ , and the
> >
> > > top level vmlinuz doesn''t seem to work with GRUB (or
I''m
> >
> > passing the
> >
> > > wrong arguments, I''m not sure).
> > >
> > > Here''s my grub boot line:
> > > 	kernel (hd0,0)/vmlinuz-2.6.15-xen-dom0 root=/dev/hda5
> > > 	initrd (hd0,0)/initrd-2.6.15-xen-dom0
> > >
> > > I get a GRUB error "Error 13: Invalid or unsupported
executable
> > > format" when I try to boot, though.
> >
> > kernel line should reference the Xen image, then a module line to
> > reference the dom0 kernel image, then a second module line
> > (not initrd line) to reference the initrd image.
> >
> > Your current grub boot lines are trying to load a xenlinux image
> > directly as if it were a native linux boot image.
>
> Which Xen image?  arch/i386 has a boot-xen directory, but
> arch/x86_64 does not, and I don''t see anything that looks like
> a xen build image in the directory structure.
>
> -Mark Langsdorf
> AMD, Inc.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
-- 
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

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