All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
@ 2016-08-01  9:16 Paul Durrant
  2016-08-01  9:35 ` Paul Durrant
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-01  9:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel; +Cc: Paul Durrant, Stefano Stabellini, Anthony Perard

VMs created on older versions on Xen will not have been provisioned with
pages to support creation of non-default ioreq servers. In this case
the ioreq server API is not supported and QEMU's only option is to fall
back to using the default ioreq server pages as it did prior to
commit 3996e85c ("Xen: Use the ioreq-server API when available").

This patch therefore changes the code in xen_common.h to stop considering
a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
as an indication that the guest is too old to support the ioreq server
API. Instead a boolean is set to cause reversion to old behaviour such
that the default ioreq server is then used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
---
 include/hw/xen/xen_common.h | 123 +++++++++++++++++++++++++++++++-------------
 trace-events                |   1 +
 xen-hvm.c                   |   6 +--
 3 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 640c31e..8707adc 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -107,6 +107,42 @@ static inline int xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
 
 #endif
 
+static inline int xen_get_default_ioreq_server_info(xc_interface *xc, domid_t dom,
+                                                    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;
+}
+
 /* Xen before 4.5 */
 #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 450
 
@@ -154,10 +190,9 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
 {
 }
 
-static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
-                                          ioservid_t *ioservid)
+static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
+                                           ioservid_t *ioservid)
 {
-    return 0;
 }
 
 static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
@@ -171,35 +206,8 @@ static inline int xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
                                             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;
+    return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn, bufioreq_pfn,
+                                             bufioreq_evtchn);
 }
 
 static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
@@ -212,6 +220,8 @@ static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
 /* Xen 4.5 */
 #else
 
+static bool use_default_ioreq_server;
+
 static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
                                           ioservid_t ioservid,
                                           MemoryRegionSection *section)
@@ -220,6 +230,10 @@ static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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);
@@ -233,6 +247,11 @@ static inline void xen_unmap_memory_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
+
     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);
@@ -246,6 +265,11 @@ static inline void xen_map_io_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
+
     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);
@@ -259,6 +283,10 @@ static inline void xen_unmap_io_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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);
@@ -268,6 +296,10 @@ static inline void xen_map_pcidev(xc_interface *xc, domid_t dom,
                                   ioservid_t ioservid,
                                   PCIDevice *pci_dev)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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,
@@ -280,6 +312,10 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
                                     ioservid_t ioservid,
                                     PCIDevice *pci_dev)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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,
@@ -288,22 +324,29 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
                                           PCI_FUNC(pci_dev->devfn));
 }
 
-static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
-                                          ioservid_t *ioservid)
+static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
+                                           ioservid_t *ioservid)
 {
     int rc = xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC,
                                         ioservid);
 
     if (rc == 0) {
         trace_xen_ioreq_server_create(*ioservid);
+        return;
     }
 
-    return rc;
+    *ioservid = 0;
+    use_default_ioreq_server = true;
+    trace_xen_default_ioreq_server();
 }
 
 static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
                                             ioservid_t ioservid)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     trace_xen_ioreq_server_destroy(ioservid);
     xc_hvm_destroy_ioreq_server(xc, dom, ioservid);
 }
@@ -314,6 +357,12 @@ static inline int xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
                                             xen_pfn_t *bufioreq_pfn,
                                             evtchn_port_t *bufioreq_evtchn)
 {
+    if (use_default_ioreq_server) {
+        return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
+                                                 bufioreq_pfn,
+                                                 bufioreq_evtchn);
+    }
+
     return xc_hvm_get_ioreq_server_info(xc, dom, ioservid,
                                         ioreq_pfn, bufioreq_pfn,
                                         bufioreq_evtchn);
@@ -323,6 +372,10 @@ static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
                                              ioservid_t ioservid,
                                              bool enable)
 {
+    if (use_default_ioreq_server) {
+        return 0;
+    }
+
     trace_xen_ioreq_server_state(ioservid, enable);
     return xc_hvm_set_ioreq_server_state(xc, dom, ioservid, enable);
 }
diff --git a/trace-events b/trace-events
index 52c6a6c..616cc52 100644
--- a/trace-events
+++ b/trace-events
@@ -60,6 +60,7 @@ spice_vmc_event(int event) "spice vmc event %d"
 # 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_default_ioreq_server(void) ""
 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"
diff --git a/xen-hvm.c b/xen-hvm.c
index eb57792..cc3d4b0 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1203,11 +1203,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
         goto err;
     }
 
-    rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
-    if (rc < 0) {
-        perror("xen: ioreq server create");
-        goto err;
-    }
+    xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
 
     state->exit.notify = xen_exit_notifier;
     qemu_add_exit_notifier(&state->exit);
-- 
2.1.4

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-01  9:16 [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages Paul Durrant
  2016-08-01  9:35 ` Paul Durrant
@ 2016-08-01  9:35 ` Paul Durrant
  2016-08-09 15:19 ` Anthony PERARD
  2016-08-09 15:19 ` [Qemu-devel] " Anthony PERARD
  3 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-01  9:35 UTC (permalink / raw)
  To: Paul Durrant, xen-devel, qemu-devel
  Cc: Stefano Stabellini, Anthony Perard, Olaf Hering

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 01 August 2016 10:16
> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org
> Cc: Paul Durrant; Stefano Stabellini; Anthony Perard
> Subject: [PATCH] xen: handle inbound migration of VMs without ioreq server
> pages
> 
> VMs created on older versions on Xen will not have been provisioned with
> pages to support creation of non-default ioreq servers. In this case
> the ioreq server API is not supported and QEMU's only option is to fall
> back to using the default ioreq server pages as it did prior to
> commit 3996e85c ("Xen: Use the ioreq-server API when available").
> 
> This patch therefore changes the code in xen_common.h to stop considering
> a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> as an indication that the guest is too old to support the ioreq server
> API. Instead a boolean is set to cause reversion to old behaviour such
> that the default ioreq server is then used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>

Apologies, this should also be

Reported-by: Olaf Hering <olaf@aepfle.de>

> ---
>  include/hw/xen/xen_common.h | 123
> +++++++++++++++++++++++++++++++-------------
>  trace-events                |   1 +
>  xen-hvm.c                   |   6 +--
>  3 files changed, 90 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> index 640c31e..8707adc 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -107,6 +107,42 @@ static inline int
> xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
> 
>  #endif
> 
> +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> domid_t dom,
> +                                                    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;
> +}
> +
>  /* Xen before 4.5 */
>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 450
> 
> @@ -154,10 +190,9 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>  {
>  }
> 
> -static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> -                                          ioservid_t *ioservid)
> +static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> +                                           ioservid_t *ioservid)
>  {
> -    return 0;
>  }
> 
>  static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
> @@ -171,35 +206,8 @@ static inline int
> xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
>                                              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;
> +    return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
> bufioreq_pfn,
> +                                             bufioreq_evtchn);
>  }
> 
>  static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
> @@ -212,6 +220,8 @@ static inline int
> xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
>  /* Xen 4.5 */
>  #else
> 
> +static bool use_default_ioreq_server;
> +
>  static inline void xen_map_memory_section(xc_interface *xc, domid_t
> dom,
>                                            ioservid_t ioservid,
>                                            MemoryRegionSection *section)
> @@ -220,6 +230,10 @@ static inline void
> xen_map_memory_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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);
> @@ -233,6 +247,11 @@ static inline void
> xen_unmap_memory_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
> +
>      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);
> @@ -246,6 +265,11 @@ static inline void xen_map_io_section(xc_interface
> *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
> +
>      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);
> @@ -259,6 +283,10 @@ static inline void
> xen_unmap_io_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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);
> @@ -268,6 +296,10 @@ static inline void xen_map_pcidev(xc_interface *xc,
> domid_t dom,
>                                    ioservid_t ioservid,
>                                    PCIDevice *pci_dev)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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,
> @@ -280,6 +312,10 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>                                      ioservid_t ioservid,
>                                      PCIDevice *pci_dev)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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,
> @@ -288,22 +324,29 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>                                            PCI_FUNC(pci_dev->devfn));
>  }
> 
> -static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> -                                          ioservid_t *ioservid)
> +static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> +                                           ioservid_t *ioservid)
>  {
>      int rc = xc_hvm_create_ioreq_server(xc, dom,
> HVM_IOREQSRV_BUFIOREQ_ATOMIC,
>                                          ioservid);
> 
>      if (rc == 0) {
>          trace_xen_ioreq_server_create(*ioservid);
> +        return;
>      }
> 
> -    return rc;
> +    *ioservid = 0;
> +    use_default_ioreq_server = true;
> +    trace_xen_default_ioreq_server();
>  }
> 
>  static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
>                                              ioservid_t ioservid)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      trace_xen_ioreq_server_destroy(ioservid);
>      xc_hvm_destroy_ioreq_server(xc, dom, ioservid);
>  }
> @@ -314,6 +357,12 @@ static inline int
> xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
>                                              xen_pfn_t *bufioreq_pfn,
>                                              evtchn_port_t *bufioreq_evtchn)
>  {
> +    if (use_default_ioreq_server) {
> +        return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
> +                                                 bufioreq_pfn,
> +                                                 bufioreq_evtchn);
> +    }
> +
>      return xc_hvm_get_ioreq_server_info(xc, dom, ioservid,
>                                          ioreq_pfn, bufioreq_pfn,
>                                          bufioreq_evtchn);
> @@ -323,6 +372,10 @@ static inline int
> xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
>                                               ioservid_t ioservid,
>                                               bool enable)
>  {
> +    if (use_default_ioreq_server) {
> +        return 0;
> +    }
> +
>      trace_xen_ioreq_server_state(ioservid, enable);
>      return xc_hvm_set_ioreq_server_state(xc, dom, ioservid, enable);
>  }
> diff --git a/trace-events b/trace-events
> index 52c6a6c..616cc52 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -60,6 +60,7 @@ spice_vmc_event(int event) "spice vmc event %d"
>  # 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_default_ioreq_server(void) ""
>  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"
> diff --git a/xen-hvm.c b/xen-hvm.c
> index eb57792..cc3d4b0 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1203,11 +1203,7 @@ void xen_hvm_init(PCMachineState *pcms,
> MemoryRegion **ram_memory)
>          goto err;
>      }
> 
> -    rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
> -    if (rc < 0) {
> -        perror("xen: ioreq server create");
> -        goto err;
> -    }
> +    xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
> 
>      state->exit.notify = xen_exit_notifier;
>      qemu_add_exit_notifier(&state->exit);
> --
> 2.1.4

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-01  9:16 [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages Paul Durrant
@ 2016-08-01  9:35 ` Paul Durrant
  2016-08-01  9:35 ` [Qemu-devel] " Paul Durrant
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-01  9:35 UTC (permalink / raw)
  To: Paul Durrant, xen-devel, qemu-devel
  Cc: Anthony Perard, Olaf Hering, Stefano Stabellini

> -----Original Message-----
> From: Paul Durrant [mailto:paul.durrant@citrix.com]
> Sent: 01 August 2016 10:16
> To: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org
> Cc: Paul Durrant; Stefano Stabellini; Anthony Perard
> Subject: [PATCH] xen: handle inbound migration of VMs without ioreq server
> pages
> 
> VMs created on older versions on Xen will not have been provisioned with
> pages to support creation of non-default ioreq servers. In this case
> the ioreq server API is not supported and QEMU's only option is to fall
> back to using the default ioreq server pages as it did prior to
> commit 3996e85c ("Xen: Use the ioreq-server API when available").
> 
> This patch therefore changes the code in xen_common.h to stop considering
> a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> as an indication that the guest is too old to support the ioreq server
> API. Instead a boolean is set to cause reversion to old behaviour such
> that the default ioreq server is then used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>

Apologies, this should also be

Reported-by: Olaf Hering <olaf@aepfle.de>

> ---
>  include/hw/xen/xen_common.h | 123
> +++++++++++++++++++++++++++++++-------------
>  trace-events                |   1 +
>  xen-hvm.c                   |   6 +--
>  3 files changed, 90 insertions(+), 40 deletions(-)
> 
> diff --git a/include/hw/xen/xen_common.h
> b/include/hw/xen/xen_common.h
> index 640c31e..8707adc 100644
> --- a/include/hw/xen/xen_common.h
> +++ b/include/hw/xen/xen_common.h
> @@ -107,6 +107,42 @@ static inline int
> xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
> 
>  #endif
> 
> +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> domid_t dom,
> +                                                    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;
> +}
> +
>  /* Xen before 4.5 */
>  #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 450
> 
> @@ -154,10 +190,9 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>  {
>  }
> 
> -static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> -                                          ioservid_t *ioservid)
> +static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> +                                           ioservid_t *ioservid)
>  {
> -    return 0;
>  }
> 
>  static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
> @@ -171,35 +206,8 @@ static inline int
> xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
>                                              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;
> +    return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
> bufioreq_pfn,
> +                                             bufioreq_evtchn);
>  }
> 
>  static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
> @@ -212,6 +220,8 @@ static inline int
> xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
>  /* Xen 4.5 */
>  #else
> 
> +static bool use_default_ioreq_server;
> +
>  static inline void xen_map_memory_section(xc_interface *xc, domid_t
> dom,
>                                            ioservid_t ioservid,
>                                            MemoryRegionSection *section)
> @@ -220,6 +230,10 @@ static inline void
> xen_map_memory_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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);
> @@ -233,6 +247,11 @@ static inline void
> xen_unmap_memory_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
> +
>      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);
> @@ -246,6 +265,11 @@ static inline void xen_map_io_section(xc_interface
> *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
> +
>      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);
> @@ -259,6 +283,10 @@ static inline void
> xen_unmap_io_section(xc_interface *xc, domid_t dom,
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr end_addr = start_addr + size - 1;
> 
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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);
> @@ -268,6 +296,10 @@ static inline void xen_map_pcidev(xc_interface *xc,
> domid_t dom,
>                                    ioservid_t ioservid,
>                                    PCIDevice *pci_dev)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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,
> @@ -280,6 +312,10 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>                                      ioservid_t ioservid,
>                                      PCIDevice *pci_dev)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      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,
> @@ -288,22 +324,29 @@ static inline void xen_unmap_pcidev(xc_interface
> *xc, domid_t dom,
>                                            PCI_FUNC(pci_dev->devfn));
>  }
> 
> -static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> -                                          ioservid_t *ioservid)
> +static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
> +                                           ioservid_t *ioservid)
>  {
>      int rc = xc_hvm_create_ioreq_server(xc, dom,
> HVM_IOREQSRV_BUFIOREQ_ATOMIC,
>                                          ioservid);
> 
>      if (rc == 0) {
>          trace_xen_ioreq_server_create(*ioservid);
> +        return;
>      }
> 
> -    return rc;
> +    *ioservid = 0;
> +    use_default_ioreq_server = true;
> +    trace_xen_default_ioreq_server();
>  }
> 
>  static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
>                                              ioservid_t ioservid)
>  {
> +    if (use_default_ioreq_server) {
> +        return;
> +    }
> +
>      trace_xen_ioreq_server_destroy(ioservid);
>      xc_hvm_destroy_ioreq_server(xc, dom, ioservid);
>  }
> @@ -314,6 +357,12 @@ static inline int
> xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
>                                              xen_pfn_t *bufioreq_pfn,
>                                              evtchn_port_t *bufioreq_evtchn)
>  {
> +    if (use_default_ioreq_server) {
> +        return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
> +                                                 bufioreq_pfn,
> +                                                 bufioreq_evtchn);
> +    }
> +
>      return xc_hvm_get_ioreq_server_info(xc, dom, ioservid,
>                                          ioreq_pfn, bufioreq_pfn,
>                                          bufioreq_evtchn);
> @@ -323,6 +372,10 @@ static inline int
> xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
>                                               ioservid_t ioservid,
>                                               bool enable)
>  {
> +    if (use_default_ioreq_server) {
> +        return 0;
> +    }
> +
>      trace_xen_ioreq_server_state(ioservid, enable);
>      return xc_hvm_set_ioreq_server_state(xc, dom, ioservid, enable);
>  }
> diff --git a/trace-events b/trace-events
> index 52c6a6c..616cc52 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -60,6 +60,7 @@ spice_vmc_event(int event) "spice vmc event %d"
>  # 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_default_ioreq_server(void) ""
>  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"
> diff --git a/xen-hvm.c b/xen-hvm.c
> index eb57792..cc3d4b0 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -1203,11 +1203,7 @@ void xen_hvm_init(PCMachineState *pcms,
> MemoryRegion **ram_memory)
>          goto err;
>      }
> 
> -    rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
> -    if (rc < 0) {
> -        perror("xen: ioreq server create");
> -        goto err;
> -    }
> +    xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
> 
>      state->exit.notify = xen_exit_notifier;
>      qemu_add_exit_notifier(&state->exit);
> --
> 2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-01  9:16 [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages Paul Durrant
                   ` (2 preceding siblings ...)
  2016-08-09 15:19 ` Anthony PERARD
@ 2016-08-09 15:19 ` Anthony PERARD
  2016-08-09 15:23   ` Paul Durrant
  2016-08-09 15:23   ` [Qemu-devel] " Paul Durrant
  3 siblings, 2 replies; 14+ messages in thread
