All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 REPOST 2/2] Use ioreq-server API
@ 2014-12-03 11:03 Paul Durrant
  2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface Paul Durrant
  2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 2/2] Xen: Use the ioreq-server API when available Paul Durrant
  0 siblings, 2 replies; 9+ messages in thread
From: Paul Durrant @ 2014-12-03 11:03 UTC (permalink / raw)
  To: qemu-devel

This patch series is a re-post v4 of what was originally the single
patch "Xen: Use the ioreq-server API when available". It was originally
posted on October 16th and pinged on October 29th. I am still awaiting
an ack/nack for patch #1, hence the re-post.

v2 of the series moved the code that added the PCI bus listener
to patch #1 and the remainder of the changes to patch #2. Patch #2
was then re-worked to constrain the #ifdefing to xen_common.h, as
requested by Stefano.

v3 of the series modifies patch #1 to add the listener interface
into the core qdev, rather than the PCI bus code. This change only
requires trivial modification to patch #2, to only act on realize/
unrealize of PCI devices. Patch #2 was also modified at Stefano's
request to remove an extra identity check of memory sections
against the ram region.

v4 of the series replaces the use of a vmstate pre_save callback
with extra code in the existing runstate change notification
callback. It also tidies up some things in xen-hvm.c pointed out
by Stefano and adds his ack to patch #2.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 11:03 [Qemu-devel] [PATCH v4 REPOST 2/2] Use ioreq-server API Paul Durrant
@ 2014-12-03 11:03 ` Paul Durrant
  2014-12-03 12:29   ` Stefano Stabellini
  2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 2/2] Xen: Use the ioreq-server API when available Paul Durrant
  1 sibling, 1 reply; 9+ messages in thread
From: Paul Durrant @ 2014-12-03 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, Christian Borntraeger, Paul Durrant,
	Igor Mammedov, Paolo Bonzini, Andreas Faerber"

The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
models explicitly register with Xen for config space accesses. This patch
adds a listener interface into qdev-core which can be used by the Xen
interface code to monitor for arrival and departure of PCI devices.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Andreas Faerber" <afaerber@suse.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Thomas Huth <thuth@linux.vnet.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/core/qdev.c          |   54 +++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/qdev-core.h  |   10 +++++++++
 include/qemu/typedefs.h |    1 +
 3 files changed, 65 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index fcb1638..4a9c1f6 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
     return 0;
 }
 
+static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
+    = QTAILQ_HEAD_INITIALIZER(qdev_listeners);
+
+enum ListenerDirection { Forward, Reverse };
+
+#define QDEV_LISTENER_CALL(_callback, _direction, _args...)     \
+    do {                                                        \
+        DeviceListener *_listener;                              \
+                                                                \
+        switch (_direction) {                                   \
+        case Forward:                                           \
+            QTAILQ_FOREACH(_listener, &qdev_listeners, link) {  \
+                if (_listener->_callback) {                     \
+                    _listener->_callback(_listener, ##_args);   \
+                }                                               \
+            }                                                   \
+            break;                                              \
+        case Reverse:                                           \
+            QTAILQ_FOREACH_REVERSE(_listener, &qdev_listeners,  \
+                                   qdev_listeners, link) {      \
+                if (_listener->_callback) {                     \
+                    _listener->_callback(_listener, ##_args);   \
+                }                                               \
+            }                                                   \
+            break;                                              \
+        default:                                                \
+            abort();                                            \
+        }                                                       \
+    } while (0)
+
+static int qdev_listener_add(DeviceState *dev, void *opaque)
+{
+    QDEV_LISTENER_CALL(realize, Forward, dev);
+
+    return 0;
+}
+
+void qdev_listener_register(DeviceListener *listener)
+{
+    QTAILQ_INSERT_TAIL(&qdev_listeners, listener, link);
+
+    qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
+                       NULL, NULL);
+}
+
+void qdev_listener_unregister(DeviceListener *listener)
+{
+    QTAILQ_REMOVE(&qdev_listeners, listener, link);
+}
+
 static void device_realize(DeviceState *dev, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+
+    QDEV_LISTENER_CALL(realize, Forward, dev);
 }
 
 static void device_unrealize(DeviceState *dev, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
+    QDEV_LISTENER_CALL(unrealize, Reverse, dev);
+
     if (dc->exit) {
         int rc = dc->exit(dev);
         if (rc < 0) {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 178fee2..f2dc267 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -167,6 +167,12 @@ struct DeviceState {
     int alias_required_for_version;
 };
 
+struct DeviceListener {
+    void (*realize)(DeviceListener *listener, DeviceState *dev);
+    void (*unrealize)(DeviceListener *listener, DeviceState *dev);
+    QTAILQ_ENTRY(DeviceListener) link;
+};
+
 #define TYPE_BUS "bus"
 #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
 #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
@@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
                              QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
     bus->allow_hotplug = 1;
 }
+
+void qdev_listener_register(DeviceListener *listener);
+void qdev_listener_unregister(DeviceListener *listener);
+
 #endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 04df51b..e32bca2 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -20,6 +20,7 @@ typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
 typedef struct CompatProperty CompatProperty;
 typedef struct DeviceState DeviceState;
+typedef struct DeviceListener DeviceListener;
 typedef struct BusState BusState;
 typedef struct BusClass BusClass;
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v4 REPOST 2/2] Xen: Use the ioreq-server API when available
  2014-12-03 11:03 [Qemu-devel] [PATCH v4 REPOST 2/2] Use ioreq-server API Paul Durrant
  2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface Paul Durrant
@ 2014-12-03 11:03 ` Paul Durrant
  1 sibling, 0 replies; 9+ messages in thread
From: Paul Durrant @ 2014-12-03 11:03 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Olaf Hering, Alexey Kardashevskiy, Stefan Weil,
	Michael Tokarev, Alexander Graf, Paul Durrant, Gerd Hoffmann,
	Stefan Hajnoczi, Paolo Bonzini

The ioreq-server API added to Xen 4.5 offers better security than
the existing Xen/QEMU interface because the shared pages that are
used to pass emulation request/results back and forth are removed
from the guest's memory space before any requests are serviced.
This prevents the guest from mapping these pages (they are in a
well known location) and attempting to attack QEMU by synthesizing
its own request structures. Hence, this patch modifies configure
to detect whether the API is available, and adds the necessary
code to use the API if it is.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael Tokarev <mjt@tls.msk.ru>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefan Weil <sw@weilnetz.de>
Cc: Olaf Hering <olaf@aepfle.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: Alexander Graf <agraf@suse.de>
---
 configure                   |   29 ++++++
 include/hw/xen/xen_common.h |  223 +++++++++++++++++++++++++++++++++++++++++++
 trace-events                |    9 ++
 xen-hvm.c                   |  156 ++++++++++++++++++++++++++----
 4 files changed, 396 insertions(+), 21 deletions(-)

diff --git a/configure b/configure
index 9ac2600..c2db574 100755
--- a/configure
+++ b/configure
@@ -1876,6 +1876,32 @@ int main(void) {
   xc_gnttab_open(NULL, 0);
   xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
   xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000);
+  xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
+  return 0;
+}
+EOF
+      compile_prog "" "$xen_libs"
+    then
+    xen_ctrl_version=450
+    xen=yes
+
+  elif
+      cat > $TMPC <<EOF &&
+#include <xenctrl.h>
+#include <xenstore.h>
+#include <stdint.h>
+#include <xen/hvm/hvm_info_table.h>
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xc_interface *xc;
+  xs_daemon_open();
+  xc = xc_interface_open(0, 0, 0);
+  xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+  xc_gnttab_open(NULL, 0);
+  xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+  xc_hvm_inject_msi(xc, 0, 0xf0000000, 0x00000000);
   return 0;
 }
 EOF
@@ -4282,6 +4308,9 @@ if test -n "$sparc_cpu"; then
     echo "Target Sparc Arch $sparc_cpu"
 fi
 echo "xen support       $xen"
+if test "$xen" = "yes" ; then
+  echo "xen ctrl version  $xen_ctrl_version"
+fi
 echo "brlapi support    $brlapi"
 echo "bluez  support    $bluez"
 echo "Documentation     $docs"
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 07731b9..aec1372 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -16,7 +16,9 @@
 
 #include "hw/hw.h"
 #include "hw/xen/xen.h"
+#include "hw/pci/pci.h"
 #include "qemu/queue.h"
+#include "trace.h"
 
 /*
  * We don't support Xen prior to 3.3.0.
@@ -164,4 +166,225 @@ void destroy_hvm_domain(bool reboot);
 /* shutdown/destroy current domain because of an error */
 void xen_shutdown_fatal_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 
+/* Xen before 4.5 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 450
+
+#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
+#define HVM_PARAM_BUFIOREQ_EVTCHN 26
+#endif
+
+#define IOREQ_TYPE_PCI_CONFIG 2
+
+typedef uint32_t ioservid_t;
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+                                          ioservid_t ioservid,
+                                          MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid,
+                                            MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+                                      ioservid_t ioservid,
+                                      MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+                                        ioservid_t ioservid,
+                                        MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+                                  ioservid_t ioservid,
+                                  PCIDevice *pci_dev)
+{
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+                                    ioservid_t ioservid,
+                                    PCIDevice *pci_dev)
+{
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+                                          ioservid_t *ioservid)
+{
+    return 0;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid)
+{
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid,
+                                            xen_pfn_t *ioreq_pfn,
+                                            xen_pfn_t *bufioreq_pfn,
+                                            evtchn_port_t *bufioreq_evtchn)
+{
+    unsigned long param;
+    int rc;
+
+    rc = xc_get_hvm_param(xc, dom, HVM_PARAM_IOREQ_PFN, &param);
+    if (rc < 0) {
+        fprintf(stderr, "failed to get HVM_PARAM_IOREQ_PFN\n");
+        return -1;
+    }
+
+    *ioreq_pfn = param;
+
+    rc = xc_get_hvm_param(xc, dom, HVM_PARAM_BUFIOREQ_PFN, &param);
+    if (rc < 0) {
+        fprintf(stderr, "failed to get HVM_PARAM_BUFIOREQ_PFN\n");
+        return -1;
+    }
+
+    *bufioreq_pfn = param;
+
+    rc = xc_get_hvm_param(xc, dom, HVM_PARAM_BUFIOREQ_EVTCHN,
+                          &param);
+    if (rc < 0) {
+        fprintf(stderr, "failed to get HVM_PARAM_BUFIOREQ_EVTCHN\n");
+        return -1;
+    }
+
+    *bufioreq_evtchn = param;
+
+    return 0;
+}
+
+static inline int xen_set_ioreq_server_state(XenXC xc, domid_t dom,
+                                             ioservid_t ioservid,
+                                             bool enable)
+{
+    return 0;
+}
+
+/* Xen 4.5 */
+#else
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+                                          ioservid_t ioservid,
+                                          MemoryRegionSection *section)
+{
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    hwaddr end_addr = start_addr + size - 1;
+
+    trace_xen_map_mmio_range(ioservid, start_addr, end_addr);
+    xc_hvm_map_io_range_to_ioreq_server(xc, dom, ioservid, 1,
+                                        start_addr, end_addr);
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid,
+                                            MemoryRegionSection *section)
+{
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    hwaddr end_addr = start_addr + size - 1;
+
+    trace_xen_unmap_mmio_range(ioservid, start_addr, end_addr);
+    xc_hvm_unmap_io_range_from_ioreq_server(xc, dom, ioservid, 1,
+                                            start_addr, end_addr);
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+                                      ioservid_t ioservid,
+                                      MemoryRegionSection *section)
+{
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    hwaddr end_addr = start_addr + size - 1;
+
+    trace_xen_map_portio_range(ioservid, start_addr, end_addr);
+    xc_hvm_map_io_range_to_ioreq_server(xc, dom, ioservid, 0,
+                                        start_addr, end_addr);
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+                                        ioservid_t ioservid,
+                                        MemoryRegionSection *section)
+{
+    hwaddr start_addr = section->offset_within_address_space;
+    ram_addr_t size = int128_get64(section->size);
+    hwaddr end_addr = start_addr + size - 1;
+
+    trace_xen_unmap_portio_range(ioservid, start_addr, end_addr);
+    xc_hvm_unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
+                                            start_addr, end_addr);
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+                                  ioservid_t ioservid,
+                                  PCIDevice *pci_dev)
+{
+    trace_xen_map_pcidev(ioservid, pci_bus_num(pci_dev->bus),
+                         PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn));
+    xc_hvm_map_pcidev_to_ioreq_server(xc, dom, ioservid,
+                                      0, pci_bus_num(pci_dev->bus),
+                                      PCI_SLOT(pci_dev->devfn),
+                                      PCI_FUNC(pci_dev->devfn));
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+                                    ioservid_t ioservid,
+                                    PCIDevice *pci_dev)
+{
+    trace_xen_unmap_pcidev(ioservid, pci_bus_num(pci_dev->bus),
+                           PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn));
+    xc_hvm_unmap_pcidev_from_ioreq_server(xc, dom, ioservid,
+                                          0, pci_bus_num(pci_dev->bus),
+                                          PCI_SLOT(pci_dev->devfn),
+                                          PCI_FUNC(pci_dev->devfn));
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+                                          ioservid_t *ioservid)
+{
+    int rc = xc_hvm_create_ioreq_server(xc, dom, 1, ioservid);
+
+    if (rc == 0) {
+        trace_xen_ioreq_server_create(*ioservid);
+    }
+
+    return rc;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid)
+{
+    trace_xen_ioreq_server_destroy(ioservid);
+    xc_hvm_destroy_ioreq_server(xc, dom, ioservid);
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+                                            ioservid_t ioservid,
+                                            xen_pfn_t *ioreq_pfn,
+                                            xen_pfn_t *bufioreq_pfn,
+                                            evtchn_port_t *bufioreq_evtchn)
+{
+    return xc_hvm_get_ioreq_server_info(xc, dom, ioservid,
+                                        ioreq_pfn, bufioreq_pfn,
+                                        bufioreq_evtchn);
+}
+
+static inline int xen_set_ioreq_server_state(XenXC xc, domid_t dom,
+                                             ioservid_t ioservid,
+                                             bool enable)
+{
+    trace_xen_ioreq_server_state(ioservid, enable);
+    return xc_hvm_set_ioreq_server_state(xc, dom, ioservid, enable);
+}
+
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
diff --git a/trace-events b/trace-events
index 011d105..0126ef5 100644
--- a/trace-events
+++ b/trace-events
@@ -895,6 +895,15 @@ pvscsi_tx_rings_num_pages(const char* label, uint32_t num) "Number of %s pages:
 # xen-hvm.c
 xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: %#lx, size %#lx"
 xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "%#"PRIx64" size %#lx, log_dirty %i"
+xen_ioreq_server_create(uint32_t id) "id: %u"
+xen_ioreq_server_destroy(uint32_t id) "id: %u"
+xen_ioreq_server_state(uint32_t id, bool enable) "id: %u: enable: %i"
+xen_map_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_unmap_mmio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_map_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_unmap_portio_range(uint32_t id, uint64_t start_addr, uint64_t end_addr) "id: %u start: %#"PRIx64" end: %#"PRIx64
+xen_map_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
+xen_unmap_pcidev(uint32_t id, uint8_t bus, uint8_t dev, uint8_t func) "id: %u bdf: %02x.%02x.%02x"
 
 # xen-mapcache.c
 xen_map_cache(uint64_t phys_addr) "want %#"PRIx64
diff --git a/xen-hvm.c b/xen-hvm.c
index 05e522c..79ecfbb 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -62,9 +62,6 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t *shared_page, int vcpu)
 }
 #  define FMT_ioreq_size "u"
 #endif
-#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
-#define HVM_PARAM_BUFIOREQ_EVTCHN 26
-#endif
 
 #define BUFFER_IO_MAX_DELAY  100
 
@@ -78,6 +75,7 @@ typedef struct XenPhysmap {
 } XenPhysmap;
 
 typedef struct XenIOState {
+    ioservid_t ioservid;
     shared_iopage_t *shared_page;
     buffered_iopage_t *buffered_io_page;
     QEMUTimer *buffered_io_timer;
@@ -92,6 +90,8 @@ typedef struct XenIOState {
 
     struct xs_handle *xenstore;
     MemoryListener memory_listener;
+    MemoryListener io_listener;
+    DeviceListener device_listener;
     QLIST_HEAD(, XenPhysmap) physmap;
     hwaddr free_phys_offset;
     const XenPhysmap *log_for_dirtybit;
@@ -442,12 +442,23 @@ static void xen_set_memory(struct MemoryListener *listener,
     bool log_dirty = memory_region_is_logging(section->mr);
     hvmmem_type_t mem_type;
 
+    if (section->mr == &ram_memory) {
+        return;
+    } else {
+        if (add) {
+            xen_map_memory_section(xen_xc, xen_domid, state->ioservid,
+                                   section);
+        } else {
+            xen_unmap_memory_section(xen_xc, xen_domid, state->ioservid,
+                                     section);
+        }
+    }
+
     if (!memory_region_is_ram(section->mr)) {
         return;
     }
 
-    if (!(section->mr != &ram_memory
-          && ( (log_dirty && add) || (!log_dirty && !add)))) {
+    if (log_dirty != add) {
         return;
     }
 
@@ -490,6 +501,50 @@ static void xen_region_del(MemoryListener *listener,
     memory_region_unref(section->mr);
 }
 
+static void xen_io_add(MemoryListener *listener,
+                       MemoryRegionSection *section)
+{
+    XenIOState *state = container_of(listener, XenIOState, io_listener);
+
+    memory_region_ref(section->mr);
+
+    xen_map_io_section(xen_xc, xen_domid, state->ioservid, section);
+}
+
+static void xen_io_del(MemoryListener *listener,
+                       MemoryRegionSection *section)
+{
+    XenIOState *state = container_of(listener, XenIOState, io_listener);
+
+    xen_unmap_io_section(xen_xc, xen_domid, state->ioservid, section);
+
+    memory_region_unref(section->mr);
+}
+
+static void xen_device_realize(DeviceListener *listener,
+			       DeviceState *dev)
+{
+    XenIOState *state = container_of(listener, XenIOState, device_listener);
+
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        PCIDevice *pci_dev = PCI_DEVICE(dev);
+
+        xen_map_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev);
+    }
+}
+
+static void xen_device_unrealize(DeviceListener *listener,
+				 DeviceState *dev)
+{
+    XenIOState *state = container_of(listener, XenIOState, device_listener);
+
+    if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
+        PCIDevice *pci_dev = PCI_DEVICE(dev);
+
+        xen_unmap_pcidev(xen_xc, xen_domid, state->ioservid, pci_dev);
+    }
+}
+
 static void xen_sync_dirty_bitmap(XenIOState *state,
                                   hwaddr start_addr,
                                   ram_addr_t size)
@@ -590,6 +645,17 @@ static MemoryListener xen_memory_listener = {
     .priority = 10,
 };
 
+static MemoryListener xen_io_listener = {
+    .region_add = xen_io_add,
+    .region_del = xen_io_del,
+    .priority = 10,
+};
+
+static DeviceListener xen_device_listener = {
+    .realize = xen_device_realize,
+    .unrealize = xen_device_unrealize,
+};
+
 /* get the ioreq packets from share mem */
 static ioreq_t *cpu_get_ioreq_from_shared_memory(XenIOState *state, int vcpu)
 {
@@ -792,6 +858,27 @@ static void handle_ioreq(ioreq_t *req)
         case IOREQ_TYPE_INVALIDATE:
             xen_invalidate_map_cache();
             break;
+        case IOREQ_TYPE_PCI_CONFIG: {
+            uint32_t sbdf = req->addr >> 32;
+            uint32_t val;
+
+            /* Fake a write to port 0xCF8 so that
+             * the config space access will target the
+             * correct device model.
+             */
+            val = (1u << 31) |
+                  ((req->addr & 0x0f00) << 16) |
+                  ((sbdf & 0xffff) << 8) |
+                  (req->addr & 0xfc);
+            do_outp(0xcf8, 4, val);
+
+            /* Now issue the config space access via
+             * port 0xCFC
+             */
+            req->addr = 0xcfc | (req->addr & 0x03);
+            cpu_ioreq_pio(req);
+            break;
+        }
         default:
             hw_error("Invalid ioreq type 0x%x\n", req->type);
     }
@@ -916,6 +1003,10 @@ static void xen_hvm_change_state_handler(void *opaque, int running,
     if (running) {
         xen_main_loop_prepare(xstate);
     }
+
+    xen_set_ioreq_server_state(xen_xc, xen_domid,
+                               xstate->ioservid,
+                               (rstate == RUN_STATE_RUNNING));
 }
 
 static void xen_exit_notifier(Notifier *n, void *data)
@@ -984,8 +1075,9 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
                  MemoryRegion **ram_memory)
 {
     int i, rc;
-    unsigned long ioreq_pfn;
-    unsigned long bufioreq_evtchn;
+    xen_pfn_t ioreq_pfn;
+    xen_pfn_t bufioreq_pfn;
+    evtchn_port_t bufioreq_evtchn;
     XenIOState *state;
 
     state = g_malloc0(sizeof (XenIOState));
@@ -1002,6 +1094,12 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
         return -1;
     }
 
+    rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
+    if (rc < 0) {
+        perror("xen: ioreq server create");
+        return -1;
+    }
+
     state->exit.notify = xen_exit_notifier;
     qemu_add_exit_notifier(&state->exit);
 
@@ -1011,8 +1109,18 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
     state->wakeup.notify = xen_wakeup_notifier;
     qemu_register_wakeup_notifier(&state->wakeup);
 
-    xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_IOREQ_PFN, &ioreq_pfn);
+    rc = xen_get_ioreq_server_info(xen_xc, xen_domid, state->ioservid,
+                                   &ioreq_pfn, &bufioreq_pfn,
+                                   &bufioreq_evtchn);
+    if (rc < 0) {
+        hw_error("failed to get ioreq server info: error %d handle=" XC_INTERFACE_FMT,
+                 errno, xen_xc);
+    }
+
     DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
+    DPRINTF("buffered io page at pfn %lx\n", bufioreq_pfn);
+    DPRINTF("buffered io evtchn is %x\n", bufioreq_evtchn);
+
     state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
                                               PROT_READ|PROT_WRITE, ioreq_pfn);
     if (state->shared_page == NULL) {
@@ -1020,14 +1128,20 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
                  errno, xen_xc);
     }
 
-    xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_BUFIOREQ_PFN, &ioreq_pfn);
-    DPRINTF("buffered io page at pfn %lx\n", ioreq_pfn);
-    state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
-                                                   PROT_READ|PROT_WRITE, ioreq_pfn);
+    state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid,
+                                                   XC_PAGE_SIZE,
+                                                   PROT_READ|PROT_WRITE,
+                                                   bufioreq_pfn);
     if (state->buffered_io_page == NULL) {
         hw_error("map buffered IO page returned error %d", errno);
     }
 
+    rc = xen_set_ioreq_server_state(xen_xc, xen_domid, state->ioservid, true);
+    if (rc < 0) {
+        hw_error("failed to enable ioreq server info: error %d handle=" XC_INTERFACE_FMT,
+                 errno, xen_xc);
+    }
+
     state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t));
 
     /* FIXME: how about if we overflow the page here? */
@@ -1035,22 +1149,16 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
         rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
                                         xen_vcpu_eport(state->shared_page, i));
         if (rc == -1) {
-            fprintf(stderr, "bind interdomain ioctl error %d\n", errno);
+            fprintf(stderr, "shared evtchn %d bind error %d\n", i, errno);
             return -1;
         }
         state->ioreq_local_port[i] = rc;
     }
 
-    rc = xc_get_hvm_param(xen_xc, xen_domid, HVM_PARAM_BUFIOREQ_EVTCHN,
-            &bufioreq_evtchn);
-    if (rc < 0) {
-        fprintf(stderr, "failed to get HVM_PARAM_BUFIOREQ_EVTCHN\n");
-        return -1;
-    }
     rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
-            (uint32_t)bufioreq_evtchn);
+                                    bufioreq_evtchn);
     if (rc == -1) {
-        fprintf(stderr, "bind interdomain ioctl error %d\n", errno);
+        fprintf(stderr, "buffered evtchn bind error %d\n", errno);
         return -1;
     }
     state->bufioreq_local_port = rc;
@@ -1066,6 +1174,12 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
     memory_listener_register(&state->memory_listener, &address_space_memory);
     state->log_for_dirtybit = NULL;
 
+    state->io_listener = xen_io_listener;
+    memory_listener_register(&state->io_listener, &address_space_io);
+
+    state->device_listener = xen_device_listener;
+    qdev_listener_register(&state->device_listener);
+
     /* Initialize backend core & drivers */
     if (xen_be_init() != 0) {
         fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface Paul Durrant
@ 2014-12-03 12:29   ` Stefano Stabellini
  2014-12-03 13:40     ` Michael S. Tsirkin
  2014-12-03 14:26     ` Andreas Färber
  0 siblings, 2 replies; 9+ messages in thread
From: Stefano Stabellini @ 2014-12-03 12:29 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Peter Crosthwaite, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, qemu-devel, Christian Borntraeger,
	Paolo Bonzini, Igor Mammedov, Andreas Faerber

The second patch is already Acked.
You just need a review on this one to move forward, right?

Andreas, Michael?

On Wed, 3 Dec 2014, Paul Durrant wrote:
> The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
> models explicitly register with Xen for config space accesses. This patch
> adds a listener interface into qdev-core which can be used by the Xen
> interface code to monitor for arrival and departure of PCI devices.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Andreas Faerber" <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Thomas Huth <thuth@linux.vnet.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/core/qdev.c          |   54 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/qdev-core.h  |   10 +++++++++
>  include/qemu/typedefs.h |    1 +
>  3 files changed, 65 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index fcb1638..4a9c1f6 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
>      return 0;
>  }
>  
> +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
> +    = QTAILQ_HEAD_INITIALIZER(qdev_listeners);
> +
> +enum ListenerDirection { Forward, Reverse };
> +
> +#define QDEV_LISTENER_CALL(_callback, _direction, _args...)     \
> +    do {                                                        \
> +        DeviceListener *_listener;                              \
> +                                                                \
> +        switch (_direction) {                                   \
> +        case Forward:                                           \
> +            QTAILQ_FOREACH(_listener, &qdev_listeners, link) {  \
> +                if (_listener->_callback) {                     \
> +                    _listener->_callback(_listener, ##_args);   \
> +                }                                               \
> +            }                                                   \
> +            break;                                              \
> +        case Reverse:                                           \
> +            QTAILQ_FOREACH_REVERSE(_listener, &qdev_listeners,  \
> +                                   qdev_listeners, link) {      \
> +                if (_listener->_callback) {                     \
> +                    _listener->_callback(_listener, ##_args);   \
> +                }                                               \
> +            }                                                   \
> +            break;                                              \
> +        default:                                                \
> +            abort();                                            \
> +        }                                                       \
> +    } while (0)
> +
> +static int qdev_listener_add(DeviceState *dev, void *opaque)
> +{
> +    QDEV_LISTENER_CALL(realize, Forward, dev);
> +
> +    return 0;
> +}
> +
> +void qdev_listener_register(DeviceListener *listener)
> +{
> +    QTAILQ_INSERT_TAIL(&qdev_listeners, listener, link);
> +
> +    qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
> +                       NULL, NULL);
> +}
> +
> +void qdev_listener_unregister(DeviceListener *listener)
> +{
> +    QTAILQ_REMOVE(&qdev_listeners, listener, link);
> +}
> +
>  static void device_realize(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error **errp)
>              return;
>          }
>      }
> +
> +    QDEV_LISTENER_CALL(realize, Forward, dev);
>  }
>  
>  static void device_unrealize(DeviceState *dev, Error **errp)
>  {
>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>  
> +    QDEV_LISTENER_CALL(unrealize, Reverse, dev);
> +
>      if (dc->exit) {
>          int rc = dc->exit(dev);
>          if (rc < 0) {
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 178fee2..f2dc267 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -167,6 +167,12 @@ struct DeviceState {
>      int alias_required_for_version;
>  };
>  
> +struct DeviceListener {
> +    void (*realize)(DeviceListener *listener, DeviceState *dev);
> +    void (*unrealize)(DeviceListener *listener, DeviceState *dev);
> +    QTAILQ_ENTRY(DeviceListener) link;
> +};
> +
>  #define TYPE_BUS "bus"
>  #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
>  #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
> @@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
>                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
>      bus->allow_hotplug = 1;
>  }
> +
> +void qdev_listener_register(DeviceListener *listener);
> +void qdev_listener_unregister(DeviceListener *listener);
> +
>  #endif
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 04df51b..e32bca2 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -20,6 +20,7 @@ typedef struct Property Property;
>  typedef struct PropertyInfo PropertyInfo;
>  typedef struct CompatProperty CompatProperty;
>  typedef struct DeviceState DeviceState;
> +typedef struct DeviceListener DeviceListener;
>  typedef struct BusState BusState;
>  typedef struct BusClass BusClass;
>  
> -- 
> 1.7.10.4
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 12:29   ` Stefano Stabellini
@ 2014-12-03 13:40     ` Michael S. Tsirkin
  2014-12-03 13:55       ` Paolo Bonzini
  2014-12-03 14:26     ` Andreas Färber
  1 sibling, 1 reply; 9+ messages in thread
From: Michael S. Tsirkin @ 2014-12-03 13:40 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Peter Crosthwaite, Thomas Huth, Markus Armbruster, qemu-devel,
	Christian Borntraeger, Paul Durrant, Paolo Bonzini,
	Igor Mammedov, Andreas Faerber

On Wed, Dec 03, 2014 at 12:29:43PM +0000, Stefano Stabellini wrote:
> The second patch is already Acked.
> You just need a review on this one to move forward, right?
> 
> Andreas, Michael?

Looks like a generic qdev thing, nothing to do with me.

> On Wed, 3 Dec 2014, Paul Durrant wrote:
> > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
> > models explicitly register with Xen for config space accesses. This patch
> > adds a listener interface into qdev-core which can be used by the Xen
> > interface code to monitor for arrival and departure of PCI devices.
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Andreas Faerber" <afaerber@suse.de>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > Cc: Igor Mammedov <imammedo@redhat.com>
> > Cc: Markus Armbruster <armbru@redhat.com>
> > Cc: Thomas Huth <thuth@linux.vnet.ibm.com>
> > Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> > ---
> >  hw/core/qdev.c          |   54 +++++++++++++++++++++++++++++++++++++++++++++++
> >  include/hw/qdev-core.h  |   10 +++++++++
> >  include/qemu/typedefs.h |    1 +
> >  3 files changed, 65 insertions(+)
> > 
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index fcb1638..4a9c1f6 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
> >      return 0;
> >  }
> >  
> > +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
> > +    = QTAILQ_HEAD_INITIALIZER(qdev_listeners);
> > +
> > +enum ListenerDirection { Forward, Reverse };
> > +
> > +#define QDEV_LISTENER_CALL(_callback, _direction, _args...)     \
> > +    do {                                                        \
> > +        DeviceListener *_listener;                              \
> > +                                                                \
> > +        switch (_direction) {                                   \
> > +        case Forward:                                           \
> > +            QTAILQ_FOREACH(_listener, &qdev_listeners, link) {  \
> > +                if (_listener->_callback) {                     \
> > +                    _listener->_callback(_listener, ##_args);   \
> > +                }                                               \
> > +            }                                                   \
> > +            break;                                              \
> > +        case Reverse:                                           \
> > +            QTAILQ_FOREACH_REVERSE(_listener, &qdev_listeners,  \
> > +                                   qdev_listeners, link) {      \
> > +                if (_listener->_callback) {                     \
> > +                    _listener->_callback(_listener, ##_args);   \
> > +                }                                               \
> > +            }                                                   \
> > +            break;                                              \
> > +        default:                                                \
> > +            abort();                                            \
> > +        }                                                       \
> > +    } while (0)
> > +
> > +static int qdev_listener_add(DeviceState *dev, void *opaque)
> > +{
> > +    QDEV_LISTENER_CALL(realize, Forward, dev);
> > +
> > +    return 0;
> > +}
> > +
> > +void qdev_listener_register(DeviceListener *listener)
> > +{
> > +    QTAILQ_INSERT_TAIL(&qdev_listeners, listener, link);
> > +
> > +    qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
> > +                       NULL, NULL);
> > +}
> > +
> > +void qdev_listener_unregister(DeviceListener *listener)
> > +{
> > +    QTAILQ_REMOVE(&qdev_listeners, listener, link);
> > +}
> > +
> >  static void device_realize(DeviceState *dev, Error **errp)
> >  {
> >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> > @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error **errp)
> >              return;
> >          }
> >      }
> > +
> > +    QDEV_LISTENER_CALL(realize, Forward, dev);
> >  }
> >  
> >  static void device_unrealize(DeviceState *dev, Error **errp)
> >  {
> >      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> >  
> > +    QDEV_LISTENER_CALL(unrealize, Reverse, dev);
> > +
> >      if (dc->exit) {
> >          int rc = dc->exit(dev);
> >          if (rc < 0) {
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 178fee2..f2dc267 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -167,6 +167,12 @@ struct DeviceState {
> >      int alias_required_for_version;
> >  };
> >  
> > +struct DeviceListener {
> > +    void (*realize)(DeviceListener *listener, DeviceState *dev);
> > +    void (*unrealize)(DeviceListener *listener, DeviceState *dev);
> > +    QTAILQ_ENTRY(DeviceListener) link;
> > +};
> > +
> >  #define TYPE_BUS "bus"
> >  #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
> >  #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
> > @@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
> >                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
> >      bus->allow_hotplug = 1;
> >  }
> > +
> > +void qdev_listener_register(DeviceListener *listener);
> > +void qdev_listener_unregister(DeviceListener *listener);
> > +
> >  #endif
> > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> > index 04df51b..e32bca2 100644
> > --- a/include/qemu/typedefs.h
> > +++ b/include/qemu/typedefs.h
> > @@ -20,6 +20,7 @@ typedef struct Property Property;
> >  typedef struct PropertyInfo PropertyInfo;
> >  typedef struct CompatProperty CompatProperty;
> >  typedef struct DeviceState DeviceState;
> > +typedef struct DeviceListener DeviceListener;
> >  typedef struct BusState BusState;
> >  typedef struct BusClass BusClass;
> >  
> > -- 
> > 1.7.10.4
> > 
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 13:40     ` Michael S. Tsirkin
@ 2014-12-03 13:55       ` Paolo Bonzini
  2014-12-03 14:11         ` Paul Durrant
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2014-12-03 13:55 UTC (permalink / raw)
  To: Michael S. Tsirkin, Stefano Stabellini
  Cc: Peter Crosthwaite, Thomas Huth, Markus Armbruster, qemu-devel,
	Christian Borntraeger, Paul Durrant, Igor Mammedov,
	Andreas Faerber



On 03/12/2014 14:40, Michael S. Tsirkin wrote:
> On Wed, Dec 03, 2014 at 12:29:43PM +0000, Stefano Stabellini wrote:
>> The second patch is already Acked.
>> You just need a review on this one to move forward, right?
>>
>> Andreas, Michael?
> 
> Looks like a generic qdev thing, nothing to do with me.
> 
>> On Wed, 3 Dec 2014, Paul Durrant wrote:
>>> The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
>>> models explicitly register with Xen for config space accesses. This patch
>>> adds a listener interface into qdev-core which can be used by the Xen
>>> interface code to monitor for arrival and departure of PCI devices.
>>>
>>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Andreas Faerber" <afaerber@suse.de>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>> Cc: Markus Armbruster <armbru@redhat.com>
>>> Cc: Thomas Huth <thuth@linux.vnet.ibm.com>
>>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>>  hw/core/qdev.c          |   54 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  include/hw/qdev-core.h  |   10 +++++++++
>>>  include/qemu/typedefs.h |    1 +
>>>  3 files changed, 65 insertions(+)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index fcb1638..4a9c1f6 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
>>>      return 0;
>>>  }
>>>  
>>> +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
>>> +    = QTAILQ_HEAD_INITIALIZER(qdev_listeners);
>>> +
>>> +enum ListenerDirection { Forward, Reverse };
>>> +
>>> +#define QDEV_LISTENER_CALL(_callback, _direction, _args...)     \
>>> +    do {                                                        \
>>> +        DeviceListener *_listener;                              \
>>> +                                                                \
>>> +        switch (_direction) {                                   \
>>> +        case Forward:                                           \
>>> +            QTAILQ_FOREACH(_listener, &qdev_listeners, link) {  \
>>> +                if (_listener->_callback) {                     \
>>> +                    _listener->_callback(_listener, ##_args);   \
>>> +                }                                               \
>>> +            }                                                   \
>>> +            break;                                              \
>>> +        case Reverse:                                           \
>>> +            QTAILQ_FOREACH_REVERSE(_listener, &qdev_listeners,  \
>>> +                                   qdev_listeners, link) {      \
>>> +                if (_listener->_callback) {                     \
>>> +                    _listener->_callback(_listener, ##_args);   \
>>> +                }                                               \
>>> +            }                                                   \
>>> +            break;                                              \
>>> +        default:                                                \
>>> +            abort();                                            \
>>> +        }                                                       \
>>> +    } while (0)
>>> +
>>> +static int qdev_listener_add(DeviceState *dev, void *opaque)
>>> +{
>>> +    QDEV_LISTENER_CALL(realize, Forward, dev);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void qdev_listener_register(DeviceListener *listener)
>>> +{
>>> +    QTAILQ_INSERT_TAIL(&qdev_listeners, listener, link);
>>> +
>>> +    qbus_walk_children(sysbus_get_default(), NULL, NULL, qdev_listener_add,
>>> +                       NULL, NULL);
>>> +}
>>> +
>>> +void qdev_listener_unregister(DeviceListener *listener)
>>> +{
>>> +    QTAILQ_REMOVE(&qdev_listeners, listener, link);
>>> +}
>>> +
>>>  static void device_realize(DeviceState *dev, Error **errp)
>>>  {
>>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>> @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev, Error **errp)
>>>              return;
>>>          }
>>>      }
>>> +
>>> +    QDEV_LISTENER_CALL(realize, Forward, dev);
>>>  }
>>>  
>>>  static void device_unrealize(DeviceState *dev, Error **errp)
>>>  {
>>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
>>>  
>>> +    QDEV_LISTENER_CALL(unrealize, Reverse, dev);

These need to be in device_set_realized.  device_realize and
device_unrealize are just for backwards-compatibility to devices that
still use init/exit.

Also, this has to be call to be _after_ unrealization, i.e. after
setting dev->pending_deleted_event in device_set_realize.

Paolo

>>>      if (dc->exit) {
>>>          int rc = dc->exit(dev);
>>>          if (rc < 0) {
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index 178fee2..f2dc267 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -167,6 +167,12 @@ struct DeviceState {
>>>      int alias_required_for_version;
>>>  };
>>>  
>>> +struct DeviceListener {
>>> +    void (*realize)(DeviceListener *listener, DeviceState *dev);
>>> +    void (*unrealize)(DeviceListener *listener, DeviceState *dev);
>>> +    QTAILQ_ENTRY(DeviceListener) link;
>>> +};
>>> +
>>>  #define TYPE_BUS "bus"
>>>  #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
>>>  #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
>>> @@ -368,4 +374,8 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
>>>                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
>>>      bus->allow_hotplug = 1;
>>>  }
>>> +
>>> +void qdev_listener_register(DeviceListener *listener);
>>> +void qdev_listener_unregister(DeviceListener *listener);
>>> +
>>>  #endif
>>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>>> index 04df51b..e32bca2 100644
>>> --- a/include/qemu/typedefs.h
>>> +++ b/include/qemu/typedefs.h
>>> @@ -20,6 +20,7 @@ typedef struct Property Property;
>>>  typedef struct PropertyInfo PropertyInfo;
>>>  typedef struct CompatProperty CompatProperty;
>>>  typedef struct DeviceState DeviceState;
>>> +typedef struct DeviceListener DeviceListener;
>>>  typedef struct BusState BusState;
>>>  typedef struct BusClass BusClass;
>>>  
>>> -- 
>>> 1.7.10.4
>>>
>>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 13:55       ` Paolo Bonzini
@ 2014-12-03 14:11         ` Paul Durrant
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Durrant @ 2014-12-03 14:11 UTC (permalink / raw)
  To: Paolo Bonzini, Michael S. Tsirkin, Stefano Stabellini
  Cc: Peter Crosthwaite, Thomas Huth, Markus Armbruster, qemu-devel,
	Christian Borntraeger, Igor Mammedov, Andreas Faerber

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: 03 December 2014 13:56
> To: Michael S. Tsirkin; Stefano Stabellini
> Cc: Paul Durrant; qemu-devel@nongnu.org; Peter Crosthwaite; Thomas
> Huth; Markus Armbruster; Christian Borntraeger; Igor Mammedov; Andreas
> Faerber
> Subject: Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener
> interface
> 
> 
> 
> On 03/12/2014 14:40, Michael S. Tsirkin wrote:
> > On Wed, Dec 03, 2014 at 12:29:43PM +0000, Stefano Stabellini wrote:
> >> The second patch is already Acked.
> >> You just need a review on this one to move forward, right?
> >>
> >> Andreas, Michael?
> >
> > Looks like a generic qdev thing, nothing to do with me.
> >
> >> On Wed, 3 Dec 2014, Paul Durrant wrote:
> >>> The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
> >>> models explicitly register with Xen for config space accesses. This patch
> >>> adds a listener interface into qdev-core which can be used by the Xen
> >>> interface code to monitor for arrival and departure of PCI devices.
> >>>
> >>> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> >>> Cc: Michael S. Tsirkin <mst@redhat.com>
> >>> Cc: Andreas Faerber" <afaerber@suse.de>
> >>> Cc: Paolo Bonzini <pbonzini@redhat.com>
> >>> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >>> Cc: Igor Mammedov <imammedo@redhat.com>
> >>> Cc: Markus Armbruster <armbru@redhat.com>
> >>> Cc: Thomas Huth <thuth@linux.vnet.ibm.com>
> >>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> >>> ---
> >>>  hw/core/qdev.c          |   54
> +++++++++++++++++++++++++++++++++++++++++++++++
> >>>  include/hw/qdev-core.h  |   10 +++++++++
> >>>  include/qemu/typedefs.h |    1 +
> >>>  3 files changed, 65 insertions(+)
> >>>
> >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >>> index fcb1638..4a9c1f6 100644
> >>> --- a/hw/core/qdev.c
> >>> +++ b/hw/core/qdev.c
> >>> @@ -175,6 +175,56 @@ int qdev_init(DeviceState *dev)
> >>>      return 0;
> >>>  }
> >>>
> >>> +static QTAILQ_HEAD(qdev_listeners, DeviceListener) qdev_listeners
> >>> +    = QTAILQ_HEAD_INITIALIZER(qdev_listeners);
> >>> +
> >>> +enum ListenerDirection { Forward, Reverse };
> >>> +
> >>> +#define QDEV_LISTENER_CALL(_callback, _direction, _args...)     \
> >>> +    do {                                                        \
> >>> +        DeviceListener *_listener;                              \
> >>> +                                                                \
> >>> +        switch (_direction) {                                   \
> >>> +        case Forward:                                           \
> >>> +            QTAILQ_FOREACH(_listener, &qdev_listeners, link) {  \
> >>> +                if (_listener->_callback) {                     \
> >>> +                    _listener->_callback(_listener, ##_args);   \
> >>> +                }                                               \
> >>> +            }                                                   \
> >>> +            break;                                              \
> >>> +        case Reverse:                                           \
> >>> +            QTAILQ_FOREACH_REVERSE(_listener, &qdev_listeners,  \
> >>> +                                   qdev_listeners, link) {      \
> >>> +                if (_listener->_callback) {                     \
> >>> +                    _listener->_callback(_listener, ##_args);   \
> >>> +                }                                               \
> >>> +            }                                                   \
> >>> +            break;                                              \
> >>> +        default:                                                \
> >>> +            abort();                                            \
> >>> +        }                                                       \
> >>> +    } while (0)
> >>> +
> >>> +static int qdev_listener_add(DeviceState *dev, void *opaque)
> >>> +{
> >>> +    QDEV_LISTENER_CALL(realize, Forward, dev);
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +void qdev_listener_register(DeviceListener *listener)
> >>> +{
> >>> +    QTAILQ_INSERT_TAIL(&qdev_listeners, listener, link);
> >>> +
> >>> +    qbus_walk_children(sysbus_get_default(), NULL, NULL,
> qdev_listener_add,
> >>> +                       NULL, NULL);
> >>> +}
> >>> +
> >>> +void qdev_listener_unregister(DeviceListener *listener)
> >>> +{
> >>> +    QTAILQ_REMOVE(&qdev_listeners, listener, link);
> >>> +}
> >>> +
> >>>  static void device_realize(DeviceState *dev, Error **errp)
> >>>  {
> >>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> >>> @@ -186,12 +236,16 @@ static void device_realize(DeviceState *dev,
> Error **errp)
> >>>              return;
> >>>          }
> >>>      }
> >>> +
> >>> +    QDEV_LISTENER_CALL(realize, Forward, dev);
> >>>  }
> >>>
> >>>  static void device_unrealize(DeviceState *dev, Error **errp)
> >>>  {
> >>>      DeviceClass *dc = DEVICE_GET_CLASS(dev);
> >>>
> >>> +    QDEV_LISTENER_CALL(unrealize, Reverse, dev);
> 
> These need to be in device_set_realized.  device_realize and
> device_unrealize are just for backwards-compatibility to devices that
> still use init/exit.
> 
> Also, this has to be call to be _after_ unrealization, i.e. after
> setting dev->pending_deleted_event in device_set_realize.
> 

Ok. Thanks. I'll move the calls.

  Paul

> Paolo
> 
> >>>      if (dc->exit) {
> >>>          int rc = dc->exit(dev);
> >>>          if (rc < 0) {
> >>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> >>> index 178fee2..f2dc267 100644
> >>> --- a/include/hw/qdev-core.h
> >>> +++ b/include/hw/qdev-core.h
> >>> @@ -167,6 +167,12 @@ struct DeviceState {
> >>>      int alias_required_for_version;
> >>>  };
> >>>
> >>> +struct DeviceListener {
> >>> +    void (*realize)(DeviceListener *listener, DeviceState *dev);
> >>> +    void (*unrealize)(DeviceListener *listener, DeviceState *dev);
> >>> +    QTAILQ_ENTRY(DeviceListener) link;
> >>> +};
> >>> +
> >>>  #define TYPE_BUS "bus"
> >>>  #define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
> >>>  #define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass),
> TYPE_BUS)
> >>> @@ -368,4 +374,8 @@ static inline void
> qbus_set_hotplug_handler(BusState *bus, DeviceState *handler,
> >>>                               QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
> >>>      bus->allow_hotplug = 1;
> >>>  }
> >>> +
> >>> +void qdev_listener_register(DeviceListener *listener);
> >>> +void qdev_listener_unregister(DeviceListener *listener);
> >>> +
> >>>  #endif
> >>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> >>> index 04df51b..e32bca2 100644
> >>> --- a/include/qemu/typedefs.h
> >>> +++ b/include/qemu/typedefs.h
> >>> @@ -20,6 +20,7 @@ typedef struct Property Property;
> >>>  typedef struct PropertyInfo PropertyInfo;
> >>>  typedef struct CompatProperty CompatProperty;
> >>>  typedef struct DeviceState DeviceState;
> >>> +typedef struct DeviceListener DeviceListener;
> >>>  typedef struct BusState BusState;
> >>>  typedef struct BusClass BusClass;
> >>>
> >>> --
> >>> 1.7.10.4
> >>>
> >>>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 12:29   ` Stefano Stabellini
  2014-12-03 13:40     ` Michael S. Tsirkin
@ 2014-12-03 14:26     ` Andreas Färber
  2014-12-05 10:28       ` Paul Durrant
  1 sibling, 1 reply; 9+ messages in thread
From: Andreas Färber @ 2014-12-03 14:26 UTC (permalink / raw)
  To: Stefano Stabellini, Paul Durrant
  Cc: Peter Crosthwaite, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, qemu-devel, Christian Borntraeger,
	Paolo Bonzini, Igor Mammedov

Am 03.12.2014 um 13:29 schrieb Stefano Stabellini:
> The second patch is already Acked.
> You just need a review on this one to move forward, right?
> 
> Andreas, Michael?

Once again, I would appreciate if such patches consistently used device_
/ DEVICE_ rather than qdev_ / QDEV_. In particular given that
DeviceListener is being used already and this doesn't seem to rely on
old qdev. But other than that no objections. Will be back in the office
next week.

Regards,
Andreas

-- 
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface
  2014-12-03 14:26     ` Andreas Färber
@ 2014-12-05 10:28       ` Paul Durrant
  0 siblings, 0 replies; 9+ messages in thread
From: Paul Durrant @ 2014-12-05 10:28 UTC (permalink / raw)
  To: Andreas Färber, Stefano Stabellini
  Cc: Peter Crosthwaite, Thomas Huth, Michael S. Tsirkin,
	Markus Armbruster, qemu-devel, Christian Borntraeger,
	Paolo Bonzini, Igor Mammedov

> -----Original Message-----
> From: Andreas Färber [mailto:afaerber@suse.de]
> Sent: 03 December 2014 14:27
> To: Stefano Stabellini; Paul Durrant
> Cc: qemu-devel@nongnu.org; Peter Crosthwaite; Thomas Huth; Michael S.
> Tsirkin; Markus Armbruster; Christian Borntraeger; Igor Mammedov; Paolo
> Bonzini
> Subject: Re: [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener
> interface
> 
> Am 03.12.2014 um 13:29 schrieb Stefano Stabellini:
> > The second patch is already Acked.
> > You just need a review on this one to move forward, right?
> >
> > Andreas, Michael?
> 
> Once again, I would appreciate if such patches consistently used device_
> / DEVICE_ rather than qdev_ / QDEV_. In particular given that
> DeviceListener is being used already and this doesn't seem to rely on
> old qdev. But other than that no objections. Will be back in the office
> next week.
> 

Ok. I'll fix the naming.

  Paul

> Regards,
> Andreas
> 
> --
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-12-05 10:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-03 11:03 [Qemu-devel] [PATCH v4 REPOST 2/2] Use ioreq-server API Paul Durrant
2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 1/2] Add device listener interface Paul Durrant
2014-12-03 12:29   ` Stefano Stabellini
2014-12-03 13:40     ` Michael S. Tsirkin
2014-12-03 13:55       ` Paolo Bonzini
2014-12-03 14:11         ` Paul Durrant
2014-12-03 14:26     ` Andreas Färber
2014-12-05 10:28       ` Paul Durrant
2014-12-03 11:03 ` [Qemu-devel] [PATCH v4 REPOST 2/2] Xen: Use the ioreq-server API when available Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.