From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:32857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Semd1-0006mb-G7 for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:24:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Semcz-000091-If for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:24:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50382) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Semcz-00008t-AE for qemu-devel@nongnu.org; Wed, 13 Jun 2012 08:24:49 -0400 Date: Wed, 13 Jun 2012 15:25:21 +0300 From: "Michael S. Tsirkin" Message-ID: <20120613122519.GA18934@redhat.com> References: <20120612200243.2994.18161.stgit@bling.home> <4FD86EF1.9000305@siemens.com> <20120613112131.GE18001@redhat.com> <4FD87810.6080707@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4FD87810.6080707@siemens.com> 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: Jan Kiszka Cc: Alex Williamson , "qemu-devel@nongnu.org" On Wed, Jun 13, 2012 at 01:22:56PM +0200, Jan Kiszka wrote: > On 2012-06-13 13:21, Michael S. Tsirkin wrote: > > On Wed, Jun 13, 2012 at 12:44:01PM +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. > >> > >> Back then, I only included a generic memory region name. That can be > >> improved, but without bothering the caller. Just derive it from > >> PCIDevice::name. > >> > >> Jan > > > > I think callers must initialize the BAR regions. > > This is because BAR can include other stuff besides MSI-X. > > MSI-X adds its own subregion. > > That's the non-common case handled by msix_init. I don't see this as > typical for emulated devices. > > Jan Devices create other bars, easier to let them create msix bar as well, and easier to figure out what's going on. > -- > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux