[Qemu-devel] [PULL 0/3] xen-2015-10-26

[Qemu-devel] [PULL 0/3] xen-2015-10-26

[Stefano Stabellini]

The following changes since commit af25e7277d3e95a3ea31023f31d8097ab5e2ac84:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2015-10-23 18:14:42 +0100)

are available in the git repository at:


  git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-2015-10-26

for you to fetch changes up to b1ecd51bdbb0fc0a7026662b03e7e7df9d129ca0:

  xen-platform: Replace assert() with appropriate error reporting (2015-10-26 11:32:24 +0000)

----------------------------------------------------------------
Xen 2015-10-26

----------------------------------------------------------------
Eduardo Habkost (1):
      xen-platform: Replace assert() with appropriate error reporting

Lan Tianyu (1):
      Qemu/Xen: Fix early freeing MSIX MMIO memory region

Stefano Stabellini (1):
      xen_platform: switch to realize

 hw/i386/xen/xen_platform.c  |   12 +++++++-----
 hw/xen/xen_pt.c             |    8 ++++++++
 hw/xen/xen_pt.h             |    1 +
 hw/xen/xen_pt_config_init.c |    2 +-
 hw/xen/xen_pt_msi.c         |   13 ++++++++++++-
 5 files changed, 29 insertions(+), 7 deletions(-)

[Qemu-devel] [PULL 1/3] Qemu/Xen: Fix early freeing MSIX MMIO memory region

[Stefano Stabellini]

From: Lan Tianyu <tianyu.lan@intel.com>

msix->mmio is added to XenPCIPassthroughState's object as property.
object_finalize_child_property is called for XenPCIPassthroughState's
object, which calls object_property_del_all, which is going to try to
delete msix->mmio. object_finalize_child_property() will access
msix->mmio's obj. But the whole msix struct has already been freed
by xen_pt_msix_delete. This will cause segment fault when msix->mmio
has been overwritten.

This patch is to fix the issue.

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xen/xen_pt.c             |    8 ++++++++
 hw/xen/xen_pt.h             |    1 +
 hw/xen/xen_pt_config_init.c |    2 +-
 hw/xen/xen_pt_msi.c         |   13 ++++++++++++-
 4 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 2b54f52..aa96288 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -938,10 +938,18 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
     dc->props = xen_pci_passthrough_properties;
 };
 
+static void xen_pci_passthrough_finalize(Object *obj)
+{
+    XenPCIPassthroughState *s = XEN_PT_DEVICE(obj);
+
+    xen_pt_msix_delete(s);
+}
+
 static const TypeInfo xen_pci_passthrough_info = {
     .name = TYPE_XEN_PT_DEVICE,
     .parent = TYPE_PCI_DEVICE,
     .instance_size = sizeof(XenPCIPassthroughState),
+    .instance_finalize = xen_pci_passthrough_finalize,
     .class_init = xen_pci_passthrough_class_init,
 };
 
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 3bc22eb..c545280 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -305,6 +305,7 @@ void xen_pt_msi_disable(XenPCIPassthroughState *s);
 
 int xen_pt_msix_init(XenPCIPassthroughState *s, uint32_t base);
 void xen_pt_msix_delete(XenPCIPassthroughState *s);
+void xen_pt_msix_unmap(XenPCIPassthroughState *s);
 int xen_pt_msix_update(XenPCIPassthroughState *s);
 int xen_pt_msix_update_remap(XenPCIPassthroughState *s, int bar_index);
 void xen_pt_msix_disable(XenPCIPassthroughState *s);
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 4a5bc11..0efee11 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -2079,7 +2079,7 @@ void xen_pt_config_delete(XenPCIPassthroughState *s)
 
     /* free MSI/MSI-X info table */
     if (s->msix) {
-        xen_pt_msix_delete(s);
+        xen_pt_msix_unmap(s);
     }
     g_free(s->msi);
 
diff --git a/hw/xen/xen_pt_msi.c b/hw/xen/xen_pt_msi.c
index e3d7194..82de2bc 100644
--- a/hw/xen/xen_pt_msi.c
+++ b/hw/xen/xen_pt_msi.c
@@ -610,7 +610,7 @@ error_out:
     return rc;
 }
 
-void xen_pt_msix_delete(XenPCIPassthroughState *s)
+void xen_pt_msix_unmap(XenPCIPassthroughState *s)
 {
     XenPTMSIX *msix = s->msix;
 
@@ -627,6 +627,17 @@ void xen_pt_msix_delete(XenPCIPassthroughState *s)
     }
 
     memory_region_del_subregion(&s->bar[msix->bar_index], &msix->mmio);