From: Anthony PERARD @ 2016-08-09 15:19 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, qemu-devel, Stefano Stabellini

On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> VMs created on older versions on Xen will not have been provisioned with
> pages to support creation of non-default ioreq servers. In this case
> the ioreq server API is not supported and QEMU's only option is to fall
> back to using the default ioreq server pages as it did prior to
> commit 3996e85c ("Xen: Use the ioreq-server API when available").
> 
> This patch therefore changes the code in xen_common.h to stop considering
> a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> as an indication that the guest is too old to support the ioreq server
> API. Instead a boolean is set to cause reversion to old behaviour such
> that the default ioreq server is then used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>

There are just two lines too long:

WARNING: line over 80 characters
#31: FILE: include/hw/xen/xen_common.h:110:
+static inline int xen_get_default_ioreq_server_info(xc_interface *xc, domid_t dom,

WARNING: line over 80 characters
#34: FILE: include/hw/xen/xen_common.h:113:
+                                                    evtchn_port_t *bufioreq_evtchn)

With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-01  9:16 [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages Paul Durrant
  2016-08-01  9:35 ` Paul Durrant
  2016-08-01  9:35 ` [Qemu-devel] " Paul Durrant
@ 2016-08-09 15:19 ` Anthony PERARD
  2016-08-09 15:19 ` [Qemu-devel] " Anthony PERARD
  3 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2016-08-09 15:19 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Stefano Stabellini, qemu-devel

On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> VMs created on older versions on Xen will not have been provisioned with
> pages to support creation of non-default ioreq servers. In this case
> the ioreq server API is not supported and QEMU's only option is to fall
> back to using the default ioreq server pages as it did prior to
> commit 3996e85c ("Xen: Use the ioreq-server API when available").
> 
> This patch therefore changes the code in xen_common.h to stop considering
> a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> as an indication that the guest is too old to support the ioreq server
> API. Instead a boolean is set to cause reversion to old behaviour such
> that the default ioreq server is then used.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>

There are just two lines too long:

WARNING: line over 80 characters
#31: FILE: include/hw/xen/xen_common.h:110:
+static inline int xen_get_default_ioreq_server_info(xc_interface *xc, domid_t dom,

WARNING: line over 80 characters
#34: FILE: include/hw/xen/xen_common.h:113:
+                                                    evtchn_port_t *bufioreq_evtchn)

With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-09 15:19 ` [Qemu-devel] " Anthony PERARD
  2016-08-09 15:23   ` Paul Durrant
@ 2016-08-09 15:23   ` Paul Durrant
  2016-08-11 20:07     ` Stefano Stabellini
  2016-08-11 20:07     ` Stefano Stabellini
  1 sibling, 2 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-09 15:23 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, qemu-devel, Stefano Stabellini

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 09 August 2016 16:19
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> Stabellini
> Subject: Re: [PATCH] xen: handle inbound migration of VMs without ioreq
> server pages
> 
> On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > VMs created on older versions on Xen will not have been provisioned with
> > pages to support creation of non-default ioreq servers. In this case
> > the ioreq server API is not supported and QEMU's only option is to fall
> > back to using the default ioreq server pages as it did prior to
> > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> >
> > This patch therefore changes the code in xen_common.h to stop
> considering
> > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > as an indication that the guest is too old to support the ioreq server
> > API. Instead a boolean is set to cause reversion to old behaviour such
> > that the default ioreq server is then used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> There are just two lines too long:
> 
> WARNING: line over 80 characters
> #31: FILE: include/hw/xen/xen_common.h:110:
> +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> domid_t dom,
> 
> WARNING: line over 80 characters
> #34: FILE: include/hw/xen/xen_common.h:113:
> +                                                    evtchn_port_t *bufioreq_evtchn)
> 
> With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Thanks,
> 

Ok, I'll post v2 with those fixes and your ack. Thanks,

  Paul

> --
> Anthony PERARD

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-09 15:19 ` [Qemu-devel] " Anthony PERARD
@ 2016-08-09 15:23   ` Paul Durrant
  2016-08-09 15:23   ` [Qemu-devel] " Paul Durrant
  1 sibling, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-09 15:23 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, Stefano Stabellini, qemu-devel

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 09 August 2016 16:19
> To: Paul Durrant
> Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> Stabellini
> Subject: Re: [PATCH] xen: handle inbound migration of VMs without ioreq
> server pages
> 
> On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > VMs created on older versions on Xen will not have been provisioned with
> > pages to support creation of non-default ioreq servers. In this case
> > the ioreq server API is not supported and QEMU's only option is to fall
> > back to using the default ioreq server pages as it did prior to
> > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> >
> > This patch therefore changes the code in xen_common.h to stop
> considering
> > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > as an indication that the guest is too old to support the ioreq server
> > API. Instead a boolean is set to cause reversion to old behaviour such
> > that the default ioreq server is then used.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> There are just two lines too long:
> 
> WARNING: line over 80 characters
> #31: FILE: include/hw/xen/xen_common.h:110:
> +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> domid_t dom,
> 
> WARNING: line over 80 characters
> #34: FILE: include/hw/xen/xen_common.h:113:
> +                                                    evtchn_port_t *bufioreq_evtchn)
> 
> With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> Thanks,
> 

