[Qemu-devel] Re: PATCH/RFC: PCI memory mapping

[Tristan Gingold]

On Apr 2, 2009, at 4:57 PM, Brian Wheeler wrote:

> [first off, if there's an easier way to do this, let me know!]
>
> This patch adds an address mapping function to the PCI bus so the host
> chipset can remap PCI generated addresses to the appropriate physical
> addresses.

I have been thinking about this for a while as many 64bits machines  
have an IOTLB or IOMMU.
And with VTd and AMD IOMMU it is becoming common.

> It adds two parameters to pci_register_bus:
>
> PCIBus *pci_register_bus(pci_set_irq_fn set_irq, pci_map_irq_fn  
> map_irq,
>                         qemu_irq *pic, int devfn_min, int nirq,
>                          pci_map_addr_fn map_addr, void *map_addr_opaque)

map_addr_opaque should be simply opaque (in case of new callbacks are  
added).

> The map_addr is a function which has the prototype:
>
> typedef target_phys_addr_t (*pci_map_addr_fn)(void *opaque, int  
> bus_num,
>                                               target_phys_addr_t addr);

I think this is not complete enough.

I'd replace int bus_num with PCIDevice *dev.  You can get the pci bus  
from the device.
The IOTLB mapping may depend on the device so we really need it.

Second you need to pass and return a length as the mapping is valid  
only for a certain length.

In the future the returned length could be 0 to simulate a pci abort...

> and the map_addr_opaque data is a pointer to the IOTLB buffer in the
> chipset.
>
> When a pci device is doing dma or needs to write to the system, it can
> use the
>
> target_phys_addr_t pci_phys_addr(PCIDevice *device,
>                                  target_phys_addr_t addr);
>
> function to get the mapped target address.
>
> If map_addr is NULL the function just returns the address it was  
> passed.
> Otherwise the map_addr function is called and the address is  
> translated.

I think we should do this in new functions such as pci_memory_read/ 
pci_memory_write which will
replace cpu_physical_memory_read/cpu_physical_memory_write.


Thank you for working on this,
Tristan.