Re: [PATCH] Xen/i386 cleanups - AGP bus/phys cleanups

[Paul Mackerras <paulus@samba.org>]

Keir Fraser writes:

> Yes, Xen will break w/o them, because physical addresses are an 
> illusory trick that the guest OS plays on itself to give itself the 
> impression of a contiguous memory map. We use _to_machine and _to_bus 
> macros to get 'real' physical addresses.

This code needs real physical addresses, which are not the same things
as bus addresses.  I thought from what Rik said that virt_to_phys
should give you real physical addresses, while virt_to_pfn would give
you the contiguous "physical" page numbers - is that the case?

> For allocating the GATT itself, using dma_alloc_coherent() as done in 
> my patch certainly seems sane -- the bus base address of that table is 
> poked into a chipset register, right?

Well, a northbridge register.  The northbridge is in a privileged
position in that it controls the memory and can access all of it
directly using physical addresses, something which PCI or other
devices can't.  The address you poke into a register to define the
base of the table is a physical address.  How could it be a bus
address, when the whole point of the table is to translate bus
addresses to physical addresses?  If it was a bus address you would
have to know where the table was before you could work out where the
table was.

> As for poking entries into the GATT, I guess I'm not sure what ought to 
> be used. virt_to_phys() doesn't sound right to me: the GART is a bridge 
> between two buses, so some sort of bus mapping would still be in order 

No, the GART is a bridge between a bus and memory, and the GATT
entries are physical addresses.

> So: I would very much like you to take the patches I made to generic.c 
> that replace __get_free_pages() calls with dma_alloc_coherent(). For 

This is also wrong - the base address of the GATT is a physical
address not a bus address.  This change will break agpgart on ppc64
systems and I won't be able to play bzflag on my G5 any more. :)
dma_alloc_coherent allocates iommu entries and returns a bus address,
but the addresses coming out of the GART don't go through a further
translation through the iommu.

> now, the patch lines that poke into the GATT I guess stay as they are. 
> We can maintain an out-of-tree patch for Xen, or perhaps if 
> virt_to_phys() is not used very much we can override its definition.

It sounds like xen is trying to overload the concepts of physical and
bus addresses to represent the mapping from "logical" addresses seen
by the kernel to "absolute" addresses (the "real" physical
addresses).  IMHO that is a mistake and will only lead to trouble.

Regards,
Paul.