* [PATCH v3 1/8] xen_backend: add grant table helpers
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
@ 2018-05-04 19:26 ` Paul Durrant
2018-05-04 19:26 ` [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab Paul Durrant
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
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-04 19:26 ` [PATCH v3 3/8] xen: remove other " Paul Durrant
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
2018-05-04 19:26 ` [PATCH v3 1/8] xen_backend: add grant table helpers 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 ` Paul Durrant
2018-05-04 19:26 ` [PATCH v3 4/8] xen_backend: add an emulation of grant copy Paul Durrant
` (11 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 4/8] xen_backend: add an emulation of grant copy
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
` (2 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-04 19:26 ` [PATCH v3 5/8] xen_disk: remove use of grant map/unmap Paul Durrant
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
` (3 preceding siblings ...)
2018-05-04 19:26 ` [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 6/8] xen_backend: make the xen_feature_grant_copy flag private Paul Durrant
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 6/8] xen_backend: make the xen_feature_grant_copy flag private
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
` (4 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-04 19:26 ` [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
` (8 subsequent siblings)
14 siblings, 1 reply; 19+ 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] 19+ messages in thread* [PATCH v3 7/8] xen_disk: use a single entry iovec
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
` (5 preceding siblings ...)
2018-05-04 19:26 ` [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-04 19:26 ` [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev Paul Durrant
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread* [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
[not found] <1525461967-32174-1-git-send-email-paul.durrant@citrix.com>
` (6 preceding siblings ...)
2018-05-04 19:26 ` [PATCH v3 7/8] xen_disk: use a single entry iovec Paul Durrant
@ 2018-05-04 19:26 ` Paul Durrant
[not found] ` <1525461967-32174-2-git-send-email-paul.durrant@citrix.com>
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ 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] 19+ messages in thread[parent not found: <1525461967-32174-2-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 1/8] xen_backend: add grant table helpers
[not found] ` <1525461967-32174-2-git-send-email-paul.durrant@citrix.com>
@ 2018-05-16 13:50 ` Anthony PERARD
2018-05-16 14:00 ` Paul Durrant
0 siblings, 1 reply; 19+ 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] 19+ messages in thread* Re: [PATCH v3 1/8] xen_backend: add grant table helpers
2018-05-16 13:50 ` [PATCH v3 1/8] xen_backend: add grant table helpers Anthony PERARD
@ 2018-05-16 14:00 ` Paul Durrant
0 siblings, 0 replies; 19+ messages in thread
From: Paul Durrant @ 2018-05-16 14:00 UTC (permalink / raw)
To: Anthony Perard
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini,
qemu-devel@nongnu.org, qemu-block@nongnu.org
> -----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] 19+ messages in thread
[parent not found: <1525461967-32174-3-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 2/8] xen_disk: remove open-coded use of libxengnttab
[not found] ` <1525461967-32174-3-git-send-email-paul.durrant@citrix.com>
@ 2018-05-16 14:02 ` Anthony PERARD
0 siblings, 0 replies; 19+ 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] 19+ messages in thread
[parent not found: <1525461967-32174-4-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
[not found] ` <1525461967-32174-4-git-send-email-paul.durrant@citrix.com>
@ 2018-05-16 14:14 ` Anthony PERARD
[not found] ` <20180516141424.GH2057@perard.uk.xensource.com>
1 sibling, 0 replies; 19+ 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] 19+ messages in thread[parent not found: <20180516141424.GH2057@perard.uk.xensource.com>]
* Re: [PATCH v3 3/8] xen: remove other open-coded use of libxengnttab
[not found] ` <20180516141424.GH2057@perard.uk.xensource.com>
@ 2018-05-16 14:22 ` Paul Durrant
0 siblings, 0 replies; 19+ messages in thread
From: Paul Durrant @ 2018-05-16 14:22 UTC (permalink / raw)
To: Anthony Perard
Cc: Stefano Stabellini, qemu-block@nongnu.org, Jason Wang,
qemu-devel@nongnu.org, Greg Kurz, Gerd Hoffmann, Paolo Bonzini,
xen-devel@lists.xenproject.org
> -----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] 19+ messages in thread
[parent not found: <1525461967-32174-5-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
[not found] ` <1525461967-32174-5-git-send-email-paul.durrant@citrix.com>
@ 2018-05-16 14:30 ` Anthony PERARD
2018-05-16 14:34 ` Paul Durrant
0 siblings, 1 reply; 19+ 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] 19+ messages in thread* Re: [PATCH v3 4/8] xen_backend: add an emulation of grant copy
2018-05-16 14:30 ` [PATCH v3 4/8] xen_backend: add an emulation of grant copy Anthony PERARD
@ 2018-05-16 14:34 ` Paul Durrant
0 siblings, 0 replies; 19+ messages in thread
From: Paul Durrant @ 2018-05-16 14:34 UTC (permalink / raw)
To: Anthony Perard
Cc: xen-devel@lists.xenproject.org, Stefano Stabellini,
qemu-devel@nongnu.org, qemu-block@nongnu.org
> -----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] 19+ messages in thread
[parent not found: <1525461967-32174-6-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 5/8] xen_disk: remove use of grant map/unmap
[not found] ` <1525461967-32174-6-git-send-email-paul.durrant@citrix.com>
@ 2018-05-17 10:31 ` Anthony PERARD
0 siblings, 0 replies; 19+ 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] 19+ messages in thread
[parent not found: <1525461967-32174-8-git-send-email-paul.durrant@citrix.com>]
[parent not found: <1525461967-32174-9-git-send-email-paul.durrant@citrix.com>]
* Re: [PATCH v3 8/8] xen_disk: be consistent with use of xendev and blkdev->xendev
[not found] ` <1525461967-32174-9-git-send-email-paul.durrant@citrix.com>
@ 2018-05-17 11:07 ` Anthony PERARD
0 siblings, 0 replies; 19+ 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] 19+ messages in thread