From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH 1/2] msix: Allow msix_init on a device with existing MSI-X capability Date: Sat, 23 Oct 2010 18:18:58 +0200 Message-ID: <20101023161858.GB11488@redhat.com> References: <20101022193845.10161.52907.stgit@s20.home> <20101022203151.10161.17562.stgit@s20.home> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org To: Alex Williamson Return-path: Received: from mx1.redhat.com ([209.132.183.28]:34861 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756926Ab0JWQZ2 (ORCPT ); Sat, 23 Oct 2010 12:25:28 -0400 Content-Disposition: inline In-Reply-To: <20101022203151.10161.17562.stgit@s20.home> Sender: kvm-owner@vger.kernel.org List-ID: On Fri, Oct 22, 2010 at 02:40:31PM -0600, Alex Williamson wrote: > To enable common msix support to be used with pass through devices, > don't attempt to change the BAR if the device already has an > MSI-X capability. This also means we want to pay closer attention > to the size when we map the msix table page, as it isn't necessarily > covering the entire end of the BAR. > > Signed-off-by: Alex Williamson > --- > > hw/msix.c | 67 +++++++++++++++++++++++++++++++++++-------------------------- > 1 files changed, 38 insertions(+), 29 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index 43efbd2..4122395 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -167,35 +167,43 @@ static int msix_add_config(struct PCIDevice *pdev, unsigned short nentries, > { > int config_offset; > uint8_t *config; > - uint32_t new_size; > > - if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) > - return -EINVAL; > - if (bar_size > 0x80000000) > - return -ENOSPC; > - > - /* Add space for MSI-X structures */ > - if (!bar_size) { > - new_size = MSIX_PAGE_SIZE; > - } else if (bar_size < MSIX_PAGE_SIZE) { > - bar_size = MSIX_PAGE_SIZE; > - new_size = MSIX_PAGE_SIZE * 2; > - } else { > - new_size = bar_size * 2; > - } > - > - pdev->msix_bar_size = new_size; > - config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX, MSIX_CAP_LENGTH); > - if (config_offset < 0) > - return config_offset; > - config = pdev->config + config_offset; > - > - pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1); > - /* Table on top of BAR */ > - pci_set_long(config + MSIX_TABLE_OFFSET, bar_size | bar_nr); > - /* Pending bits on top of that */ > - pci_set_long(config + MSIX_PBA_OFFSET, (bar_size + MSIX_PAGE_PENDING) | > - bar_nr); > + pdev->msix_bar_size = bar_size; > + > + config_offset = pci_find_capability(pdev, PCI_CAP_ID_MSIX); > + > + if (!config_offset) { > + uint32_t new_size; > + > + if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) > + return -EINVAL; > + if (bar_size > 0x80000000) > + return -ENOSPC; > + > + /* Add space for MSI-X structures */ > + if (!bar_size) { > + new_size = MSIX_PAGE_SIZE; > + } else if (bar_size < MSIX_PAGE_SIZE) { > + bar_size = MSIX_PAGE_SIZE; > + new_size = MSIX_PAGE_SIZE * 2; > + } else { > + new_size = bar_size * 2; > + } > + > + pdev->msix_bar_size = new_size; > + config_offset = pci_add_capability(pdev, PCI_CAP_ID_MSIX, > + MSIX_CAP_LENGTH); > + if (config_offset < 0) > + return config_offset; > + config = pdev->config + config_offset; > + > + pci_set_word(config + PCI_MSIX_FLAGS, nentries - 1); > + /* Table on top of BAR */ > + pci_set_long(config + MSIX_TABLE_OFFSET, bar_size | bar_nr); > + /* Pending bits on top of that */ > + pci_set_long(config + MSIX_PBA_OFFSET, (bar_size + MSIX_PAGE_PENDING) | > + bar_nr); > + } > pdev->msix_cap = config_offset; > /* Make flags bit writeable. */ > pdev->wmask[config_offset + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK | > @@ -337,7 +345,8 @@ void msix_mmio_map(PCIDevice *d, int region_num, > return; > if (size <= offset) > return; > - cpu_register_physical_memory(addr + offset, size - offset, > + cpu_register_physical_memory(addr + offset, > + MIN(size - offset, MSIX_PAGE_SIZE), This is wrong I think, the table might not fit in a single page. You would need to read table size out of from device config. > d->msix_mmio_index); > } >