[PATCHv5 1/5] qemu/msi: fix segfault in msix_save

[PATCHv5 1/5] 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..d0f9695 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

[Qemu-devel] [PATCHv5 1/5] 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..d0f9695 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] [PATCHv5 1/5] qemu/msi: fix segfault in msix_save

[Luiz Capitulino]

On Sun, 5 Jul 2009 15:58:23 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> 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>

 Any reason on why this wasn't applied yet?

 Spent some time bisecting this just to find out it's already
fixed. I'm unable to migrate w/o this one.

Re: [Qemu-devel] [PATCHv5 1/5] qemu/msi: fix segfault in msix_save

[Luiz Capitulino]

On Sun, 5 Jul 2009 15:58:23 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> 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>

 Any reason on why this wasn't applied yet?

 Spent some time bisecting this just to find out it's already
fixed. I'm unable to migrate w/o this one.

Re: [Qemu-devel] [PATCHv5 1/5] qemu/msi: fix segfault in msix_save

[Anthony Liguori]

Luiz Capitulino wrote:
> On Sun, 5 Jul 2009 15:58:23 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>   
>> 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>
>>     
>
>  Any reason on why this wasn't applied yet?
>
>  Spent some time bisecting this just to find out it's already
> fixed. I'm unable to migrate w/o this one.
>   

It was just posted on Sunday, give a chance for people to review it and 
apply.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCHv5 1/5] qemu/msi: fix segfault in msix_save

[Anthony Liguori]

Luiz Capitulino wrote:
> On Sun, 5 Jul 2009 15:58:23 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>   
>> 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>
>>     
>
>  Any reason on why this wasn't applied yet?
>
>  Spent some time bisecting this just to find out it's already
> fixed. I'm unable to migrate w/o this one.
>   

It was just posted on Sunday, give a chance for people to review it and 
apply.

-- 
Regards,

Anthony Liguori

[PATCHv5 2/5] qemu/virtio: remove control vector save

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] [PATCHv5 2/5] 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

[PATCHv5 3/5] qemu/msi: clean used vectors state on load

[Michael S. Tsirkin]

Clean up msix vector usage state on load. Since guest might have control
over it through the device, the device will have to load this state from
file.

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

diff --git a/hw/msix.c b/hw/msix.c
index d0f9695..80038d3 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);
 }
-- 
1.6.2.2

[Qemu-devel] [PATCHv5 3/5] qemu/msi: clean used vectors state on load

[Michael S. Tsirkin]

Clean up msix vector usage state on load. Since guest might have control
over it through the device, the device will have to load this state from
file.

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

diff --git a/hw/msix.c b/hw/msix.c
index d0f9695..80038d3 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);
 }
-- 
1.6.2.2

[PATCHv5 4/5] qemu/msi: missing braces

[Michael S. Tsirkin]

MSIX present bit is tested incorrectly, and only happens to work because
the bit we are testing is 0x1.  Add braces to fix this.

Reported-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/msix.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 80038d3..4224d8f 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -298,9 +298,9 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
 {
     unsigned n = dev->msix_entries_nr;
 
-    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
+    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);
-- 
1.6.2.2

[Qemu-devel] [PATCHv5 4/5] qemu/msi: missing braces

[Michael S. Tsirkin]

MSIX present bit is tested incorrectly, and only happens to work because
the bit we are testing is 0x1.  Add braces to fix this.

Reported-by: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/msix.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 80038d3..4224d8f 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -298,9 +298,9 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
 {
     unsigned n = dev->msix_entries_nr;
 
-    if (!dev->cap_present & QEMU_PCI_CAP_MSIX)
+    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);
-- 
1.6.2.2

[PATCHv5 5/5] qemu/virtio: mark msi vectors used on load

[Michael S. Tsirkin]

Usage of msi vectors is 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/virtio-pci.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index f7da503..19ba5b8 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -126,11 +126,18 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
     VirtIOPCIProxy *proxy = opaque;
     int ret;
     ret = pci_device_load(&proxy->pci_dev, f);
-    if (ret)
+    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);
+    } else {
+        proxy->vdev->config_vector = VIRTIO_NO_VECTOR;
+    }
+    if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
+    }
     return 0;
 }
 
@@ -138,10 +145,15 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint16_t vector;
-    if (!msix_present(&proxy->pci_dev))
-        return 0;
-    qemu_get_be16s(f, &vector);
+    if (msix_present(&proxy->pci_dev)) {
+        qemu_get_be16s(f, &vector);
+    } else {
+        vector = VIRTIO_NO_VECTOR;
+    }
     virtio_queue_set_vector(proxy->vdev, n, vector);
+    if (vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, vector);
+    }
     return 0;
 }
 
-- 
1.6.2.2

[Qemu-devel] [PATCHv5 5/5] qemu/virtio: mark msi vectors used on load

[Michael S. Tsirkin]

Usage of msi vectors is 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/virtio-pci.c |   22 +++++++++++++++++-----
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index f7da503..19ba5b8 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -126,11 +126,18 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
     VirtIOPCIProxy *proxy = opaque;
     int ret;
     ret = pci_device_load(&proxy->pci_dev, f);
-    if (ret)
+    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);
+    } else {
+        proxy->vdev->config_vector = VIRTIO_NO_VECTOR;
+    }
+    if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, proxy->vdev->config_vector);
+    }
     return 0;
 }
 
@@ -138,10 +145,15 @@ static int virtio_pci_load_queue(void * opaque, int n, QEMUFile *f)
 {
     VirtIOPCIProxy *proxy = opaque;
     uint16_t vector;
-    if (!msix_present(&proxy->pci_dev))
-        return 0;
-    qemu_get_be16s(f, &vector);
+    if (msix_present(&proxy->pci_dev)) {
+        qemu_get_be16s(f, &vector);
+    } else {
+        vector = VIRTIO_NO_VECTOR;
+    }
     virtio_queue_set_vector(proxy->vdev, n, vector);
+    if (vector != VIRTIO_NO_VECTOR) {
+        return msix_vector_use(&proxy->pci_dev, vector);
+    }
     return 0;
 }
 
-- 
1.6.2.2