* [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
[not found] <cover.1246782566.git.mst@redhat.com>
@ 2009-07-05 10:24 ` Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
@ 2009-07-05 10:24 ` Michael S. Tsirkin
0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
(?)
@ 2009-07-05 10:45 ` Stuart Brady
2009-07-05 19:47 ` Michael S. Tsirkin
-1 siblings, 1 reply; 13+ messages in thread
From: Stuart Brady @ 2009-07-05 10:45 UTC (permalink / raw)
To: qemu-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
2009-07-05 10:45 ` Stuart Brady
@ 2009-07-05 19:47 ` Michael S. Tsirkin
2009-07-05 20:12 ` Blue Swirl
0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 19:47 UTC (permalink / raw)
To: Stuart Brady; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
2009-07-05 19:47 ` Michael S. Tsirkin
@ 2009-07-05 20:12 ` Blue Swirl
0 siblings, 0 replies; 13+ messages in thread
From: Blue Swirl @ 2009-07-05 20:12 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
@ 2009-07-05 10:46 ` Blue Swirl
-1 siblings, 0 replies; 13+ messages in thread
From: Blue Swirl @ 2009-07-05 10:46 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: qemu-devel, avi, kvm, aliguori, kwolf
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
@ 2009-07-05 10:46 ` Blue Swirl
0 siblings, 0 replies; 13+ messages in thread
From: Blue Swirl @ 2009-07-05 10:46 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kwolf, aliguori, qemu-devel, kvm, avi
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
2009-07-05 10:46 ` Blue Swirl
@ 2009-07-05 11:10 ` Michael S. Tsirkin
-1 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 11:10 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, avi, kvm, aliguori, kwolf
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!
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCHv3 1/3] qemu/msi: fix segfault in msix_save
@ 2009-07-05 11:10 ` Michael S. Tsirkin
0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 11:10 UTC (permalink / raw)
To: Blue Swirl; +Cc: kwolf, aliguori, qemu-devel, kvm, avi
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!
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCHv3 2/3] qemu/virtio: remove control vector save
[not found] <cover.1246782566.git.mst@redhat.com>
@ 2009-07-05 10:24 ` Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCHv3 2/3] qemu/virtio: remove control vector save
@ 2009-07-05 10:24 ` Michael S. Tsirkin
0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCHv3 3/3] qemu/virtio: mark msi vectors used on load
[not found] <cover.1246782566.git.mst@redhat.com>
@ 2009-07-05 10:24 ` Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2009-07-05 10:24 ` [Qemu-devel] " Michael S. Tsirkin
2 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCHv3 3/3] qemu/virtio: mark msi vectors used on load
@ 2009-07-05 10:24 ` Michael S. Tsirkin
0 siblings, 0 replies; 13+ messages in thread
From: Michael S. Tsirkin @ 2009-07-05 10:24 UTC (permalink / raw)
To: qemu-devel, avi, kvm, aliguori, kwolf
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
^ permalink raw reply related [flat|nested] 13+ messages in thread