All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup
@ 2018-05-04 19:25 Paul Durrant
  2018-05-04 19:26 ` [PATCH v3 1/8] xen_backend: add grant table helpers Paul Durrant
                   ` (14 more replies)
  0 siblings, 15 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:25 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Anthony Perard, Gerd Hoffmann, Greg Kurz,
	Jason Wang, Kevin Wolf, Max Reitz, Paolo Bonzini,
	Stefano Stabellini

The grant copy operation was added to libxengnttab in Xen 4.8.0 (released
nearly 18 months ago) but the xen_disk PV backend QEMU is still carrying
a significant amount of code purely to remain compatible with older
versions of Xen.

As can be inferred from the diff stats below, removing this support for
older versions of Xen from QEMU reduces the size of the xen_disk source by
around 320 lines (~25%).
 
This versionseries maintains compatibility with older Xen, and OS
not supporting the grant copy operation, by adding an emulation of it
into the xen_backend code. Thus xen_disk can be simplified without
regressing support for any environment. This series also performs
general cleanup of the code by introducing and consistently using
helper functions for calling into libxenttab.

Paul Durrant (8):
  xen_backend: add grant table helpers
  xen_disk: remove open-coded use of libxengnttab
  xen: remove other open-coded use of libxengnttab
  xen_backend: add an emulation of grant copy
  xen_disk: remove use of grant map/unmap
  xen_backend: make the xen_feature_grant_copy flag private
  xen_disk: use a single entry iovec
  xen_disk: be consistent with use of xendev and blkdev->xendev

 hw/9pfs/xen-9p-backend.c     |  32 ++-
 hw/block/xen_disk.c          | 614 +++++++------------------------------------
 hw/char/xen_console.c        |   9 +-
 hw/net/xen_nic.c             |  34 ++-
 hw/usb/xen-usb.c             |  37 ++-
 hw/xen/xen_backend.c         | 178 ++++++++++++-
 include/hw/xen/xen_backend.h |  34 ++-
 7 files changed, 351 insertions(+), 587 deletions(-)
---
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Greg Kurz <groug@kaod.org>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>

-- 
2.1.4

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

* [Qemu-devel] [PATCH v3 1/8] xen_backend: add grant table helpers
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
  2018-05-04 19:26 ` [PATCH v3 1/8] xen_backend: add grant table helpers Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-16 13:50     ` Anthony PERARD
  2018-05-04 19:26 ` [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab Paul Durrant
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard

This patch adds grant table helper functions to the xen_backend code to
localize error reporting and use of xen_domid.

The patch also defers the call to xengnttab_open() until just before the
initialise method in XenDevOps is invoked. This method is responsible for
mapping the shared ring. No prior method requires access to the grant table.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c         | 123 ++++++++++++++++++++++++++++++++++++++-----
 include/hw/xen/xen_backend.h |  33 ++++++++++++
 2 files changed, 144 insertions(+), 12 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 7445b50..50412d6 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state)
     return 0;
 }
 
+void xen_be_set_max_grant_refs(struct XenDevice *xendev,
+                               unsigned int nr_refs)
+{
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
+                      strerror(errno));
+    }
+}
+
+void *xen_be_map_grant_refs(struct XenDevice *xendev, uint32_t *refs,
+                            unsigned int nr_refs, int prot)
+{
+    void *ptr;
+
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    ptr = xengnttab_map_domain_grant_refs(xendev->gnttabdev, nr_refs,
+                                          xen_domid, refs, prot);
+    if (!ptr) {
+        xen_pv_printf(xendev, 0,
+                      "xengnttab_map_domain_grant_refs failed: %s\n",
+                      strerror(errno));
+    }
+
+    return ptr;
+}
+
+void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
+                             unsigned int nr_refs)
+{
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    if (xengnttab_unmap(xendev->gnttabdev, ptr, nr_refs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_unmap failed: %s\n",
+                      strerror(errno));
+    }
+}
+
+int xen_be_copy_grant_refs(struct XenDevice *xendev,
+                           bool to_domain,
+                           XenGrantCopySegment segs[],
+                           unsigned int nr_segs)
+{
+    xengnttab_grant_copy_segment_t *xengnttab_segs;
+    unsigned int i;
+    int rc;
+
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+        xengnttab_grant_copy_segment_t *xengnttab_seg = &xengnttab_segs[i];
+
+        if (to_domain) {
+            xengnttab_seg->flags = GNTCOPY_dest_gref;
+            xengnttab_seg->dest.foreign.domid = xen_domid;
+            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
+            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
+            xengnttab_seg->source.virt = seg->source.virt;
+        } else {
+            xengnttab_seg->flags = GNTCOPY_source_gref;
+            xengnttab_seg->source.foreign.domid = xen_domid;
+            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
+            xengnttab_seg->source.foreign.offset =
+                seg->source.foreign.offset;
+            xengnttab_seg->dest.virt = seg->dest.virt;
+        }
+
+        xengnttab_seg->len = seg->len;
+    }
+
+    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs, xengnttab_segs);
+
+    if (rc) {
+        xen_pv_printf(xendev, 0, "xengnttab_copy failed: %s\n",
+                      strerror(errno));
+    }
+
+    for (i = 0; i < nr_segs; i++) {
+        xengnttab_grant_copy_segment_t *xengnttab_seg =
+            &xengnttab_segs[i];
+
+        if (xengnttab_seg->status != GNTST_okay) {
+            xen_pv_printf(xendev, 0, "segment[%u] status: %d\n", i,
+                          xengnttab_seg->status);
+            rc = -1;
+        }
+    }
+
+    g_free(xengnttab_segs);
+    return rc;
+}
+
 /*
  * get xen backend device, allocate a new one if it doesn't exist.
  */
@@ -149,18 +246,6 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
     }
     qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
 
-    if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
-        xendev->gnttabdev = xengnttab_open(NULL, 0);
-        if (xendev->gnttabdev == NULL) {
-            xen_pv_printf(NULL, 0, "can't open gnttab device\n");
-            xenevtchn_close(xendev->evtchndev);
-            qdev_unplug(DEVICE(xendev), NULL);
-            return NULL;
-        }
-    } else {
-        xendev->gnttabdev = NULL;
-    }
-
     xen_pv_insert_xendev(xendev);
 
     if (xendev->ops->alloc) {
@@ -322,6 +407,16 @@ static int xen_be_try_initialise(struct XenDevice *xendev)
         }
     }
 
+    if (xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
+        xendev->gnttabdev = xengnttab_open(NULL, 0);
+        if (xendev->gnttabdev == NULL) {
+            xen_pv_printf(NULL, 0, "can't open gnttab device\n");
+            return -1;
+        }
+    } else {
+        xendev->gnttabdev = NULL;
+    }
+
     if (xendev->ops->initialise) {
         rc = xendev->ops->initialise(xendev);
     }
@@ -369,6 +464,10 @@ static void xen_be_disconnect(struct XenDevice *xendev, enum xenbus_state state)
         xendev->ops->disconnect) {
         xendev->ops->disconnect(xendev);
     }
+    if (xendev->gnttabdev) {
+        xengnttab_close(xendev->gnttabdev);
+        xendev->gnttabdev = NULL;
+    }
     if (xendev->be_state != state) {
         xen_be_set_state(xendev, state);
     }
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 3a27692..29bf1c3 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -42,6 +42,39 @@ void xen_be_register_common(void);
 int xen_be_register(const char *type, struct XenDevOps *ops);
 int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state);
 int xen_be_bind_evtchn(struct XenDevice *xendev);
+void xen_be_set_max_grant_refs(struct XenDevice *xendev,
+                               unsigned int nr_refs);
+void *xen_be_map_grant_refs(struct XenDevice *xendev, uint32_t *refs,
+                            unsigned int nr_refs, int prot);
+void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
+                             unsigned int nr_refs);
+
+typedef struct XenGrantCopySegment {
+    union {
+        void *virt;
+        struct {
+            uint32_t ref;
+            off_t offset;
+        } foreign;
+    } source, dest;
+    size_t len;
+} XenGrantCopySegment;
+
+int xen_be_copy_grant_refs(struct XenDevice *xendev,
+                           bool to_domain, XenGrantCopySegment segs[],
+                           unsigned int nr_segs);
+
+static inline void *xen_be_map_grant_ref(struct XenDevice *xendev,
+                                         uint32_t ref, int prot)
+{
+    return xen_be_map_grant_refs(xendev, &ref, 1, prot);
+}
+
+static inline void xen_be_unmap_grant_ref(struct XenDevice *xendev,
+                                          void *ptr)
+{
+    return xen_be_unmap_grant_refs(xendev, ptr, 1);
+}
 
 /* actual backend drivers */
 extern struct XenDevOps xen_console_ops;      /* xen_console.c     */
-- 
2.1.4

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

* [PATCH v3 1/8] xen_backend: add grant table helpers
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Paul Durrant, Stefano Stabellini

This patch adds grant table helper functions to the xen_backend code to
localize error reporting and use of xen_domid.

The patch also defers the call to xengnttab_open() until just before the
initialise method in XenDevOps is invoked. This method is responsible for
mapping the shared ring. No prior method requires access to the grant table.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c         | 123 ++++++++++++++++++++++++++++++++++++++-----
 include/hw/xen/xen_backend.h |  33 ++++++++++++
 2 files changed, 144 insertions(+), 12 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 7445b50..50412d6 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state)
     return 0;
 }
 
+void xen_be_set_max_grant_refs(struct XenDevice *xendev,
+                               unsigned int nr_refs)
+{
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
+                      strerror(errno));
+    }
+}
+
+void *xen_be_map_grant_refs(struct XenDevice *xendev, uint32_t *refs,
+                            unsigned int nr_refs, int prot)
+{
+    void *ptr;
+
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    ptr = xengnttab_map_domain_grant_refs(xendev->gnttabdev, nr_refs,
+                                          xen_domid, refs, prot);
+    if (!ptr) {
+        xen_pv_printf(xendev, 0,
+                      "xengnttab_map_domain_grant_refs failed: %s\n",
+                      strerror(errno));
+    }
+
+    return ptr;
+}
+
+void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
+                             unsigned int nr_refs)
+{
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    if (xengnttab_unmap(xendev->gnttabdev, ptr, nr_refs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_unmap failed: %s\n",
+                      strerror(errno));
+    }
+}
+
+int xen_be_copy_grant_refs(struct XenDevice *xendev,
+                           bool to_domain,
+                           XenGrantCopySegment segs[],
+                           unsigned int nr_segs)
+{
+    xengnttab_grant_copy_segment_t *xengnttab_segs;
+    unsigned int i;
+    int rc;
+
+    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
+
+    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+        xengnttab_grant_copy_segment_t *xengnttab_seg = &xengnttab_segs[i];
+
+        if (to_domain) {
+            xengnttab_seg->flags = GNTCOPY_dest_gref;
+            xengnttab_seg->dest.foreign.domid = xen_domid;
+            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
+            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
+            xengnttab_seg->source.virt = seg->source.virt;
+        } else {
+            xengnttab_seg->flags = GNTCOPY_source_gref;
+            xengnttab_seg->source.foreign.domid = xen_domid;
+            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
+            xengnttab_seg->source.foreign.offset =
+                seg->source.foreign.offset;
+            xengnttab_seg->dest.virt = seg->dest.virt;
+        }
+
+        xengnttab_seg->len = seg->len;
+    }
+
+    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs, xengnttab_segs);
+
+    if (rc) {
+        xen_pv_printf(xendev, 0, "xengnttab_copy failed: %s\n",
+                      strerror(errno));
+    }
+
+    for (i = 0; i < nr_segs; i++) {
+        xengnttab_grant_copy_segment_t *xengnttab_seg =
+            &xengnttab_segs[i];
+
+        if (xengnttab_seg->status != GNTST_okay) {
+            xen_pv_printf(xendev, 0, "segment[%u] status: %d\n", i,
+                          xengnttab_seg->status);
+            rc = -1;
+        }
+    }
+
+    g_free(xengnttab_segs);
+    return rc;
+}
+
 /*
  * get xen backend device, allocate a new one if it doesn't exist.
  */
@@ -149,18 +246,6 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev,
     }
     qemu_set_cloexec(xenevtchn_fd(xendev->evtchndev));
 
-    if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
-        xendev->gnttabdev = xengnttab_open(NULL, 0);
-        if (xendev->gnttabdev == NULL) {
-            xen_pv_printf(NULL, 0, "can't open gnttab device\n");
-            xenevtchn_close(xendev->evtchndev);
-            qdev_unplug(DEVICE(xendev), NULL);
-            return NULL;
-        }
-    } else {
-        xendev->gnttabdev = NULL;
-    }
-
     xen_pv_insert_xendev(xendev);
 
     if (xendev->ops->alloc) {
@@ -322,6 +407,16 @@ static int xen_be_try_initialise(struct XenDevice *xendev)
         }
     }
 
+    if (xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
+        xendev->gnttabdev = xengnttab_open(NULL, 0);
+        if (xendev->gnttabdev == NULL) {
+            xen_pv_printf(NULL, 0, "can't open gnttab device\n");
+            return -1;
+        }
+    } else {
+        xendev->gnttabdev = NULL;
+    }
+
     if (xendev->ops->initialise) {
         rc = xendev->ops->initialise(xendev);
     }
@@ -369,6 +464,10 @@ static void xen_be_disconnect(struct XenDevice *xendev, enum xenbus_state state)
         xendev->ops->disconnect) {
         xendev->ops->disconnect(xendev);
     }
+    if (xendev->gnttabdev) {
+        xengnttab_close(xendev->gnttabdev);
+        xendev->gnttabdev = NULL;
+    }
     if (xendev->be_state != state) {
         xen_be_set_state(xendev, state);
     }
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 3a27692..29bf1c3 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -42,6 +42,39 @@ void xen_be_register_common(void);
 int xen_be_register(const char *type, struct XenDevOps *ops);
 int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state);
 int xen_be_bind_evtchn(struct XenDevice *xendev);
+void xen_be_set_max_grant_refs(struct XenDevice *xendev,
+                               unsigned int nr_refs);
+void *xen_be_map_grant_refs(struct XenDevice *xendev, uint32_t *refs,
+                            unsigned int nr_refs, int prot);
+void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
+                             unsigned int nr_refs);
+
+typedef struct XenGrantCopySegment {
+    union {
+        void *virt;
+        struct {
+            uint32_t ref;
+            off_t offset;
+        } foreign;
+    } source, dest;
+    size_t len;
+} XenGrantCopySegment;
+
+int xen_be_copy_grant_refs(struct XenDevice *xendev,
+                           bool to_domain, XenGrantCopySegment segs[],
+                           unsigned int nr_segs);
+
+static inline void *xen_be_map_grant_ref(struct XenDevice *xendev,
+                                         uint32_t ref, int prot)
+{
+    return xen_be_map_grant_refs(xendev, &ref, 1, prot);
+}
+
+static inline void xen_be_unmap_grant_ref(struct XenDevice *xendev,
+                                          void *ptr)
+{
+    return xen_be_unmap_grant_refs(xendev, ptr, 1);
+}
 
 /* actual backend drivers */
 extern struct XenDevOps xen_console_ops;      /* xen_console.c     */
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (2 preceding siblings ...)
  2018-05-04 19:26 ` [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-16 14:02     ` Anthony PERARD
  2018-05-04 19:26 ` [PATCH v3 3/8] xen: remove other " Paul Durrant
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard, Kevin Wolf, Max Reitz

Now that helpers are present in xen_backend, this patch removes open-coded
calls to libxengnttab from the xen_disk code.

This patch also fixes one whitspace error in the assignment of the
XenDevOps initialise method.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - New in v2
---
 hw/block/xen_disk.c | 122 ++++++++++++++--------------------------------------
 1 file changed, 32 insertions(+), 90 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index f74fcd4..66ed2b7 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -68,7 +68,6 @@ struct ioreq {
     uint8_t             mapped;
 
     /* grant mapping */
-    uint32_t            domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int                 prot;
     void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
@@ -142,7 +141,6 @@ static void ioreq_reset(struct ioreq *ioreq)
     ioreq->presync = 0;
     ioreq->mapped = 0;
 
-    memset(ioreq->domids, 0, sizeof(ioreq->domids));
     memset(ioreq->refs, 0, sizeof(ioreq->refs));
     ioreq->prot = 0;
     memset(ioreq->page, 0, sizeof(ioreq->page));
@@ -168,16 +166,12 @@ static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
 static void destroy_grant(gpointer pgnt)
 {
     PersistentGrant *grant = pgnt;
-    xengnttab_handle *gnt = grant->blkdev->xendev.gnttabdev;
+    struct XenBlkDev *blkdev = grant->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
-    if (xengnttab_unmap(gnt, grant->page, 1) != 0) {
-        xen_pv_printf(&grant->blkdev->xendev, 0,
-                      "xengnttab_unmap failed: %s\n",
-                      strerror(errno));
-    }
+    xen_be_unmap_grant_ref(xendev, grant->page);
     grant->blkdev->persistent_gnt_count--;
-    xen_pv_printf(&grant->blkdev->xendev, 3,
-                  "unmapped grant %p\n", grant->page);
+    xen_pv_printf(xendev, 3, "unmapped grant %p\n", grant->page);
     g_free(grant);
 }
 
@@ -185,15 +179,10 @@ static void remove_persistent_region(gpointer data, gpointer dev)
 {
     PersistentRegion *region = data;
     struct XenBlkDev *blkdev = dev;
-    xengnttab_handle *gnt = blkdev->xendev.gnttabdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
-    if (xengnttab_unmap(gnt, region->addr, region->num) != 0) {
-        xen_pv_printf(&blkdev->xendev, 0,
-                      "xengnttab_unmap region %p failed: %s\n",
-                      region->addr, strerror(errno));
-    }
-    xen_pv_printf(&blkdev->xendev, 3,
-                  "unmapped grant region %p with %d pages\n",
+    xen_be_unmap_grant_refs(xendev, region->addr, region->num);
+    xen_pv_printf(xendev, 3, "unmapped grant region %p with %d pages\n",
                   region->addr, region->num);
     g_free(region);
 }
@@ -304,7 +293,6 @@ static int ioreq_parse(struct ioreq *ioreq)
             goto err;
         }
 
-        ioreq->domids[i] = blkdev->xendev.dom;
         ioreq->refs[i]   = ioreq->req.seg[i].gref;
 
         mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
@@ -324,7 +312,8 @@ err:
 
 static void ioreq_unmap(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     int i;
 
     if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
@@ -334,11 +323,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
         if (!ioreq->pages) {
             return;
         }
-        if (xengnttab_unmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                          "xengnttab_unmap failed: %s\n",
-                          strerror(errno));
-        }
+        xen_be_unmap_grant_refs(xendev, ioreq->pages, ioreq->num_unmap);
         ioreq->blkdev->cnt_map -= ioreq->num_unmap;
         ioreq->pages = NULL;
     } else {
@@ -346,11 +331,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
             if (!ioreq->page[i]) {
                 continue;
             }
-            if (xengnttab_unmap(gnt, ioreq->page[i], 1) != 0) {
-                xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                              "xengnttab_unmap failed: %s\n",
-                              strerror(errno));
-            }
+            xen_be_unmap_grant_ref(xendev, ioreq->page[i]);
             ioreq->blkdev->cnt_map--;
             ioreq->page[i] = NULL;
         }
@@ -360,14 +341,14 @@ static void ioreq_unmap(struct ioreq *ioreq)
 
 static int ioreq_map(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
-    uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, j, new_maps = 0;
     PersistentGrant *grant;
     PersistentRegion *region;
-    /* domids and refs variables will contain the information necessary
+    /* refs variable will contain the information necessary
      * to map the grants that are needed to fulfill this request.
      *
      * After mapping the needed grants, the page array will contain the
@@ -392,7 +373,6 @@ static int ioreq_map(struct ioreq *ioreq)
                     /* Add the grant to the list of grants that
                      * should be mapped
                      */
-                    domids[new_maps] = ioreq->domids[i];
                     refs[new_maps] = ioreq->refs[i];
                     page[i] = NULL;
                     new_maps++;
@@ -405,14 +385,13 @@ static int ioreq_map(struct ioreq *ioreq)
     } else {
         /* All grants in the request should be mapped */
         memcpy(refs, ioreq->refs, sizeof(refs));
-        memcpy(domids, ioreq->domids, sizeof(domids));
         memset(page, 0, sizeof(page));
         new_maps = ioreq->v.niov;
     }
 
     if (batch_maps && new_maps) {
-        ioreq->pages = xengnttab_map_grant_refs
-            (gnt, new_maps, domids, refs, ioreq->prot);
+        ioreq->pages = xen_be_map_grant_refs(xendev, refs, new_maps,
+                                             ioreq->prot);
         if (ioreq->pages == NULL) {
             xen_pv_printf(&ioreq->blkdev->xendev, 0,
                           "can't map %d grant refs (%s, %d maps)\n",
@@ -427,8 +406,8 @@ static int ioreq_map(struct ioreq *ioreq)
         ioreq->blkdev->cnt_map += new_maps;
     } else if (new_maps)  {
         for (i = 0; i < new_maps; i++) {
-            ioreq->page[i] = xengnttab_map_grant_ref
-                (gnt, domids[i], refs[i], ioreq->prot);
+            ioreq->page[i] = xen_be_map_grant_ref(xendev, refs[i],
+                                                  ioreq->prot);
             if (ioreq->page[i] == NULL) {
                 xen_pv_printf(&ioreq->blkdev->xendev, 0,
                               "can't map grant ref %d (%s, %d maps)\n",
@@ -529,10 +508,12 @@ static int ioreq_init_copy_buffers(struct ioreq *ioreq)
 
 static int ioreq_grant_copy(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
-    xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
+    XenGrantCopySegment segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, count, rc;
     int64_t file_blk = ioreq->blkdev->file_blk;
+    bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
 
     if (ioreq->v.niov == 0) {
         return 0;
@@ -541,16 +522,12 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     count = ioreq->v.niov;
 
     for (i = 0; i < count; i++) {
-        if (ioreq->req.operation == BLKIF_OP_READ) {
-            segs[i].flags = GNTCOPY_dest_gref;
+        if (to_domain) {
             segs[i].dest.foreign.ref = ioreq->refs[i];
-            segs[i].dest.foreign.domid = ioreq->domids[i];
             segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
             segs[i].source.virt = ioreq->v.iov[i].iov_base;
         } else {
-            segs[i].flags = GNTCOPY_source_gref;
             segs[i].source.foreign.ref = ioreq->refs[i];
-            segs[i].source.foreign.domid = ioreq->domids[i];
             segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
             segs[i].dest.virt = ioreq->v.iov[i].iov_base;
         }
@@ -558,7 +535,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
                        - ioreq->req.seg[i].first_sect + 1) * file_blk;
     }
 
-    rc = xengnttab_grant_copy(gnt, count, segs);
+    rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
 
     if (rc) {
         xen_pv_printf(&ioreq->blkdev->xendev, 0,
@@ -567,16 +544,6 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
         return -1;
     }
 
-    for (i = 0; i < count; i++) {
-        if (segs[i].status != GNTST_okay) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                          "failed to copy data %d for gref %d, domid %d\n",
-                          segs[i].status, ioreq->refs[i], ioreq->domids[i]);
-            ioreq->aio_errors++;
-            rc = -1;
-        }
-    }
-
     return rc;
 }
 #else
@@ -1085,7 +1052,6 @@ static int blk_connect(struct XenDevice *xendev)
     int order, ring_ref;
     unsigned int ring_size, max_grants;
     unsigned int i;
-    uint32_t *domids;
 
     trace_xen_disk_connect(xendev->name);
 
@@ -1247,31 +1213,11 @@ static int blk_connect(struct XenDevice *xendev)
     /* Add on the number needed for the ring pages */
     max_grants += blkdev->nr_ring_ref;
 
-    blkdev->xendev.gnttabdev = xengnttab_open(NULL, 0);
-    if (blkdev->xendev.gnttabdev == NULL) {
-        xen_pv_printf(xendev, 0, "xengnttab_open failed: %s\n",
-                      strerror(errno));
-        return -1;
-    }
-    if (xengnttab_set_max_grants(blkdev->xendev.gnttabdev, max_grants)) {
-        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
-                      strerror(errno));
-        return -1;
-    }
-
-    domids = g_new0(uint32_t, blkdev->nr_ring_ref);
-    for (i = 0; i < blkdev->nr_ring_ref; i++) {
-        domids[i] = blkdev->xendev.dom;
-    }
-
-    blkdev->sring = xengnttab_map_grant_refs(blkdev->xendev.gnttabdev,
-                                             blkdev->nr_ring_ref,
-                                             domids,
-                                             blkdev->ring_ref,
-                                             PROT_READ | PROT_WRITE);
-
-    g_free(domids);
+    xen_be_set_max_grant_refs(xendev, max_grants);
 
+    blkdev->sring = xen_be_map_grant_refs(xendev, blkdev->ring_ref,
+                                          blkdev->nr_ring_ref,
+                                          PROT_READ | PROT_WRITE);
     if (!blkdev->sring) {
         return -1;
     }
@@ -1344,8 +1290,8 @@ static void blk_disconnect(struct XenDevice *xendev)
     aio_context_release(blkdev->ctx);
 
     if (blkdev->sring) {
-        xengnttab_unmap(blkdev->xendev.gnttabdev, blkdev->sring,
-                        blkdev->nr_ring_ref);
+        xen_be_unmap_grant_refs(xendev, blkdev->sring,
+                                blkdev->nr_ring_ref);
         blkdev->cnt_map--;
         blkdev->sring = NULL;
     }
@@ -1369,11 +1315,6 @@ static void blk_disconnect(struct XenDevice *xendev)
         }
         blkdev->feature_persistent = false;
     }
-
-    if (blkdev->xendev.gnttabdev) {
-        xengnttab_close(blkdev->xendev.gnttabdev);
-        blkdev->xendev.gnttabdev = NULL;
-    }
 }
 
 static int blk_free(struct XenDevice *xendev)
@@ -1410,10 +1351,11 @@ static void blk_event(struct XenDevice *xendev)
 }
 
 struct XenDevOps xen_blkdev_ops = {
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .size       = sizeof(struct XenBlkDev),
     .alloc      = blk_alloc,
     .init       = blk_init,
-    .initialise    = blk_connect,
+    .initialise = blk_connect,
     .disconnect = blk_disconnect,
     .event      = blk_event,
     .free       = blk_free,
-- 
2.1.4

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

* [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
  2018-05-04 19:26 ` [PATCH v3 1/8] xen_backend: add grant table helpers Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Kevin Wolf, Paul Durrant, Stefano Stabellini, Max Reitz

Now that helpers are present in xen_backend, this patch removes open-coded
calls to libxengnttab from the xen_disk code.

This patch also fixes one whitspace error in the assignment of the
XenDevOps initialise method.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - New in v2
---
 hw/block/xen_disk.c | 122 ++++++++++++++--------------------------------------
 1 file changed, 32 insertions(+), 90 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index f74fcd4..66ed2b7 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -68,7 +68,6 @@ struct ioreq {
     uint8_t             mapped;
 
     /* grant mapping */
-    uint32_t            domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int                 prot;
     void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
@@ -142,7 +141,6 @@ static void ioreq_reset(struct ioreq *ioreq)
     ioreq->presync = 0;
     ioreq->mapped = 0;
 
-    memset(ioreq->domids, 0, sizeof(ioreq->domids));
     memset(ioreq->refs, 0, sizeof(ioreq->refs));
     ioreq->prot = 0;
     memset(ioreq->page, 0, sizeof(ioreq->page));
@@ -168,16 +166,12 @@ static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
 static void destroy_grant(gpointer pgnt)
 {
     PersistentGrant *grant = pgnt;
-    xengnttab_handle *gnt = grant->blkdev->xendev.gnttabdev;
+    struct XenBlkDev *blkdev = grant->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
-    if (xengnttab_unmap(gnt, grant->page, 1) != 0) {
-        xen_pv_printf(&grant->blkdev->xendev, 0,
-                      "xengnttab_unmap failed: %s\n",
-                      strerror(errno));
-    }
+    xen_be_unmap_grant_ref(xendev, grant->page);
     grant->blkdev->persistent_gnt_count--;
-    xen_pv_printf(&grant->blkdev->xendev, 3,
-                  "unmapped grant %p\n", grant->page);
+    xen_pv_printf(xendev, 3, "unmapped grant %p\n", grant->page);
     g_free(grant);
 }
 
@@ -185,15 +179,10 @@ static void remove_persistent_region(gpointer data, gpointer dev)
 {
     PersistentRegion *region = data;
     struct XenBlkDev *blkdev = dev;
-    xengnttab_handle *gnt = blkdev->xendev.gnttabdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
-    if (xengnttab_unmap(gnt, region->addr, region->num) != 0) {
-        xen_pv_printf(&blkdev->xendev, 0,
-                      "xengnttab_unmap region %p failed: %s\n",
-                      region->addr, strerror(errno));
-    }
-    xen_pv_printf(&blkdev->xendev, 3,
-                  "unmapped grant region %p with %d pages\n",
+    xen_be_unmap_grant_refs(xendev, region->addr, region->num);
+    xen_pv_printf(xendev, 3, "unmapped grant region %p with %d pages\n",
                   region->addr, region->num);
     g_free(region);
 }
@@ -304,7 +293,6 @@ static int ioreq_parse(struct ioreq *ioreq)
             goto err;
         }
 
-        ioreq->domids[i] = blkdev->xendev.dom;
         ioreq->refs[i]   = ioreq->req.seg[i].gref;
 
         mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
@@ -324,7 +312,8 @@ err:
 
 static void ioreq_unmap(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     int i;
 
     if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
@@ -334,11 +323,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
         if (!ioreq->pages) {
             return;
         }
-        if (xengnttab_unmap(gnt, ioreq->pages, ioreq->num_unmap) != 0) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                          "xengnttab_unmap failed: %s\n",
-                          strerror(errno));
-        }
+        xen_be_unmap_grant_refs(xendev, ioreq->pages, ioreq->num_unmap);
         ioreq->blkdev->cnt_map -= ioreq->num_unmap;
         ioreq->pages = NULL;
     } else {
@@ -346,11 +331,7 @@ static void ioreq_unmap(struct ioreq *ioreq)
             if (!ioreq->page[i]) {
                 continue;
             }
-            if (xengnttab_unmap(gnt, ioreq->page[i], 1) != 0) {
-                xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                              "xengnttab_unmap failed: %s\n",
-                              strerror(errno));
-            }
+            xen_be_unmap_grant_ref(xendev, ioreq->page[i]);
             ioreq->blkdev->cnt_map--;
             ioreq->page[i] = NULL;
         }
@@ -360,14 +341,14 @@ static void ioreq_unmap(struct ioreq *ioreq)
 
 static int ioreq_map(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
-    uint32_t domids[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, j, new_maps = 0;
     PersistentGrant *grant;
     PersistentRegion *region;
-    /* domids and refs variables will contain the information necessary
+    /* refs variable will contain the information necessary
      * to map the grants that are needed to fulfill this request.
      *
      * After mapping the needed grants, the page array will contain the
@@ -392,7 +373,6 @@ static int ioreq_map(struct ioreq *ioreq)
                     /* Add the grant to the list of grants that
                      * should be mapped
                      */
-                    domids[new_maps] = ioreq->domids[i];
                     refs[new_maps] = ioreq->refs[i];
                     page[i] = NULL;
                     new_maps++;
@@ -405,14 +385,13 @@ static int ioreq_map(struct ioreq *ioreq)
     } else {
         /* All grants in the request should be mapped */
         memcpy(refs, ioreq->refs, sizeof(refs));
-        memcpy(domids, ioreq->domids, sizeof(domids));
         memset(page, 0, sizeof(page));
         new_maps = ioreq->v.niov;
     }
 
     if (batch_maps && new_maps) {
-        ioreq->pages = xengnttab_map_grant_refs
-            (gnt, new_maps, domids, refs, ioreq->prot);
+        ioreq->pages = xen_be_map_grant_refs(xendev, refs, new_maps,
+                                             ioreq->prot);
         if (ioreq->pages == NULL) {
             xen_pv_printf(&ioreq->blkdev->xendev, 0,
                           "can't map %d grant refs (%s, %d maps)\n",
@@ -427,8 +406,8 @@ static int ioreq_map(struct ioreq *ioreq)
         ioreq->blkdev->cnt_map += new_maps;
     } else if (new_maps)  {
         for (i = 0; i < new_maps; i++) {
-            ioreq->page[i] = xengnttab_map_grant_ref
-                (gnt, domids[i], refs[i], ioreq->prot);
+            ioreq->page[i] = xen_be_map_grant_ref(xendev, refs[i],
+                                                  ioreq->prot);
             if (ioreq->page[i] == NULL) {
                 xen_pv_printf(&ioreq->blkdev->xendev, 0,
                               "can't map grant ref %d (%s, %d maps)\n",
@@ -529,10 +508,12 @@ static int ioreq_init_copy_buffers(struct ioreq *ioreq)
 
 static int ioreq_grant_copy(struct ioreq *ioreq)
 {
-    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
-    xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
+    XenGrantCopySegment segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, count, rc;
     int64_t file_blk = ioreq->blkdev->file_blk;
+    bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
 
     if (ioreq->v.niov == 0) {
         return 0;
@@ -541,16 +522,12 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     count = ioreq->v.niov;
 
     for (i = 0; i < count; i++) {
-        if (ioreq->req.operation == BLKIF_OP_READ) {
-            segs[i].flags = GNTCOPY_dest_gref;
+        if (to_domain) {
             segs[i].dest.foreign.ref = ioreq->refs[i];
-            segs[i].dest.foreign.domid = ioreq->domids[i];
             segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
             segs[i].source.virt = ioreq->v.iov[i].iov_base;
         } else {
-            segs[i].flags = GNTCOPY_source_gref;
             segs[i].source.foreign.ref = ioreq->refs[i];
-            segs[i].source.foreign.domid = ioreq->domids[i];
             segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
             segs[i].dest.virt = ioreq->v.iov[i].iov_base;
         }
@@ -558,7 +535,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
                        - ioreq->req.seg[i].first_sect + 1) * file_blk;
     }
 
-    rc = xengnttab_grant_copy(gnt, count, segs);
+    rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
 
     if (rc) {
         xen_pv_printf(&ioreq->blkdev->xendev, 0,
@@ -567,16 +544,6 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
         return -1;
     }
 
-    for (i = 0; i < count; i++) {
-        if (segs[i].status != GNTST_okay) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                          "failed to copy data %d for gref %d, domid %d\n",
-                          segs[i].status, ioreq->refs[i], ioreq->domids[i]);
-            ioreq->aio_errors++;
-            rc = -1;
-        }
-    }
-
     return rc;
 }
 #else
@@ -1085,7 +1052,6 @@ static int blk_connect(struct XenDevice *xendev)
     int order, ring_ref;
     unsigned int ring_size, max_grants;
     unsigned int i;
-    uint32_t *domids;
 
     trace_xen_disk_connect(xendev->name);
 
@@ -1247,31 +1213,11 @@ static int blk_connect(struct XenDevice *xendev)
     /* Add on the number needed for the ring pages */
     max_grants += blkdev->nr_ring_ref;
 
-    blkdev->xendev.gnttabdev = xengnttab_open(NULL, 0);
-    if (blkdev->xendev.gnttabdev == NULL) {
-        xen_pv_printf(xendev, 0, "xengnttab_open failed: %s\n",
-                      strerror(errno));
-        return -1;
-    }
-    if (xengnttab_set_max_grants(blkdev->xendev.gnttabdev, max_grants)) {
-        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
-                      strerror(errno));
-        return -1;
-    }
-
-    domids = g_new0(uint32_t, blkdev->nr_ring_ref);
-    for (i = 0; i < blkdev->nr_ring_ref; i++) {
-        domids[i] = blkdev->xendev.dom;
-    }
-
-    blkdev->sring = xengnttab_map_grant_refs(blkdev->xendev.gnttabdev,
-                                             blkdev->nr_ring_ref,
-                                             domids,
-                                             blkdev->ring_ref,
-                                             PROT_READ | PROT_WRITE);
-
-    g_free(domids);
+    xen_be_set_max_grant_refs(xendev, max_grants);
 
+    blkdev->sring = xen_be_map_grant_refs(xendev, blkdev->ring_ref,
+                                          blkdev->nr_ring_ref,
+                                          PROT_READ | PROT_WRITE);
     if (!blkdev->sring) {
         return -1;
     }
@@ -1344,8 +1290,8 @@ static void blk_disconnect(struct XenDevice *xendev)
     aio_context_release(blkdev->ctx);
 
     if (blkdev->sring) {
-        xengnttab_unmap(blkdev->xendev.gnttabdev, blkdev->sring,
-                        blkdev->nr_ring_ref);
+        xen_be_unmap_grant_refs(xendev, blkdev->sring,
+                                blkdev->nr_ring_ref);
         blkdev->cnt_map--;
         blkdev->sring = NULL;
     }
@@ -1369,11 +1315,6 @@ static void blk_disconnect(struct XenDevice *xendev)
         }
         blkdev->feature_persistent = false;
     }
-
-    if (blkdev->xendev.gnttabdev) {
-        xengnttab_close(blkdev->xendev.gnttabdev);
-        blkdev->xendev.gnttabdev = NULL;
-    }
 }
 
 static int blk_free(struct XenDevice *xendev)
@@ -1410,10 +1351,11 @@ static void blk_event(struct XenDevice *xendev)
 }
 
 struct XenDevOps xen_blkdev_ops = {
+    .flags      = DEVOPS_FLAG_NEED_GNTDEV,
     .size       = sizeof(struct XenBlkDev),
     .alloc      = blk_alloc,
     .init       = blk_init,
-    .initialise    = blk_connect,
+    .initialise = blk_connect,
     .disconnect = blk_disconnect,
     .event      = blk_event,
     .free       = blk_free,
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (4 preceding siblings ...)
  2018-05-04 19:26 ` [PATCH v3 3/8] xen: remove other " Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-16 14:14   ` Anthony PERARD
  2018-05-16 14:14   ` Anthony PERARD
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy Paul Durrant
                   ` (8 subsequent siblings)
  14 siblings, 2 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard, Greg Kurz,
	Paolo Bonzini, Jason Wang, Gerd Hoffmann

Now that helpers are available in xen_backend, use them throughout all
Xen PV backends.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Greg Kurz <groug@kaod.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

v2:
 - New in v2
---
 hw/9pfs/xen-9p-backend.c | 32 +++++++++++++++-----------------
 hw/char/xen_console.c    |  9 ++++-----
 hw/net/xen_nic.c         | 34 +++++++++++++++-------------------
 hw/usb/xen-usb.c         | 37 +++++++++++++++++--------------------
 4 files changed, 51 insertions(+), 61 deletions(-)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 95e50c4..6026780 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -331,14 +331,14 @@ static int xen_9pfs_free(struct XenDevice *xendev)
 
     for (i = 0; i < xen_9pdev->num_rings; i++) {
         if (xen_9pdev->rings[i].data != NULL) {
-            xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
-                    xen_9pdev->rings[i].data,
-                    (1 << xen_9pdev->rings[i].ring_order));
+            xen_be_unmap_grant_refs(&xen_9pdev->xendev,
+                                    xen_9pdev->rings[i].data,
+                                    (1 << xen_9pdev->rings[i].ring_order));
         }
         if (xen_9pdev->rings[i].intf != NULL) {
-            xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
-                    xen_9pdev->rings[i].intf,
-                    1);
+            xen_be_unmap_grant_refs(&xen_9pdev->xendev,
+                                    xen_9pdev->rings[i].intf,
+                                    1);
         }
         if (xen_9pdev->rings[i].bh != NULL) {
             qemu_bh_delete(xen_9pdev->rings[i].bh);
@@ -390,11 +390,10 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
         }
         g_free(str);
 
-        xen_9pdev->rings[i].intf =  xengnttab_map_grant_ref(
-                xen_9pdev->xendev.gnttabdev,
-                xen_9pdev->xendev.dom,
-                xen_9pdev->rings[i].ref,
-                PROT_READ | PROT_WRITE);
+        xen_9pdev->rings[i].intf =
+            xen_be_map_grant_ref(&xen_9pdev->xendev,
+                                 xen_9pdev->rings[i].ref,
+                                 PROT_READ | PROT_WRITE);
         if (!xen_9pdev->rings[i].intf) {
             goto out;
         }
@@ -403,12 +402,11 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
             goto out;
         }
         xen_9pdev->rings[i].ring_order = ring_order;
-        xen_9pdev->rings[i].data = xengnttab_map_domain_grant_refs(
-                xen_9pdev->xendev.gnttabdev,
-                (1 << ring_order),
-                xen_9pdev->xendev.dom,
-                xen_9pdev->rings[i].intf->ref,
-                PROT_READ | PROT_WRITE);
+        xen_9pdev->rings[i].data =
+            xen_be_map_grant_refs(&xen_9pdev->xendev,
+                                  xen_9pdev->rings[i].intf->ref,
+                                  (1 << ring_order),
+                                  PROT_READ | PROT_WRITE);
         if (!xen_9pdev->rings[i].data) {
             goto out;
         }
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index bdfaa40..8b4b4bf 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -233,12 +233,11 @@ static int con_initialise(struct XenDevice *xendev)
     if (!xendev->dev) {
         xen_pfn_t mfn = con->ring_ref;
         con->sring = xenforeignmemory_map(xen_fmem, con->xendev.dom,
-                                          PROT_READ|PROT_WRITE,
+                                          PROT_READ | PROT_WRITE,
                                           1, &mfn, NULL);
     } else {
-        con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
-                                             con->ring_ref,
-                                             PROT_READ|PROT_WRITE);
+        con->sring = xen_be_map_grant_ref(xendev, con->ring_ref,
+                                          PROT_READ | PROT_WRITE);
     }
     if (!con->sring)
 	return -1;
@@ -267,7 +266,7 @@ static void con_disconnect(struct XenDevice *xendev)
         if (!xendev->dev) {
             xenforeignmemory_unmap(xen_fmem, con->sring, 1);
         } else {
-            xengnttab_unmap(xendev->gnttabdev, con->sring, 1);
+            xen_be_unmap_grant_ref(xendev, con->sring);
         }
         con->sring = NULL;
     }
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 20c43a6..73d6f1b 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev *netdev)
                           (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
                           (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
 
-            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                           netdev->xendev.dom,
-                                           txreq.gref, PROT_READ);
+            page = xen_be_map_grant_refs(&netdev->xendev,
+                                         &txreq.gref, 1, PROT_READ);
             if (page == NULL) {
                 xen_pv_printf(&netdev->xendev, 0,
                               "error: tx gref dereference failed (%d)\n",
@@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
                 qemu_send_packet(qemu_get_queue(netdev->nic),
                                  page + txreq.offset, txreq.size);
             }
-            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
+            xen_be_unmap_grant_ref(&netdev->xendev, page);
             net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
         }
         if (!netdev->tx_work) {
@@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
     memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
     netdev->rx_ring.req_cons = ++rc;
 
-    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                   netdev->xendev.dom,
-                                   rxreq.gref, PROT_WRITE);
+    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
+                                 PROT_WRITE);
     if (page == NULL) {
         xen_pv_printf(&netdev->xendev, 0,
                       "error: rx gref dereference failed (%d)\n",
@@ -265,7 +263,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
         return -1;
     }
     memcpy(page + NET_IP_ALIGN, buf, size);
-    xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
+    xen_be_unmap_grant_ref(&netdev->xendev, page);
     net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
 
     return size;
@@ -338,19 +336,17 @@ static int net_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    netdev->txs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                          netdev->xendev.dom,
-                                          netdev->tx_ring_ref,
-                                          PROT_READ | PROT_WRITE);
+    netdev->txs = xen_be_map_grant_ref(&netdev->xendev,
+                                       netdev->tx_ring_ref,
+                                       PROT_READ | PROT_WRITE);
     if (!netdev->txs) {
         return -1;
     }
-    netdev->rxs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                          netdev->xendev.dom,
-                                          netdev->rx_ring_ref,
-                                          PROT_READ | PROT_WRITE);
+    netdev->rxs = xen_be_map_grant_ref(&netdev->xendev,
+                                       netdev->rx_ring_ref,
+                                       PROT_READ | PROT_WRITE);
     if (!netdev->rxs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs);
         netdev->txs = NULL;
         return -1;
     }
@@ -375,11 +371,11 @@ static void net_disconnect(struct XenDevice *xendev)
     xen_pv_unbind_evtchn(&netdev->xendev);
 
     if (netdev->txs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs);
         netdev->txs = NULL;
     }
     if (netdev->rxs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->rxs);
         netdev->rxs = NULL;
     }
 }
diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index b3a90c0..5b2e21e 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -173,8 +173,9 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
         for (i = 0; i < usbback_req->nr_buffer_segs; i++) {
             ref[i] = usbback_req->req.seg[i].gref;
         }
-        usbback_req->buffer = xengnttab_map_domain_grant_refs(xendev->gnttabdev,
-            usbback_req->nr_buffer_segs, xendev->dom, ref, prot);
+        usbback_req->buffer =
+            xen_be_map_grant_refs(xendev, ref, usbback_req->nr_buffer_segs,
+                                  prot);
 
         if (!usbback_req->buffer) {
             return -ENOMEM;
@@ -206,8 +207,9 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
     for (i = 0; i < usbback_req->nr_extra_segs; i++) {
         ref[i] = usbback_req->req.seg[i + usbback_req->req.nr_buffer_segs].gref;
     }
-    usbback_req->isoc_buffer = xengnttab_map_domain_grant_refs(
-         xendev->gnttabdev, usbback_req->nr_extra_segs, xendev->dom, ref, prot);
+    usbback_req->isoc_buffer =
+        xen_be_map_grant_refs(xendev, ref, usbback_req->nr_extra_segs,
+                              prot);
 
     if (!usbback_req->isoc_buffer) {
         return -ENOMEM;
@@ -291,14 +293,14 @@ static void usbback_do_response(struct usbback_req *usbback_req, int32_t status,
     }
 
     if (usbback_req->buffer) {
-        xengnttab_unmap(xendev->gnttabdev, usbback_req->buffer,
-                        usbback_req->nr_buffer_segs);
+        xen_be_unmap_grant_refs(xendev, usbback_req->buffer,
+                                usbback_req->nr_buffer_segs);
         usbback_req->buffer = NULL;
     }
 
     if (usbback_req->isoc_buffer) {
-        xengnttab_unmap(xendev->gnttabdev, usbback_req->isoc_buffer,
-                        usbback_req->nr_extra_segs);
+        xen_be_unmap_grant_refs(xendev, usbback_req->isoc_buffer,
+                                usbback_req->nr_extra_segs);
         usbback_req->isoc_buffer = NULL;
     }
 
@@ -834,11 +836,11 @@ static void usbback_disconnect(struct XenDevice *xendev)
     xen_pv_unbind_evtchn(xendev);
 
     if (usbif->urb_sring) {
-        xengnttab_unmap(xendev->gnttabdev, usbif->urb_sring, 1);
+        xen_be_unmap_grant_ref(xendev, usbif->urb_sring);
         usbif->urb_sring = NULL;
     }
     if (usbif->conn_sring) {
-        xengnttab_unmap(xendev->gnttabdev, usbif->conn_sring, 1);
+        xen_be_unmap_grant_ref(xendev, usbif->conn_sring);
         usbif->conn_sring = NULL;
     }
 
@@ -877,12 +879,10 @@ static int usbback_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    usbif->urb_sring = xengnttab_map_grant_ref(xendev->gnttabdev, xendev->dom,
-                                               urb_ring_ref,
-                                               PROT_READ | PROT_WRITE);
-    usbif->conn_sring = xengnttab_map_grant_ref(xendev->gnttabdev, xendev->dom,
-                                                conn_ring_ref,
-                                                PROT_READ | PROT_WRITE);
+    usbif->urb_sring = xen_be_map_grant_ref(xendev, urb_ring_ref,
+                                            PROT_READ | PROT_WRITE);
+    usbif->conn_sring = xen_be_map_grant_ref(xendev, conn_ring_ref,
+                                             PROT_READ | PROT_WRITE);
     if (!usbif->urb_sring || !usbif->conn_sring) {
         xen_pv_printf(xendev, 0, "error mapping rings\n");
         usbback_disconnect(xendev);
@@ -1024,10 +1024,7 @@ static void usbback_alloc(struct XenDevice *xendev)
 
     /* max_grants: for each request and for the rings (request and connect). */
     max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
-    if (xengnttab_set_max_grants(xendev->gnttabdev, max_grants) < 0) {
-        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
-                      strerror(errno));
-    }
+    xen_be_set_max_grant_refs(xendev, max_grants);
 }
 
 static int usbback_free(struct XenDevice *xendev)
-- 
2.1.4

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

* [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (3 preceding siblings ...)
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Stefano Stabellini, Jason Wang, Greg Kurz, Paul Durrant,
	Gerd Hoffmann, Anthony Perard, Paolo Bonzini

Now that helpers are available in xen_backend, use them throughout all
Xen PV backends.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Greg Kurz <groug@kaod.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>

v2:
 - New in v2
---
 hw/9pfs/xen-9p-backend.c | 32 +++++++++++++++-----------------
 hw/char/xen_console.c    |  9 ++++-----
 hw/net/xen_nic.c         | 34 +++++++++++++++-------------------
 hw/usb/xen-usb.c         | 37 +++++++++++++++++--------------------
 4 files changed, 51 insertions(+), 61 deletions(-)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 95e50c4..6026780 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -331,14 +331,14 @@ static int xen_9pfs_free(struct XenDevice *xendev)
 
     for (i = 0; i < xen_9pdev->num_rings; i++) {
         if (xen_9pdev->rings[i].data != NULL) {
-            xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
-                    xen_9pdev->rings[i].data,
-                    (1 << xen_9pdev->rings[i].ring_order));
+            xen_be_unmap_grant_refs(&xen_9pdev->xendev,
+                                    xen_9pdev->rings[i].data,
+                                    (1 << xen_9pdev->rings[i].ring_order));
         }
         if (xen_9pdev->rings[i].intf != NULL) {
-            xengnttab_unmap(xen_9pdev->xendev.gnttabdev,
-                    xen_9pdev->rings[i].intf,
-                    1);
+            xen_be_unmap_grant_refs(&xen_9pdev->xendev,
+                                    xen_9pdev->rings[i].intf,
+                                    1);
         }
         if (xen_9pdev->rings[i].bh != NULL) {
             qemu_bh_delete(xen_9pdev->rings[i].bh);
@@ -390,11 +390,10 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
         }
         g_free(str);
 
-        xen_9pdev->rings[i].intf =  xengnttab_map_grant_ref(
-                xen_9pdev->xendev.gnttabdev,
-                xen_9pdev->xendev.dom,
-                xen_9pdev->rings[i].ref,
-                PROT_READ | PROT_WRITE);
+        xen_9pdev->rings[i].intf =
+            xen_be_map_grant_ref(&xen_9pdev->xendev,
+                                 xen_9pdev->rings[i].ref,
+                                 PROT_READ | PROT_WRITE);
         if (!xen_9pdev->rings[i].intf) {
             goto out;
         }
@@ -403,12 +402,11 @@ static int xen_9pfs_connect(struct XenDevice *xendev)
             goto out;
         }
         xen_9pdev->rings[i].ring_order = ring_order;
-        xen_9pdev->rings[i].data = xengnttab_map_domain_grant_refs(
-                xen_9pdev->xendev.gnttabdev,
-                (1 << ring_order),
-                xen_9pdev->xendev.dom,
-                xen_9pdev->rings[i].intf->ref,
-                PROT_READ | PROT_WRITE);
+        xen_9pdev->rings[i].data =
+            xen_be_map_grant_refs(&xen_9pdev->xendev,
+                                  xen_9pdev->rings[i].intf->ref,
+                                  (1 << ring_order),
+                                  PROT_READ | PROT_WRITE);
         if (!xen_9pdev->rings[i].data) {
             goto out;
         }
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index bdfaa40..8b4b4bf 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -233,12 +233,11 @@ static int con_initialise(struct XenDevice *xendev)
     if (!xendev->dev) {
         xen_pfn_t mfn = con->ring_ref;
         con->sring = xenforeignmemory_map(xen_fmem, con->xendev.dom,
-                                          PROT_READ|PROT_WRITE,
+                                          PROT_READ | PROT_WRITE,
                                           1, &mfn, NULL);
     } else {
-        con->sring = xengnttab_map_grant_ref(xendev->gnttabdev, con->xendev.dom,
-                                             con->ring_ref,
-                                             PROT_READ|PROT_WRITE);
+        con->sring = xen_be_map_grant_ref(xendev, con->ring_ref,
+                                          PROT_READ | PROT_WRITE);
     }
     if (!con->sring)
 	return -1;
@@ -267,7 +266,7 @@ static void con_disconnect(struct XenDevice *xendev)
         if (!xendev->dev) {
             xenforeignmemory_unmap(xen_fmem, con->sring, 1);
         } else {
-            xengnttab_unmap(xendev->gnttabdev, con->sring, 1);
+            xen_be_unmap_grant_ref(xendev, con->sring);
         }
         con->sring = NULL;
     }
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 20c43a6..73d6f1b 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev *netdev)
                           (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
                           (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
 
-            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                           netdev->xendev.dom,
-                                           txreq.gref, PROT_READ);
+            page = xen_be_map_grant_refs(&netdev->xendev,
+                                         &txreq.gref, 1, PROT_READ);
             if (page == NULL) {
                 xen_pv_printf(&netdev->xendev, 0,
                               "error: tx gref dereference failed (%d)\n",
@@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
                 qemu_send_packet(qemu_get_queue(netdev->nic),
                                  page + txreq.offset, txreq.size);
             }
-            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
+            xen_be_unmap_grant_ref(&netdev->xendev, page);
             net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
         }
         if (!netdev->tx_work) {
@@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
     memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
     netdev->rx_ring.req_cons = ++rc;
 
-    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                   netdev->xendev.dom,
-                                   rxreq.gref, PROT_WRITE);
+    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
+                                 PROT_WRITE);
     if (page == NULL) {
         xen_pv_printf(&netdev->xendev, 0,
                       "error: rx gref dereference failed (%d)\n",
@@ -265,7 +263,7 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
         return -1;
     }
     memcpy(page + NET_IP_ALIGN, buf, size);
-    xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
+    xen_be_unmap_grant_ref(&netdev->xendev, page);
     net_rx_response(netdev, &rxreq, NETIF_RSP_OKAY, NET_IP_ALIGN, size, 0);
 
     return size;
@@ -338,19 +336,17 @@ static int net_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    netdev->txs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                          netdev->xendev.dom,
-                                          netdev->tx_ring_ref,
-                                          PROT_READ | PROT_WRITE);
+    netdev->txs = xen_be_map_grant_ref(&netdev->xendev,
+                                       netdev->tx_ring_ref,
+                                       PROT_READ | PROT_WRITE);
     if (!netdev->txs) {
         return -1;
     }
-    netdev->rxs = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
-                                          netdev->xendev.dom,
-                                          netdev->rx_ring_ref,
-                                          PROT_READ | PROT_WRITE);
+    netdev->rxs = xen_be_map_grant_ref(&netdev->xendev,
+                                       netdev->rx_ring_ref,
+                                       PROT_READ | PROT_WRITE);
     if (!netdev->rxs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs);
         netdev->txs = NULL;
         return -1;
     }
@@ -375,11 +371,11 @@ static void net_disconnect(struct XenDevice *xendev)
     xen_pv_unbind_evtchn(&netdev->xendev);
 
     if (netdev->txs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->txs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->txs);
         netdev->txs = NULL;
     }
     if (netdev->rxs) {
-        xengnttab_unmap(netdev->xendev.gnttabdev, netdev->rxs, 1);
+        xen_be_unmap_grant_ref(&netdev->xendev, netdev->rxs);
         netdev->rxs = NULL;
     }
 }
diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index b3a90c0..5b2e21e 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -173,8 +173,9 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
         for (i = 0; i < usbback_req->nr_buffer_segs; i++) {
             ref[i] = usbback_req->req.seg[i].gref;
         }
-        usbback_req->buffer = xengnttab_map_domain_grant_refs(xendev->gnttabdev,
-            usbback_req->nr_buffer_segs, xendev->dom, ref, prot);
+        usbback_req->buffer =
+            xen_be_map_grant_refs(xendev, ref, usbback_req->nr_buffer_segs,
+                                  prot);
 
         if (!usbback_req->buffer) {
             return -ENOMEM;
@@ -206,8 +207,9 @@ static int usbback_gnttab_map(struct usbback_req *usbback_req)
     for (i = 0; i < usbback_req->nr_extra_segs; i++) {
         ref[i] = usbback_req->req.seg[i + usbback_req->req.nr_buffer_segs].gref;
     }
-    usbback_req->isoc_buffer = xengnttab_map_domain_grant_refs(
-         xendev->gnttabdev, usbback_req->nr_extra_segs, xendev->dom, ref, prot);
+    usbback_req->isoc_buffer =
+        xen_be_map_grant_refs(xendev, ref, usbback_req->nr_extra_segs,
+                              prot);
 
     if (!usbback_req->isoc_buffer) {
         return -ENOMEM;
@@ -291,14 +293,14 @@ static void usbback_do_response(struct usbback_req *usbback_req, int32_t status,
     }
 
     if (usbback_req->buffer) {
-        xengnttab_unmap(xendev->gnttabdev, usbback_req->buffer,
-                        usbback_req->nr_buffer_segs);
+        xen_be_unmap_grant_refs(xendev, usbback_req->buffer,
+                                usbback_req->nr_buffer_segs);
         usbback_req->buffer = NULL;
     }
 
     if (usbback_req->isoc_buffer) {
-        xengnttab_unmap(xendev->gnttabdev, usbback_req->isoc_buffer,
-                        usbback_req->nr_extra_segs);
+        xen_be_unmap_grant_refs(xendev, usbback_req->isoc_buffer,
+                                usbback_req->nr_extra_segs);
         usbback_req->isoc_buffer = NULL;
     }
 
@@ -834,11 +836,11 @@ static void usbback_disconnect(struct XenDevice *xendev)
     xen_pv_unbind_evtchn(xendev);
 
     if (usbif->urb_sring) {
-        xengnttab_unmap(xendev->gnttabdev, usbif->urb_sring, 1);
+        xen_be_unmap_grant_ref(xendev, usbif->urb_sring);
         usbif->urb_sring = NULL;
     }
     if (usbif->conn_sring) {
-        xengnttab_unmap(xendev->gnttabdev, usbif->conn_sring, 1);
+        xen_be_unmap_grant_ref(xendev, usbif->conn_sring);
         usbif->conn_sring = NULL;
     }
 
@@ -877,12 +879,10 @@ static int usbback_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    usbif->urb_sring = xengnttab_map_grant_ref(xendev->gnttabdev, xendev->dom,
-                                               urb_ring_ref,
-                                               PROT_READ | PROT_WRITE);
-    usbif->conn_sring = xengnttab_map_grant_ref(xendev->gnttabdev, xendev->dom,
-                                                conn_ring_ref,
-                                                PROT_READ | PROT_WRITE);
+    usbif->urb_sring = xen_be_map_grant_ref(xendev, urb_ring_ref,
+                                            PROT_READ | PROT_WRITE);
+    usbif->conn_sring = xen_be_map_grant_ref(xendev, conn_ring_ref,
+                                             PROT_READ | PROT_WRITE);
     if (!usbif->urb_sring || !usbif->conn_sring) {
         xen_pv_printf(xendev, 0, "error mapping rings\n");
         usbback_disconnect(xendev);
@@ -1024,10 +1024,7 @@ static void usbback_alloc(struct XenDevice *xendev)
 
     /* max_grants: for each request and for the rings (request and connect). */
     max_grants = USBIF_MAX_SEGMENTS_PER_REQUEST * USB_URB_RING_SIZE + 2;
-    if (xengnttab_set_max_grants(xendev->gnttabdev, max_grants) < 0) {
-        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
-                      strerror(errno));
-    }
+    xen_be_set_max_grant_refs(xendev, max_grants);
 }
 
 static int usbback_free(struct XenDevice *xendev)
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (5 preceding siblings ...)
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-16 14:30     ` Anthony PERARD
  2018-05-04 19:26 ` Paul Durrant
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard

Not all Xen environments support the xengnttab_grant_copy() operation.
E.g. where the OS is FreeBSD or Xen is older than 4.8.0.

This patch introduces an emulation of that operation using
xengnttab_map_domain_grant_refs() and memcpy() for those environments.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 50412d6..3c3fc2c 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -146,6 +146,55 @@ void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
     }
 }
 
