Re: [Qemu-devel] [PATCH 2/3] Add PCI memory region registration

[Avi Kivity <avi@redhat.com>]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>>>
>>>> +PCIIORegionComponent *pci_register_physical_memory(PCIDevice 
>>>> *pci_dev,
>>>> +                                                   int region_num,
>>>> +                                                   
>>>> target_phys_addr_t size,
>>>> +                                                   ram_addr_t 
>>>> ram_addr);
>>>>   
>>>
>>> PCI devices should not care about physical memory.  They should only 
>>> care about IO regions.  It ought to look something like:
>>>
>>> enum {
>>>   PCI_REGION_IO,
>>>   PCI_REGION_MEM,
>>> };
>>>
>>> typedef struct PCIIOFunction
>>> {
>>>   uint32_t (*read)(void *opaque, uint64_t offset, int size);
>>>       void (*write)(void *opaque, uint64_t offset, int size, 
>>> uint64_t value);
>>> } PCIIOFunction;
>>
>> That doesn't work for direct-mapped memory like the cirrus 
>> framebuffer or host devices.
>
> That's because it's an internal performance hack.  We should just 
> avoid the PCI routines for that device, if we can, although that 
> suggests we need a map hook which is ugly.  Clever ideas are welcome.

My original proposal.  Note it uses ram addresses, not cpu physical 
addresses.

>
> The above API is needed to sanely deal with PCI busses that do either 
> IOMMU translation or endianness conversion.

IOMMUs or endian conversions can be handled by allocating a new ram 
address whose callbacks to the required translation, then perform the 
operation on the original ram address.  If we teach 
cpu_physical_memory_map() about them, then we can even do dma through an 
iommu.

>
> Whether we use the existing IO memory functions verses a new set is 
> not important to me.  What I care most about is that we have a region 
> vs. memory API, we don't have knowledge of where the region is mapped 
> (so we can get rid of map), 

My proposal provides this.  For example BAR reprogramming will move the 
region (whether it is callback based or direct map) without the device 
knowing about it.

> and that we provide a read/write indirection layer that is hookable by 
> the PCI bus.

This is also covered, though of course the code is missing.

> Direct mapping is a very special case that can be handled 
> differently.  We shouldn't design the API around it.

It happens to be a very important case.  I don't want to neglect it or 
have very different APIs for direct mapped and callback based.

-- 
error compiling committee.c: too many arguments to function