All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/2] qemu-qdisk: Implementation of grant copy operation.
@ 2016-08-16  6:55 Paulina Szubarczyk
  2016-08-16  6:55   ` Paulina Szubarczyk
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Paulina Szubarczyk @ 2016-08-16  6:55 UTC (permalink / raw)
  To: xen-devel, roger.pau
  Cc: wei.liu2, ian.jackson, david.vrabel, sstabellini, anthony.perard,
	qemu-devel, Paulina Szubarczyk

Hi,

It is a proposition for implementation of grant copy operation in qemu-qdisk and interface in libxc/libs. 

Changes since v3:
Interface:
- revert to cast from xengnttab_grant_copy_segment_t
  to ioctl_gntdev_grant_copy.
- added compile-time check to compare the libs
  xengnttab_grant_copy_segment_t with the ioctl structure.
  The patch relies on Wei patch introducing XENGNTTAB_BUILD_BUG_ON in libs/gnttab.

qemu-qdisk:
- qemu_memalign/qemu_free is used instead function allocating
  memory from xc.
- removed the get_buffer function instead there is a direct call
  to qemu_memalign.
- moved ioreq_copy for write operation to ioreq_runio_qemu_aio.
- added struct xengnttab_grant_copy_segment_t and stub in
  xen_common.h for version of Xen earlier then 480.
- added checking for version 480 to configure. The test repeats
  all the operation that are required for version < 480 and
  checks if xengnttab_grant_copy() is implemented.

Changes since v2:
Interface:
- dropped the changes in libxc/include/xenctrl_compat
- changed the MINOR version in Makefile
- replaced 'return -1' -> 'abort()'in libs/gnttab/gnttab_unimp.c
- moved the struct 'xengnttab_copy_grant_segment' to 
  libs/gnttab/include/xengnttab.h
- added explicit assingment to ioctl_gntdev_grant_copy_segment 
  to the linux part

qemu-qdisk:
- to use the xengnttab_* function directly added -lxengnttab to configure
  and include <xengnttab.h> in include/hw/xen/xen_common.h
- in ioreq_copy removed an out path, changed a log level, made explicit 
  assignement to 'xengnttab_copy_grant_segment'
* I did not change the way of testing if grant_copy operation is implemented.
  As far as I understand if the code from gnttab_unimp.c is used then the gnttab 
  device is unavailable and the handler to gntdev would be invalid. But 
  if the handler is valid then the ioctl should return operation unimplemented 
  if the gntdev does not implement the operation.


Changes since v1:
Interface:
- changed the interface to call grant copy operation to match ioctl
	int xengnttab_grant_copy(xengnttab_handle *xgt,
                         	 uint32_t count,
                         	 xengnttab_grant_copy_segment_t* segs)

- added a struct 'xengnttab_copy_grant_segment' definition to tools/libs	
  /gnttab/private.h, tools/libxc/include/xenctrl_compat.h

- changed the function 'osdep_gnttab_grant_copy' which right now just
  call the ioctl

- added a new VER1.1 to tools/libs/gnttab/libxengnttab.map 

qemu-qdisk:
- removed the 'ioreq_write','ioreq_read_init','ioreq_read' functions 
- implemented 'ioreq_init_copy_buffers', 'ioreq_copy' 
- reverted the removal of grant map and introduced conditional invoking
  grant copy or grant map
- resigned from caching the local buffers on behalf of allocating the 
  required amount of pages at once. The cached structure would require 
  to have an lock guard and I suppose that the performance improvement 
  would degraded. 
 

For the functional test I attached the device with a qdisk backend to the guest, 
mounted, performed some reads and writes.

I run fio tests[1] with different iodepth and size of the block. The test can be 
accessed on my github[2] but mainly after the warm up I run for 60 seconds:
    fio --time_based \
		--clocksource=clock_gettime \
		--rw=randread \
		--random_distribution=pareto:0.9 \
		--size=10g \
	    --direct='1' \
	    --ioengine=libaio \
		--filename=$DEV \
		--iodepth=$IODEPTH \
		--bs=$BS \
		--name=$NAME \
		--runtime=$RUNTIME >> $FILENAME
The test were repeated at least three times. 

[1] https://docs.google.com/spreadsheets/d/1E6AMiB8ceJpExL6jWpH9u2yy6DZxzhmDUyFf-eUuJ0c/edit?usp=sharing

[2] https://github.com/paulina-szubarczyk/xen-benchmark
    - multitest_with_iodepth.sh


Thanks and regards, 
Paulina

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

* [Qemu-devel] [PATCH v5 1/2] libs/gnttab: introduce grant copy interface
  2016-08-16  6:55 [Qemu-devel] [PATCH v4 0/2] qemu-qdisk: Implementation of grant copy operation Paulina Szubarczyk
@ 2016-08-16  6:55   ` Paulina Szubarczyk
  2016-08-16  6:55 ` [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation Paulina Szubarczyk
  2016-08-16  6:55 ` [Qemu-devel] " Paulina Szubarczyk
  2 siblings, 0 replies; 7+ messages in thread
From: Paulina Szubarczyk @ 2016-08-16  6:55 UTC (permalink / raw)
  To: xen-devel, roger.pau
  Cc: wei.liu2, ian.jackson, david.vrabel, sstabellini, anthony.perard,
	qemu-devel, Paulina Szubarczyk

In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
system call is invoked. In mini-os the operation is yet not
implemented. For the OSs that does not implement gnttab the
call of the grant copy operation causes abort.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
Changes since v4:
- changed the subject line
- changed the comment in libs/gnttab/include/xengnttab.h according
  to the David's suggestion.
- removed unnecessary braces.
---
 tools/include/xen-sys/Linux/gntdev.h  | 21 ++++++++++
 tools/libs/gnttab/Makefile            |  2 +-
 tools/libs/gnttab/gnttab_core.c       |  6 +++
 tools/libs/gnttab/gnttab_unimp.c      |  6 +++
 tools/libs/gnttab/include/xengnttab.h | 28 +++++++++++++
 tools/libs/gnttab/libxengnttab.map    |  5 +++
 tools/libs/gnttab/linux.c             | 77 +++++++++++++++++++++++++++++++++++
 tools/libs/gnttab/minios.c            |  6 +++
 tools/libs/gnttab/private.h           |  4 ++
 9 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/tools/include/xen-sys/Linux/gntdev.h b/tools/include/xen-sys/Linux/gntdev.h
index caf6fb4..0ca07c9 100644
--- a/tools/include/xen-sys/Linux/gntdev.h
+++ b/tools/include/xen-sys/Linux/gntdev.h
@@ -147,4 +147,25 @@ struct ioctl_gntdev_unmap_notify {
 /* Send an interrupt on the indicated event channel */
 #define UNMAP_NOTIFY_SEND_EVENT 0x2
 
+struct ioctl_gntdev_grant_copy_segment {
+    union {
+        void *virt;
+        struct {
+            uint32_t ref;
+            uint16_t offset;
+            uint16_t domid;
+        } foreign;
+    } source, dest;
+    uint16_t len;
+    uint16_t flags;
+    int16_t status;
+};
+
+#define IOCTL_GNTDEV_GRANT_COPY \
+_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
+struct ioctl_gntdev_grant_copy {
+    unsigned int count;
+    struct ioctl_gntdev_grant_copy_segment *segments;
+};
+
 #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
index af64542..95c2cd8 100644
--- a/tools/libs/gnttab/Makefile
+++ b/tools/libs/gnttab/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 MAJOR    = 1
-MINOR    = 0
+MINOR    = 1
 SHLIB_LDFLAGS += -Wl,--version-script=libxengnttab.map
 
 CFLAGS   += -Werror -Wmissing-prototypes
diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
index 5d0474d..968c833 100644
--- a/tools/libs/gnttab/gnttab_core.c
+++ b/tools/libs/gnttab/gnttab_core.c
@@ -113,6 +113,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count)
     return osdep_gnttab_unmap(xgt, start_address, count);
 }
 
+int xengnttab_grant_copy(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_grant_copy_segment_t *segs)
+{
+    return osdep_gnttab_grant_copy(xgt, count, segs);
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/gnttab_unimp.c b/tools/libs/gnttab/gnttab_unimp.c
index b3a4a20..829eced 100644
--- a/tools/libs/gnttab/gnttab_unimp.c
+++ b/tools/libs/gnttab/gnttab_unimp.c
@@ -78,6 +78,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count)
     abort();
 }
 
+int xengnttab_copy_grant(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_copy_grant_segment_t *segs)
+{
+    abort();
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/include/xengnttab.h b/tools/libs/gnttab/include/xengnttab.h
index 0431dcf..35be6c1 100644
--- a/tools/libs/gnttab/include/xengnttab.h
+++ b/tools/libs/gnttab/include/xengnttab.h
@@ -258,6 +258,34 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count);
 int xengnttab_set_max_grants(xengnttab_handle *xgt,
                              uint32_t nr_grants);
 
+struct xengnttab_grant_copy_segment {
+    union xengnttab_copy_ptr {
+        void *virt;
+        struct {
+            uint32_t ref;
+            uint16_t offset;
+            uint16_t domid;
+        } foreign;
+    } source, dest;
+    uint16_t len;
+    uint16_t flags;
+    int16_t status;
+};
+
+typedef struct xengnttab_grant_copy_segment xengnttab_grant_copy_segment_t;
+
+/**
+ * Copy memory from or to grant references. The information of each operations
+ * are contained in 'xengnttab_grant_copy_segment_t'. The @flag value indicate
+ * the direction of an operation (GNTCOPY_source_gref\GNTCOPY_dest_gref).
+ *
+ * For each segment, @virt may cross a page boundary but @offset + @len
+ * must not exceed XEN_PAGE_SIZE.
+ */
+int xengnttab_grant_copy(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_grant_copy_segment_t *segs);
+
 /*
  * Grant Sharing Interface (allocating and granting pages to others)
  */
diff --git a/tools/libs/gnttab/libxengnttab.map b/tools/libs/gnttab/libxengnttab.map
index dc737ac..f78da22 100644
--- a/tools/libs/gnttab/libxengnttab.map
+++ b/tools/libs/gnttab/libxengnttab.map
@@ -21,3 +21,8 @@ VERS_1.0 {
 		xengntshr_unshare;
 	local: *; /* Do not expose anything by default */
 };
+
+VERS_1.1 {
+    global:
+        xengnttab_grant_copy;
+} VERS_1.0;
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
index 7b0fba4..6bd9bd2 100644
--- a/tools/libs/gnttab/linux.c
+++ b/tools/libs/gnttab/linux.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#include <stddef.h>
 
 #include <sys/ioctl.h>
 #include <sys/mman.h>