Ok, I'll post v2 with those fixes and your ack. Thanks,

  Paul

> --
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-09 15:23   ` [Qemu-devel] " Paul Durrant
@ 2016-08-11 20:07     ` Stefano Stabellini
  2016-08-12  8:55         ` Paul Durrant
  2016-08-11 20:07     ` Stefano Stabellini
  1 sibling, 1 reply; 14+ messages in thread
From: Stefano Stabellini @ 2016-08-11 20:07 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Anthony Perard, xen-devel, qemu-devel, Stefano Stabellini

On Tue, 9 Aug 2016, Paul Durrant wrote:
> > -----Original Message-----
> > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > Sent: 09 August 2016 16:19
> > To: Paul Durrant
> > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > Stabellini
> > Subject: Re: [PATCH] xen: handle inbound migration of VMs without ioreq
> > server pages
> > 
> > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > VMs created on older versions on Xen will not have been provisioned with
> > > pages to support creation of non-default ioreq servers. In this case
> > > the ioreq server API is not supported and QEMU's only option is to fall
> > > back to using the default ioreq server pages as it did prior to
> > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > >
> > > This patch therefore changes the code in xen_common.h to stop
> > considering
> > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > as an indication that the guest is too old to support the ioreq server
> > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > that the default ioreq server is then used.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > 
> > There are just two lines too long:
> > 
> > WARNING: line over 80 characters
> > #31: FILE: include/hw/xen/xen_common.h:110:
> > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > domid_t dom,
> > 
> > WARNING: line over 80 characters
> > #34: FILE: include/hw/xen/xen_common.h:113:
> > +                                                    evtchn_port_t *bufioreq_evtchn)
> > 
> > With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > Thanks,
> > 
> 
> Ok, I'll post v2 with those fixes and your ack. Thanks,

Thank you for fixing this bug and Thanks Anthony for the review.

I was about to commit it but my sense of style rebelled: I really don't
like all the if statements. Too many! Sorry for coming in so late in
commenting on a patch, I realize that it is unfair.

Would you be up for rewriting the fix so that instead of introducing

    if (use_default_ioreq_server) {
        return;
    }

in many functions, we turn xc_hvm_* calls into function pointers calls
that get set to the right behavior depending on the success of
xc_hvm_create_ioreq_server?


The call sites would be something like:

    ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
                                            start_addr, end_addr);

At boot time, if xc_hvm_create_ioreq_server returns error:

    ioreq_server = empty_stubs_functions;

else

    ioreq_server = useful_functions;


What do you guys think?

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-09 15:23   ` [Qemu-devel] " Paul Durrant
  2016-08-11 20:07     ` Stefano Stabellini
@ 2016-08-11 20:07     ` Stefano Stabellini
  1 sibling, 0 replies; 14+ messages in thread
From: Stefano Stabellini @ 2016-08-11 20:07 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Anthony Perard, xen-devel, Stefano Stabellini, qemu-devel

On Tue, 9 Aug 2016, Paul Durrant wrote:
> > -----Original Message-----
> > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > Sent: 09 August 2016 16:19
> > To: Paul Durrant
> > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > Stabellini
> > Subject: Re: [PATCH] xen: handle inbound migration of VMs without ioreq
> > server pages
> > 
> > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > VMs created on older versions on Xen will not have been provisioned with
> > > pages to support creation of non-default ioreq servers. In this case
> > > the ioreq server API is not supported and QEMU's only option is to fall
> > > back to using the default ioreq server pages as it did prior to
> > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > >
> > > This patch therefore changes the code in xen_common.h to stop
> > considering
> > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > as an indication that the guest is too old to support the ioreq server
> > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > that the default ioreq server is then used.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > 
> > There are just two lines too long:
> > 
> > WARNING: line over 80 characters
> > #31: FILE: include/hw/xen/xen_common.h:110:
> > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > domid_t dom,
> > 
> > WARNING: line over 80 characters
> > #34: FILE: include/hw/xen/xen_common.h:113:
> > +                                                    evtchn_port_t *bufioreq_evtchn)
> > 
> > With that fixed: Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> > 
> > Thanks,
> > 
> 
> Ok, I'll post v2 with those fixes and your ack. Thanks,

Thank you for fixing this bug and Thanks Anthony for the review.

I was about to commit it but my sense of style rebelled: I really don't
like all the if statements. Too many! Sorry for coming in so late in
commenting on a patch, I realize that it is unfair.

Would you be up for rewriting the fix so that instead of introducing

    if (use_default_ioreq_server) {
        return;
    }

in many functions, we turn xc_hvm_* calls into function pointers calls
that get set to the right behavior depending on the success of
xc_hvm_create_ioreq_server?


The call sites would be something like:

    ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
                                            start_addr, end_addr);

At boot time, if xc_hvm_create_ioreq_server returns error:

    ioreq_server = empty_stubs_functions;

else

    ioreq_server = useful_functions;


What do you guys think?

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-11 20:07     ` Stefano Stabellini
@ 2016-08-12  8:55         ` Paul Durrant
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-12  8:55 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Anthony Perard, xen-devel, qemu-devel

> -----Original Message-----
> From: Stefano Stabellini [mailto:sstabellini@kernel.org]
> Sent: 11 August 2016 21:07
> To: Paul Durrant
> Cc: Anthony Perard; xen-devel@lists.xenproject.org; qemu-
> devel@nongnu.org; Stefano Stabellini
> Subject: RE: [PATCH] xen: handle inbound migration of VMs without ioreq
> server pages
> 
> On Tue, 9 Aug 2016, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > > Sent: 09 August 2016 16:19
> > > To: Paul Durrant
> > > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > > Stabellini
> > > Subject: Re: [PATCH] xen: handle inbound migration of VMs without
> ioreq
> > > server pages
> > >
> > > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > > VMs created on older versions on Xen will not have been provisioned
> with
> > > > pages to support creation of non-default ioreq servers. In this case
> > > > the ioreq server API is not supported and QEMU's only option is to fall
> > > > back to using the default ioreq server pages as it did prior to
> > > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > > >
> > > > This patch therefore changes the code in xen_common.h to stop
> > > considering
> > > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > > as an indication that the guest is too old to support the ioreq server
> > > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > > that the default ioreq server is then used.
> > > >
> > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > >
> > > There are just two lines too long:
> > >
> > > WARNING: line over 80 characters
> > > #31: FILE: include/hw/xen/xen_common.h:110:
> > > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > > domid_t dom,
> > >
> > > WARNING: line over 80 characters
> > > #34: FILE: include/hw/xen/xen_common.h:113:
> > > +                                                    evtchn_port_t *bufioreq_evtchn)
> > >
> > > With that fixed: Acked-by: Anthony PERARD
> <anthony.perard@citrix.com>
> > >
> > > Thanks,
> > >
> >
> > Ok, I'll post v2 with those fixes and your ack. Thanks,
> 
> Thank you for fixing this bug and Thanks Anthony for the review.
> 
> I was about to commit it but my sense of style rebelled: I really don't
> like all the if statements. Too many! Sorry for coming in so late in
> commenting on a patch, I realize that it is unfair.
> 
> Would you be up for rewriting the fix so that instead of introducing
> 
>     if (use_default_ioreq_server) {
>         return;
>     }
> 
> in many functions, we turn xc_hvm_* calls into function pointers calls
> that get set to the right behavior depending on the success of
> xc_hvm_create_ioreq_server?
> 
> 
> The call sites would be something like:
> 
>     ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
>                                             start_addr, end_addr);
> 
> At boot time, if xc_hvm_create_ioreq_server returns error:
> 
>     ioreq_server = empty_stubs_functions;
> 
> else
> 
>     ioreq_server = useful_functions;
> 
> 
> What do you guys think?

Personally I don't think it's worth it. This is not a performance critical codepath but if you insist I will re-factor the code.

  Paul

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
@ 2016-08-12  8:55         ` Paul Durrant
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-12  8:55 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Anthony Perard, xen-devel, qemu-devel

