From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56570) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlRdq-0000v1-1i for qemu-devel@nongnu.org; Mon, 25 Jul 2011 16:20:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QlRdo-00056P-U6 for qemu-devel@nongnu.org; Mon, 25 Jul 2011 16:20:41 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:65350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QlRdo-00056L-Pn for qemu-devel@nongnu.org; Mon, 25 Jul 2011 16:20:40 -0400 Received: by ywb3 with SMTP id 3so2956043ywb.4 for ; Mon, 25 Jul 2011 13:20:40 -0700 (PDT) Message-ID: <4E2DD014.5040700@codemonkey.ws> Date: Mon, 25 Jul 2011 15:20:36 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1311602584-23409-1-git-send-email-avi@redhat.com> <1311602584-23409-22-git-send-email-avi@redhat.com> In-Reply-To: <1311602584-23409-22-git-send-email-avi@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 21/23] pci: add MemoryRegion based BAR management API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org 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 > 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 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);