[Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Michael S. Tsirkin]

This fixes segfault reported by Kevin Wolf,
and simplifies the code in msix_save.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/msix.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 4ab6da6..98c62a5 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -284,11 +284,13 @@ int msix_uninit(PCIDevice *dev)
 
 void msix_save(PCIDevice *dev, QEMUFile *f)
 {
-    unsigned nentries = (pci_get_word(dev->config + PCI_MSIX_FLAGS) &
-                         PCI_MSIX_FLAGS_QSIZE) + 1;
-    qemu_put_buffer(f, dev->msix_table_page, nentries * MSIX_ENTRY_SIZE);
-    qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING,
-                    (nentries + 7) / 8);
+    unsigned n = dev->msix_entries_nr;
+
+    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
+        return;
+
+    qemu_put_buffer(f, dev->msix_table_page, n * MSIX_ENTRY_SIZE);
+    qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
 }
 
 /* Should be called after restoring the config space. */
-- 
1.6.2.2

Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Stuart Brady]

On Sun, Jul 05, 2009 at 01:24:24PM +0300, Michael S. Tsirkin wrote:
> +    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
> +        return;

Same bug as fixed by 98846d7 ('Fix Sparse warning').

Should be:

    if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
        return;
    }

Cheers,
-- 
Stuart Brady

Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Michael S. Tsirkin]

On Sun, Jul 05, 2009 at 11:45:17AM +0100, Stuart Brady wrote:
> On Sun, Jul 05, 2009 at 01:24:24PM +0300, Michael S. Tsirkin wrote:
> > +    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
> > +        return;
> 
> Same bug as fixed by 98846d7 ('Fix Sparse warning').
> 
> Should be:
> 
>     if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
>         return;
>     }
> 
> Cheers,

BTW what's the command to run sparse on qemu?

-- 
MST

Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Blue Swirl]

On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Jul 05, 2009 at 11:45:17AM +0100, Stuart Brady wrote:
>  > On Sun, Jul 05, 2009 at 01:24:24PM +0300, Michael S. Tsirkin wrote:
>  > > +    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
>  > > +        return;
>  >
>  > Same bug as fixed by 98846d7 ('Fix Sparse warning').
>  >
>  > Should be:
>  >
>  >     if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
>  >         return;
>  >     }
>  >
>  > Cheers,
>
>  BTW what's the command to run sparse on qemu?

Just install sparse (sparse and cgcc) somewhere along $PATH, then
configure with --enable-sparse. Rebuild QEMU, sparse will be used as a
wrapper for real GCC.

Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Blue Swirl]

On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote:
> This fixes segfault reported by Kevin Wolf,
>  and simplifies the code in msix_save.

>  +    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
>  +        return;

Dubious: !x & y. You also forgot the braces.

Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save

[Michael S. Tsirkin]

On Sun, Jul 05, 2009 at 01:46:53PM +0300, Blue Swirl wrote:
> On 7/5/09, Michael S. Tsirkin <mst@redhat.com> wrote:
> > This fixes segfault reported by Kevin Wolf,
> >  and simplifies the code in msix_save.
> 
> >  +    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
> >  +        return;
> 
> Dubious: !x & y. You also forgot the braces.

Yes. Happens to work because QEMU_PCI_CAP_MSIX is 0x1.
I'll fix that, thanks!

[Qemu-devel] [PATCHv3 2/3] qemu/virtio: remove control vector save

[Michael S. Tsirkin]

control vector is already saved/restored by virtio-pci,
it does not need to be done in virtio.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/virtio.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index 843be86..41e7ca2 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -626,9 +626,6 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_be32(f, vdev->config_len);
     qemu_put_buffer(f, vdev->config, vdev->config_len);
 
-    if (vdev->nvectors)
-        qemu_put_be16s(f, &vdev->config_vector);
-
     for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
         if (vdev->vq[i].vring.num == 0)
             break;
-- 
1.6.2.2

[Qemu-devel] [PATCHv3 3/3] qemu/virtio: mark msi vectors used on load

[Michael S. Tsirkin]

Which msi vectors are used controlled by the guest and so needs to be
restored on load. Do this for msi vectors used by the virtio device.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/msix.c       |    1 +
 hw/virtio-pci.c |    6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 98c62a5..46925d9 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -301,6 +301,7 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
     if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
         return;
 
+    msix_free_irq_entries(dev);
     qemu_get_buffer(f, dev->msix_table_page, n * MSIX_ENTRY_SIZE);
     qemu_get_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
 }
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index f7da503..66c779e 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -129,8 +129,10 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
     if (ret)
         return ret;
     msix_load(&proxy->pci_dev, f);
-    if (msix_present(&proxy->pci_dev))
+    if (msix_present(&proxy->pci_dev)) {
         qemu_get_be16s(f, &proxy->vdev->config_vector);
+        return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
+    }
     return 0;
 }
 
@@ -142,7 +144,7 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
         return 0;
     qemu_get_be16s(f, &vector);
     virtio_queue_set_vector(proxy->vdev, n, vector);
-    return 0;
+    return msix_vector_use(&proxy->pci_dev, vector);
 }
 
 static void virtio_pci_reset(void *opaque)
-- 
1.6.2.2