[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>
  • From: "Langsdorf, Mark" <mark.langsdorf@xxxxxxx>
  • Date: Thu, 19 Jan 2006 18:10:11 -0600
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 20 Jan 2006 00:18:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcYdLlzDSZAbffs/TcCw+gCgntJWewAJh3yw
  • Thread-topic: [Xen-devel] Support for AGP aperture as IOMMU in AMD64 mode [2/2]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx 
> [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of 
> Keir Fraser
> Sent: Thursday, January 19, 2006 1:32 PM
> To: Langsdorf, Mark
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> 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.


> 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



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.