> -----Original Message-----
> From: Stefano Stabellini [mailto:sstabellini@kernel.org]
> Sent: 11 August 2016 21:07
> To: Paul Durrant
> Cc: Anthony Perard; xen-devel@lists.xenproject.org; qemu-
> devel@nongnu.org; Stefano Stabellini
> Subject: RE: [PATCH] xen: handle inbound migration of VMs without ioreq
> server pages
> 
> On Tue, 9 Aug 2016, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > > Sent: 09 August 2016 16:19
> > > To: Paul Durrant
> > > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > > Stabellini
> > > Subject: Re: [PATCH] xen: handle inbound migration of VMs without
> ioreq
> > > server pages
> > >
> > > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > > VMs created on older versions on Xen will not have been provisioned
> with
> > > > pages to support creation of non-default ioreq servers. In this case
> > > > the ioreq server API is not supported and QEMU's only option is to fall
> > > > back to using the default ioreq server pages as it did prior to
> > > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > > >
> > > > This patch therefore changes the code in xen_common.h to stop
> > > considering
> > > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > > as an indication that the guest is too old to support the ioreq server
> > > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > > that the default ioreq server is then used.
> > > >
> > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > >
> > > There are just two lines too long:
> > >
> > > WARNING: line over 80 characters
> > > #31: FILE: include/hw/xen/xen_common.h:110:
> > > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > > domid_t dom,
> > >
> > > WARNING: line over 80 characters
> > > #34: FILE: include/hw/xen/xen_common.h:113:
> > > +                                                    evtchn_port_t *bufioreq_evtchn)
> > >
> > > With that fixed: Acked-by: Anthony PERARD
> <anthony.perard@citrix.com>
> > >
> > > Thanks,
> > >
> >
> > Ok, I'll post v2 with those fixes and your ack. Thanks,
> 
> Thank you for fixing this bug and Thanks Anthony for the review.
> 
> I was about to commit it but my sense of style rebelled: I really don't
> like all the if statements. Too many! Sorry for coming in so late in
> commenting on a patch, I realize that it is unfair.
> 
> Would you be up for rewriting the fix so that instead of introducing
> 
>     if (use_default_ioreq_server) {
>         return;
>     }
> 
> in many functions, we turn xc_hvm_* calls into function pointers calls
> that get set to the right behavior depending on the success of
> xc_hvm_create_ioreq_server?
> 
> 
> The call sites would be something like:
> 
>     ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
>                                             start_addr, end_addr);
> 
> At boot time, if xc_hvm_create_ioreq_server returns error:
> 
>     ioreq_server = empty_stubs_functions;
> 
> else
> 
>     ioreq_server = useful_functions;
> 
> 
> What do you guys think?

Personally I don't think it's worth it. This is not a performance critical codepath but if you insist I will re-factor the code.

  Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* Re: [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-12  8:55         ` Paul Durrant
  (?)
  (?)
@ 2016-08-12 22:09         ` Stefano Stabellini
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefano Stabellini @ 2016-08-12 22:09 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Stefano Stabellini, Anthony Perard, xen-devel, qemu-devel

On Fri, 12 Aug 2016, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:sstabellini@kernel.org]
> > Sent: 11 August 2016 21:07
> > To: Paul Durrant
> > Cc: Anthony Perard; xen-devel@lists.xenproject.org; qemu-
> > devel@nongnu.org; Stefano Stabellini
> > Subject: RE: [PATCH] xen: handle inbound migration of VMs without ioreq
> > server pages
> > 
> > On Tue, 9 Aug 2016, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > > > Sent: 09 August 2016 16:19
> > > > To: Paul Durrant
> > > > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > > > Stabellini
> > > > Subject: Re: [PATCH] xen: handle inbound migration of VMs without
> > ioreq
> > > > server pages
> > > >
> > > > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > > > VMs created on older versions on Xen will not have been provisioned
> > with
> > > > > pages to support creation of non-default ioreq servers. In this case
> > > > > the ioreq server API is not supported and QEMU's only option is to fall
> > > > > back to using the default ioreq server pages as it did prior to
> > > > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > > > >
> > > > > This patch therefore changes the code in xen_common.h to stop
> > > > considering
> > > > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > > > as an indication that the guest is too old to support the ioreq server
> > > > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > > > that the default ioreq server is then used.
> > > > >
> > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > > >
> > > > There are just two lines too long:
> > > >
> > > > WARNING: line over 80 characters
> > > > #31: FILE: include/hw/xen/xen_common.h:110:
> > > > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > > > domid_t dom,
> > > >
> > > > WARNING: line over 80 characters
> > > > #34: FILE: include/hw/xen/xen_common.h:113:
> > > > +                                                    evtchn_port_t *bufioreq_evtchn)
> > > >
> > > > With that fixed: Acked-by: Anthony PERARD
> > <anthony.perard@citrix.com>
> > > >
> > > > Thanks,
> > > >
> > >
> > > Ok, I'll post v2 with those fixes and your ack. Thanks,
> > 
> > Thank you for fixing this bug and Thanks Anthony for the review.
> > 
> > I was about to commit it but my sense of style rebelled: I really don't
> > like all the if statements. Too many! Sorry for coming in so late in
> > commenting on a patch, I realize that it is unfair.
> > 
> > Would you be up for rewriting the fix so that instead of introducing
> > 
> >     if (use_default_ioreq_server) {
> >         return;
> >     }
> > 
> > in many functions, we turn xc_hvm_* calls into function pointers calls
> > that get set to the right behavior depending on the success of
> > xc_hvm_create_ioreq_server?
> > 
> > 
> > The call sites would be something like:
> > 
> >     ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
> >                                             start_addr, end_addr);
> > 
> > At boot time, if xc_hvm_create_ioreq_server returns error:
> > 
> >     ioreq_server = empty_stubs_functions;
> > 
> > else
> > 
> >     ioreq_server = useful_functions;
> > 
> > 
> > What do you guys think?
> 
> Personally I don't think it's worth it. This is not a performance critical codepath but if you insist I will re-factor the code.

All right, given that Anthony already acked it, it's two vs. one. I'll
commit it as is.

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

* Re: [PATCH] xen: handle inbound migration of VMs without ioreq server pages
  2016-08-12  8:55         ` Paul Durrant
  (?)
@ 2016-08-12 22:09         ` Stefano Stabellini
  -1 siblings, 0 replies; 14+ messages in thread
From: Stefano Stabellini @ 2016-08-12 22:09 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Anthony Perard, xen-devel, Stefano Stabellini, qemu-devel

On Fri, 12 Aug 2016, Paul Durrant wrote:
> > -----Original Message-----
> > From: Stefano Stabellini [mailto:sstabellini@kernel.org]
> > Sent: 11 August 2016 21:07
> > To: Paul Durrant
> > Cc: Anthony Perard; xen-devel@lists.xenproject.org; qemu-
> > devel@nongnu.org; Stefano Stabellini
> > Subject: RE: [PATCH] xen: handle inbound migration of VMs without ioreq
> > server pages
> > 
> > On Tue, 9 Aug 2016, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> > > > Sent: 09 August 2016 16:19
> > > > To: Paul Durrant
> > > > Cc: xen-devel@lists.xenproject.org; qemu-devel@nongnu.org; Stefano
> > > > Stabellini
> > > > Subject: Re: [PATCH] xen: handle inbound migration of VMs without
> > ioreq
> > > > server pages
> > > >
> > > > On Mon, Aug 01, 2016 at 10:16:25AM +0100, Paul Durrant wrote:
> > > > > VMs created on older versions on Xen will not have been provisioned
> > with
> > > > > pages to support creation of non-default ioreq servers. In this case
> > > > > the ioreq server API is not supported and QEMU's only option is to fall
> > > > > back to using the default ioreq server pages as it did prior to
> > > > > commit 3996e85c ("Xen: Use the ioreq-server API when available").
> > > > >
> > > > > This patch therefore changes the code in xen_common.h to stop
> > > > considering
> > > > > a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
> > > > > as an indication that the guest is too old to support the ioreq server
> > > > > API. Instead a boolean is set to cause reversion to old behaviour such
> > > > > that the default ioreq server is then used.
> > > > >
> > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > > > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > > > > Cc: Anthony Perard <anthony.perard@citrix.com>
> > > >
> > > > There are just two lines too long:
> > > >
> > > > WARNING: line over 80 characters
> > > > #31: FILE: include/hw/xen/xen_common.h:110:
> > > > +static inline int xen_get_default_ioreq_server_info(xc_interface *xc,
> > > > domid_t dom,
> > > >
> > > > WARNING: line over 80 characters
> > > > #34: FILE: include/hw/xen/xen_common.h:113:
> > > > +                                                    evtchn_port_t *bufioreq_evtchn)
> > > >
> > > > With that fixed: Acked-by: Anthony PERARD
> > <anthony.perard@citrix.com>
> > > >
> > > > Thanks,
> > > >
> > >
> > > Ok, I'll post v2 with those fixes and your ack. Thanks,
> > 
> > Thank you for fixing this bug and Thanks Anthony for the review.
> > 
> > I was about to commit it but my sense of style rebelled: I really don't
> > like all the if statements. Too many! Sorry for coming in so late in
> > commenting on a patch, I realize that it is unfair.
> > 
> > Would you be up for rewriting the fix so that instead of introducing
> > 
> >     if (use_default_ioreq_server) {
> >         return;
> >     }
> > 
> > in many functions, we turn xc_hvm_* calls into function pointers calls
> > that get set to the right behavior depending on the success of
> > xc_hvm_create_ioreq_server?
> > 
> > 
> > The call sites would be something like:
> > 
> >     ioreq_server->unmap_io_range_from_ioreq_server(xc, dom, ioservid, 0,
> >                                             start_addr, end_addr);
> > 
> > At boot time, if xc_hvm_create_ioreq_server returns error:
> > 
> >     ioreq_server = empty_stubs_functions;
> > 
> > else
> > 
> >     ioreq_server = useful_functions;
> > 
> > 
> > What do you guys think?
> 
> Personally I don't think it's worth it. This is not a performance critical codepath but if you insist I will re-factor the code.

All right, given that Anthony already acked it, it's two vs. one. I'll
commit it as is.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

* [PATCH] xen: handle inbound migration of VMs without ioreq server pages
@ 2016-08-01  9:16 Paul Durrant
  0 siblings, 0 replies; 14+ messages in thread
From: Paul Durrant @ 2016-08-01  9:16 UTC (permalink / raw)
  To: xen-devel, qemu-devel; +Cc: Anthony Perard, Paul Durrant, Stefano Stabellini

VMs created on older versions on Xen will not have been provisioned with
pages to support creation of non-default ioreq servers. In this case
the ioreq server API is not supported and QEMU's only option is to fall
back to using the default ioreq server pages as it did prior to
commit 3996e85c ("Xen: Use the ioreq-server API when available").

This patch therefore changes the code in xen_common.h to stop considering
a failure of xc_hvm_create_ioreq_server() as a hard failure but simply
as an indication that the guest is too old to support the ioreq server
API. Instead a boolean is set to cause reversion to old behaviour such
that the default ioreq server is then used.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
---
 include/hw/xen/xen_common.h | 123 +++++++++++++++++++++++++++++++-------------
 trace-events                |   1 +
 xen-hvm.c                   |   6 +--
 3 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 640c31e..8707adc 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -107,6 +107,42 @@ static inline int xen_get_vmport_regs_pfn(xc_interface *xc, domid_t dom,
 
 #endif
 
+static inline int xen_get_default_ioreq_server_info(xc_interface *xc, domid_t dom,
+                                                    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;
+}
+
 /* Xen before 4.5 */
 #if CONFIG_XEN_CTRL_INTERFACE_VERSION < 450
 
@@ -154,10 +190,9 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
 {
 }
 
-static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
-                                          ioservid_t *ioservid)
+static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
+                                           ioservid_t *ioservid)
 {
-    return 0;
 }
 
 static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
@@ -171,35 +206,8 @@ static inline int xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
                                             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;
+    return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn, bufioreq_pfn,
+                                             bufioreq_evtchn);
 }
 
 static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
@@ -212,6 +220,8 @@ static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
 /* Xen 4.5 */
 #else
 
+static bool use_default_ioreq_server;
+
 static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
                                           ioservid_t ioservid,
                                           MemoryRegionSection *section)
@@ -220,6 +230,10 @@ static inline void xen_map_memory_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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);
@@ -233,6 +247,11 @@ static inline void xen_unmap_memory_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
+
     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);
@@ -246,6 +265,11 @@ static inline void xen_map_io_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
+
     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);
@@ -259,6 +283,10 @@ static inline void xen_unmap_io_section(xc_interface *xc, domid_t dom,
     ram_addr_t size = int128_get64(section->size);
     hwaddr end_addr = start_addr + size - 1;
 
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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);
@@ -268,6 +296,10 @@ static inline void xen_map_pcidev(xc_interface *xc, domid_t dom,
                                   ioservid_t ioservid,
                                   PCIDevice *pci_dev)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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,
@@ -280,6 +312,10 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
                                     ioservid_t ioservid,
                                     PCIDevice *pci_dev)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     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,
@@ -288,22 +324,29 @@ static inline void xen_unmap_pcidev(xc_interface *xc, domid_t dom,
                                           PCI_FUNC(pci_dev->devfn));
 }
 
-static inline int xen_create_ioreq_server(xc_interface *xc, domid_t dom,
-                                          ioservid_t *ioservid)
+static inline void xen_create_ioreq_server(xc_interface *xc, domid_t dom,
+                                           ioservid_t *ioservid)
 {
     int rc = xc_hvm_create_ioreq_server(xc, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC,
                                         ioservid);
 
     if (rc == 0) {
         trace_xen_ioreq_server_create(*ioservid);
+        return;
     }
 
-    return rc;
+    *ioservid = 0;
+    use_default_ioreq_server = true;
+    trace_xen_default_ioreq_server();
 }
 
 static inline void xen_destroy_ioreq_server(xc_interface *xc, domid_t dom,
                                             ioservid_t ioservid)
 {
+    if (use_default_ioreq_server) {
+        return;
+    }
+
     trace_xen_ioreq_server_destroy(ioservid);
     xc_hvm_destroy_ioreq_server(xc, dom, ioservid);
 }
@@ -314,6 +357,12 @@ static inline int xen_get_ioreq_server_info(xc_interface *xc, domid_t dom,
                                             xen_pfn_t *bufioreq_pfn,
                                             evtchn_port_t *bufioreq_evtchn)
 {
+    if (use_default_ioreq_server) {
+        return xen_get_default_ioreq_server_info(xc, dom, ioreq_pfn,
+                                                 bufioreq_pfn,
+                                                 bufioreq_evtchn);
+    }
+
     return xc_hvm_get_ioreq_server_info(xc, dom, ioservid,
                                         ioreq_pfn, bufioreq_pfn,
                                         bufioreq_evtchn);
@@ -323,6 +372,10 @@ static inline int xen_set_ioreq_server_state(xc_interface *xc, domid_t dom,
                                              ioservid_t ioservid,
                                              bool enable)
 {
+    if (use_default_ioreq_server) {
+        return 0;
+    }
+
     trace_xen_ioreq_server_state(ioservid, enable);
     return xc_hvm_set_ioreq_server_state(xc, dom, ioservid, enable);
 }
diff --git a/trace-events b/trace-events
index 52c6a6c..616cc52 100644
--- a/trace-events
+++ b/trace-events
@@ -60,6 +60,7 @@ spice_vmc_event(int event) "spice vmc event %d"
 # 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_default_ioreq_server(void) ""
 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"
diff --git a/xen-hvm.c b/xen-hvm.c
index eb57792..cc3d4b0 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -1203,11 +1203,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
         goto err;
     }
 
-    rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
-    if (rc < 0) {
-        perror("xen: ioreq server create");
-        goto err;
-    }
+    xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
 
     state->exit.notify = xen_exit_notifier;
     qemu_add_exit_notifier(&state->exit);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

end of thread, other threads:[~2016-08-12 22:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-01  9:16 [Qemu-devel] [PATCH] xen: handle inbound migration of VMs without ioreq server pages Paul Durrant
2016-08-01  9:35 ` Paul Durrant
2016-08-01  9:35 ` [Qemu-devel] " Paul Durrant
2016-08-09 15:19 ` Anthony PERARD
2016-08-09 15:19 ` [Qemu-devel] " Anthony PERARD
2016-08-09 15:23   ` Paul Durrant
2016-08-09 15:23   ` [Qemu-devel] " Paul Durrant
2016-08-11 20:07     ` Stefano Stabellini
2016-08-12  8:55       ` Paul Durrant
2016-08-12  8:55         ` Paul Durrant
2016-08-12 22:09         ` Stefano Stabellini
2016-08-12 22:09         ` [Qemu-devel] " Stefano Stabellini
2016-08-11 20:07     ` Stefano Stabellini
2016-08-01  9:16 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.