@@ -235,6 +236,82 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
     return 0;
 }
 
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs)
+{
+    int rc;
+    int fd = xgt->fd;
+    struct ioctl_gntdev_grant_copy copy;
+
+    XENGNTTAB_BUILD_BUG_ON(sizeof(struct ioctl_gntdev_grant_copy_segment) !=
+                           sizeof(xengnttab_grant_copy_segment_t));
+
+    XENGNTTAB_BUILD_BUG_ON(__alignof__(struct ioctl_gntdev_grant_copy_segment) !=
+                           __alignof__(xengnttab_grant_copy_segment_t));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.virt) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.virt));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.ref) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.offset) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign.offset));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.domid) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign.domid));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.virt) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.virt));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.ref) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.offset) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign.offset));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.domid) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign.domid));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    len) !=
+                           offsetof(xengnttab_grant_copy_segment_t, len));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    flags) !=
+                           offsetof(xengnttab_grant_copy_segment_t, flags));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    status) !=
+                           offsetof(xengnttab_grant_copy_segment_t, status));
+
+    copy.segments = (struct ioctl_gntdev_grant_copy_segment *)segs;
+    copy.count = count;
+
+    rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, &copy);
+    if (rc)
+        GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
+
+    return rc;
+}
+
 int osdep_gntshr_open(xengntshr_handle *xgs)
 {
     int fd = open(DEVXEN "gntalloc", O_RDWR);
diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
index 7e04174..0951bc9 100644
--- a/tools/libs/gnttab/minios.c
+++ b/tools/libs/gnttab/minios.c
@@ -106,6 +106,12 @@ int osdep_gnttab_set_max_grants(xengnttab_handle *xgt, uint32_t count)
     return ret;
 }
 
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs)
+{
+    return -1;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index 2bdc0f2..f8d234a 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -30,6 +30,10 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 int osdep_gnttab_unmap(xengnttab_handle *xgt,
                        void *start_address,
                        uint32_t count);
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs);
+
 int osdep_gntshr_open(xengntshr_handle *xgs);
 int osdep_gntshr_close(xengntshr_handle *xgs);
 
-- 
1.9.1

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

* [PATCH v5 1/2] libs/gnttab: introduce grant copy interface
@ 2016-08-16  6:55   ` Paulina Szubarczyk
  0 siblings, 0 replies; 7+ messages in thread
From: Paulina Szubarczyk @ 2016-08-16  6:55 UTC (permalink / raw)
  To: xen-devel, roger.pau
  Cc: sstabellini, wei.liu2, Paulina Szubarczyk, ian.jackson,
	qemu-devel, david.vrabel, anthony.perard

In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
system call is invoked. In mini-os the operation is yet not
implemented. For the OSs that does not implement gnttab the
call of the grant copy operation causes abort.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
Changes since v4:
- changed the subject line
- changed the comment in libs/gnttab/include/xengnttab.h according
  to the David's suggestion.
- removed unnecessary braces.
---
 tools/include/xen-sys/Linux/gntdev.h  | 21 ++++++++++
 tools/libs/gnttab/Makefile            |  2 +-
 tools/libs/gnttab/gnttab_core.c       |  6 +++
 tools/libs/gnttab/gnttab_unimp.c      |  6 +++
 tools/libs/gnttab/include/xengnttab.h | 28 +++++++++++++
 tools/libs/gnttab/libxengnttab.map    |  5 +++
 tools/libs/gnttab/linux.c             | 77 +++++++++++++++++++++++++++++++++++
 tools/libs/gnttab/minios.c            |  6 +++
 tools/libs/gnttab/private.h           |  4 ++
 9 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/tools/include/xen-sys/Linux/gntdev.h b/tools/include/xen-sys/Linux/gntdev.h
index caf6fb4..0ca07c9 100644
--- a/tools/include/xen-sys/Linux/gntdev.h
+++ b/tools/include/xen-sys/Linux/gntdev.h
@@ -147,4 +147,25 @@ struct ioctl_gntdev_unmap_notify {
 /* Send an interrupt on the indicated event channel */
 #define UNMAP_NOTIFY_SEND_EVENT 0x2
 
+struct ioctl_gntdev_grant_copy_segment {
+    union {
+        void *virt;
+        struct {
+            uint32_t ref;
+            uint16_t offset;
+            uint16_t domid;
+        } foreign;
+    } source, dest;
+    uint16_t len;
+    uint16_t flags;
+    int16_t status;
+};
+
+#define IOCTL_GNTDEV_GRANT_COPY \
+_IOC(_IOC_NONE, 'G', 8, sizeof(struct ioctl_gntdev_grant_copy))
+struct ioctl_gntdev_grant_copy {
+    unsigned int count;
+    struct ioctl_gntdev_grant_copy_segment *segments;
+};
+
 #endif /* __LINUX_PUBLIC_GNTDEV_H__ */
diff --git a/tools/libs/gnttab/Makefile b/tools/libs/gnttab/Makefile
index af64542..95c2cd8 100644
--- a/tools/libs/gnttab/Makefile
+++ b/tools/libs/gnttab/Makefile
@@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../..
 include $(XEN_ROOT)/tools/Rules.mk
 
 MAJOR    = 1
-MINOR    = 0
+MINOR    = 1
 SHLIB_LDFLAGS += -Wl,--version-script=libxengnttab.map
 
 CFLAGS   += -Werror -Wmissing-prototypes
diff --git a/tools/libs/gnttab/gnttab_core.c b/tools/libs/gnttab/gnttab_core.c
index 5d0474d..968c833 100644
--- a/tools/libs/gnttab/gnttab_core.c
+++ b/tools/libs/gnttab/gnttab_core.c
@@ -113,6 +113,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count)
     return osdep_gnttab_unmap(xgt, start_address, count);
 }
 
+int xengnttab_grant_copy(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_grant_copy_segment_t *segs)
+{
+    return osdep_gnttab_grant_copy(xgt, count, segs);
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/gnttab_unimp.c b/tools/libs/gnttab/gnttab_unimp.c
index b3a4a20..829eced 100644
--- a/tools/libs/gnttab/gnttab_unimp.c
+++ b/tools/libs/gnttab/gnttab_unimp.c
@@ -78,6 +78,12 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count)
     abort();
 }
 
+int xengnttab_copy_grant(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_copy_grant_segment_t *segs)
+{
+    abort();
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/include/xengnttab.h b/tools/libs/gnttab/include/xengnttab.h
index 0431dcf..35be6c1 100644
--- a/tools/libs/gnttab/include/xengnttab.h
+++ b/tools/libs/gnttab/include/xengnttab.h
@@ -258,6 +258,34 @@ int xengnttab_unmap(xengnttab_handle *xgt, void *start_address, uint32_t count);
 int xengnttab_set_max_grants(xengnttab_handle *xgt,
                              uint32_t nr_grants);
 
+struct xengnttab_grant_copy_segment {
+    union xengnttab_copy_ptr {
+        void *virt;
+        struct {
+            uint32_t ref;
+            uint16_t offset;
+            uint16_t domid;
+        } foreign;
+    } source, dest;
+    uint16_t len;
+    uint16_t flags;
+    int16_t status;
+};
+
+typedef struct xengnttab_grant_copy_segment xengnttab_grant_copy_segment_t;
+
+/**
+ * Copy memory from or to grant references. The information of each operations
+ * are contained in 'xengnttab_grant_copy_segment_t'. The @flag value indicate
+ * the direction of an operation (GNTCOPY_source_gref\GNTCOPY_dest_gref).
+ *
+ * For each segment, @virt may cross a page boundary but @offset + @len
+ * must not exceed XEN_PAGE_SIZE.
+ */
+int xengnttab_grant_copy(xengnttab_handle *xgt,
+                         uint32_t count,
+                         xengnttab_grant_copy_segment_t *segs);
+
 /*
  * Grant Sharing Interface (allocating and granting pages to others)
  */
diff --git a/tools/libs/gnttab/libxengnttab.map b/tools/libs/gnttab/libxengnttab.map
index dc737ac..f78da22 100644
--- a/tools/libs/gnttab/libxengnttab.map
+++ b/tools/libs/gnttab/libxengnttab.map
@@ -21,3 +21,8 @@ VERS_1.0 {
 		xengntshr_unshare;
 	local: *; /* Do not expose anything by default */
 };
+
+VERS_1.1 {
+    global:
+        xengnttab_grant_copy;
+} VERS_1.0;
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c
index 7b0fba4..6bd9bd2 100644
--- a/tools/libs/gnttab/linux.c
+++ b/tools/libs/gnttab/linux.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <stdint.h>
 #include <string.h>
+#include <stddef.h>
 
 #include <sys/ioctl.h>
 #include <sys/mman.h>
@@ -235,6 +236,82 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
     return 0;
 }
 
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs)
+{
+    int rc;
+    int fd = xgt->fd;
+    struct ioctl_gntdev_grant_copy copy;
+
+    XENGNTTAB_BUILD_BUG_ON(sizeof(struct ioctl_gntdev_grant_copy_segment) !=
+                           sizeof(xengnttab_grant_copy_segment_t));
+
+    XENGNTTAB_BUILD_BUG_ON(__alignof__(struct ioctl_gntdev_grant_copy_segment) !=
+                           __alignof__(xengnttab_grant_copy_segment_t));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.virt) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.virt));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.ref) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.offset) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign.offset));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    source.foreign.domid) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    source.foreign.domid));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.virt) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.virt));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.ref) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.offset) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign.offset));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    dest.foreign.domid) !=
+                           offsetof(xengnttab_grant_copy_segment_t,
+                                    dest.foreign.domid));
+
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    len) !=
+                           offsetof(xengnttab_grant_copy_segment_t, len));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    flags) !=
+                           offsetof(xengnttab_grant_copy_segment_t, flags));
+    XENGNTTAB_BUILD_BUG_ON(offsetof(struct ioctl_gntdev_grant_copy_segment,
+                                    status) !=
+                           offsetof(xengnttab_grant_copy_segment_t, status));
+
+    copy.segments = (struct ioctl_gntdev_grant_copy_segment *)segs;
+    copy.count = count;
+
+    rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, &copy);
+    if (rc)
+        GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
+
+    return rc;
+}
+
 int osdep_gntshr_open(xengntshr_handle *xgs)
 {
     int fd = open(DEVXEN "gntalloc", O_RDWR);
diff --git a/tools/libs/gnttab/minios.c b/tools/libs/gnttab/minios.c
index 7e04174..0951bc9 100644
--- a/tools/libs/gnttab/minios.c
+++ b/tools/libs/gnttab/minios.c
@@ -106,6 +106,12 @@ int osdep_gnttab_set_max_grants(xengnttab_handle *xgt, uint32_t count)
     return ret;
 }
 
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs)
+{
+    return -1;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/gnttab/private.h b/tools/libs/gnttab/private.h
index 2bdc0f2..f8d234a 100644
--- a/tools/libs/gnttab/private.h
+++ b/tools/libs/gnttab/private.h
@@ -30,6 +30,10 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt,
 int osdep_gnttab_unmap(xengnttab_handle *xgt,
                        void *start_address,
                        uint32_t count);
+int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
+                            uint32_t count,
+                            xengnttab_grant_copy_segment_t *segs);
+
 int osdep_gntshr_open(xengntshr_handle *xgs);
 int osdep_gntshr_close(xengntshr_handle *xgs);
 
-- 
1.9.1


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

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

* [Qemu-devel] [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation
  2016-08-16  6:55 [Qemu-devel] [PATCH v4 0/2] qemu-qdisk: Implementation of grant copy operation Paulina Szubarczyk
  2016-08-16  6:55   ` Paulina Szubarczyk
  2016-08-16  6:55 ` [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation Paulina Szubarczyk
@ 2016-08-16  6:55 ` Paulina Szubarczyk
  2016-09-06  9:19   ` Anthony PERARD
  2016-09-06  9:19   ` [Qemu-devel] " Anthony PERARD
  2 siblings, 2 replies; 7+ messages in thread
