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

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel