| Message ID | 1311602584-23409-22-git-send-email-avi@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/25/2011 09:03 AM, Avi Kivity wrote: > Allow registering a BAR using a MemoryRegion. Once all users are converted, > pci_register_bar() and pci_register_bar_simple() will be removed. > > Signed-off-by: Avi Kivity<avi@redhat.com> > diff --git a/hw/pci.h b/hw/pci.h > index cfeb042..c51156d 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -94,6 +94,7 @@ typedef struct PCIIORegion { > uint8_t type; > PCIMapIORegionFunc *map_func; > ram_addr_t ram_addr; > + MemoryRegion *memory; > } PCIIORegion; > > #define PCI_ROM_SLOT 6 > @@ -204,6 +205,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, > PCIMapIORegionFunc *map_func); > void pci_register_bar_simple(PCIDevice *pci_dev, int region_num, > pcibus_t size, uint8_t attr, ram_addr_t ram_addr); > +void pci_register_bar_region(PCIDevice *pci_dev, int region_num, > + uint8_t attr, MemoryRegion *memory); This ends up being a very nice API. I had always thought this should be a PCI specific set of callbacks but I do see the benefits of having the callbacks be generic. Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> One thing I'm curious about, what's the symmetric view of this API? Would you see a device doing something like: memory_region_read(&dev->pci_bus->memory, addr, &data, sizeof(data)) ? Regards, Anthony Liguori > int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > uint8_t offset, uint8_t size);
On 07/25/2011 11:20 PM, Anthony Liguori wrote: > On 07/25/2011 09:03 AM, Avi Kivity wrote: >> Allow registering a BAR using a MemoryRegion. Once all users are >> converted, >> pci_register_bar() and pci_register_bar_simple() will be removed. >> >> Signed-off-by: Avi Kivity<avi@redhat.com> > >> diff --git a/hw/pci.h b/hw/pci.h >> index cfeb042..c51156d 100644 >> --- a/hw/pci.h >> +++ b/hw/pci.h >> @@ -94,6 +94,7 @@ typedef struct PCIIORegion { >> uint8_t type; >> PCIMapIORegionFunc *map_func; >> ram_addr_t ram_addr; >> + MemoryRegion *memory; >> } PCIIORegion; >> >> #define PCI_ROM_SLOT 6 >> @@ -204,6 +205,8 @@ void pci_register_bar(PCIDevice *pci_dev, int >> region_num, >> PCIMapIORegionFunc *map_func); >> void pci_register_bar_simple(PCIDevice *pci_dev, int region_num, >> pcibus_t size, uint8_t attr, >> ram_addr_t ram_addr); >> +void pci_register_bar_region(PCIDevice *pci_dev, int region_num, >> + uint8_t attr, MemoryRegion *memory); > > This ends up being a very nice API. I had always thought this should > be a PCI specific set of callbacks but I do see the benefits of having > the callbacks be generic. > > Reviewed-by: Anthony Liguori <aliguori@us.ibm.com> > > One thing I'm curious about, what's the symmetric view of this API? > > Would you see a device doing something like: > > memory_region_read(&dev->pci_bus->memory, addr, &data, sizeof(data)) > I haven't really considered this, but it makes sense to consider the initiating point for DMA. This allows us to do IOMMU transformations naturally.
diff --git a/hw/pci.c b/hw/pci.c index cf16f3b..36db58b 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -844,10 +844,15 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev) if (r->type == PCI_BASE_ADDRESS_SPACE_IO) { isa_unassign_ioport(r->addr, r->filtered_size); } else { - cpu_register_physical_memory(pci_to_cpu_addr(pci_dev->bus, - r->addr), - r->filtered_size, - IO_MEM_UNASSIGNED); + if (r->memory) { + memory_region_del_subregion(pci_dev->bus->address_space, + r->memory); + } else { + cpu_register_physical_memory(pci_to_cpu_addr(pci_dev->bus, + r->addr), + r->filtered_size, + IO_MEM_UNASSIGNED); + } } } } @@ -893,6 +898,7 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, r->type = type; r->map_func = map_func; r->ram_addr = IO_MEM_UNASSIGNED; + r->memory = NULL; wmask = ~(size - 1); addr = pci_bar(pci_dev, region_num); @@ -918,6 +924,16 @@ static void pci_simple_bar_mapfunc(PCIDevice *pci_dev, int region_num, pci_dev->io_regions[region_num].ram_addr); } +static void pci_simple_bar_mapfunc_region(PCIDevice *pci_dev, int region_num, + pcibus_t addr, pcibus_t size, + int type) +{ + memory_region_add_subregion_overlap(pci_dev->bus->address_space, + addr, + pci_dev->io_regions[region_num].memory, + 1); +} + void pci_register_bar_simple(PCIDevice *pci_dev, int region_num, pcibus_t size, uint8_t attr, ram_addr_t ram_addr) { @@ -927,6 +943,15 @@ void pci_register_bar_simple(PCIDevice *pci_dev, int region_num, pci_dev->io_regions[region_num].ram_addr = ram_addr; } +void pci_register_bar_region(PCIDevice *pci_dev, int region_num, + uint8_t attr, MemoryRegion *memory) +{ + pci_register_bar(pci_dev, region_num, memory_region_size(memory), + PCI_BASE_ADDRESS_SPACE_MEMORY | attr, + pci_simple_bar_mapfunc_region); + pci_dev->io_regions[region_num].memory = memory; +} + static void pci_bridge_filter(PCIDevice *d, pcibus_t *addr, pcibus_t *size, uint8_t type) { @@ -1065,10 +1090,16 @@ static void pci_update_mappings(PCIDevice *d) isa_unassign_ioport(r->addr, r->filtered_size); } } else { - cpu_register_physical_memory(pci_to_cpu_addr(d->bus, r->addr), - r->filtered_size, - IO_MEM_UNASSIGNED); - qemu_unregister_coalesced_mmio(r->addr, r->filtered_size); + if (r->memory) { + memory_region_del_subregion(d->bus->address_space, + r->memory); + } else { + cpu_register_physical_memory(pci_to_cpu_addr(d->bus, + r->addr), + r->filtered_size, + IO_MEM_UNASSIGNED); + qemu_unregister_coalesced_mmio(r->addr, r->filtered_size); + } } } r->addr = new_addr; diff --git a/hw/pci.h b/hw/pci.h index cfeb042..c51156d 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -94,6 +94,7 @@ typedef struct PCIIORegion { uint8_t type; PCIMapIORegionFunc *map_func; ram_addr_t ram_addr; + MemoryRegion *memory; } PCIIORegion; #define PCI_ROM_SLOT 6 @@ -204,6 +205,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num, PCIMapIORegionFunc *map_func); void pci_register_bar_simple(PCIDevice *pci_dev, int region_num, pcibus_t size, uint8_t attr, ram_addr_t ram_addr); +void pci_register_bar_region(PCIDevice *pci_dev, int region_num, + uint8_t attr, MemoryRegion *memory); int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t offset, uint8_t size);
Allow registering a BAR using a MemoryRegion. Once all users are converted, pci_register_bar() and pci_register_bar_simple() will be removed. Signed-off-by: Avi Kivity <avi@redhat.com> --- hw/pci.c | 47 +++++++++++++++++++++++++++++++++++++++-------- hw/pci.h | 3 +++ 2 files changed, 42 insertions(+), 8 deletions(-)