From: Paulina Szubarczyk @ 2016-08-16  6:55 UTC (permalink / raw)
  To: xen-devel, roger.pau
  Cc: wei.liu2, ian.jackson, david.vrabel, sstabellini, anthony.perard,
	qemu-devel, Paulina Szubarczyk

Copy data operated on during request from/to local buffers to/from
the grant references.

Before grant copy operation local buffers must be allocated what is
done by calling ioreq_init_copy_buffers. For the 'read' operation,
first, the qemu device invokes the read operation on local buffers
and on the completion grant copy is called and buffers are freed.
For the 'write' operation grant copy is performed before invoking
write by qemu device.

A new value 'feature_grant_copy' is added to recognize when the
grant copy operation is supported by a guest.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
---
Changes since v4:
- in the configure file check only if xengnttab_grant_copy is
  implemented to verify 480 version of Xen.
- remove r variable and initialization of count to 0 in
  ioreq_copy.

- surround free_buffers, ioreq_init_copy_buffers and ioreq_copy
  by "#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 480" abort in else
  path because the function should not be called in that case.
- replace the definition of struct xengnttab_grant_copy_segment
  and a typedef to it with
  'typedef void* xengnttab_grant_copy_segment_t'.
- moved the new code in the xen_common.h to the end of the file.
---
 configure                   |  37 +++++++++++
 hw/block/xen_disk.c         | 157 ++++++++++++++++++++++++++++++++++++++++++--
 include/hw/xen/xen_common.h |  14 ++++
 3 files changed, 203 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 8d84919..68b3374 100755
--- a/configure
+++ b/configure
@@ -1956,6 +1956,43 @@ EOF
 /*
  * If we have stable libs the we don't want the libxc compat
  * layers, regardless of what CFLAGS we may have been given.
+ *
+ * Also, check if xengnttab_grant_copy_segment_t is defined and
+ * grant copy operation is implemented.
+ */
+#undef XC_WANT_COMPAT_EVTCHN_API
+#undef XC_WANT_COMPAT_GNTTAB_API
+#undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#include <xenctrl.h>
+#include <xenstore.h>
+#include <xenevtchn.h>
+#include <xengnttab.h>
+#include <xenforeignmemory.h>
+#include <stdint.h>
+#include <xen/hvm/hvm_info_table.h>
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xengnttab_handle *xg;
+  xengnttab_grant_copy_segment_t* seg = NULL;
+
+  xg = xengnttab_open(0, 0);
+  xengnttab_map_grant_ref(xg, 0, 0, 0);
+  xengnttab_grant_copy(xg, 0, seg);
+
+  return 0;
+}
+EOF
+      compile_prog "" "$xen_libs $xen_stable_libs"
+    then
+    xen_ctrl_version=480
+    xen=yes
+  elif
+      cat > $TMPC <<EOF &&
+/*
+ * If we have stable libs the we don't want the libxc compat
+ * layers, regardless of what CFLAGS we may have been given.
  */
 #undef XC_WANT_COMPAT_EVTCHN_API
 #undef XC_WANT_COMPAT_GNTTAB_API
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 3b8ad33..3739e13 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -119,6 +119,9 @@ struct XenBlkDev {
     unsigned int        persistent_gnt_count;
     unsigned int        max_grants;
 
+    /* Grant copy */
+    gboolean            feature_grant_copy;
+
     /* qemu block driver */
     DriveInfo           *dinfo;
     BlockBackend        *blk;
@@ -489,6 +492,108 @@ static int ioreq_map(struct ioreq *ioreq)
     return 0;
 }
 
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 480
+
+static void free_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_copy(struct ioreq *ioreq)
+{
+    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
+    xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    int i, count, rc;
+    int64_t file_blk = ioreq->blkdev->file_blk;
+
+    if (ioreq->v.niov == 0) {
+        return 0;
+    }
+
+    count = ioreq->v.niov;
+
+    for (i = 0; i < count; i++) {
+
+        if (ioreq->req.operation == BLKIF_OP_READ) {
+            segs[i].flags = GNTCOPY_dest_gref;
+            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;
+        }
+        segs[i].len = (ioreq->req.seg[i].last_sect
+                       - ioreq->req.seg[i].first_sect + 1) * file_blk;
+
+    }
+
+    rc = xengnttab_grant_copy(gnt, count, segs);
+
+    if (rc) {
+        xen_be_printf(&ioreq->blkdev->xendev, 0,
+                      "failed to copy data %d\n", rc);
+        ioreq->aio_errors++;
+        return -1;
+    }
+
+    for (i = 0; i < count; i++) {
+        if (segs[i].status != GNTST_okay) {
+            xen_be_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
+static void free_buffers(struct ioreq *ioreq)
+{
+    abort();
+}
+
+static int ioreq_init_copy_buffers(struct ioreq *ioreq)
+{
+    abort();
+}
+
+static int ioreq_copy(struct ioreq *ioreq)
+{
+    abort();
+}
+#endif
+
 static int ioreq_runio_qemu_aio(struct ioreq *ioreq);
 
 static void qemu_aio_complete(void *opaque, int ret)
@@ -511,8 +616,31 @@ static void qemu_aio_complete(void *opaque, int ret)
         return;
     }
 
+    if (ioreq->blkdev->feature_grant_copy) {
+        switch (ioreq->req.operation) {
+        case BLKIF_OP_READ:
+            /* in case of failure ioreq->aio_errors is increased */
+            if (ret == 0) {
+                ioreq_copy(ioreq);
+            }
+            free_buffers(ioreq);
+            break;
+        case BLKIF_OP_WRITE:
+        case BLKIF_OP_FLUSH_DISKCACHE:
+            if (!ioreq->req.nr_segments) {
+                break;
+            }
+            free_buffers(ioreq);
+            break;
+        default:
+            break;
+        }
+    }
+
     ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
-    ioreq_unmap(ioreq);
+    if (!ioreq->blkdev->feature_grant_copy) {
+        ioreq_unmap(ioreq);
+    }
     ioreq_finish(ioreq);
     switch (ioreq->req.operation) {
     case BLKIF_OP_WRITE:
@@ -538,8 +666,20 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    if (ioreq->req.nr_segments && ioreq_map(ioreq) == -1) {
-        goto err_no_map;
+    if (ioreq->blkdev->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)) {
+            if (ioreq_copy(ioreq)) {
+                free_buffers(ioreq);
+                goto err;
+            }
+        }
+
+    } else {
+        if (ioreq->req.nr_segments && ioreq_map(ioreq)) {
+            goto err;
+        }
     }
 
     ioreq->aio_inflight++;
@@ -582,6 +722,9 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     }
     default:
         /* unknown operation (shouldn't happen -- parse catches this) */
+        if (!ioreq->blkdev->feature_grant_copy) {
+            ioreq_unmap(ioreq);
+        }
         goto err;
     }
 
