From mboxrd@z Thu Jan  1 00:00:00 1970
From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [PATCH 21/23] pci: add MemoryRegion based BAR
 management API
Date: Mon, 25 Jul 2011 15:20:36 -0500
Message-ID: <4E2DD014.5040700@codemonkey.ws>
References: <1311602584-23409-1-git-send-email-avi@redhat.com>
	<1311602584-23409-22-git-send-email-avi@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: qemu-devel@nongnu.org, kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
In-Reply-To: <1311602584-23409-22-git-send-email-avi@redhat.com>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: </archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
List-Id: 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<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);