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

[Anthony Liguori]

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.

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

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), and that we provide a read/write indirection layer 
that is hookable by the PCI bus.

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

Regards,

Anthony Liguori