@@ -590,8 +733,6 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     return 0;
 
 err:
-    ioreq_unmap(ioreq);
-err_no_map:
     ioreq_finish(ioreq);
     ioreq->status = BLKIF_RSP_ERROR;
     return -1;
@@ -1032,6 +1173,12 @@ static int blk_connect(struct XenDevice *xendev)
 
     xen_be_bind_evtchn(&blkdev->xendev);
 
+    blkdev->feature_grant_copy =
+                (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);
+
+    xen_be_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
+                  blkdev->feature_grant_copy ? "enabled" : "disabled");
+
     xen_be_printf(&blkdev->xendev, 1, "ok: proto %s, ring-ref %d, "
                   "remote port %d, local port %d\n",
                   blkdev->xendev.protocol, blkdev->ring_ref,
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 640c31e..6b0776f 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -369,4 +369,18 @@ static inline int xen_domain_create(xc_interface *xc, uint32_t ssidref,
 #endif
 #endif
 
+/* Xen before 4.8 */
+
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 480
+
+
+typedef void *xengnttab_grant_copy_segment_t;
+
+static inline int xengnttab_grant_copy(xengnttab_handle *xgt, uint32_t count,
+                                       xengnttab_grant_copy_segment_t *segs)
+{
+    return -ENOSYS;
+}
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
-- 
1.9.1

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

* [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation
  2016-08-16  6:55 [Qemu-devel] [PATCH v4 0/2] qemu-qdisk: Implementation of grant copy operation Paulina Szubarczyk
  2016-08-16  6:55   ` Paulina Szubarczyk
@ 2016-08-16  6:55 ` Paulina Szubarczyk
  2016-08-16  6:55 ` [Qemu-devel] " Paulina Szubarczyk
  2 siblings, 0 replies; 7+ messages in thread
From: Paulina Szubarczyk @ 2016-08-16  6:55 UTC (permalink / raw)
  To: xen-devel, roger.pau
  Cc: sstabellini, wei.liu2, Paulina Szubarczyk, ian.jackson,
	qemu-devel, david.vrabel, anthony.perard

Copy data operated on during request from/to local buffers to/from
the grant references.

Before grant copy operation local buffers must be allocated what is
done by calling ioreq_init_copy_buffers. For the 'read' operation,
first, the qemu device invokes the read operation on local buffers
and on the completion grant copy is called and buffers are freed.
For the 'write' operation grant copy is performed before invoking
write by qemu device.

A new value 'feature_grant_copy' is added to recognize when the
grant copy operation is supported by a guest.

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
---
Changes since v4:
- in the configure file check only if xengnttab_grant_copy is
  implemented to verify 480 version of Xen.
- remove r variable and initialization of count to 0 in
  ioreq_copy.

- surround free_buffers, ioreq_init_copy_buffers and ioreq_copy
  by "#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 480" abort in else
  path because the function should not be called in that case.
- replace the definition of struct xengnttab_grant_copy_segment
  and a typedef to it with
  'typedef void* xengnttab_grant_copy_segment_t'.
- moved the new code in the xen_common.h to the end of the file.
---
 configure                   |  37 +++++++++++
 hw/block/xen_disk.c         | 157 ++++++++++++++++++++++++++++++++++++++++++--
 include/hw/xen/xen_common.h |  14 ++++
 3 files changed, 203 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 8d84919..68b3374 100755
--- a/configure
+++ b/configure
@@ -1956,6 +1956,43 @@ EOF
 /*
  * If we have stable libs the we don't want the libxc compat
  * layers, regardless of what CFLAGS we may have been given.
+ *
+ * Also, check if xengnttab_grant_copy_segment_t is defined and
+ * grant copy operation is implemented.
+ */
+#undef XC_WANT_COMPAT_EVTCHN_API
+#undef XC_WANT_COMPAT_GNTTAB_API
+#undef XC_WANT_COMPAT_MAP_FOREIGN_API
+#include <xenctrl.h>
+#include <xenstore.h>
+#include <xenevtchn.h>
+#include <xengnttab.h>
+#include <xenforeignmemory.h>
+#include <stdint.h>
+#include <xen/hvm/hvm_info_table.h>
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+  xengnttab_handle *xg;
+  xengnttab_grant_copy_segment_t* seg = NULL;
+
+  xg = xengnttab_open(0, 0);
+  xengnttab_map_grant_ref(xg, 0, 0, 0);
+  xengnttab_grant_copy(xg, 0, seg);
+
+  return 0;
+}
+EOF
+      compile_prog "" "$xen_libs $xen_stable_libs"
+    then
+    xen_ctrl_version=480
+    xen=yes
+  elif
+      cat > $TMPC <<EOF &&
+/*
+ * If we have stable libs the we don't want the libxc compat
+ * layers, regardless of what CFLAGS we may have been given.
  */
 #undef XC_WANT_COMPAT_EVTCHN_API
 #undef XC_WANT_COMPAT_GNTTAB_API
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 3b8ad33..3739e13 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -119,6 +119,9 @@ struct XenBlkDev {
     unsigned int        persistent_gnt_count;
     unsigned int        max_grants;
 
+    /* Grant copy */
+    gboolean            feature_grant_copy;
+
     /* qemu block driver */
     DriveInfo           *dinfo;
     BlockBackend        *blk;
@@ -489,6 +492,108 @@ static int ioreq_map(struct ioreq *ioreq)
     return 0;
 }
 
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION >= 480
+
+static void free_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_copy(struct ioreq *ioreq)
+{
+    xengnttab_handle *gnt = ioreq->blkdev->xendev.gnttabdev;
+    xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+    int i, count, rc;
+    int64_t file_blk = ioreq->blkdev->file_blk;
+
+    if (ioreq->v.niov == 0) {
+        return 0;
+    }
+
+    count = ioreq->v.niov;
+
+    for (i = 0; i < count; i++) {
+
+        if (ioreq->req.operation == BLKIF_OP_READ) {
+            segs[i].flags = GNTCOPY_dest_gref;
+            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;
+        }
+        segs[i].len = (ioreq->req.seg[i].last_sect
+                       - ioreq->req.seg[i].first_sect + 1) * file_blk;
+
+    }
+
+    rc = xengnttab_grant_copy(gnt, count, segs);
+
+    if (rc) {
+        xen_be_printf(&ioreq->blkdev->xendev, 0,
+                      "failed to copy data %d\n", rc);
+        ioreq->aio_errors++;
+        return -1;
+    }
+
+    for (i = 0; i < count; i++) {
+        if (segs[i].status != GNTST_okay) {
+            xen_be_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
+static void free_buffers(struct ioreq *ioreq)
+{
+    abort();
+}
+
+static int ioreq_init_copy_buffers(struct ioreq *ioreq)
+{
+    abort();
+}
+
+static int ioreq_copy(struct ioreq *ioreq)
+{
+    abort();
+}
+#endif
+
 static int ioreq_runio_qemu_aio(struct ioreq *ioreq);
 
 static void qemu_aio_complete(void *opaque, int ret)
@@ -511,8 +616,31 @@ static void qemu_aio_complete(void *opaque, int ret)
         return;
     }
 
+    if (ioreq->blkdev->feature_grant_copy) {
+        switch (ioreq->req.operation) {
+        case BLKIF_OP_READ:
+            /* in case of failure ioreq->aio_errors is increased */
+            if (ret == 0) {
+                ioreq_copy(ioreq);
+            }
+            free_buffers(ioreq);
+            break;
+        case BLKIF_OP_WRITE:
+        case BLKIF_OP_FLUSH_DISKCACHE:
+            if (!ioreq->req.nr_segments) {
+                break;
+            }
+            free_buffers(ioreq);
+            break;
+        default:
+            break;
+        }
+    }
+
     ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
-    ioreq_unmap(ioreq);
+    if (!ioreq->blkdev->feature_grant_copy) {
+        ioreq_unmap(ioreq);
+    }
     ioreq_finish(ioreq);
     switch (ioreq->req.operation) {
     case BLKIF_OP_WRITE:
@@ -538,8 +666,20 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
 {
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
-    if (ioreq->req.nr_segments && ioreq_map(ioreq) == -1) {
-        goto err_no_map;
+    if (ioreq->blkdev->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)) {
+            if (ioreq_copy(ioreq)) {
+                free_buffers(ioreq);
+                goto err;
+            }
+        }
+
+    } else {
+        if (ioreq->req.nr_segments && ioreq_map(ioreq)) {
+            goto err;
+        }
     }
 
     ioreq->aio_inflight++;
@@ -582,6 +722,9 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     }
     default:
         /* unknown operation (shouldn't happen -- parse catches this) */
+        if (!ioreq->blkdev->feature_grant_copy) {
+            ioreq_unmap(ioreq);
+        }
         goto err;
     }
 
@@ -590,8 +733,6 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq)
     return 0;
 
 err:
-    ioreq_unmap(ioreq);
-err_no_map:
     ioreq_finish(ioreq);
     ioreq->status = BLKIF_RSP_ERROR;
     return -1;
@@ -1032,6 +1173,12 @@ static int blk_connect(struct XenDevice *xendev)
 
     xen_be_bind_evtchn(&blkdev->xendev);
 
+    blkdev->feature_grant_copy =
+                (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);
+
+    xen_be_printf(&blkdev->xendev, 3, "grant copy operation %s\n",
+                  blkdev->feature_grant_copy ? "enabled" : "disabled");
+
     xen_be_printf(&blkdev->xendev, 1, "ok: proto %s, ring-ref %d, "
                   "remote port %d, local port %d\n",
                   blkdev->xendev.protocol, blkdev->ring_ref,
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 640c31e..6b0776f 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -369,4 +369,18 @@ static inline int xen_domain_create(xc_interface *xc, uint32_t ssidref,
 #endif
 #endif
 
+/* Xen before 4.8 */
+
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION < 480
+
+
+typedef void *xengnttab_grant_copy_segment_t;
+
+static inline int xengnttab_grant_copy(xengnttab_handle *xgt, uint32_t count,
+                                       xengnttab_grant_copy_segment_t *segs)
+{
+    return -ENOSYS;
+}
+#endif
+
 #endif /* QEMU_HW_XEN_COMMON_H */
-- 
1.9.1


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

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

* Re: [Qemu-devel] [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation
  2016-08-16  6:55 ` [Qemu-devel] " Paulina Szubarczyk
  2016-09-06  9:19   ` Anthony PERARD
@ 2016-09-06  9:19   ` Anthony PERARD
  1 sibling, 0 replies; 7+ messages in thread
From: Anthony PERARD @ 2016-09-06  9:19 UTC (permalink / raw)
  To: Paulina Szubarczyk
  Cc: xen-devel, roger.pau, wei.liu2, ian.jackson, david.vrabel,
	sstabellini, qemu-devel

On Tue, Aug 16, 2016 at 08:55:56AM +0200, Paulina Szubarczyk wrote:
> diff --git a/configure b/configure
> index 8d84919..68b3374 100755
> --- a/configure
> +++ b/configure
> @@ -1956,6 +1956,43 @@ EOF
>  /*
>   * If we have stable libs the we don't want the libxc compat
>   * layers, regardless of what CFLAGS we may have been given.
> + *
> + * Also, check if xengnttab_grant_copy_segment_t is defined and
> + * grant copy operation is implemented.
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#include <xenctrl.h>
> +#include <xenstore.h>
> +#include <xenevtchn.h>
> +#include <xengnttab.h>
> +#include <xenforeignmemory.h>
> +#include <stdint.h>
> +#include <xen/hvm/hvm_info_table.h>
> +#if !defined(HVM_MAX_VCPUS)
> +# error HVM_MAX_VCPUS not defined
> +#endif
> +int main(void) {
> +  xengnttab_handle *xg;
> +  xengnttab_grant_copy_segment_t* seg = NULL;
> +
> +  xg = xengnttab_open(0, 0);
> +  xengnttab_map_grant_ref(xg, 0, 0, 0);
> +  xengnttab_grant_copy(xg, 0, seg);
> +
> +  return 0;
> +}

I think we should check every interface that qemu use, for here that
just mean copy-paste the previous version check and add grant_copy to
it.


With that fix, I think the patch will be ready to go.

Thanks,

-- 
Anthony PERARD

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

* Re: [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation
  2016-08-16  6:55 ` [Qemu-devel] " Paulina Szubarczyk
@ 2016-09-06  9:19   ` Anthony PERARD
  2016-09-06  9:19   ` [Qemu-devel] " Anthony PERARD
  1 sibling, 0 replies; 7+ messages in thread
From: Anthony PERARD @ 2016-09-06  9:19 UTC (permalink / raw)
  To: Paulina Szubarczyk
  Cc: sstabellini, wei.liu2, ian.jackson, qemu-devel, david.vrabel,
	xen-devel, roger.pau

On Tue, Aug 16, 2016 at 08:55:56AM +0200, Paulina Szubarczyk wrote:
> diff --git a/configure b/configure
> index 8d84919..68b3374 100755
> --- a/configure
> +++ b/configure
> @@ -1956,6 +1956,43 @@ EOF
>  /*
>   * If we have stable libs the we don't want the libxc compat
>   * layers, regardless of what CFLAGS we may have been given.
> + *
> + * Also, check if xengnttab_grant_copy_segment_t is defined and
> + * grant copy operation is implemented.
> + */
> +#undef XC_WANT_COMPAT_EVTCHN_API
> +#undef XC_WANT_COMPAT_GNTTAB_API
> +#undef XC_WANT_COMPAT_MAP_FOREIGN_API
> +#include <xenctrl.h>
> +#include <xenstore.h>
> +#include <xenevtchn.h>
> +#include <xengnttab.h>
> +#include <xenforeignmemory.h>
> +#include <stdint.h>
> +#include <xen/hvm/hvm_info_table.h>
> +#if !defined(HVM_MAX_VCPUS)
> +# error HVM_MAX_VCPUS not defined
> +#endif
> +int main(void) {
> +  xengnttab_handle *xg;
> +  xengnttab_grant_copy_segment_t* seg = NULL;
> +
> +  xg = xengnttab_open(0, 0);
> +  xengnttab_map_grant_ref(xg, 0, 0, 0);
> +  xengnttab_grant_copy(xg, 0, seg);
> +
> +  return 0;
> +}

I think we should check every interface that qemu use, for here that
just mean copy-paste the previous version check and add grant_copy to
it.


With that fix, I think the patch will be ready to go.

Thanks,

-- 
Anthony PERARD

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

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

end of thread, other threads:[~2016-09-06  9:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16  6:55 [Qemu-devel] [PATCH v4 0/2] qemu-qdisk: Implementation of grant copy operation Paulina Szubarczyk
2016-08-16  6:55 ` [Qemu-devel] [PATCH v5 1/2] libs/gnttab: introduce grant copy interface Paulina Szubarczyk
2016-08-16  6:55   ` Paulina Szubarczyk
2016-08-16  6:55 ` [PATCH v5 2/2] qdisk - hw/block/xen_disk: grant copy implementation Paulina Szubarczyk
2016-08-16  6:55 ` [Qemu-devel] " Paulina Szubarczyk
2016-09-06  9:19   ` Anthony PERARD
2016-09-06  9:19   ` [Qemu-devel] " Anthony PERARD

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.