+static int compat_copy_grant_refs(struct XenDevice *xendev,
+                                  bool to_domain,
+                                  XenGrantCopySegment segs[],
+                                  unsigned int nr_segs)
+{
+    uint32_t *refs = g_new(uint32_t, nr_segs);
+    int prot = to_domain ? PROT_WRITE : PROT_READ;
+    void *pages;
+    unsigned int i;
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+
+        refs[i] = to_domain ?
+            seg->dest.foreign.ref : seg->source.foreign.ref;
+    }
+
+    pages = xengnttab_map_domain_grant_refs(xendev->gnttabdev, nr_segs,
+                                            xen_domid, refs, prot);
+    if (!pages) {
+        xen_pv_printf(xendev, 0,
+                      "xengnttab_map_domain_grant_refs failed: %s\n",
+                      strerror(errno));
+        g_free(refs);
+        return -1;
+    }
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+        void *page = pages + (i * XC_PAGE_SIZE);
+
+        if (to_domain) {
+            memcpy(page + seg->dest.foreign.offset, seg->source.virt,
+                   seg->len);
+        } else {
+            memcpy(seg->dest.virt, page + seg->source.foreign.offset,
+                   seg->len);
+        }
+    }
+
+    if (xengnttab_unmap(xendev->gnttabdev, pages, nr_segs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_unmap failed: %s\n",
+                      strerror(errno));
+    }
+
+    g_free(refs);
+    return 0;
+}
+
 int xen_be_copy_grant_refs(struct XenDevice *xendev,
                            bool to_domain,
                            XenGrantCopySegment segs[],
@@ -157,6 +206,10 @@ int xen_be_copy_grant_refs(struct XenDevice *xendev,
 
     assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
 
+    if (!xen_feature_grant_copy) {
+        return compat_copy_grant_refs(xendev, to_domain, segs, nr_segs);
+    }
+
     xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
 
     for (i = 0; i < nr_segs; i++) {
-- 
2.1.4

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

* [PATCH v3 4/8] xen_backend: add an emulation of grant copy
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (6 preceding siblings ...)
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [PATCH v3 5/8] xen_disk: remove use of grant map/unmap Paul Durrant
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Paul Durrant, Stefano Stabellini

Not all Xen environments support the xengnttab_grant_copy() operation.
E.g. where the OS is FreeBSD or Xen is older than 4.8.0.

This patch introduces an emulation of that operation using
xengnttab_map_domain_grant_refs() and memcpy() for those environments.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 50412d6..3c3fc2c 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -146,6 +146,55 @@ void xen_be_unmap_grant_refs(struct XenDevice *xendev, void *ptr,
     }
 }
 
+static int compat_copy_grant_refs(struct XenDevice *xendev,
+                                  bool to_domain,
+                                  XenGrantCopySegment segs[],
+                                  unsigned int nr_segs)
+{
+    uint32_t *refs = g_new(uint32_t, nr_segs);
+    int prot = to_domain ? PROT_WRITE : PROT_READ;
+    void *pages;
+    unsigned int i;
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+
+        refs[i] = to_domain ?
+            seg->dest.foreign.ref : seg->source.foreign.ref;
+    }
+
+    pages = xengnttab_map_domain_grant_refs(xendev->gnttabdev, nr_segs,
+                                            xen_domid, refs, prot);
+    if (!pages) {
+        xen_pv_printf(xendev, 0,
+                      "xengnttab_map_domain_grant_refs failed: %s\n",
+                      strerror(errno));
+        g_free(refs);
+        return -1;
+    }
+
+    for (i = 0; i < nr_segs; i++) {
+        XenGrantCopySegment *seg = &segs[i];
+        void *page = pages + (i * XC_PAGE_SIZE);
+
+        if (to_domain) {
+            memcpy(page + seg->dest.foreign.offset, seg->source.virt,
+                   seg->len);
+        } else {
+            memcpy(seg->dest.virt, page + seg->source.foreign.offset,
+                   seg->len);
+        }
+    }
+
+    if (xengnttab_unmap(xendev->gnttabdev, pages, nr_segs)) {
+        xen_pv_printf(xendev, 0, "xengnttab_unmap failed: %s\n",
+                      strerror(errno));
+    }
+
+    g_free(refs);
+    return 0;
+}
+
 int xen_be_copy_grant_refs(struct XenDevice *xendev,
                            bool to_domain,
                            XenGrantCopySegment segs[],
@@ -157,6 +206,10 @@ int xen_be_copy_grant_refs(struct XenDevice *xendev,
 
     assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
 
+    if (!xen_feature_grant_copy) {
+        return compat_copy_grant_refs(xendev, to_domain, segs, nr_segs);
+    }
+
     xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
 
     for (i = 0; i < nr_segs; i++) {
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (8 preceding siblings ...)
  2018-05-04 19:26 ` [PATCH v3 5/8] xen_disk: remove use of grant map/unmap Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-17 10:31   ` Anthony PERARD
  2018-05-17 10:31   ` Anthony PERARD
  2018-05-04 19:26   ` Paul Durrant
                   ` (4 subsequent siblings)
  14 siblings, 2 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard, Kevin Wolf, Max Reitz

Now that the (native or emulated) xen_be_copy_grant_refs() helper is
always available, the xen_disk code can be significantly simplified by
removing direct use of grant map and unmap operations.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - Squashed in separate patche removing persistent grant use
 - Re-based
---
 hw/block/xen_disk.c | 370 ++++------------------------------------------------
 1 file changed, 25 insertions(+), 345 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 66ed2b7..28be8b6 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -36,27 +36,9 @@
 
 /* ------------------------------------------------------------- */
 
-static int batch_maps   = 0;
-
-/* ------------------------------------------------------------- */
-
 #define BLOCK_SIZE  512
 #define IOCB_COUNT  (BLKIF_MAX_SEGMENTS_PER_REQUEST + 2)
 
-struct PersistentGrant {
-    void *page;
-    struct XenBlkDev *blkdev;
-};
-
-typedef struct PersistentGrant PersistentGrant;
-
-struct PersistentRegion {
-    void *addr;
-    int num;
-};
-
-typedef struct PersistentRegion PersistentRegion;
-
 struct ioreq {
     blkif_request_t     req;
     int16_t             status;
@@ -65,14 +47,11 @@ struct ioreq {
     off_t               start;
     QEMUIOVector        v;
     int                 presync;
-    uint8_t             mapped;
 
     /* grant mapping */
     uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    int                 prot;
     void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     void                *pages;
-    int                 num_unmap;
 
     /* aio status */
     int                 aio_inflight;
@@ -103,7 +82,6 @@ struct XenBlkDev {
     int                 protocol;
     blkif_back_rings_t  rings;
     int                 more_work;
-    int                 cnt_map;
 
     /* request lists */
     QLIST_HEAD(inflight_head, ioreq) inflight;
@@ -114,13 +92,7 @@ struct XenBlkDev {
     int                 requests_finished;
     unsigned int        max_requests;
 
-    /* Persistent grants extension */
     gboolean            feature_discard;
-    gboolean            feature_persistent;
-    GTree               *persistent_gnts;
-    GSList              *persistent_regions;
-    unsigned int        persistent_gnt_count;
-    unsigned int        max_grants;
 
     /* qemu block driver */
     DriveInfo           *dinfo;
@@ -139,10 +111,8 @@ static void ioreq_reset(struct ioreq *ioreq)
     ioreq->status = 0;
     ioreq->start = 0;
     ioreq->presync = 0;
-    ioreq->mapped = 0;
 
     memset(ioreq->refs, 0, sizeof(ioreq->refs));
-    ioreq->prot = 0;
     memset(ioreq->page, 0, sizeof(ioreq->page));
     ioreq->pages = NULL;
 
@@ -156,37 +126,6 @@ static void ioreq_reset(struct ioreq *ioreq)
     qemu_iovec_reset(&ioreq->v);
 }
 
-static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
-{
-    uint ua = GPOINTER_TO_UINT(a);
-    uint ub = GPOINTER_TO_UINT(b);
-    return (ua > ub) - (ua < ub);
-}
-
-static void destroy_grant(gpointer pgnt)
-{
-    PersistentGrant *grant = pgnt;
-    struct XenBlkDev *blkdev = grant->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-
-    xen_be_unmap_grant_ref(xendev, grant->page);
-    grant->blkdev->persistent_gnt_count--;
-    xen_pv_printf(xendev, 3, "unmapped grant %p\n", grant->page);
-    g_free(grant);
-}
-
-static void remove_persistent_region(gpointer data, gpointer dev)
-{
-    PersistentRegion *region = data;
-    struct XenBlkDev *blkdev = dev;
-    struct XenDevice *xendev = &blkdev->xendev;
-
-    xen_be_unmap_grant_refs(xendev, region->addr, region->num);
-    xen_pv_printf(xendev, 3, "unmapped grant region %p with %d pages\n",
-                  region->addr, region->num);
-    g_free(region);
-}
-
 static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
 {
     struct ioreq *ioreq = NULL;
@@ -254,7 +193,6 @@ static int ioreq_parse(struct ioreq *ioreq)
                   ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
-        ioreq->prot = PROT_WRITE; /* to memory */
         break;
     case BLKIF_OP_FLUSH_DISKCACHE:
         ioreq->presync = 1;
@@ -263,7 +201,6 @@ static int ioreq_parse(struct ioreq *ioreq)
         }
         /* fall through */
     case BLKIF_OP_WRITE:
-        ioreq->prot = PROT_READ; /* from memory */
         break;
     case BLKIF_OP_DISCARD:
         return 0;
@@ -310,173 +247,6 @@ err:
     return -1;
 }
 
-static void ioreq_unmap(struct ioreq *ioreq)
-{
-    struct XenBlkDev *blkdev = ioreq->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-    int i;
-
-    if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
-        return;
-    }
-    if (batch_maps) {
-        if (!ioreq->pages) {
-            return;
-        }
-        xen_be_unmap_grant_refs(xendev, ioreq->pages, ioreq->num_unmap);
-        ioreq->blkdev->cnt_map -= ioreq->num_unmap;
-        ioreq->pages = NULL;
-    } else {
-        for (i = 0; i < ioreq->num_unmap; i++) {
-            if (!ioreq->page[i]) {
-                continue;
-            }
-            xen_be_unmap_grant_ref(xendev, ioreq->page[i]);
-            ioreq->blkdev->cnt_map--;
-            ioreq->page[i] = NULL;
-        }
-    }
-    ioreq->mapped = 0;
-}
-
-static int ioreq_map(struct ioreq *ioreq)
-{
-    struct XenBlkDev *blkdev = ioreq->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-    uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    int i, j, new_maps = 0;
-    PersistentGrant *grant;
-    PersistentRegion *region;
-    /* refs variable will contain the information necessary
-     * to map the grants that are needed to fulfill this request.
-     *
-     * After mapping the needed grants, the page array will contain the
-     * memory address of each granted page in the order specified in ioreq
-     * (disregarding if it's a persistent grant or not).
-     */
-
-    if (ioreq->v.niov == 0 || ioreq->mapped == 1) {
-        return 0;
-    }
-    if (ioreq->blkdev->feature_persistent) {
-        for (i = 0; i < ioreq->v.niov; i++) {
-            grant = g_tree_lookup(ioreq->blkdev->persistent_gnts,
-                                    GUINT_TO_POINTER(ioreq->refs[i]));
-
-            if (grant != NULL) {
-                page[i] = grant->page;
-                xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                              "using persistent-grant %" PRIu32 "\n",
-                              ioreq->refs[i]);
-            } else {
-                    /* Add the grant to the list of grants that
-                     * should be mapped
-                     */
-                    refs[new_maps] = ioreq->refs[i];
-                    page[i] = NULL;
-                    new_maps++;
-            }
-        }
-        /* Set the protection to RW, since grants may be reused later
-         * with a different protection than the one needed for this request
-         */
-        ioreq->prot = PROT_WRITE | PROT_READ;
-    } else {
-        /* All grants in the request should be mapped */
-        memcpy(refs, ioreq->refs, sizeof(refs));
-        memset(page, 0, sizeof(page));
-        new_maps = ioreq->v.niov;
-    }
-
-    if (batch_maps && new_maps) {
-        ioreq->pages = xen_be_map_grant_refs(xendev, refs, new_maps,
-                                             ioreq->prot);
-        if (ioreq->pages == NULL) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                          "can't map %d grant refs (%s, %d maps)\n",
-                          new_maps, strerror(errno), ioreq->blkdev->cnt_map);
-            return -1;
-        }
-        for (i = 0, j = 0; i < ioreq->v.niov; i++) {
-            if (page[i] == NULL) {
-                page[i] = ioreq->pages + (j++) * XC_PAGE_SIZE;
-            }
-        }
-        ioreq->blkdev->cnt_map += new_maps;
-    } else if (new_maps)  {
-        for (i = 0; i < new_maps; i++) {
-            ioreq->page[i] = xen_be_map_grant_ref(xendev, refs[i],
-                                                  ioreq->prot);
-            if (ioreq->page[i] == NULL) {
-                xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                              "can't map grant ref %d (%s, %d maps)\n",
-                              refs[i], strerror(errno), ioreq->blkdev->cnt_map);
-                ioreq->mapped = 1;
-                ioreq_unmap(ioreq);
-                return -1;
-            }
-            ioreq->blkdev->cnt_map++;
-        }
-        for (i = 0, j = 0; i < ioreq->v.niov; i++) {
-            if (page[i] == NULL) {
-                page[i] = ioreq->page[j++];
-            }
-        }
-    }
-    if (ioreq->blkdev->feature_persistent && new_maps != 0 &&
-        (!batch_maps || (ioreq->blkdev->persistent_gnt_count + new_maps <=
-        ioreq->blkdev->max_grants))) {
-        /*
-         * If we are using persistent grants and batch mappings only
-         * add the new maps to the list of persistent grants if the whole
-         * area can be persistently mapped.
-         */
-        if (batch_maps) {
-            region = g_malloc0(sizeof(*region));
-            region->addr = ioreq->pages;
-            region->num = new_maps;
-            ioreq->blkdev->persistent_regions = g_slist_append(
-                                            ioreq->blkdev->persistent_regions,
-                                            region);
-        }
-        while ((ioreq->blkdev->persistent_gnt_count < ioreq->blkdev->max_grants)
-              && new_maps) {
-            /* Go through the list of newly mapped grants and add as many
-             * as possible to the list of persistently mapped grants.
-             *
-             * Since we start at the end of ioreq->page(s), we only need
-             * to decrease new_maps to prevent this granted pages from
-             * being unmapped in ioreq_unmap.
-             */
-            grant = g_malloc0(sizeof(*grant));
-            new_maps--;
-            if (batch_maps) {
-                grant->page = ioreq->pages + (new_maps) * XC_PAGE_SIZE;
-            } else {
-                grant->page = ioreq->page[new_maps];
-            }
-            grant->blkdev = ioreq->blkdev;
-            xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                          "adding grant %" PRIu32 " page: %p\n",
-                          refs[new_maps], grant->page);
-            g_tree_insert(ioreq->blkdev->persistent_gnts,
-                          GUINT_TO_POINTER(refs[new_maps]),
-                          grant);
-            ioreq->blkdev->persistent_gnt_count++;
-        }
-        assert(!batch_maps || new_maps == 0);
-    }
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->v.iov[i].iov_base += (uintptr_t)page[i];
-    }
-    ioreq->mapped = 1;
-    ioreq->num_unmap = new_maps;
-    return 0;
-}
-
-#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40800
-
 static void ioreq_free_copy_buffers(struct ioreq *ioreq)
 {
     int i;
@@ -546,22 +316,6 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
 
     return rc;
 }
-#else
-static void ioreq_free_copy_buffers(struct ioreq *ioreq)
-{
-    abort();
-}
-
-static int ioreq_init_copy_buffers(struct ioreq *ioreq)
-{
-    abort();
-}
-
-static int ioreq_grant_copy(struct ioreq *ioreq)
-{
-    abort();
-}
-#endif
 
 static int ioreq_runio_qemu_aio(struct ioreq *ioreq);
 
@@ -588,32 +342,28 @@ static void qemu_aio_complete(void *opaque, int ret)
         goto done;
     }
 
-    if (xen_feature_grant_copy) {
-        switch (ioreq->req.operation) {
-        case BLKIF_OP_READ:
-            /* in case of failure ioreq->aio_errors is increased */
-            if (ret == 0) {
-                ioreq_grant_copy(ioreq);
-            }
-            ioreq_free_copy_buffers(ioreq);
-            break;
-        case BLKIF_OP_WRITE:
-        case BLKIF_OP_FLUSH_DISKCACHE:
-            if (!ioreq->req.nr_segments) {
-                break;
-            }
-            ioreq_free_copy_buffers(ioreq);
-            break;
-        default:
+    switch (ioreq->req.operation) {
+    case BLKIF_OP_READ:
+        /* in case of failure ioreq->aio_errors is increased */
+        if (ret == 0) {
+            ioreq_grant_copy(ioreq);
+        }
+        ioreq_free_copy_buffers(ioreq);
+        break;
+    case BLKIF_OP_WRITE:
+    case BLKIF_OP_FLUSH_DISKCACHE:
+        if (!ioreq->req.nr_segments) {
             break;
         }
+        ioreq_free_copy_buffers(ioreq);
+        break;
+    default:
+        break;
     }
 
     ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
-    if (!xen_feature_grant_copy) {
-        ioreq_unmap(ioreq);
-    }
     ioreq_finish(ioreq);
+
     switch (ioreq->req.operation) {
     case BLKIF_OP_WRITE:
     case BLKIF_OP_FLUSH_DISKCACHE:
@@ -673,18 +423,13 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    if (xen_feature_grant_copy) {
-        ioreq_init_copy_buffers(ioreq);
-        if (ioreq->req.nr_segments && (ioreq->req.operation == BLKIF_OP_WRITE ||
-            ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
-            ioreq_grant_copy(ioreq)) {
-                ioreq_free_copy_buffers(ioreq);
-                goto err;
-        }
-    } else {
-        if (ioreq->req.nr_segments && ioreq_map(ioreq)) {
-            goto err;
-        }
+    ioreq_init_copy_buffers(ioreq);
+    if (ioreq->req.nr_segments &&
+        (ioreq->req.operation == BLKIF_OP_WRITE ||
+         ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
+        ioreq_grant_copy(ioreq)) {
+        ioreq_free_copy_buffers(ioreq);
+        goto err;
     }
 
     ioreq->aio_inflight++;
@@ -725,9 +470,6 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     }
     default:
         /* unknown operation (shouldn't happen -- parse catches this) */
-        if (!xen_feature_grant_copy) {
-            ioreq_unmap(ioreq);
-        }
         goto err;
     }
 
@@ -913,10 +655,6 @@ static void blk_alloc(struct XenDevice *xendev)
 
     blkdev->ctx = iothread_get_aio_context(blkdev->iothread);
     blkdev->bh = aio_bh_new(blkdev->ctx, blk_bh, blkdev);
-
-    if (xen_mode != XEN_EMULATE) {
-        batch_maps = 1;
-    }
 }
 
 static void blk_parse_discard(struct XenBlkDev *blkdev)
@@ -999,15 +737,10 @@ static int blk_init(struct XenDevice *xendev)
 
     blkdev->file_blk  = BLOCK_SIZE;
 
-    xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
-                  xen_feature_grant_copy ? "enabled" : "disabled");
-
     /* fill info
      * blk_connect supplies sector-size and sectors
      */
     xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
-    xenstore_write_be_int(&blkdev->xendev, "feature-persistent",
-                          !xen_feature_grant_copy);
     xenstore_write_be_int(&blkdev->xendev, "info", info);
 
     xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order",
@@ -1034,19 +767,10 @@ out_error:
     return -1;
 }
 
-/*
- * We need to account for the grant allocations requiring contiguous
- * chunks; the worst case number would be
- *     max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1,
- * but in order to keep things simple just use
- *     2 * max_req * max_seg.
- */
-#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg))
-
 static int blk_connect(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
-    int pers, index, qflags;
+    int index, qflags;
     bool readonly = true;
     bool writethrough = true;
     int order, ring_ref;
@@ -1168,11 +892,6 @@ static int blk_connect(struct XenDevice *xendev)
                              &blkdev->xendev.remote_port) == -1) {
         return -1;
     }
-    if (xenstore_read_fe_int(&blkdev->xendev, "feature-persistent", &pers)) {
-        blkdev->feature_persistent = FALSE;
-    } else {
-        blkdev->feature_persistent = !!pers;
-    }
 
     if (!blkdev->xendev.protocol) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
@@ -1207,11 +926,8 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    /* Calculate the maximum number of grants needed by ioreqs */
-    max_grants = MAX_GRANTS(blkdev->max_requests,
-                            BLKIF_MAX_SEGMENTS_PER_REQUEST);
     /* Add on the number needed for the ring pages */
-    max_grants += blkdev->nr_ring_ref;
+    max_grants = blkdev->nr_ring_ref;
 
     xen_be_set_max_grant_refs(xendev, max_grants);
 
@@ -1222,8 +938,6 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    blkdev->cnt_map++;
-
     switch (blkdev->protocol) {
     case BLKIF_PROTOCOL_NATIVE:
     {
@@ -1247,19 +961,6 @@ static int blk_connect(struct XenDevice *xendev)
     }
     }
 
-    if (blkdev->feature_persistent) {
-        /* Init persistent grants */
-        blkdev->max_grants = blkdev->max_requests *
-            BLKIF_MAX_SEGMENTS_PER_REQUEST;
-        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
-                                             NULL, NULL,
-                                             batch_maps ?
-                                             (GDestroyNotify)g_free :
-                                             (GDestroyNotify)destroy_grant);
-        blkdev->persistent_regions = NULL;
-        blkdev->persistent_gnt_count = 0;
-    }
-
     blk_set_aio_context(blkdev->blk, blkdev->ctx);
 
     xen_be_bind_evtchn(&blkdev->xendev);
@@ -1292,29 +993,8 @@ static void blk_disconnect(struct XenDevice *xendev)
     if (blkdev->sring) {
         xen_be_unmap_grant_refs(xendev, blkdev->sring,
                                 blkdev->nr_ring_ref);
-        blkdev->cnt_map--;
         blkdev->sring = NULL;
     }
-
-    /*
-     * Unmap persistent grants before switching to the closed state
-     * so the frontend can free them.
-     *
-     * In the !batch_maps case g_tree_destroy will take care of unmapping
-     * the grant, but in the batch_maps case we need to iterate over every
-     * region in persistent_regions and unmap it.
-     */
-    if (blkdev->feature_persistent) {
-        g_tree_destroy(blkdev->persistent_gnts);
-        assert(batch_maps || blkdev->persistent_gnt_count == 0);
-        if (batch_maps) {
-            blkdev->persistent_gnt_count = 0;
-            g_slist_foreach(blkdev->persistent_regions,
-                            (GFunc)remove_persistent_region, blkdev);
-            g_slist_free(blkdev->persistent_regions);
-        }
-        blkdev->feature_persistent = false;
-    }
 }
 
 static int blk_free(struct XenDevice *xendev)
-- 
2.1.4

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

* [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (7 preceding siblings ...)
  2018-05-04 19:26 ` Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Kevin Wolf, Paul Durrant, Stefano Stabellini, Max Reitz

Now that the (native or emulated) xen_be_copy_grant_refs() helper is
always available, the xen_disk code can be significantly simplified by
removing direct use of grant map and unmap operations.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - Squashed in separate patche removing persistent grant use
 - Re-based
---
 hw/block/xen_disk.c | 370 ++++------------------------------------------------
 1 file changed, 25 insertions(+), 345 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 66ed2b7..28be8b6 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -36,27 +36,9 @@
 
 /* ------------------------------------------------------------- */
 
-static int batch_maps   = 0;
-
-/* ------------------------------------------------------------- */
-
 #define BLOCK_SIZE  512
 #define IOCB_COUNT  (BLKIF_MAX_SEGMENTS_PER_REQUEST + 2)
 
-struct PersistentGrant {
-    void *page;
-    struct XenBlkDev *blkdev;
-};
-
-typedef struct PersistentGrant PersistentGrant;
-
-struct PersistentRegion {
-    void *addr;
-    int num;
-};
-
-typedef struct PersistentRegion PersistentRegion;
-
 struct ioreq {
     blkif_request_t     req;
     int16_t             status;
@@ -65,14 +47,11 @@ struct ioreq {
     off_t               start;
     QEMUIOVector        v;
     int                 presync;
-    uint8_t             mapped;
 
     /* grant mapping */
     uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    int                 prot;
     void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     void                *pages;
-    int                 num_unmap;
 
     /* aio status */
     int                 aio_inflight;
@@ -103,7 +82,6 @@ struct XenBlkDev {
     int                 protocol;
     blkif_back_rings_t  rings;
     int                 more_work;
-    int                 cnt_map;
 
     /* request lists */
     QLIST_HEAD(inflight_head, ioreq) inflight;
@@ -114,13 +92,7 @@ struct XenBlkDev {
     int                 requests_finished;
     unsigned int        max_requests;
 
-    /* Persistent grants extension */
     gboolean            feature_discard;
-    gboolean            feature_persistent;
-    GTree               *persistent_gnts;
-    GSList              *persistent_regions;
-    unsigned int        persistent_gnt_count;
-    unsigned int        max_grants;
 
     /* qemu block driver */
     DriveInfo           *dinfo;
@@ -139,10 +111,8 @@ static void ioreq_reset(struct ioreq *ioreq)
     ioreq->status = 0;
     ioreq->start = 0;
     ioreq->presync = 0;
-    ioreq->mapped = 0;
 
     memset(ioreq->refs, 0, sizeof(ioreq->refs));
-    ioreq->prot = 0;
     memset(ioreq->page, 0, sizeof(ioreq->page));
     ioreq->pages = NULL;
 
@@ -156,37 +126,6 @@ static void ioreq_reset(struct ioreq *ioreq)
     qemu_iovec_reset(&ioreq->v);
 }
 
-static gint int_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
-{
-    uint ua = GPOINTER_TO_UINT(a);
-    uint ub = GPOINTER_TO_UINT(b);
-    return (ua > ub) - (ua < ub);
-}
-
-static void destroy_grant(gpointer pgnt)
-{
-    PersistentGrant *grant = pgnt;
-    struct XenBlkDev *blkdev = grant->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-
-    xen_be_unmap_grant_ref(xendev, grant->page);
-    grant->blkdev->persistent_gnt_count--;
-    xen_pv_printf(xendev, 3, "unmapped grant %p\n", grant->page);
-    g_free(grant);
-}
-
-static void remove_persistent_region(gpointer data, gpointer dev)
-{
-    PersistentRegion *region = data;
-    struct XenBlkDev *blkdev = dev;
-    struct XenDevice *xendev = &blkdev->xendev;
-
-    xen_be_unmap_grant_refs(xendev, region->addr, region->num);
-    xen_pv_printf(xendev, 3, "unmapped grant region %p with %d pages\n",
-                  region->addr, region->num);
-    g_free(region);
-}
-
 static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
 {
     struct ioreq *ioreq = NULL;
@@ -254,7 +193,6 @@ static int ioreq_parse(struct ioreq *ioreq)
                   ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
-        ioreq->prot = PROT_WRITE; /* to memory */
         break;
     case BLKIF_OP_FLUSH_DISKCACHE:
         ioreq->presync = 1;
@@ -263,7 +201,6 @@ static int ioreq_parse(struct ioreq *ioreq)
         }
         /* fall through */
     case BLKIF_OP_WRITE:
-        ioreq->prot = PROT_READ; /* from memory */
         break;
     case BLKIF_OP_DISCARD:
         return 0;
@@ -310,173 +247,6 @@ err:
     return -1;
 }
 
-static void ioreq_unmap(struct ioreq *ioreq)
-{
-    struct XenBlkDev *blkdev = ioreq->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-    int i;
-
-    if (ioreq->num_unmap == 0 || ioreq->mapped == 0) {
-        return;
-    }
-    if (batch_maps) {
-        if (!ioreq->pages) {
-            return;
-        }
-        xen_be_unmap_grant_refs(xendev, ioreq->pages, ioreq->num_unmap);
-        ioreq->blkdev->cnt_map -= ioreq->num_unmap;
-        ioreq->pages = NULL;
-    } else {
-        for (i = 0; i < ioreq->num_unmap; i++) {
-            if (!ioreq->page[i]) {
-                continue;
-            }
-            xen_be_unmap_grant_ref(xendev, ioreq->page[i]);
-            ioreq->blkdev->cnt_map--;
-            ioreq->page[i] = NULL;
-        }
-    }
-    ioreq->mapped = 0;
-}
-
-static int ioreq_map(struct ioreq *ioreq)
-{
-    struct XenBlkDev *blkdev = ioreq->blkdev;
-    struct XenDevice *xendev = &blkdev->xendev;
-    uint32_t refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    int i, j, new_maps = 0;
-    PersistentGrant *grant;
-    PersistentRegion *region;
-    /* refs variable will contain the information necessary
-     * to map the grants that are needed to fulfill this request.
-     *
-     * After mapping the needed grants, the page array will contain the
-     * memory address of each granted page in the order specified in ioreq
-     * (disregarding if it's a persistent grant or not).
-     */
-
-    if (ioreq->v.niov == 0 || ioreq->mapped == 1) {
-        return 0;
-    }
-    if (ioreq->blkdev->feature_persistent) {
-        for (i = 0; i < ioreq->v.niov; i++) {
-            grant = g_tree_lookup(ioreq->blkdev->persistent_gnts,
-                                    GUINT_TO_POINTER(ioreq->refs[i]));
-
-            if (grant != NULL) {
-                page[i] = grant->page;
-                xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                              "using persistent-grant %" PRIu32 "\n",
-                              ioreq->refs[i]);
-            } else {
-                    /* Add the grant to the list of grants that
-                     * should be mapped
-                     */
-                    refs[new_maps] = ioreq->refs[i];
-                    page[i] = NULL;
-                    new_maps++;
-            }
-        }
-        /* Set the protection to RW, since grants may be reused later
-         * with a different protection than the one needed for this request
-         */
-        ioreq->prot = PROT_WRITE | PROT_READ;
-    } else {
-        /* All grants in the request should be mapped */
-        memcpy(refs, ioreq->refs, sizeof(refs));
-        memset(page, 0, sizeof(page));
-        new_maps = ioreq->v.niov;
-    }
-
-    if (batch_maps && new_maps) {
-        ioreq->pages = xen_be_map_grant_refs(xendev, refs, new_maps,
-                                             ioreq->prot);
-        if (ioreq->pages == NULL) {
-            xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                          "can't map %d grant refs (%s, %d maps)\n",
-                          new_maps, strerror(errno), ioreq->blkdev->cnt_map);
-            return -1;
-        }
-        for (i = 0, j = 0; i < ioreq->v.niov; i++) {
-            if (page[i] == NULL) {
-                page[i] = ioreq->pages + (j++) * XC_PAGE_SIZE;
-            }
-        }
-        ioreq->blkdev->cnt_map += new_maps;
-    } else if (new_maps)  {
-        for (i = 0; i < new_maps; i++) {
-            ioreq->page[i] = xen_be_map_grant_ref(xendev, refs[i],
-                                                  ioreq->prot);
-            if (ioreq->page[i] == NULL) {
-                xen_pv_printf(&ioreq->blkdev->xendev, 0,
-                              "can't map grant ref %d (%s, %d maps)\n",
-                              refs[i], strerror(errno), ioreq->blkdev->cnt_map);
-                ioreq->mapped = 1;
-                ioreq_unmap(ioreq);
-                return -1;
-            }
-            ioreq->blkdev->cnt_map++;
-        }
-        for (i = 0, j = 0; i < ioreq->v.niov; i++) {
-            if (page[i] == NULL) {
-                page[i] = ioreq->page[j++];
-            }
-        }
-    }
-    if (ioreq->blkdev->feature_persistent && new_maps != 0 &&
-        (!batch_maps || (ioreq->blkdev->persistent_gnt_count + new_maps <=
-        ioreq->blkdev->max_grants))) {
-        /*
-         * If we are using persistent grants and batch mappings only
-         * add the new maps to the list of persistent grants if the whole
-         * area can be persistently mapped.
-         */
-        if (batch_maps) {
-            region = g_malloc0(sizeof(*region));
-            region->addr = ioreq->pages;
-            region->num = new_maps;
-            ioreq->blkdev->persistent_regions = g_slist_append(
-                                            ioreq->blkdev->persistent_regions,
-                                            region);
-        }
-        while ((ioreq->blkdev->persistent_gnt_count < ioreq->blkdev->max_grants)
-              && new_maps) {
-            /* Go through the list of newly mapped grants and add as many
-             * as possible to the list of persistently mapped grants.
-             *
-             * Since we start at the end of ioreq->page(s), we only need
-             * to decrease new_maps to prevent this granted pages from
-             * being unmapped in ioreq_unmap.
-             */
-            grant = g_malloc0(sizeof(*grant));
-            new_maps--;
-            if (batch_maps) {
-                grant->page = ioreq->pages + (new_maps) * XC_PAGE_SIZE;
-            } else {
-                grant->page = ioreq->page[new_maps];
-            }
-            grant->blkdev = ioreq->blkdev;
-            xen_pv_printf(&ioreq->blkdev->xendev, 3,
-                          "adding grant %" PRIu32 " page: %p\n",
-                          refs[new_maps], grant->page);
-            g_tree_insert(ioreq->blkdev->persistent_gnts,
-                          GUINT_TO_POINTER(refs[new_maps]),
-                          grant);
-            ioreq->blkdev->persistent_gnt_count++;
-        }
-        assert(!batch_maps || new_maps == 0);
-    }
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->v.iov[i].iov_base += (uintptr_t)page[i];
-    }
-    ioreq->mapped = 1;
-    ioreq->num_unmap = new_maps;
-    return 0;
-}
-
-#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 40800
-
 static void ioreq_free_copy_buffers(struct ioreq *ioreq)
 {
     int i;
@@ -546,22 +316,6 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
 
     return rc;
 }
-#else
-static void ioreq_free_copy_buffers(struct ioreq *ioreq)
-{
-    abort();
-}
-
-static int ioreq_init_copy_buffers(struct ioreq *ioreq)
-{
-    abort();
-}
-
-static int ioreq_grant_copy(struct ioreq *ioreq)
-{
-    abort();
-}
-#endif
 
 static int ioreq_runio_qemu_aio(struct ioreq *ioreq);
 
@@ -588,32 +342,28 @@ static void qemu_aio_complete(void *opaque, int ret)
         goto done;
     }
 
-    if (xen_feature_grant_copy) {
-        switch (ioreq->req.operation) {
-        case BLKIF_OP_READ:
-            /* in case of failure ioreq->aio_errors is increased */
-            if (ret == 0) {
-                ioreq_grant_copy(ioreq);
-            }
-            ioreq_free_copy_buffers(ioreq);
-            break;
-        case BLKIF_OP_WRITE:
-        case BLKIF_OP_FLUSH_DISKCACHE:
-            if (!ioreq->req.nr_segments) {
-                break;
-            }
-            ioreq_free_copy_buffers(ioreq);
-            break;
-        default:
+    switch (ioreq->req.operation) {
+    case BLKIF_OP_READ:
+        /* in case of failure ioreq->aio_errors is increased */
+        if (ret == 0) {
+            ioreq_grant_copy(ioreq);
+        }
+        ioreq_free_copy_buffers(ioreq);
+        break;
+    case BLKIF_OP_WRITE:
+    case BLKIF_OP_FLUSH_DISKCACHE:
+        if (!ioreq->req.nr_segments) {
             break;
         }
+        ioreq_free_copy_buffers(ioreq);
+        break;
+    default:
+        break;
     }
 
     ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
-    if (!xen_feature_grant_copy) {
-        ioreq_unmap(ioreq);
-    }
     ioreq_finish(ioreq);
+
     switch (ioreq->req.operation) {
     case BLKIF_OP_WRITE:
     case BLKIF_OP_FLUSH_DISKCACHE:
@@ -673,18 +423,13 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    if (xen_feature_grant_copy) {
-        ioreq_init_copy_buffers(ioreq);
-        if (ioreq->req.nr_segments && (ioreq->req.operation == BLKIF_OP_WRITE ||
-            ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
-            ioreq_grant_copy(ioreq)) {
-                ioreq_free_copy_buffers(ioreq);
-                goto err;
-        }
-    } else {
-        if (ioreq->req.nr_segments && ioreq_map(ioreq)) {
-            goto err;
-        }
+    ioreq_init_copy_buffers(ioreq);
+    if (ioreq->req.nr_segments &&
+        (ioreq->req.operation == BLKIF_OP_WRITE ||
+         ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
+        ioreq_grant_copy(ioreq)) {
+        ioreq_free_copy_buffers(ioreq);
+        goto err;
     }
 
     ioreq->aio_inflight++;
@@ -725,9 +470,6 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     }
     default:
         /* unknown operation (shouldn't happen -- parse catches this) */
-        if (!xen_feature_grant_copy) {
-            ioreq_unmap(ioreq);
-        }
         goto err;
     }
 
@@ -913,10 +655,6 @@ static void blk_alloc(struct XenDevice *xendev)
 
     blkdev->ctx = iothread_get_aio_context(blkdev->iothread);
     blkdev->bh = aio_bh_new(blkdev->ctx, blk_bh, blkdev);
-
-    if (xen_mode != XEN_EMULATE) {
-        batch_maps = 1;
-    }
 }
 
 static void blk_parse_discard(struct XenBlkDev *blkdev)
@@ -999,15 +737,10 @@ static int blk_init(struct XenDevice *xendev)
 
     blkdev->file_blk  = BLOCK_SIZE;
 
-    xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
-                  xen_feature_grant_copy ? "enabled" : "disabled");
-
     /* fill info
      * blk_connect supplies sector-size and sectors
      */
     xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
-    xenstore_write_be_int(&blkdev->xendev, "feature-persistent",
-                          !xen_feature_grant_copy);
     xenstore_write_be_int(&blkdev->xendev, "info", info);
 
     xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order",
@@ -1034,19 +767,10 @@ out_error:
     return -1;
 }
 
-/*
- * We need to account for the grant allocations requiring contiguous
- * chunks; the worst case number would be
- *     max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1,
- * but in order to keep things simple just use
- *     2 * max_req * max_seg.
- */
-#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg))
-
 static int blk_connect(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
-    int pers, index, qflags;
+    int index, qflags;
     bool readonly = true;
     bool writethrough = true;
     int order, ring_ref;
@@ -1168,11 +892,6 @@ static int blk_connect(struct XenDevice *xendev)
                              &blkdev->xendev.remote_port) == -1) {
         return -1;
     }
-    if (xenstore_read_fe_int(&blkdev->xendev, "feature-persistent", &pers)) {
-        blkdev->feature_persistent = FALSE;
-    } else {
-        blkdev->feature_persistent = !!pers;
-    }
 
     if (!blkdev->xendev.protocol) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
@@ -1207,11 +926,8 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    /* Calculate the maximum number of grants needed by ioreqs */
-    max_grants = MAX_GRANTS(blkdev->max_requests,
-                            BLKIF_MAX_SEGMENTS_PER_REQUEST);
     /* Add on the number needed for the ring pages */
-    max_grants += blkdev->nr_ring_ref;
+    max_grants = blkdev->nr_ring_ref;
 
     xen_be_set_max_grant_refs(xendev, max_grants);
 
@@ -1222,8 +938,6 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    blkdev->cnt_map++;
-
     switch (blkdev->protocol) {
     case BLKIF_PROTOCOL_NATIVE:
     {
@@ -1247,19 +961,6 @@ static int blk_connect(struct XenDevice *xendev)
     }
     }
 
-    if (blkdev->feature_persistent) {
-        /* Init persistent grants */
-        blkdev->max_grants = blkdev->max_requests *
-            BLKIF_MAX_SEGMENTS_PER_REQUEST;
-        blkdev->persistent_gnts = g_tree_new_full((GCompareDataFunc)int_cmp,
-                                             NULL, NULL,
-                                             batch_maps ?
-                                             (GDestroyNotify)g_free :
-                                             (GDestroyNotify)destroy_grant);
-        blkdev->persistent_regions = NULL;
-        blkdev->persistent_gnt_count = 0;
-    }
-
     blk_set_aio_context(blkdev->blk, blkdev->ctx);
 
     xen_be_bind_evtchn(&blkdev->xendev);
@@ -1292,29 +993,8 @@ static void blk_disconnect(struct XenDevice *xendev)
     if (blkdev->sring) {
         xen_be_unmap_grant_refs(xendev, blkdev->sring,
                                 blkdev->nr_ring_ref);
-        blkdev->cnt_map--;
         blkdev->sring = NULL;
     }
-
-    /*
-     * Unmap persistent grants before switching to the closed state
-     * so the frontend can free them.
-     *
-     * In the !batch_maps case g_tree_destroy will take care of unmapping
-     * the grant, but in the batch_maps case we need to iterate over every
-     * region in persistent_regions and unmap it.
-     */
-    if (blkdev->feature_persistent) {
-        g_tree_destroy(blkdev->persistent_gnts);
-        assert(batch_maps || blkdev->persistent_gnt_count == 0);
-        if (batch_maps) {
-            blkdev->persistent_gnt_count = 0;
-            g_slist_foreach(blkdev->persistent_regions,
-                            (GFunc)remove_persistent_region, blkdev);
-            g_slist_free(blkdev->persistent_regions);
-        }
-        blkdev->feature_persistent = false;
-    }
 }
 
 static int blk_free(struct XenDevice *xendev)
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
@ 2018-05-04 19:26   ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
                     ` (13 subsequent siblings)
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard

There is no longer any use of this flag outside of the xen_backend code.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c         | 2 +-
 include/hw/xen/xen_backend.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 3c3fc2c..9a8e877 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -44,9 +44,9 @@ BusState *xen_sysbus;
 /* public */
 struct xs_handle *xenstore = NULL;
 const char *xen_protocol;
-bool xen_feature_grant_copy;
 
 /* private */
+static bool xen_feature_grant_copy;
 static int debug;
 
 int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const char *val)
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 29bf1c3..9c17fdd 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -16,7 +16,6 @@
 /* variables */
 extern struct xs_handle *xenstore;
 extern const char *xen_protocol;
-extern bool xen_feature_grant_copy;
 extern DeviceState *xen_sysdev;
 extern BusState *xen_sysbus;
 
-- 
2.1.4

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

* [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private
@ 2018-05-04 19:26   ` Paul Durrant
  0 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Paul Durrant, Stefano Stabellini

There is no longer any use of this flag outside of the xen_backend code.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>

v2:
 - New in v2
---
 hw/xen/xen_backend.c         | 2 +-
 include/hw/xen/xen_backend.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index 3c3fc2c..9a8e877 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -44,9 +44,9 @@ BusState *xen_sysbus;
 /* public */
 struct xs_handle *xenstore = NULL;
 const char *xen_protocol;
-bool xen_feature_grant_copy;
 
 /* private */
+static bool xen_feature_grant_copy;
 static int debug;
 
 int xenstore_write_be_str(struct XenDevice *xendev, const char *node, const char *val)
diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
index 29bf1c3..9c17fdd 100644
--- a/include/hw/xen/xen_backend.h
+++ b/include/hw/xen/xen_backend.h
@@ -16,7 +16,6 @@
 /* variables */
 extern struct xs_handle *xenstore;
 extern const char *xen_protocol;
-extern bool xen_feature_grant_copy;
 extern DeviceState *xen_sysdev;
 extern BusState *xen_sysbus;
 
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (10 preceding siblings ...)
  2018-05-04 19:26   ` Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-17 10:59   ` Anthony PERARD
  2018-05-17 10:59   ` [Qemu-devel] " Anthony PERARD
  2018-05-04 19:26 ` Paul Durrant
                   ` (2 subsequent siblings)
  14 siblings, 2 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard, Kevin Wolf, Max Reitz

Since xen_disk now always copies data to and from a guest there is no need
to maintain a vector entry corresponding to every page of a request.
This means there is less per-request state to maintain so the ioreq
structure can shrink significantly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v3:
 - Un-break by fixing mis-placed qemu_iovec_add()

v2:
 - Re-based
---
 hw/block/xen_disk.c | 76 +++++++++++++++--------------------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 28be8b6..28651c5 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -46,13 +46,10 @@ struct ioreq {
     /* parsed request */
     off_t               start;
     QEMUIOVector        v;
+    void                *buf;
+    size_t              size;
     int                 presync;
 
-    /* grant mapping */
-    uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void                *pages;
-
     /* aio status */
     int                 aio_inflight;
     int                 aio_errors;
@@ -110,12 +107,10 @@ static void ioreq_reset(struct ioreq *ioreq)
     memset(&ioreq->req, 0, sizeof(ioreq->req));
     ioreq->status = 0;
     ioreq->start = 0;
+    ioreq->buf = NULL;
+    ioreq->size = 0;
     ioreq->presync = 0;
 
-    memset(ioreq->refs, 0, sizeof(ioreq->refs));
-    memset(ioreq->page, 0, sizeof(ioreq->page));
-    ioreq->pages = NULL;
-
     ioreq->aio_inflight = 0;
     ioreq->aio_errors = 0;
 
@@ -138,7 +133,7 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
         ioreq = g_malloc0(sizeof(*ioreq));
         ioreq->blkdev = blkdev;
         blkdev->requests_total++;
-        qemu_iovec_init(&ioreq->v, BLKIF_MAX_SEGMENTS_PER_REQUEST);
+        qemu_iovec_init(&ioreq->v, 1);
     } else {
         /* get one from freelist */
         ioreq = QLIST_FIRST(&blkdev->freelist);
@@ -183,7 +178,6 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
 static int ioreq_parse(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
-    uintptr_t mem;
     size_t len;
     int i;
 
@@ -230,13 +224,10 @@ static int ioreq_parse(struct ioreq *ioreq)
             goto err;
         }
 
-        ioreq->refs[i]   = ioreq->req.seg[i].gref;
-
-        mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
         len = (ioreq->req.seg[i].last_sect - ioreq->req.seg[i].first_sect + 1) * blkdev->file_blk;
-        qemu_iovec_add(&ioreq->v, (void*)mem, len);
+        ioreq->size += len;
     }
-    if (ioreq->start + ioreq->v.size > blkdev->file_size) {
+    if (ioreq->start + ioreq->size > blkdev->file_size) {
         xen_pv_printf(&blkdev->xendev, 0, "error: access beyond end of file\n");
         goto err;
     }
@@ -247,35 +238,6 @@ err:
     return -1;
 }
 
-static void ioreq_free_copy_buffers(struct ioreq *ioreq)
-{
-    int i;
-
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->page[i] = NULL;
-    }
-
-    qemu_vfree(ioreq->pages);
-}
-
-static int ioreq_init_copy_buffers(struct ioreq *ioreq)
-{
-    int i;
-
-    if (ioreq->v.niov == 0) {
-        return 0;
-    }
-
-    ioreq->pages = qemu_memalign(XC_PAGE_SIZE, ioreq->v.niov * XC_PAGE_SIZE);
-
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->page[i] = ioreq->pages + i * XC_PAGE_SIZE;
-        ioreq->v.iov[i].iov_base = ioreq->page[i];
-    }
-
-    return 0;
-}
-
 static int ioreq_grant_copy(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
@@ -284,25 +246,27 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     int i, count, rc;
     int64_t file_blk = ioreq->blkdev->file_blk;
     bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
+    void *virt = ioreq->buf;
 
-    if (ioreq->v.niov == 0) {
+    if (ioreq->req.nr_segments == 0) {
         return 0;
     }
 
-    count = ioreq->v.niov;
+    count = ioreq->req.nr_segments;
 
     for (i = 0; i < count; i++) {
         if (to_domain) {
-            segs[i].dest.foreign.ref = ioreq->refs[i];
+            segs[i].dest.foreign.ref = ioreq->req.seg[i].gref;
             segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
-            segs[i].source.virt = ioreq->v.iov[i].iov_base;
+            segs[i].source.virt = virt;
         } else {
-            segs[i].source.foreign.ref = ioreq->refs[i];
+            segs[i].source.foreign.ref = ioreq->req.seg[i].gref;
             segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
-            segs[i].dest.virt = ioreq->v.iov[i].iov_base;
+            segs[i].dest.virt = virt;
         }
         segs[i].len = (ioreq->req.seg[i].last_sect
                        - ioreq->req.seg[i].first_sect + 1) * file_blk;
+        virt += segs[i].len;
     }
 
     rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
@@ -348,14 +312,14 @@ static void qemu_aio_complete(void *opaque, int ret)
         if (ret == 0) {
             ioreq_grant_copy(ioreq);
         }
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         break;
     case BLKIF_OP_WRITE:
     case BLKIF_OP_FLUSH_DISKCACHE:
         if (!ioreq->req.nr_segments) {
             break;
         }
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         break;
     default:
         break;
@@ -423,12 +387,12 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    ioreq_init_copy_buffers(ioreq);
+    ioreq->buf = qemu_memalign(XC_PAGE_SIZE, ioreq->size);
     if (ioreq->req.nr_segments &&
         (ioreq->req.operation == BLKIF_OP_WRITE ||
          ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
         ioreq_grant_copy(ioreq)) {
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         goto err;
     }
 
@@ -440,6 +404,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
+        qemu_iovec_add(&ioreq->v, ioreq->buf, ioreq->size);
         block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
                          ioreq->v.size, BLOCK_ACCT_READ);
         ioreq->aio_inflight++;
@@ -452,6 +417,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
             break;
         }
 
+        qemu_iovec_add(&ioreq->v, ioreq->buf, ioreq->size);
         block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
                          ioreq->v.size,
                          ioreq->req.operation == BLKIF_OP_WRITE ?
-- 
2.1.4

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

* [PATCH v3 7/8] xen_disk: use a single entry iovec
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (11 preceding siblings ...)
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
  2018-05-04 19:26 ` Paul Durrant
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Kevin Wolf, Paul Durrant, Stefano Stabellini, Max Reitz

Since xen_disk now always copies data to and from a guest there is no need
to maintain a vector entry corresponding to every page of a request.
This means there is less per-request state to maintain so the ioreq
structure can shrink significantly.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v3:
 - Un-break by fixing mis-placed qemu_iovec_add()

v2:
 - Re-based
---
 hw/block/xen_disk.c | 76 +++++++++++++++--------------------------------------
 1 file changed, 21 insertions(+), 55 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 28be8b6..28651c5 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -46,13 +46,10 @@ struct ioreq {
     /* parsed request */
     off_t               start;
     QEMUIOVector        v;
+    void                *buf;
+    size_t              size;
     int                 presync;
 
-    /* grant mapping */
-    uint32_t            refs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void                *page[BLKIF_MAX_SEGMENTS_PER_REQUEST];
-    void                *pages;
-
     /* aio status */
     int                 aio_inflight;
     int                 aio_errors;
@@ -110,12 +107,10 @@ static void ioreq_reset(struct ioreq *ioreq)
     memset(&ioreq->req, 0, sizeof(ioreq->req));
     ioreq->status = 0;
     ioreq->start = 0;
+    ioreq->buf = NULL;
+    ioreq->size = 0;
     ioreq->presync = 0;
 
-    memset(ioreq->refs, 0, sizeof(ioreq->refs));
-    memset(ioreq->page, 0, sizeof(ioreq->page));
-    ioreq->pages = NULL;
-
     ioreq->aio_inflight = 0;
     ioreq->aio_errors = 0;
 
@@ -138,7 +133,7 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
         ioreq = g_malloc0(sizeof(*ioreq));
         ioreq->blkdev = blkdev;
         blkdev->requests_total++;
-        qemu_iovec_init(&ioreq->v, BLKIF_MAX_SEGMENTS_PER_REQUEST);
+        qemu_iovec_init(&ioreq->v, 1);
     } else {
         /* get one from freelist */
         ioreq = QLIST_FIRST(&blkdev->freelist);
@@ -183,7 +178,6 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
 static int ioreq_parse(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
-    uintptr_t mem;
     size_t len;
     int i;
 
@@ -230,13 +224,10 @@ static int ioreq_parse(struct ioreq *ioreq)
             goto err;
         }
 
-        ioreq->refs[i]   = ioreq->req.seg[i].gref;
-
-        mem = ioreq->req.seg[i].first_sect * blkdev->file_blk;
         len = (ioreq->req.seg[i].last_sect - ioreq->req.seg[i].first_sect + 1) * blkdev->file_blk;
-        qemu_iovec_add(&ioreq->v, (void*)mem, len);
+        ioreq->size += len;
     }
-    if (ioreq->start + ioreq->v.size > blkdev->file_size) {
+    if (ioreq->start + ioreq->size > blkdev->file_size) {
         xen_pv_printf(&blkdev->xendev, 0, "error: access beyond end of file\n");
         goto err;
     }
@@ -247,35 +238,6 @@ err:
     return -1;
 }
 
-static void ioreq_free_copy_buffers(struct ioreq *ioreq)
-{
-    int i;
-
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->page[i] = NULL;
-    }
-
-    qemu_vfree(ioreq->pages);
-}
-
-static int ioreq_init_copy_buffers(struct ioreq *ioreq)
-{
-    int i;
-
-    if (ioreq->v.niov == 0) {
-        return 0;
-    }
-
-    ioreq->pages = qemu_memalign(XC_PAGE_SIZE, ioreq->v.niov * XC_PAGE_SIZE);
-
-    for (i = 0; i < ioreq->v.niov; i++) {
-        ioreq->page[i] = ioreq->pages + i * XC_PAGE_SIZE;
-        ioreq->v.iov[i].iov_base = ioreq->page[i];
-    }
-
-    return 0;
-}
-
 static int ioreq_grant_copy(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
@@ -284,25 +246,27 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     int i, count, rc;
     int64_t file_blk = ioreq->blkdev->file_blk;
     bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
+    void *virt = ioreq->buf;
 
-    if (ioreq->v.niov == 0) {
+    if (ioreq->req.nr_segments == 0) {
         return 0;
     }
 
-    count = ioreq->v.niov;
+    count = ioreq->req.nr_segments;
 
     for (i = 0; i < count; i++) {
         if (to_domain) {
-            segs[i].dest.foreign.ref = ioreq->refs[i];
+            segs[i].dest.foreign.ref = ioreq->req.seg[i].gref;
             segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
-            segs[i].source.virt = ioreq->v.iov[i].iov_base;
+            segs[i].source.virt = virt;
         } else {
-            segs[i].source.foreign.ref = ioreq->refs[i];
+            segs[i].source.foreign.ref = ioreq->req.seg[i].gref;
             segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * file_blk;
-            segs[i].dest.virt = ioreq->v.iov[i].iov_base;
+            segs[i].dest.virt = virt;
         }
         segs[i].len = (ioreq->req.seg[i].last_sect
                        - ioreq->req.seg[i].first_sect + 1) * file_blk;
+        virt += segs[i].len;
     }
 
     rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
@@ -348,14 +312,14 @@ static void qemu_aio_complete(void *opaque, int ret)
         if (ret == 0) {
             ioreq_grant_copy(ioreq);
         }
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         break;
     case BLKIF_OP_WRITE:
     case BLKIF_OP_FLUSH_DISKCACHE:
         if (!ioreq->req.nr_segments) {
             break;
         }
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         break;
     default:
         break;
@@ -423,12 +387,12 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    ioreq_init_copy_buffers(ioreq);
+    ioreq->buf = qemu_memalign(XC_PAGE_SIZE, ioreq->size);
     if (ioreq->req.nr_segments &&
         (ioreq->req.operation == BLKIF_OP_WRITE ||
          ioreq->req.operation == BLKIF_OP_FLUSH_DISKCACHE) &&
         ioreq_grant_copy(ioreq)) {
-        ioreq_free_copy_buffers(ioreq);
+        qemu_vfree(ioreq->buf);
         goto err;
     }
 
@@ -440,6 +404,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 
     switch (ioreq->req.operation) {
     case BLKIF_OP_READ:
+        qemu_iovec_add(&ioreq->v, ioreq->buf, ioreq->size);
         block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
                          ioreq->v.size, BLOCK_ACCT_READ);
         ioreq->aio_inflight++;
@@ -452,6 +417,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
             break;
         }
 
+        qemu_iovec_add(&ioreq->v, ioreq->buf, ioreq->size);
         block_acct_start(blk_get_stats(blkdev->blk), &ioreq->acct,
                          ioreq->v.size,
                          ioreq->req.operation == BLKIF_OP_WRITE ?
-- 
2.1.4


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

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

* [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (12 preceding siblings ...)
  2018-05-04 19:26 ` Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  2018-05-17 11:07   ` Anthony PERARD
  2018-05-17 11:07   ` [Qemu-devel] " Anthony PERARD
  2018-05-04 19:26 ` Paul Durrant
  14 siblings, 2 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Paul Durrant, Stefano Stabellini, Anthony Perard, Kevin Wolf, Max Reitz

Certain functions in xen_disk are called with a pointer to xendev
(struct XenDevice *). They then use continer_of() to acces the surrounding
blkdev (struct XenBlkDev) but then in various places use &blkdev->xendev
when use of the original xendev pointer is shorter to express and clearly
equivalent.

This patch is a purely cosmetic patch which makes sure there is a xendev
pointer on stack for any function where the pointer is need on multiple
occasions modified those functions to use it consistently.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - Re-based
---
 hw/block/xen_disk.c | 90 +++++++++++++++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 28651c5..9fbc0cd 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -178,10 +178,11 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
 static int ioreq_parse(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     size_t len;
     int i;
 
-    xen_pv_printf(&blkdev->xendev, 3,
+    xen_pv_printf(xendev, 3,
                   "op %d, nr %d, handle %d, id %" PRId64 ", sector %" PRId64 "\n",
                   ioreq->req.operation, ioreq->req.nr_segments,
                   ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
@@ -199,28 +200,28 @@ static int ioreq_parse(struct ioreq *ioreq)
     case BLKIF_OP_DISCARD:
         return 0;
     default:
-        xen_pv_printf(&blkdev->xendev, 0, "error: unknown operation (%d)\n",
+        xen_pv_printf(xendev, 0, "error: unknown operation (%d)\n",
                       ioreq->req.operation);
         goto err;
     };
 
     if (ioreq->req.operation != BLKIF_OP_READ && blkdev->mode[0] != 'w') {
-        xen_pv_printf(&blkdev->xendev, 0, "error: write req for ro device\n");
+        xen_pv_printf(xendev, 0, "error: write req for ro device\n");
         goto err;
     }
 
     ioreq->start = ioreq->req.sector_number * blkdev->file_blk;
     for (i = 0; i < ioreq->req.nr_segments; i++) {
         if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: nr_segments too big\n");
+            xen_pv_printf(xendev, 0, "error: nr_segments too big\n");
             goto err;
         }
         if (ioreq->req.seg[i].first_sect > ioreq->req.seg[i].last_sect) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: first > last sector\n");
+            xen_pv_printf(xendev, 0, "error: first > last sector\n");
             goto err;
         }
         if (ioreq->req.seg[i].last_sect * BLOCK_SIZE >= XC_PAGE_SIZE) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: page crossing\n");
+            xen_pv_printf(xendev, 0, "error: page crossing\n");
             goto err;
         }
 
@@ -228,7 +229,7 @@ static int ioreq_parse(struct ioreq *ioreq)
         ioreq->size += len;
     }
     if (ioreq->start + ioreq->size > blkdev->file_size) {
-        xen_pv_printf(&blkdev->xendev, 0, "error: access beyond end of file\n");
+        xen_pv_printf(xendev, 0, "error: access beyond end of file\n");
         goto err;
     }
     return 0;
@@ -244,7 +245,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     struct XenDevice *xendev = &blkdev->xendev;
     XenGrantCopySegment segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, count, rc;
-    int64_t file_blk = ioreq->blkdev->file_blk;
+    int64_t file_blk = blkdev->file_blk;
     bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
     void *virt = ioreq->buf;
 
@@ -272,7 +273,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
 
     if (rc) {
-        xen_pv_printf(&ioreq->blkdev->xendev, 0,
+        xen_pv_printf(xendev, 0,
                       "failed to copy data %d\n", rc);
         ioreq->aio_errors++;
         return -1;
@@ -287,11 +288,12 @@ static void qemu_aio_complete(void *opaque, int ret)
 {
     struct ioreq *ioreq = opaque;
     struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
     aio_context_acquire(blkdev->ctx);
 
     if (ret != 0) {
-        xen_pv_printf(&blkdev->xendev, 0, "%s I/O error\n",
+        xen_pv_printf(xendev, 0, "%s I/O error\n",
                       ioreq->req.operation == BLKIF_OP_READ ? "read" : "write");
         ioreq->aio_errors++;
     }
@@ -625,16 +627,17 @@ static void blk_alloc(struct XenDevice *xendev)
 
 static void blk_parse_discard(struct XenBlkDev *blkdev)
 {
+    struct XenDevice *xendev = &blkdev->xendev;
     int enable;
 
     blkdev->feature_discard = true;
 
-    if (xenstore_read_be_int(&blkdev->xendev, "discard-enable", &enable) == 0) {
+    if (xenstore_read_be_int(xendev, "discard-enable", &enable) == 0) {
         blkdev->feature_discard = !!enable;
     }
 
     if (blkdev->feature_discard) {
-        xenstore_write_be_int(&blkdev->xendev, "feature-discard", 1);
+        xenstore_write_be_int(xendev, "feature-discard", 1);
     }
 }
 
@@ -649,7 +652,7 @@ static int blk_init(struct XenDevice *xendev)
     /* read xenstore entries */
     if (blkdev->params == NULL) {
         char *h = NULL;
-        blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
+        blkdev->params = xenstore_read_be_str(xendev, "params");
         if (blkdev->params != NULL) {
             h = strchr(blkdev->params, ':');
         }
@@ -669,18 +672,18 @@ static int blk_init(struct XenDevice *xendev)
         blkdev->fileproto = "vpc";
     }
     if (blkdev->mode == NULL) {
-        blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
+        blkdev->mode = xenstore_read_be_str(xendev, "mode");
     }
     if (blkdev->type == NULL) {
-        blkdev->type = xenstore_read_be_str(&blkdev->xendev, "type");
+        blkdev->type = xenstore_read_be_str(xendev, "type");
     }
     if (blkdev->dev == NULL) {
-        blkdev->dev = xenstore_read_be_str(&blkdev->xendev, "dev");
+        blkdev->dev = xenstore_read_be_str(xendev, "dev");
     }
     if (blkdev->devtype == NULL) {
-        blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
+        blkdev->devtype = xenstore_read_be_str(xendev, "device-type");
     }
-    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
+    directiosafe = xenstore_read_be_str(xendev, "direct-io-safe");
     blkdev->directiosafe = (directiosafe && atoi(directiosafe));
 
     /* do we have all we need? */
@@ -706,10 +709,10 @@ static int blk_init(struct XenDevice *xendev)
     /* fill info
      * blk_connect supplies sector-size and sectors
      */
-    xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
-    xenstore_write_be_int(&blkdev->xendev, "info", info);
+    xenstore_write_be_int(xendev, "feature-flush-cache", 1);
+    xenstore_write_be_int(xendev, "info", info);
 
-    xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order",
+    xenstore_write_be_int(xendev, "max-ring-page-order",
                           MAX_RING_PAGE_ORDER);
 
     blk_parse_discard(blkdev);
@@ -761,7 +764,7 @@ static int blk_connect(struct XenDevice *xendev)
     }
 
     /* init qemu block driver */
-    index = (blkdev->xendev.dev - 202 * 256) / 16;
+    index = (xendev->dev - 202 * 256) / 16;
     blkdev->dinfo = drive_get(IF_XEN, 0, index);
     if (!blkdev->dinfo) {
         Error *local_err = NULL;
@@ -773,11 +776,11 @@ static int blk_connect(struct XenDevice *xendev)
         }
 
         /* setup via xenbus -> create new block driver instance */
-        xen_pv_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
+        xen_pv_printf(xendev, 2, "create new bdrv (xenbus setup)\n");
         blkdev->blk = blk_new_open(blkdev->filename, NULL, options,
                                    qflags, &local_err);
         if (!blkdev->blk) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: %s\n",
+            xen_pv_printf(xendev, 0, "error: %s\n",
                           error_get_pretty(local_err));
             error_free(local_err);
             return -1;
@@ -785,11 +788,11 @@ static int blk_connect(struct XenDevice *xendev)
         blk_set_enable_write_cache(blkdev->blk, !writethrough);
     } else {
         /* setup via qemu cmdline -> already setup for us */
-        xen_pv_printf(&blkdev->xendev, 2,
+        xen_pv_printf(xendev, 2,
                       "get configured bdrv (cmdline setup)\n");
         blkdev->blk = blk_by_legacy_dinfo(blkdev->dinfo);
         if (blk_is_read_only(blkdev->blk) && !readonly) {
-            xen_pv_printf(&blkdev->xendev, 0, "Unexpected read-only drive");
+            xen_pv_printf(xendev, 0, "Unexpected read-only drive");
             blkdev->blk = NULL;
             return -1;
         }
@@ -802,7 +805,7 @@ static int blk_connect(struct XenDevice *xendev)
     if (blkdev->file_size < 0) {
         BlockDriverState *bs = blk_bs(blkdev->blk);
         const char *drv_name = bs ? bdrv_get_format_name(bs) : NULL;
-        xen_pv_printf(&blkdev->xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
+        xen_pv_printf(xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
                       (int)blkdev->file_size, strerror(-blkdev->file_size),
                       drv_name ?: "-");
         blkdev->file_size = 0;
@@ -814,15 +817,15 @@ static int blk_connect(struct XenDevice *xendev)
                   blkdev->file_size, blkdev->file_size >> 20);
 
     /* Fill in number of sector size and number of sectors */
-    xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk);
-    xenstore_write_be_int64(&blkdev->xendev, "sectors",
+    xenstore_write_be_int(xendev, "sector-size", blkdev->file_blk);
+    xenstore_write_be_int64(xendev, "sectors",
                             blkdev->file_size / blkdev->file_blk);
 
-    if (xenstore_read_fe_int(&blkdev->xendev, "ring-page-order",
+    if (xenstore_read_fe_int(xendev, "ring-page-order",
                              &order) == -1) {
         blkdev->nr_ring_ref = 1;
 
-        if (xenstore_read_fe_int(&blkdev->xendev, "ring-ref",
+        if (xenstore_read_fe_int(xendev, "ring-ref",
                                  &ring_ref) == -1) {
             return -1;
         }
@@ -839,7 +842,7 @@ static int blk_connect(struct XenDevice *xendev)
                 return -1;
             }
 
-            if (xenstore_read_fe_int(&blkdev->xendev, key,
+            if (xenstore_read_fe_int(xendev, key,
                                      &ring_ref) == -1) {
                 g_free(key);
                 return -1;
@@ -854,18 +857,18 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    if (xenstore_read_fe_int(&blkdev->xendev, "event-channel",
-                             &blkdev->xendev.remote_port) == -1) {
+    if (xenstore_read_fe_int(xendev, "event-channel",
+                             &xendev->remote_port) == -1) {
         return -1;
     }
 
-    if (!blkdev->xendev.protocol) {
+    if (!xendev->protocol) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_X86_32;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_X86_64;
     } else {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
@@ -896,7 +899,6 @@ static int blk_connect(struct XenDevice *xendev)
     max_grants = blkdev->nr_ring_ref;
 
     xen_be_set_max_grant_refs(xendev, max_grants);
-
     blkdev->sring = xen_be_map_grant_refs(xendev, blkdev->ring_ref,
                                           blkdev->nr_ring_ref,
                                           PROT_READ | PROT_WRITE);
@@ -929,12 +931,12 @@ static int blk_connect(struct XenDevice *xendev)
 
     blk_set_aio_context(blkdev->blk, blkdev->ctx);
 
-    xen_be_bind_evtchn(&blkdev->xendev);
+    xen_be_bind_evtchn(xendev);
 
-    xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, nr-ring-ref %u, "
+    xen_pv_printf(xendev, 1, "ok: proto %s, nr-ring-ref %u, "
                   "remote port %d, local port %d\n",
-                  blkdev->xendev.protocol, blkdev->nr_ring_ref,
-                  blkdev->xendev.remote_port, blkdev->xendev.local_port);
+                  xendev->protocol, blkdev->nr_ring_ref,
+                  xendev->remote_port, xendev->local_port);
     return 0;
 }
 
@@ -952,7 +954,7 @@ static void blk_disconnect(struct XenDevice *xendev)
         blk_unref(blkdev->blk);
         blkdev->blk = NULL;
     }
-    xen_pv_unbind_evtchn(&blkdev->xendev);
+    xen_pv_unbind_evtchn(xendev);
 
     aio_context_release(blkdev->ctx);
 
-- 
2.1.4

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

* [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
  2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
                   ` (13 preceding siblings ...)
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
  14 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-04 19:26 UTC (permalink / raw)
  To: xen-devel, qemu-block, qemu-devel
  Cc: Anthony Perard, Kevin Wolf, Paul Durrant, Stefano Stabellini, Max Reitz

Certain functions in xen_disk are called with a pointer to xendev
(struct XenDevice *). They then use continer_of() to acces the surrounding
blkdev (struct XenBlkDev) but then in various places use &blkdev->xendev
when use of the original xendev pointer is shorter to express and clearly
equivalent.

This patch is a purely cosmetic patch which makes sure there is a xendev
pointer on stack for any function where the pointer is need on multiple
occasions modified those functions to use it consistently.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v2:
 - Re-based
---
 hw/block/xen_disk.c | 90 +++++++++++++++++++++++++++--------------------------
 1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 28651c5..9fbc0cd 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -178,10 +178,11 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
 static int ioreq_parse(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
     size_t len;
     int i;
 
-    xen_pv_printf(&blkdev->xendev, 3,
+    xen_pv_printf(xendev, 3,
                   "op %d, nr %d, handle %d, id %" PRId64 ", sector %" PRId64 "\n",
                   ioreq->req.operation, ioreq->req.nr_segments,
                   ioreq->req.handle, ioreq->req.id, ioreq->req.sector_number);
@@ -199,28 +200,28 @@ static int ioreq_parse(struct ioreq *ioreq)
     case BLKIF_OP_DISCARD:
         return 0;
     default:
-        xen_pv_printf(&blkdev->xendev, 0, "error: unknown operation (%d)\n",
+        xen_pv_printf(xendev, 0, "error: unknown operation (%d)\n",
                       ioreq->req.operation);
         goto err;
     };
 
     if (ioreq->req.operation != BLKIF_OP_READ && blkdev->mode[0] != 'w') {
-        xen_pv_printf(&blkdev->xendev, 0, "error: write req for ro device\n");
+        xen_pv_printf(xendev, 0, "error: write req for ro device\n");
         goto err;
     }
 
     ioreq->start = ioreq->req.sector_number * blkdev->file_blk;
     for (i = 0; i < ioreq->req.nr_segments; i++) {
         if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: nr_segments too big\n");
+            xen_pv_printf(xendev, 0, "error: nr_segments too big\n");
             goto err;
         }
         if (ioreq->req.seg[i].first_sect > ioreq->req.seg[i].last_sect) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: first > last sector\n");
+            xen_pv_printf(xendev, 0, "error: first > last sector\n");
             goto err;
         }
         if (ioreq->req.seg[i].last_sect * BLOCK_SIZE >= XC_PAGE_SIZE) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: page crossing\n");
+            xen_pv_printf(xendev, 0, "error: page crossing\n");
             goto err;
         }
 
@@ -228,7 +229,7 @@ static int ioreq_parse(struct ioreq *ioreq)
         ioreq->size += len;
     }
     if (ioreq->start + ioreq->size > blkdev->file_size) {
-        xen_pv_printf(&blkdev->xendev, 0, "error: access beyond end of file\n");
+        xen_pv_printf(xendev, 0, "error: access beyond end of file\n");
         goto err;
     }
     return 0;
@@ -244,7 +245,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     struct XenDevice *xendev = &blkdev->xendev;
     XenGrantCopySegment segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
     int i, count, rc;
-    int64_t file_blk = ioreq->blkdev->file_blk;
+    int64_t file_blk = blkdev->file_blk;
     bool to_domain = (ioreq->req.operation == BLKIF_OP_READ);
     void *virt = ioreq->buf;
 
@@ -272,7 +273,7 @@ static int ioreq_grant_copy(struct ioreq *ioreq)
     rc = xen_be_copy_grant_refs(xendev, to_domain, segs, count);
 
     if (rc) {
-        xen_pv_printf(&ioreq->blkdev->xendev, 0,
+        xen_pv_printf(xendev, 0,
                       "failed to copy data %d\n", rc);
         ioreq->aio_errors++;
         return -1;
@@ -287,11 +288,12 @@ static void qemu_aio_complete(void *opaque, int ret)
 {
     struct ioreq *ioreq = opaque;
     struct XenBlkDev *blkdev = ioreq->blkdev;
+    struct XenDevice *xendev = &blkdev->xendev;
 
     aio_context_acquire(blkdev->ctx);
 
     if (ret != 0) {
-        xen_pv_printf(&blkdev->xendev, 0, "%s I/O error\n",
+        xen_pv_printf(xendev, 0, "%s I/O error\n",
                       ioreq->req.operation == BLKIF_OP_READ ? "read" : "write");
         ioreq->aio_errors++;
     }
@@ -625,16 +627,17 @@ static void blk_alloc(struct XenDevice *xendev)
 
 static void blk_parse_discard(struct XenBlkDev *blkdev)
 {
+    struct XenDevice *xendev = &blkdev->xendev;
     int enable;
 
     blkdev->feature_discard = true;
 
-    if (xenstore_read_be_int(&blkdev->xendev, "discard-enable", &enable) == 0) {
+    if (xenstore_read_be_int(xendev, "discard-enable", &enable) == 0) {
         blkdev->feature_discard = !!enable;
     }
 
     if (blkdev->feature_discard) {
-        xenstore_write_be_int(&blkdev->xendev, "feature-discard", 1);
+        xenstore_write_be_int(xendev, "feature-discard", 1);
     }
 }
 
@@ -649,7 +652,7 @@ static int blk_init(struct XenDevice *xendev)
     /* read xenstore entries */
     if (blkdev->params == NULL) {
         char *h = NULL;
-        blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
+        blkdev->params = xenstore_read_be_str(xendev, "params");
         if (blkdev->params != NULL) {
             h = strchr(blkdev->params, ':');
         }
@@ -669,18 +672,18 @@ static int blk_init(struct XenDevice *xendev)
         blkdev->fileproto = "vpc";
     }
     if (blkdev->mode == NULL) {
-        blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
+        blkdev->mode = xenstore_read_be_str(xendev, "mode");
     }
     if (blkdev->type == NULL) {
-        blkdev->type = xenstore_read_be_str(&blkdev->xendev, "type");
+        blkdev->type = xenstore_read_be_str(xendev, "type");
     }
     if (blkdev->dev == NULL) {
-        blkdev->dev = xenstore_read_be_str(&blkdev->xendev, "dev");
+        blkdev->dev = xenstore_read_be_str(xendev, "dev");
     }
     if (blkdev->devtype == NULL) {
-        blkdev->devtype = xenstore_read_be_str(&blkdev->xendev, "device-type");
+        blkdev->devtype = xenstore_read_be_str(xendev, "device-type");
     }
-    directiosafe = xenstore_read_be_str(&blkdev->xendev, "direct-io-safe");
+    directiosafe = xenstore_read_be_str(xendev, "direct-io-safe");
     blkdev->directiosafe = (directiosafe && atoi(directiosafe));
 
     /* do we have all we need? */
@@ -706,10 +709,10 @@ static int blk_init(struct XenDevice *xendev)
     /* fill info
      * blk_connect supplies sector-size and sectors
      */
-    xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1);
-    xenstore_write_be_int(&blkdev->xendev, "info", info);
+    xenstore_write_be_int(xendev, "feature-flush-cache", 1);
+    xenstore_write_be_int(xendev, "info", info);
 
-    xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order",
+    xenstore_write_be_int(xendev, "max-ring-page-order",
                           MAX_RING_PAGE_ORDER);
 
     blk_parse_discard(blkdev);
@@ -761,7 +764,7 @@ static int blk_connect(struct XenDevice *xendev)
     }
 
     /* init qemu block driver */
-    index = (blkdev->xendev.dev - 202 * 256) / 16;
+    index = (xendev->dev - 202 * 256) / 16;
     blkdev->dinfo = drive_get(IF_XEN, 0, index);
     if (!blkdev->dinfo) {
         Error *local_err = NULL;
@@ -773,11 +776,11 @@ static int blk_connect(struct XenDevice *xendev)
         }
 
         /* setup via xenbus -> create new block driver instance */
-        xen_pv_printf(&blkdev->xendev, 2, "create new bdrv (xenbus setup)\n");
+        xen_pv_printf(xendev, 2, "create new bdrv (xenbus setup)\n");
         blkdev->blk = blk_new_open(blkdev->filename, NULL, options,
                                    qflags, &local_err);
         if (!blkdev->blk) {
-            xen_pv_printf(&blkdev->xendev, 0, "error: %s\n",
+            xen_pv_printf(xendev, 0, "error: %s\n",
                           error_get_pretty(local_err));
             error_free(local_err);
             return -1;
@@ -785,11 +788,11 @@ static int blk_connect(struct XenDevice *xendev)
         blk_set_enable_write_cache(blkdev->blk, !writethrough);
     } else {
         /* setup via qemu cmdline -> already setup for us */
-        xen_pv_printf(&blkdev->xendev, 2,
+        xen_pv_printf(xendev, 2,
                       "get configured bdrv (cmdline setup)\n");
         blkdev->blk = blk_by_legacy_dinfo(blkdev->dinfo);
         if (blk_is_read_only(blkdev->blk) && !readonly) {
-            xen_pv_printf(&blkdev->xendev, 0, "Unexpected read-only drive");
+            xen_pv_printf(xendev, 0, "Unexpected read-only drive");
             blkdev->blk = NULL;
             return -1;
         }
@@ -802,7 +805,7 @@ static int blk_connect(struct XenDevice *xendev)
     if (blkdev->file_size < 0) {
         BlockDriverState *bs = blk_bs(blkdev->blk);
         const char *drv_name = bs ? bdrv_get_format_name(bs) : NULL;
-        xen_pv_printf(&blkdev->xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
+        xen_pv_printf(xendev, 1, "blk_getlength: %d (%s) | drv %s\n",
                       (int)blkdev->file_size, strerror(-blkdev->file_size),
                       drv_name ?: "-");
         blkdev->file_size = 0;
@@ -814,15 +817,15 @@ static int blk_connect(struct XenDevice *xendev)
                   blkdev->file_size, blkdev->file_size >> 20);
 
     /* Fill in number of sector size and number of sectors */
-    xenstore_write_be_int(&blkdev->xendev, "sector-size", blkdev->file_blk);
-    xenstore_write_be_int64(&blkdev->xendev, "sectors",
+    xenstore_write_be_int(xendev, "sector-size", blkdev->file_blk);
+    xenstore_write_be_int64(xendev, "sectors",
                             blkdev->file_size / blkdev->file_blk);
 
-    if (xenstore_read_fe_int(&blkdev->xendev, "ring-page-order",
+    if (xenstore_read_fe_int(xendev, "ring-page-order",
                              &order) == -1) {
         blkdev->nr_ring_ref = 1;
 
-        if (xenstore_read_fe_int(&blkdev->xendev, "ring-ref",
+        if (xenstore_read_fe_int(xendev, "ring-ref",
                                  &ring_ref) == -1) {
             return -1;
         }
@@ -839,7 +842,7 @@ static int blk_connect(struct XenDevice *xendev)
                 return -1;
             }
 
-            if (xenstore_read_fe_int(&blkdev->xendev, key,
+            if (xenstore_read_fe_int(xendev, key,
                                      &ring_ref) == -1) {
                 g_free(key);
                 return -1;
@@ -854,18 +857,18 @@ static int blk_connect(struct XenDevice *xendev)
         return -1;
     }
 
-    if (xenstore_read_fe_int(&blkdev->xendev, "event-channel",
-                             &blkdev->xendev.remote_port) == -1) {
+    if (xenstore_read_fe_int(xendev, "event-channel",
+                             &xendev->remote_port) == -1) {
         return -1;
     }
 
-    if (!blkdev->xendev.protocol) {
+    if (!xendev->protocol) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_X86_32;
-    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
+    } else if (strcmp(xendev->protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
         blkdev->protocol = BLKIF_PROTOCOL_X86_64;
     } else {
         blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
@@ -896,7 +899,6 @@ static int blk_connect(struct XenDevice *xendev)
     max_grants = blkdev->nr_ring_ref;
 
     xen_be_set_max_grant_refs(xendev, max_grants);
-
     blkdev->sring = xen_be_map_grant_refs(xendev, blkdev->ring_ref,
                                           blkdev->nr_ring_ref,
                                           PROT_READ | PROT_WRITE);
@@ -929,12 +931,12 @@ static int blk_connect(struct XenDevice *xendev)
 
     blk_set_aio_context(blkdev->blk, blkdev->ctx);
 
-    xen_be_bind_evtchn(&blkdev->xendev);
+    xen_be_bind_evtchn(xendev);
 
-    xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, nr-ring-ref %u, "
+    xen_pv_printf(xendev, 1, "ok: proto %s, nr-ring-ref %u, "
                   "remote port %d, local port %d\n",
-                  blkdev->xendev.protocol, blkdev->nr_ring_ref,
-                  blkdev->xendev.remote_port, blkdev->xendev.local_port);
+                  xendev->protocol, blkdev->nr_ring_ref,
+                  xendev->remote_port, xendev->local_port);
     return 0;
 }
 
@@ -952,7 +954,7 @@ static void blk_disconnect(struct XenDevice *xendev)
         blk_unref(blkdev->blk);
         blkdev->blk = NULL;
     }
-    xen_pv_unbind_evtchn(&blkdev->xendev);
+    xen_pv_unbind_evtchn(xendev);
 
     aio_context_release(blkdev->ctx);
 
-- 
2.1.4


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

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

* Re: [Qemu-devel] [PATCH v3 1/8] xen_backend: add grant table helpers
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-16 13:50     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 13:50 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini

On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> This patch adds grant table helper functions to the xen_backend code to
> localize error reporting and use of xen_domid.
> 
> The patch also defers the call to xengnttab_open() until just before the
> initialise method in XenDevOps is invoked. This method is responsible for
> mapping the shared ring. No prior method requires access to the grant table.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> v2:
>  - New in v2
> ---
>  hw/xen/xen_backend.c         | 123 ++++++++++++++++++++++++++++++++++++++-----
>  include/hw/xen/xen_backend.h |  33 ++++++++++++
>  2 files changed, 144 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 7445b50..50412d6 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state)
>      return 0;
>  }
>  
> +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> +                               unsigned int nr_refs)

Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
seems to fail the initialisation if set_max_grants call fails. On the
other end, xen-usb.c just keep going.

> +{
> +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> +        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> +                      strerror(errno));
> +    }
> +}
> +

> +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> +                           bool to_domain,
> +                           XenGrantCopySegment segs[],
> +                           unsigned int nr_segs)
> +{
> +    xengnttab_grant_copy_segment_t *xengnttab_segs;
> +    unsigned int i;
> +    int rc;
> +
> +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
> +
> +    for (i = 0; i < nr_segs; i++) {
> +        XenGrantCopySegment *seg = &segs[i];
> +        xengnttab_grant_copy_segment_t *xengnttab_seg = &xengnttab_segs[i];
> +
> +        if (to_domain) {
> +            xengnttab_seg->flags = GNTCOPY_dest_gref;
> +            xengnttab_seg->dest.foreign.domid = xen_domid;
> +            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> +            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> +            xengnttab_seg->source.virt = seg->source.virt;
> +        } else {
> +            xengnttab_seg->flags = GNTCOPY_source_gref;
> +            xengnttab_seg->source.foreign.domid = xen_domid;
> +            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> +            xengnttab_seg->source.foreign.offset =
> +                seg->source.foreign.offset;
> +            xengnttab_seg->dest.virt = seg->dest.virt;
> +        }

That's not going to work because xengnttab_grant_copy_segment_t doesn't
exist on Xen 4.7.

> +
> +        xengnttab_seg->len = seg->len;
> +    }
> +
> +    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs, xengnttab_segs);

Thanks,

-- 
Anthony PERARD

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

* Re: [PATCH v3 1/8] xen_backend: add grant table helpers
@ 2018-05-16 13:50     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 13:50 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Stefano Stabellini, qemu-devel, qemu-block

On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> This patch adds grant table helper functions to the xen_backend code to
> localize error reporting and use of xen_domid.
> 
> The patch also defers the call to xengnttab_open() until just before the
> initialise method in XenDevOps is invoked. This method is responsible for
> mapping the shared ring. No prior method requires access to the grant table.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> 
> v2:
>  - New in v2
> ---
>  hw/xen/xen_backend.c         | 123 ++++++++++++++++++++++++++++++++++++++-----
>  include/hw/xen/xen_backend.h |  33 ++++++++++++
>  2 files changed, 144 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index 7445b50..50412d6 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state)
>      return 0;
>  }
>  
> +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> +                               unsigned int nr_refs)

Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
seems to fail the initialisation if set_max_grants call fails. On the
other end, xen-usb.c just keep going.

> +{
> +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> +        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> +                      strerror(errno));
> +    }
> +}
> +

> +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> +                           bool to_domain,
> +                           XenGrantCopySegment segs[],
> +                           unsigned int nr_segs)
> +{
> +    xengnttab_grant_copy_segment_t *xengnttab_segs;
> +    unsigned int i;
> +    int rc;
> +
> +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> +
> +    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t, nr_segs);
> +
> +    for (i = 0; i < nr_segs; i++) {
> +        XenGrantCopySegment *seg = &segs[i];
> +        xengnttab_grant_copy_segment_t *xengnttab_seg = &xengnttab_segs[i];
> +
> +        if (to_domain) {
> +            xengnttab_seg->flags = GNTCOPY_dest_gref;
> +            xengnttab_seg->dest.foreign.domid = xen_domid;
> +            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> +            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> +            xengnttab_seg->source.virt = seg->source.virt;
> +        } else {
> +            xengnttab_seg->flags = GNTCOPY_source_gref;
> +            xengnttab_seg->source.foreign.domid = xen_domid;
> +            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> +            xengnttab_seg->source.foreign.offset =
> +                seg->source.foreign.offset;
> +            xengnttab_seg->dest.virt = seg->dest.virt;
> +        }

That's not going to work because xengnttab_grant_copy_segment_t doesn't
exist on Xen 4.7.

> +
> +        xengnttab_seg->len = seg->len;
> +    }
> +
> +    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs, xengnttab_segs);

Thanks,

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 1/8] xen_backend: add grant table helpers
  2018-05-16 13:50     ` Anthony PERARD
@ 2018-05-16 14:00       ` Paul Durrant
  -1 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:00 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 14:50
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>
> Subject: Re: [PATCH v3 1/8] xen_backend: add grant table helpers
> 
> On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> > This patch adds grant table helper functions to the xen_backend code to
> > localize error reporting and use of xen_domid.
> >
> > The patch also defers the call to xengnttab_open() until just before the
> > initialise method in XenDevOps is invoked. This method is responsible for
> > mapping the shared ring. No prior method requires access to the grant
> table.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> >
> > v2:
> >  - New in v2
> > ---
> >  hw/xen/xen_backend.c         | 123
> ++++++++++++++++++++++++++++++++++++++-----
> >  include/hw/xen/xen_backend.h |  33 ++++++++++++
> >  2 files changed, 144 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> > index 7445b50..50412d6 100644
> > --- a/hw/xen/xen_backend.c
> > +++ b/hw/xen/xen_backend.c
> > @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev,
> enum xenbus_state state)
> >      return 0;
> >  }
> >
> > +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> > +                               unsigned int nr_refs)
> 
> Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
> seems to fail the initialisation if set_max_grants call fails. On the
> other end, xen-usb.c just keep going.
> 

I guess the upshot will be that a subsequent grant map would fail, so I think it should be sufficient to deal with the failure there. As you say it's use is inconsistent, and just plain missing in some cases.

> > +{
> > +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> > +        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> > +                      strerror(errno));
> > +    }
> > +}
> > +
> 
> > +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> > +                           bool to_domain,
> > +                           XenGrantCopySegment segs[],
> > +                           unsigned int nr_segs)
> > +{
> > +    xengnttab_grant_copy_segment_t *xengnttab_segs;
> > +    unsigned int i;
> > +    int rc;
> > +
> > +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t,
> nr_segs);
> > +
> > +    for (i = 0; i < nr_segs; i++) {
> > +        XenGrantCopySegment *seg = &segs[i];
> > +        xengnttab_grant_copy_segment_t *xengnttab_seg =
> &xengnttab_segs[i];
> > +
> > +        if (to_domain) {
> > +            xengnttab_seg->flags = GNTCOPY_dest_gref;
> > +            xengnttab_seg->dest.foreign.domid = xen_domid;
> > +            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> > +            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> > +            xengnttab_seg->source.virt = seg->source.virt;
> > +        } else {
> > +            xengnttab_seg->flags = GNTCOPY_source_gref;
> > +            xengnttab_seg->source.foreign.domid = xen_domid;
> > +            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> > +            xengnttab_seg->source.foreign.offset =
> > +                seg->source.foreign.offset;
> > +            xengnttab_seg->dest.virt = seg->dest.virt;
> > +        }
> 
> That's not going to work because xengnttab_grant_copy_segment_t doesn't
> exist on Xen 4.7.

Ah, I'd missed the ifdef around that in xen_disk. I'll add it here.

Cheers,

  Paul

> 
> > +
> > +        xengnttab_seg->len = seg->len;
> > +    }
> > +
> > +    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs,
> xengnttab_segs);
> 
> Thanks,
> 
> --
> Anthony PERARD

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

* Re: [PATCH v3 1/8] xen_backend: add grant table helpers
@ 2018-05-16 14:00       ` Paul Durrant
  0 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:00 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, Stefano Stabellini, qemu-devel, qemu-block

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 14:50
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>
> Subject: Re: [PATCH v3 1/8] xen_backend: add grant table helpers
> 
> On Fri, May 04, 2018 at 08:26:00PM +0100, Paul Durrant wrote:
> > This patch adds grant table helper functions to the xen_backend code to
> > localize error reporting and use of xen_domid.
> >
> > The patch also defers the call to xengnttab_open() until just before the
> > initialise method in XenDevOps is invoked. This method is responsible for
> > mapping the shared ring. No prior method requires access to the grant
> table.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Anthony Perard <anthony.perard@citrix.com>
> >
> > v2:
> >  - New in v2
> > ---
> >  hw/xen/xen_backend.c         | 123
> ++++++++++++++++++++++++++++++++++++++-----
> >  include/hw/xen/xen_backend.h |  33 ++++++++++++
> >  2 files changed, 144 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> > index 7445b50..50412d6 100644
> > --- a/hw/xen/xen_backend.c
> > +++ b/hw/xen/xen_backend.c
> > @@ -106,6 +106,103 @@ int xen_be_set_state(struct XenDevice *xendev,
> enum xenbus_state state)
> >      return 0;
> >  }
> >
> > +void xen_be_set_max_grant_refs(struct XenDevice *xendev,
> > +                               unsigned int nr_refs)
> 
> Is it fine to ignore error from set_max_grants and continue ? xen_disk.c
> seems to fail the initialisation if set_max_grants call fails. On the
> other end, xen-usb.c just keep going.
> 

I guess the upshot will be that a subsequent grant map would fail, so I think it should be sufficient to deal with the failure there. As you say it's use is inconsistent, and just plain missing in some cases.

> > +{
> > +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +    if (xengnttab_set_max_grants(xendev->gnttabdev, nr_refs)) {
> > +        xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n",
> > +                      strerror(errno));
> > +    }
> > +}
> > +
> 
> > +int xen_be_copy_grant_refs(struct XenDevice *xendev,
> > +                           bool to_domain,
> > +                           XenGrantCopySegment segs[],
> > +                           unsigned int nr_segs)
> > +{
> > +    xengnttab_grant_copy_segment_t *xengnttab_segs;
> > +    unsigned int i;
> > +    int rc;
> > +
> > +    assert(xendev->ops->flags & DEVOPS_FLAG_NEED_GNTDEV);
> > +
> > +    xengnttab_segs = g_new0(xengnttab_grant_copy_segment_t,
> nr_segs);
> > +
> > +    for (i = 0; i < nr_segs; i++) {
> > +        XenGrantCopySegment *seg = &segs[i];
> > +        xengnttab_grant_copy_segment_t *xengnttab_seg =
> &xengnttab_segs[i];
> > +
> > +        if (to_domain) {
> > +            xengnttab_seg->flags = GNTCOPY_dest_gref;
> > +            xengnttab_seg->dest.foreign.domid = xen_domid;
> > +            xengnttab_seg->dest.foreign.ref = seg->dest.foreign.ref;
> > +            xengnttab_seg->dest.foreign.offset = seg->dest.foreign.offset;
> > +            xengnttab_seg->source.virt = seg->source.virt;
> > +        } else {
> > +            xengnttab_seg->flags = GNTCOPY_source_gref;
> > +            xengnttab_seg->source.foreign.domid = xen_domid;
> > +            xengnttab_seg->source.foreign.ref = seg->source.foreign.ref;
> > +            xengnttab_seg->source.foreign.offset =
> > +                seg->source.foreign.offset;
> > +            xengnttab_seg->dest.virt = seg->dest.virt;
> > +        }
> 
> That's not going to work because xengnttab_grant_copy_segment_t doesn't
> exist on Xen 4.7.

Ah, I'd missed the ifdef around that in xen_disk. I'll add it here.

Cheers,

  Paul

> 
> > +
> > +        xengnttab_seg->len = seg->len;
> > +    }
> > +
> > +    rc = xengnttab_grant_copy(xendev->gnttabdev, nr_segs,
> xengnttab_segs);
> 
> Thanks,
> 
> --
> Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-16 14:02     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:02 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini,
	Kevin Wolf, Max Reitz

On Fri, May 04, 2018 at 08:26:01PM +0100, Paul Durrant wrote:
> Now that helpers are present in xen_backend, this patch removes open-coded
> calls to libxengnttab from the xen_disk code.
> 
> This patch also fixes one whitspace error in the assignment of the
> XenDevOps initialise method.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
@ 2018-05-16 14:02     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:02 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
	Max Reitz, xen-devel

On Fri, May 04, 2018 at 08:26:01PM +0100, Paul Durrant wrote:
> Now that helpers are present in xen_backend, this patch removes open-coded
> calls to libxengnttab from the xen_disk code.
> 
> This patch also fixes one whitspace error in the assignment of the
> XenDevOps initialise method.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-16 14:14   ` Anthony PERARD
  2018-05-16 14:22     ` Paul Durrant
  2018-05-16 14:22     ` [Qemu-devel] " Paul Durrant
  2018-05-16 14:14   ` Anthony PERARD
  1 sibling, 2 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:14 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini, Greg Kurz,
	Paolo Bonzini, Jason Wang, Gerd Hoffmann

On Fri, May 04, 2018 at 08:26:02PM +0100, Paul Durrant wrote:
> Now that helpers are available in xen_backend, use them throughout all
> Xen PV backends.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> index 20c43a6..73d6f1b 100644
> --- a/hw/net/xen_nic.c
> +++ b/hw/net/xen_nic.c
> @@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev *netdev)
>                            (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
>                            (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
>  
> -            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> -                                           netdev->xendev.dom,
> -                                           txreq.gref, PROT_READ);
> +            page = xen_be_map_grant_refs(&netdev->xendev,
> +                                         &txreq.gref, 1, PROT_READ);

xen_be_map_grant_ref instead?

>              if (page == NULL) {
>                  xen_pv_printf(&netdev->xendev, 0,
>                                "error: tx gref dereference failed (%d)\n",
> @@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
>                  qemu_send_packet(qemu_get_queue(netdev->nic),
>                                   page + txreq.offset, txreq.size);
>              }
> -            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
> +            xen_be_unmap_grant_ref(&netdev->xendev, page);
>              net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
>          }
>          if (!netdev->tx_work) {
> @@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
>      memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
>      netdev->rx_ring.req_cons = ++rc;
>  
> -    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> -                                   netdev->xendev.dom,
> -                                   rxreq.gref, PROT_WRITE);
> +    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
> +                                 PROT_WRITE);

xen_be_map_grant_ref instead?

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
  2018-05-16 14:14   ` Anthony PERARD
@ 2018-05-16 14:14   ` Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:14 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Stefano Stabellini, qemu-block, Jason Wang, qemu-devel,
	Greg Kurz, Gerd Hoffmann, Paolo Bonzini, xen-devel

On Fri, May 04, 2018 at 08:26:02PM +0100, Paul Durrant wrote:
> Now that helpers are available in xen_backend, use them throughout all
> Xen PV backends.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> index 20c43a6..73d6f1b 100644
> --- a/hw/net/xen_nic.c
> +++ b/hw/net/xen_nic.c
> @@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev *netdev)
>                            (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
>                            (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
>  
> -            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> -                                           netdev->xendev.dom,
> -                                           txreq.gref, PROT_READ);
> +            page = xen_be_map_grant_refs(&netdev->xendev,
> +                                         &txreq.gref, 1, PROT_READ);

xen_be_map_grant_ref instead?

>              if (page == NULL) {
>                  xen_pv_printf(&netdev->xendev, 0,
>                                "error: tx gref dereference failed (%d)\n",
> @@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev *netdev)
>                  qemu_send_packet(qemu_get_queue(netdev->nic),
>                                   page + txreq.offset, txreq.size);
>              }
> -            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
> +            xen_be_unmap_grant_ref(&netdev->xendev, page);
>              net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
>          }
>          if (!netdev->tx_work) {
> @@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc, const uint8_t *buf, size_t size
>      memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc), sizeof(rxreq));
>      netdev->rx_ring.req_cons = ++rc;
>  
> -    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> -                                   netdev->xendev.dom,
> -                                   rxreq.gref, PROT_WRITE);
> +    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
> +                                 PROT_WRITE);

xen_be_map_grant_ref instead?

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

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-16 14:14   ` Anthony PERARD
  2018-05-16 14:22     ` Paul Durrant
@ 2018-05-16 14:22     ` Paul Durrant
  1 sibling, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:22 UTC (permalink / raw)
  To: Anthony Perard
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini, Greg Kurz,
	Paolo Bonzini, Jason Wang, Gerd Hoffmann

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 15:14
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>; Greg Kurz
> <groug@kaod.org>; Paolo Bonzini <pbonzini@redhat.com>; Jason Wang
> <jasowang@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: Re: [PATCH v3 3/8] xen: remove other open-coded use of
> libxengnttab
> 
> On Fri, May 04, 2018 at 08:26:02PM +0100, Paul Durrant wrote:
> > Now that helpers are available in xen_backend, use them throughout all
> > Xen PV backends.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> > index 20c43a6..73d6f1b 100644
> > --- a/hw/net/xen_nic.c
> > +++ b/hw/net/xen_nic.c
> > @@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >                            (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
> >                            (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
> >
> > -            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -                                           netdev->xendev.dom,
> > -                                           txreq.gref, PROT_READ);
> > +            page = xen_be_map_grant_refs(&netdev->xendev,
> > +                                         &txreq.gref, 1, PROT_READ);
> 
> xen_be_map_grant_ref instead?
> 

Yep.

> >              if (page == NULL) {
> >                  xen_pv_printf(&netdev->xendev, 0,
> >                                "error: tx gref dereference failed (%d)\n",
> > @@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >                  qemu_send_packet(qemu_get_queue(netdev->nic),
> >                                   page + txreq.offset, txreq.size);
> >              }
> > -            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
> > +            xen_be_unmap_grant_ref(&netdev->xendev, page);
> >              net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
> >          }
> >          if (!netdev->tx_work) {
> > @@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc,
> const uint8_t *buf, size_t size
> >      memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc),
> sizeof(rxreq));
> >      netdev->rx_ring.req_cons = ++rc;
> >
> > -    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -                                   netdev->xendev.dom,
> > -                                   rxreq.gref, PROT_WRITE);
> > +    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
> > +                                 PROT_WRITE);
> 
> xen_be_map_grant_ref instead?
> 

And yep again.

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

Thanks :-)

  Paul

> --
> Anthony PERARD

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

* Re: [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
  2018-05-16 14:14   ` Anthony PERARD
@ 2018-05-16 14:22     ` Paul Durrant
  2018-05-16 14:22     ` [Qemu-devel] " Paul Durrant
  1 sibling, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:22 UTC (permalink / raw)
  To: Anthony Perard
  Cc: Stefano Stabellini, qemu-block, Jason Wang, qemu-devel,
	Greg Kurz, Gerd Hoffmann, Paolo Bonzini, xen-devel

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 15:14
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>; Greg Kurz
> <groug@kaod.org>; Paolo Bonzini <pbonzini@redhat.com>; Jason Wang
> <jasowang@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: Re: [PATCH v3 3/8] xen: remove other open-coded use of
> libxengnttab
> 
> On Fri, May 04, 2018 at 08:26:02PM +0100, Paul Durrant wrote:
> > Now that helpers are available in xen_backend, use them throughout all
> > Xen PV backends.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> > diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
> > index 20c43a6..73d6f1b 100644
> > --- a/hw/net/xen_nic.c
> > +++ b/hw/net/xen_nic.c
> > @@ -160,9 +160,8 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >                            (txreq.flags & NETTXF_more_data)      ? " more_data"      : "",
> >                            (txreq.flags & NETTXF_extra_info)     ? " extra_info"     : "");
> >
> > -            page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -                                           netdev->xendev.dom,
> > -                                           txreq.gref, PROT_READ);
> > +            page = xen_be_map_grant_refs(&netdev->xendev,
> > +                                         &txreq.gref, 1, PROT_READ);
> 
> xen_be_map_grant_ref instead?
> 

Yep.

> >              if (page == NULL) {
> >                  xen_pv_printf(&netdev->xendev, 0,
> >                                "error: tx gref dereference failed (%d)\n",
> > @@ -183,7 +182,7 @@ static void net_tx_packets(struct XenNetDev
> *netdev)
> >                  qemu_send_packet(qemu_get_queue(netdev->nic),
> >                                   page + txreq.offset, txreq.size);
> >              }
> > -            xengnttab_unmap(netdev->xendev.gnttabdev, page, 1);
> > +            xen_be_unmap_grant_ref(&netdev->xendev, page);
> >              net_tx_response(netdev, &txreq, NETIF_RSP_OKAY);
> >          }
> >          if (!netdev->tx_work) {
> > @@ -254,9 +253,8 @@ static ssize_t net_rx_packet(NetClientState *nc,
> const uint8_t *buf, size_t size
> >      memcpy(&rxreq, RING_GET_REQUEST(&netdev->rx_ring, rc),
> sizeof(rxreq));
> >      netdev->rx_ring.req_cons = ++rc;
> >
> > -    page = xengnttab_map_grant_ref(netdev->xendev.gnttabdev,
> > -                                   netdev->xendev.dom,
> > -                                   rxreq.gref, PROT_WRITE);
> > +    page = xen_be_map_grant_refs(&netdev->xendev, &rxreq.gref, 1,
> > +                                 PROT_WRITE);
> 
> xen_be_map_grant_ref instead?
> 

And yep again.

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

Thanks :-)

  Paul

> --
> Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy Paul Durrant
@ 2018-05-16 14:30     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:30 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini

On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> Not all Xen environments support the xengnttab_grant_copy() operation.
> E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> 
> This patch introduces an emulation of that operation using
> xengnttab_map_domain_grant_refs() and memcpy() for those environments.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---

The patch looks ok, but since patch 1 is going to be change, this one is
going to need to be change as well.

-- 
Anthony PERARD

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

* Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
@ 2018-05-16 14:30     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-16 14:30 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Stefano Stabellini, qemu-devel, qemu-block

On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> Not all Xen environments support the xengnttab_grant_copy() operation.
> E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> 
> This patch introduces an emulation of that operation using
> xengnttab_map_domain_grant_refs() and memcpy() for those environments.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---

The patch looks ok, but since patch 1 is going to be change, this one is
going to need to be change as well.

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy
  2018-05-16 14:30     ` Anthony PERARD
@ 2018-05-16 14:34       ` Paul Durrant
  -1 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:34 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 15:31
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>
> Subject: Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
> 
> On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> > Not all Xen environments support the xengnttab_grant_copy() operation.
> > E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> >
> > This patch introduces an emulation of that operation using
> > xengnttab_map_domain_grant_refs() and memcpy() for those
> environments.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> 
> The patch looks ok, but since patch 1 is going to be change, this one is
> going to need to be change as well.
> 

Sure. Thanks for the review.

  Paul

> --
> Anthony PERARD

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

* Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
@ 2018-05-16 14:34       ` Paul Durrant
  0 siblings, 0 replies; 39+ messages in thread
From: Paul Durrant @ 2018-05-16 14:34 UTC (permalink / raw)
  To: Anthony Perard; +Cc: xen-devel, Stefano Stabellini, qemu-devel, qemu-block

> -----Original Message-----
> From: Anthony PERARD [mailto:anthony.perard@citrix.com]
> Sent: 16 May 2018 15:31
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; qemu-block@nongnu.org; qemu-
> devel@nongnu.org; Stefano Stabellini <sstabellini@kernel.org>
> Subject: Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
> 
> On Fri, May 04, 2018 at 08:26:03PM +0100, Paul Durrant wrote:
> > Not all Xen environments support the xengnttab_grant_copy() operation.
> > E.g. where the OS is FreeBSD or Xen is older than 4.8.0.
> >
> > This patch introduces an emulation of that operation using
> > xengnttab_map_domain_grant_refs() and memcpy() for those
> environments.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > ---
> 
> The patch looks ok, but since patch 1 is going to be change, this one is
> going to need to be change as well.
> 

Sure. Thanks for the review.

  Paul

> --
> Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
@ 2018-05-17 10:31   ` Anthony PERARD
  2018-05-17 10:31   ` Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:31 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini,
	Kevin Wolf, Max Reitz

On Fri, May 04, 2018 at 08:26:04PM +0100, Paul Durrant wrote:
> Now that the (native or emulated) xen_be_copy_grant_refs() helper is
> always available, the xen_disk code can be significantly simplified by
> removing direct use of grant map and unmap operations.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
  2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
  2018-05-17 10:31   ` Anthony PERARD
@ 2018-05-17 10:31   ` Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:31 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
	Max Reitz, xen-devel

On Fri, May 04, 2018 at 08:26:04PM +0100, Paul Durrant wrote:
> Now that the (native or emulated) xen_be_copy_grant_refs() helper is
> always available, the xen_disk code can be significantly simplified by
> removing direct use of grant map and unmap operations.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private
  2018-05-04 19:26   ` Paul Durrant
@ 2018-05-17 10:31     ` Anthony PERARD
  -1 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:31 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini

On Fri, May 04, 2018 at 08:26:05PM +0100, Paul Durrant wrote:
> There is no longer any use of this flag outside of the xen_backend code.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private
@ 2018-05-17 10:31     ` Anthony PERARD
  0 siblings, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:31 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Stefano Stabellini, qemu-devel, qemu-block

On Fri, May 04, 2018 at 08:26:05PM +0100, Paul Durrant wrote:
> There is no longer any use of this flag outside of the xen_backend code.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
  2018-05-17 10:59   ` Anthony PERARD
@ 2018-05-17 10:59   ` Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:59 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini,
	Kevin Wolf, Max Reitz

On Fri, May 04, 2018 at 08:26:06PM +0100, Paul Durrant wrote:
> Since xen_disk now always copies data to and from a guest there is no need
> to maintain a vector entry corresponding to every page of a request.
> This means there is less per-request state to maintain so the ioreq
> structure can shrink significantly.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 7/8] xen_disk: use a single entry iovec
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
@ 2018-05-17 10:59   ` Anthony PERARD
  2018-05-17 10:59   ` [Qemu-devel] " Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 10:59 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
	Max Reitz, xen-devel

On Fri, May 04, 2018 at 08:26:06PM +0100, Paul Durrant wrote:
> Since xen_disk now always copies data to and from a guest there is no need
> to maintain a vector entry corresponding to every page of a request.
> This means there is less per-request state to maintain so the ioreq
> structure can shrink significantly.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

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

* Re: [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
  2018-05-17 11:07   ` Anthony PERARD
@ 2018-05-17 11:07   ` Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 11:07 UTC (permalink / raw)
  To: Paul Durrant
  Cc: xen-devel, qemu-block, qemu-devel, Stefano Stabellini,
	Kevin Wolf, Max Reitz

On Fri, May 04, 2018 at 08:26:07PM +0100, Paul Durrant wrote:
> Certain functions in xen_disk are called with a pointer to xendev
> (struct XenDevice *). They then use continer_of() to acces the surrounding

                                      ^ container_of

> blkdev (struct XenBlkDev) but then in various places use &blkdev->xendev
> when use of the original xendev pointer is shorter to express and clearly
> equivalent.
> 
> This patch is a purely cosmetic patch which makes sure there is a xendev
> pointer on stack for any function where the pointer is need on multiple
> occasions modified those functions to use it consistently.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

* Re: [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
  2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
@ 2018-05-17 11:07   ` Anthony PERARD
  2018-05-17 11:07   ` [Qemu-devel] " Anthony PERARD
  1 sibling, 0 replies; 39+ messages in thread
From: Anthony PERARD @ 2018-05-17 11:07 UTC (permalink / raw)
  To: Paul Durrant
  Cc: Kevin Wolf, Stefano Stabellini, qemu-block, qemu-devel,
	Max Reitz, xen-devel

On Fri, May 04, 2018 at 08:26:07PM +0100, Paul Durrant wrote:
> Certain functions in xen_disk are called with a pointer to xendev
> (struct XenDevice *). They then use continer_of() to acces the surrounding

                                      ^ container_of

> blkdev (struct XenBlkDev) but then in various places use &blkdev->xendev
> when use of the original xendev pointer is shorter to express and clearly
> equivalent.
> 
> This patch is a purely cosmetic patch which makes sure there is a xendev
> pointer on stack for any function where the pointer is need on multiple
> occasions modified those functions to use it consistently.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>

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

-- 
Anthony PERARD

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

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

end of thread, other threads:[~2018-05-17 11:08 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 19:25 [Qemu-devel] [PATCH v3 0/8] xen_disk: legacy code removal and cleanup Paul Durrant
2018-05-04 19:26 ` [PATCH v3 1/8] xen_backend: add grant table helpers Paul Durrant
2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
2018-05-16 13:50   ` Anthony PERARD
2018-05-16 13:50     ` Anthony PERARD
2018-05-16 14:00     ` [Qemu-devel] " Paul Durrant
2018-05-16 14:00       ` Paul Durrant
2018-05-04 19:26 ` [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab Paul Durrant
2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
2018-05-16 14:02   ` Anthony PERARD
2018-05-16 14:02     ` Anthony PERARD
2018-05-04 19:26 ` [PATCH v3 3/8] xen: remove other " Paul Durrant
2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
2018-05-16 14:14   ` Anthony PERARD
2018-05-16 14:22     ` Paul Durrant
2018-05-16 14:22     ` [Qemu-devel] " Paul Durrant
2018-05-16 14:14   ` Anthony PERARD
2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 4/8] xen_backend: add an emulation of grant copy Paul Durrant
2018-05-16 14:30   ` Anthony PERARD
2018-05-16 14:30     ` Anthony PERARD
2018-05-16 14:34     ` [Qemu-devel] " Paul Durrant
2018-05-16 14:34       ` Paul Durrant
2018-05-04 19:26 ` Paul Durrant
2018-05-04 19:26 ` [PATCH v3 5/8] xen_disk: remove use of grant map/unmap Paul Durrant
2018-05-04 19:26 ` [Qemu-devel] " Paul Durrant
2018-05-17 10:31   ` Anthony PERARD
2018-05-17 10:31   ` Anthony PERARD
2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private Paul Durrant
2018-05-04 19:26   ` Paul Durrant
2018-05-17 10:31   ` [Qemu-devel] " Anthony PERARD
2018-05-17 10:31     ` Anthony PERARD
2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
2018-05-17 10:59   ` Anthony PERARD
2018-05-17 10:59   ` [Qemu-devel] " Anthony PERARD
2018-05-04 19:26 ` Paul Durrant
2018-05-04 19:26 ` [Qemu-devel] [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
2018-05-17 11:07   ` Anthony PERARD
2018-05-17 11:07   ` [Qemu-devel] " Anthony PERARD
2018-05-04 19:26 ` 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.