Skip to contentSkip to conversation

qemu/msi: fix segfault in msix_save

PATCH Superseded Michael S. Tsirkin 3 participants 8 messages Jul 5, 2009 1 message not in the archive KVM

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Michael S. Tsirkin

author
#1 lore raw

PATCH v3 1/3 qemu/msi: fix segfault in msix_save

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

Reported-by Kevin Wolf <[email protected]>
Signed-off-by Michael S. Tsirkin <[email protected]>
---
 hw/msix.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
hw/msix.c +7 -5
index 4ab6da6..98c62a5 100644
@@ -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. */
Signature · 1 line
1.6.2.2
#2 lore raw

hw/msix.c int msix_uninit(PCIDevice *dev) · line 290

Quoted 2 lines · Michael S. Tsirkin
hw/msix.c +2 -0
+    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,

Signature · 1 line
Stuart Brady
author
#3 lore raw
Quoted 6 lines · Stuart Brady

hw/msix.c int msix_uninit(PCIDevice *dev) · line 290

Quoted 2 lines · Michael S. Tsirkin
hw/msix.c +2 -0
+    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
+        return;

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

Quoted 7 lines

Should be:

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

Cheers,

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

Signature · 1 line
MST
#4 lore raw
Quoted 9 lines · Michael S. Tsirkin
Quoted 8 lines · Stuart Brady

hw/msix.c int msix_uninit(PCIDevice *dev) · line 290

Quoted 2 lines · Michael S. Tsirkin
hw/msix.c +2 -0
+    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
+        return;

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

Should be:

Quoted 7 lines
Quoted 5 lines
    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.

#5 lore raw
In reply to Michael S. Tsirkin
Quoted 5 lines · Michael S. Tsirkin

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.

author
#6 lore raw
Quoted 8 lines · Blue Swirl
Quoted 5 lines · Michael S. Tsirkin

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!

author
#7 lore raw

PATCH v3 2/3 qemu/virtio: remove control vector save

In reply to a message not in the archive

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

Signed-off-by Michael S. Tsirkin <[email protected]>
---
 hw/virtio.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)
hw/virtio.c +0 -3
index 843be86..41e7ca2 100644
@@ -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;
Signature · 1 line
1.6.2.2
author
#8 lore raw

PATCH v3 3/3 qemu/virtio: mark msi vectors used on load

In reply to a message not in the archive

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 <[email protected]>
---
 hw/msix.c       |    1 +
 hw/virtio-pci.c |    6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)
hw/msix.c +1 -0
index 98c62a5..46925d9 100644
@@ -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);
 }
hw/virtio-pci.c +4 -2
index f7da503..66c779e 100644
@@ -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)
Signature · 1 line
1.6.2.2