+}
+
+void xen_pt_msix_delete(XenPCIPassthroughState *s)
+{
+    XenPTMSIX *msix = s->msix;
+
+    if (!msix) {
+        return;
+    }
+
+    object_unparent(OBJECT(&msix->mmio));
 
     g_free(s->msix);
     s->msix = NULL;
-- 
1.7.10.4

[Qemu-devel] [PULL 2/3] xen_platform: switch to realize

[Stefano Stabellini]

Use realize to initialize the xen_platform device

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/xen/xen_platform.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 8682c42..3dc68cb 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -382,7 +382,7 @@ static const VMStateDescription vmstate_xen_platform = {
     }
 };
 
-static int xen_platform_initfn(PCIDevice *dev)
+static void xen_platform_realize(PCIDevice *dev, Error **errp)
 {
     PCIXenPlatformState *d = XEN_PLATFORM(dev);
     uint8_t *pci_conf;
@@ -407,8 +407,6 @@ static int xen_platform_initfn(PCIDevice *dev)
                      &d->mmio_bar);
 
     platform_fixed_ioport_init(d);
-
-    return 0;
 }
 
 static void platform_reset(DeviceState *dev)
@@ -423,7 +421,7 @@ static void xen_platform_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = xen_platform_initfn;
+    k->realize = xen_platform_realize;
     k->vendor_id = PCI_VENDOR_ID_XEN;
     k->device_id = PCI_DEVICE_ID_XEN_PLATFORM;
     k->class_id = PCI_CLASS_OTHERS << 8 | 0x80;
-- 
1.7.10.4

[Qemu-devel] [PULL 3/3] xen-platform: Replace assert() with appropriate error reporting

[Stefano Stabellini]

From: Eduardo Habkost <ehabkost@redhat.com>

Commit dbb7405d8caad0814ceddd568cb49f163a847561 made it possible to
trigger an assert using "-device xen-platform". Replace it with
appropriate error reporting.

Before:

  $ qemu-system-x86_64 -device xen-platform
  qemu-system-x86_64: hw/i386/xen/xen_platform.c:391: xen_platform_initfn: Assertion `xen_enabled()' failed.
  Aborted (core dumped)
  $

After:

  $ qemu-system-x86_64 -device xen-platform
  qemu-system-x86_64: -device xen-platform: xen-platform device requires the Xen accelerator
  $

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/i386/xen/xen_platform.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 3dc68cb..de83f4e 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -33,6 +33,7 @@
 #include "trace.h"
 #include "exec/address-spaces.h"
 #include "sysemu/block-backend.h"
+#include "qemu/error-report.h"
 
 #include <xenguest.h>
 
@@ -388,7 +389,10 @@ static void xen_platform_realize(PCIDevice *dev, Error **errp)
     uint8_t *pci_conf;
 
     /* Device will crash on reset if xen is not initialized */
-    assert(xen_enabled());
+    if (!xen_enabled()) {
+        error_setg(errp, "xen-platform device requires the Xen accelerator");
+        return;
+    }
 
     pci_conf = dev->config;
 
-- 
1.7.10.4

Re: [Qemu-devel] [PULL 0/3] xen-2015-10-26

[Peter Maydell]

On 26 October 2015 at 11:35, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> The following changes since commit af25e7277d3e95a3ea31023f31d8097ab5e2ac84:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2015-10-23 18:14:42 +0100)
>
> are available in the git repository at:
>
>
>   git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-2015-10-26
>
> for you to fetch changes up to b1ecd51bdbb0fc0a7026662b03e7e7df9d129ca0:
>
>   xen-platform: Replace assert() with appropriate error reporting (2015-10-26 11:32:24 +0000)
>
> ----------------------------------------------------------------
> Xen 2015-10-26
>
> ----------------------------------------------------------------
> Eduardo Habkost (1):
>       xen-platform: Replace assert() with appropriate error reporting
>
> Lan Tianyu (1):
>       Qemu/Xen: Fix early freeing MSIX MMIO memory region
>
> Stefano Stabellini (1):
>       xen_platform: switch to realize
>
>  hw/i386/xen/xen_platform.c  |   12 +++++++-----
>  hw/xen/xen_pt.c             |    8 ++++++++
>  hw/xen/xen_pt.h             |    1 +
>  hw/xen/xen_pt_config_init.c |    2 +-
>  hw/xen/xen_pt_msi.c         |   13 ++++++++++++-
>  5 files changed, 29 insertions(+), 7 deletions(-)

Applied, thanks.

-- PMM