From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52496) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SemtW-0006Ao-K2 for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:42:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SemtU-0004Wa-48 for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:41:54 -0400 Received: from goliath.siemens.de ([192.35.17.28]:32820) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SemtT-0004WI-QQ for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:41:52 -0400 Message-ID: <4FD88A8C.7080404@siemens.com> Date: Wed, 13 Jun 2012 14:41:48 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20120612200243.2994.18161.stgit@bling.home> <4FD86EF1.9000305@siemens.com> <1339590450.24037.90.camel@bling.home> In-Reply-To: <1339590450.24037.90.camel@bling.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] msix: Support specifying offsets, BARs, and capability location List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: "qemu-devel@nongnu.org" , "mst@redhat.com" On 2012-06-13 14:27, Alex Williamson wrote: > On Wed, 2012-06-13 at 12:44 +0200, Jan Kiszka wrote: >> On 2012-06-12 22:03, Alex Williamson wrote: >>> msix_init has very little configurability as to how it lays out MSIX >>> for a device. It claims to resize BARs, but doesn't actually do this >>> anymore. This patch allows MSIX to be fully specified, which is >>> necessary both for emulated devices trying to match the physical >>> layout of a hardware device as well as for any kind of device >>> assignment. >>> >>> New functions msix_init_bar & msix_uninit_bar provide wrappers around >>> the more detailed functions for drivers that just want a simple MSIX >>> setup. >>> >>> Signed-off-by: Alex Williamson >>> --- >>> >>> hw/ivshmem.c | 9 +- >>> hw/msix.c | 299 +++++++++++++++++++++++++++++++------------------------ >>> hw/msix.h | 11 +- >>> hw/pci.h | 12 ++ >>> hw/virtio-pci.c | 15 +-- >>> 5 files changed, 192 insertions(+), 154 deletions(-) >>> >>> diff --git a/hw/ivshmem.c b/hw/ivshmem.c >>> index 05559b6..71c84a6 100644 >>> --- a/hw/ivshmem.c >>> +++ b/hw/ivshmem.c >>> @@ -563,16 +563,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { >>> >>> static void ivshmem_setup_msi(IVShmemState * s) >>> { >>> - memory_region_init(&s->msix_bar, "ivshmem-msix", 4096); >>> - if (!msix_init(&s->dev, s->vectors, &s->msix_bar, 1, 0)) { >>> - pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_MEMORY, >>> - &s->msix_bar); >>> - IVSHMEM_DPRINTF("msix initialized (%d vectors)\n", s->vectors); >>> - } else { >>> + if (msix_init_bar(&s->dev, s->vectors, &s->msix_bar, 1, "ivshmem-msix")) { >> >> I don't think the callers of msix_init_bar should have to provide the >> memory region for that bar. That can be embedded into PCIDevice, just >> like you did for the table and PBA. That was my idea with msix_init_simple. > > If it's embedded on PCIDevice, then there are questions around whether > drivers using msix_init should use that MemoryRegion for their BAR, then > if we have the MemoryRegion for one BAR embedded in PCIDevice, why not > all of them? It seems imperfect either way. To clarify this, I called my region msix_*simple*_container. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux