From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47365) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6lOB-0000e4-At for qemu-devel@nongnu.org; Wed, 29 Aug 2012 12:45:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T6lO6-0005HB-7t for qemu-devel@nongnu.org; Wed, 29 Aug 2012 12:45:11 -0400 Received: from goliath.siemens.de ([192.35.17.28]:16402) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6lO5-0005CA-Ub for qemu-devel@nongnu.org; Wed, 29 Aug 2012 12:45:06 -0400 Message-ID: <503E4701.7080300@siemens.com> Date: Wed, 29 Aug 2012 18:44:49 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20120829164056.GA7792@redhat.com> In-Reply-To: <20120829164056.GA7792@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-1.2] msix: make [un]use vectors on reset/load optional List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Anthony Liguori , Cam Macdonell , Alex Williamson , "qemu-devel@nongnu.org" , Avi Kivity On 2012-08-29 18:40, Michael S. Tsirkin wrote: > The facility to use/unuse vectors dynamically is helpful > for virtio but little else: everyone just seems to use > vectors in their init function. > > Avoid clearing msix vector use info on reset and load. > For virtio, clear it explicitly. > This should fix regressions reported with ivshmem - though > I didn't test this, I verified that virtio keeps > working like it did. > > Signed-off-by: Michael S. Tsirkin > --- > hw/msix.c | 13 +++++++++++-- > hw/virtio-pci.c | 2 ++ > 2 files changed, 13 insertions(+), 2 deletions(-) > > diff --git a/hw/msix.c b/hw/msix.c > index 800fc32..d040cc2 100644 > --- a/hw/msix.c > +++ b/hw/msix.c > @@ -340,6 +340,15 @@ static void msix_free_irq_entries(PCIDevice *dev) > } > } > > +static void msix_clear_all_vectors(PCIDevice *dev) > +{ > + int vector; > + > + for (vector = 0; vector < dev->msix_entries_nr; ++vector) { > + msix_clr_pending(dev, vector); > + } > +} > + > /* Clean up resources for the device. */ > void msix_uninit(PCIDevice *dev, MemoryRegion *table_bar, MemoryRegion *pba_bar) > { > @@ -394,7 +403,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f) > return; > } > > - msix_free_irq_entries(dev); > + msix_clear_all_vectors(dev); > qemu_get_buffer(f, dev->msix_table, n * PCI_MSIX_ENTRY_SIZE); > qemu_get_buffer(f, dev->msix_pba, (n + 7) / 8); > msix_update_function_masked(dev); > @@ -440,7 +449,7 @@ void msix_reset(PCIDevice *dev) > if (!msix_present(dev)) { > return; > } > - msix_free_irq_entries(dev); > + msix_clear_all_vectors(dev); > dev->config[dev->msix_cap + MSIX_CONTROL_OFFSET] &= > ~dev->wmask[dev->msix_cap + MSIX_CONTROL_OFFSET]; > memset(dev->msix_table, 0, dev->msix_entries_nr * PCI_MSIX_ENTRY_SIZE); > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 125eded..ca0b204 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -131,6 +131,7 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f) > if (ret) { > return ret; > } > + msix_unuse_all_vectors(&proxy->pci_dev); > msix_load(&proxy->pci_dev, f); > if (msix_present(&proxy->pci_dev)) { > qemu_get_be16s(f, &proxy->vdev->config_vector); > @@ -246,6 +247,7 @@ void virtio_pci_reset(DeviceState *d) > VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev); > virtio_pci_stop_ioeventfd(proxy); > virtio_reset(proxy->vdev); > + msix_unuse_all_vectors(&proxy->pci_dev); > proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG; > } > > Fine with me, but let's ask Cam to test. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SDP-DE Corporate Competence Center Embedded Linux