From 5feeb718d7914c3c921cc811a5844488a5644ea6 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Tue, 9 Apr 2019 17:40:38 +0100 Subject: xen-block: support feature-large-sector-size A recent Xen commit [1] clarified the semantics of sector based quantities used in the blkif protocol such that it is now safe to create a xen-block device with a logical_block_size != 512, as long as the device only connects to a frontend advertizing 'feature-large-block-size'. This patch modifies xen-block accordingly. It also uses a stack variable for the BlockBackend in xen_block_realize() to avoid repeated dereferencing of the BlockConf pointer, and changes the parameters of xen_block_dataplane_create() so that the BlockBackend pointer and sector size are passed expicitly rather than implicitly via the BlockConf. These modifications have been tested against a recent Windows PV XENVBD driver [2] using a xen-disk device with a 4kB logical block size. [1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=67e1c050e36b2c9900cca83618e56189effbad98 [2] https://winpvdrvbuild.xenproject.org:8080/job/XENVBD-master/126 Signed-off-by: Paul Durrant Reviewed-by: Anthony PERARD Message-Id: <20190409164038.25484-1-paul.durrant@citrix.com> [Edited error message] Signed-off-by: Anthony PERARD --- hw/block/dataplane/xen-block.c | 25 ++++++++++++++----------- hw/block/dataplane/xen-block.h | 3 ++- hw/block/xen-block.c | 38 ++++++++++++++++++++++++-------------- 3 files changed, 40 insertions(+), 26 deletions(-) (limited to 'hw/block') diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index f7ad452bbd..6da5c77fbb 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -58,6 +58,7 @@ struct XenBlockDataPlane { int requests_inflight; unsigned int max_requests; BlockBackend *blk; + unsigned int sector_size; QEMUBH *bh; IOThread *iothread; AioContext *ctx; @@ -167,7 +168,7 @@ static int xen_block_parse_request(XenBlockRequest *request) goto err; } - request->start = request->req.sector_number * XEN_BLKIF_SECTOR_SIZE; + request->start = request->req.sector_number * dataplane->sector_size; for (i = 0; i < request->req.nr_segments; i++) { if (i == BLKIF_MAX_SEGMENTS_PER_REQUEST) { error_report("error: nr_segments too big"); @@ -177,14 +178,14 @@ static int xen_block_parse_request(XenBlockRequest *request) error_report("error: first > last sector"); goto err; } - if (request->req.seg[i].last_sect * XEN_BLKIF_SECTOR_SIZE >= + if (request->req.seg[i].last_sect * dataplane->sector_size >= XC_PAGE_SIZE) { error_report("error: page crossing"); goto err; } len = (request->req.seg[i].last_sect - - request->req.seg[i].first_sect + 1) * XEN_BLKIF_SECTOR_SIZE; + request->req.seg[i].first_sect + 1) * dataplane->sector_size; request->size += len; } if (request->start + request->size > blk_getlength(dataplane->blk)) { @@ -218,17 +219,17 @@ static int xen_block_copy_request(XenBlockRequest *request) if (to_domain) { segs[i].dest.foreign.ref = request->req.seg[i].gref; segs[i].dest.foreign.offset = request->req.seg[i].first_sect * - XEN_BLKIF_SECTOR_SIZE; + dataplane->sector_size; segs[i].source.virt = virt; } else { segs[i].source.foreign.ref = request->req.seg[i].gref; segs[i].source.foreign.offset = request->req.seg[i].first_sect * - XEN_BLKIF_SECTOR_SIZE; + dataplane->sector_size; segs[i].dest.virt = virt; } segs[i].len = (request->req.seg[i].last_sect - request->req.seg[i].first_sect + 1) * - XEN_BLKIF_SECTOR_SIZE; + dataplane->sector_size; virt += segs[i].len; } @@ -336,12 +337,12 @@ static bool xen_block_split_discard(XenBlockRequest *request, /* Wrap around, or overflowing byte limit? */ if (sec_start + sec_count < sec_count || - sec_start + sec_count > INT64_MAX / XEN_BLKIF_SECTOR_SIZE) { + sec_start + sec_count > INT64_MAX / dataplane->sector_size) { return false; } - byte_offset = sec_start * XEN_BLKIF_SECTOR_SIZE; - byte_remaining = sec_count * XEN_BLKIF_SECTOR_SIZE; + byte_offset = sec_start * dataplane->sector_size; + byte_remaining = sec_count * dataplane->sector_size; do { byte_chunk = byte_remaining > BDRV_REQUEST_MAX_BYTES ? @@ -625,13 +626,15 @@ static void xen_block_dataplane_event(void *opaque) } XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev, - BlockConf *conf, + BlockBackend *blk, + unsigned int sector_size, IOThread *iothread) { XenBlockDataPlane *dataplane = g_new0(XenBlockDataPlane, 1); dataplane->xendev = xendev; - dataplane->blk = conf->blk; + dataplane->blk = blk; + dataplane->sector_size = sector_size; QLIST_INIT(&dataplane->inflight); QLIST_INIT(&dataplane->freelist); diff --git a/hw/block/dataplane/xen-block.h b/hw/block/dataplane/xen-block.h index d6fa6d26dd..76dcd51c3d 100644 --- a/hw/block/dataplane/xen-block.h +++ b/hw/block/dataplane/xen-block.h @@ -15,7 +15,8 @@ typedef struct XenBlockDataPlane XenBlockDataPlane; XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev, - BlockConf *conf, + BlockBackend *blk, + unsigned int sector_size, IOThread *iothread); void xen_block_dataplane_destroy(XenBlockDataPlane *dataplane); void xen_block_dataplane_start(XenBlockDataPlane *dataplane, diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c index 4de537aef4..8f224ef81d 100644 --- a/hw/block/xen-block.c +++ b/hw/block/xen-block.c @@ -52,11 +52,25 @@ static void xen_block_connect(XenDevice *xendev, Error **errp) XenBlockDevice *blockdev = XEN_BLOCK_DEVICE(xendev); const char *type = object_get_typename(OBJECT(blockdev)); XenBlockVdev *vdev = &blockdev->props.vdev; + BlockConf *conf = &blockdev->props.conf; + unsigned int feature_large_sector_size; unsigned int order, nr_ring_ref, *ring_ref, event_channel, protocol; char *str; trace_xen_block_connect(type, vdev->disk, vdev->partition); + if (xen_device_frontend_scanf(xendev, "feature-large-sector-size", "%u", + &feature_large_sector_size) != 1) { + feature_large_sector_size = 0; + } + + if (feature_large_sector_size != 1 && + conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) { + error_setg(errp, "logical_block_size != %u not supported by frontend", + XEN_BLKIF_SECTOR_SIZE); + return; + } + if (xen_device_frontend_scanf(xendev, "ring-page-order", "%u", &order) != 1) { nr_ring_ref = 1; @@ -150,7 +164,7 @@ static void xen_block_set_size(XenBlockDevice *blockdev) const char *type = object_get_typename(OBJECT(blockdev)); XenBlockVdev *vdev = &blockdev->props.vdev; BlockConf *conf = &blockdev->props.conf; - int64_t sectors = blk_getlength(conf->blk) / XEN_BLKIF_SECTOR_SIZE; + int64_t sectors = blk_getlength(conf->blk) / conf->logical_block_size; XenDevice *xendev = XEN_DEVICE(blockdev); trace_xen_block_size(type, vdev->disk, vdev->partition, sectors); @@ -185,6 +199,7 @@ static void xen_block_realize(XenDevice *xendev, Error **errp) const char *type = object_get_typename(OBJECT(blockdev)); XenBlockVdev *vdev = &blockdev->props.vdev; BlockConf *conf = &blockdev->props.conf; + BlockBackend *blk = conf->blk; Error *local_err = NULL; if (vdev->type == XEN_BLOCK_VDEV_TYPE_INVALID) { @@ -206,8 +221,8 @@ static void xen_block_realize(XenDevice *xendev, Error **errp) * The blkif protocol does not deal with removable media, so it must * always be present, even for CDRom devices. */ - assert(conf->blk); - if (!blk_is_inserted(conf->blk)) { + assert(blk); + if (!blk_is_inserted(blk)) { error_setg(errp, "device needs media, but drive is empty"); return; } @@ -224,26 +239,20 @@ static void xen_block_realize(XenDevice *xendev, Error **errp) blkconf_blocksizes(conf); - if (conf->logical_block_size != XEN_BLKIF_SECTOR_SIZE) { - error_setg(errp, "logical_block_size != %u not supported", - XEN_BLKIF_SECTOR_SIZE); - return; - } - if (conf->logical_block_size > conf->physical_block_size) { error_setg( errp, "logical_block_size > physical_block_size not supported"); return; } - blk_set_dev_ops(conf->blk, &xen_block_dev_ops, blockdev); - blk_set_guest_block_size(conf->blk, conf->logical_block_size); + blk_set_dev_ops(blk, &xen_block_dev_ops, blockdev); + blk_set_guest_block_size(blk, conf->logical_block_size); if (conf->discard_granularity == -1) { conf->discard_granularity = conf->physical_block_size; } - if (blk_get_flags(conf->blk) & BDRV_O_UNMAP) { + if (blk_get_flags(blk) & BDRV_O_UNMAP) { xen_device_backend_printf(xendev, "feature-discard", "%u", 1); xen_device_backend_printf(xendev, "discard-granularity", "%u", conf->discard_granularity); @@ -260,12 +269,13 @@ static void xen_block_realize(XenDevice *xendev, Error **errp) blockdev->device_type); xen_device_backend_printf(xendev, "sector-size", "%u", - XEN_BLKIF_SECTOR_SIZE); + conf->logical_block_size); xen_block_set_size(blockdev); blockdev->dataplane = - xen_block_dataplane_create(xendev, conf, blockdev->props.iothread); + xen_block_dataplane_create(xendev, blk, conf->logical_block_size, + blockdev->props.iothread); } static void xen_block_frontend_changed(XenDevice *xendev, -- cgit v1.2.3 From 83361a8a1f932cfac8ae4a5b86e935ad6ab1c528 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Mon, 8 Apr 2019 16:16:16 +0100 Subject: xen-bus: allow AioContext to be specified for each event channel This patch adds an AioContext parameter to xen_device_bind_event_channel() and then uses aio_set_fd_handler() to set the callback rather than qemu_set_fd_handler(). Signed-off-by: Paul Durrant Reviewed-by: Anthony PERARD Message-Id: <20190408151617.13025-3-paul.durrant@citrix.com> [Call aio_set_fd_handler() with is_external=true] Signed-off-by: Anthony PERARD --- hw/block/dataplane/xen-block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/block') diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 6da5c77fbb..aadca75644 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -806,7 +806,7 @@ void xen_block_dataplane_start(XenBlockDataPlane *dataplane, } dataplane->event_channel = - xen_device_bind_event_channel(xendev, event_channel, + xen_device_bind_event_channel(xendev, dataplane->ctx, event_channel, xen_block_dataplane_event, dataplane, &local_err); if (local_err) { -- cgit v1.2.3 From 345f42b4be9f9975ff3f7f3e95acae4f67c74f89 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Mon, 8 Apr 2019 16:16:17 +0100 Subject: xen-bus / xen-block: add support for event channel polling This patch introduces a poll callback for event channel fd-s and uses this to invoke the channel callback function. To properly support polling, it is necessary for the event channel callback function to return a boolean saying whether it has done any useful work or not. Thus xen_block_dataplane_event() is modified to directly invoke xen_block_handle_requests() and the latter only returns true if it actually processes any requests. This also means that the call to qemu_bh_schedule() is moved into xen_block_complete_aio(), which is more intuitive since the only reason for doing a deferred poll of the shared ring should be because there were previously insufficient resources to fully complete a previous poll. Signed-off-by: Paul Durrant Reviewed-by: Anthony PERARD Message-Id: <20190408151617.13025-4-paul.durrant@citrix.com> Signed-off-by: Anthony PERARD --- hw/block/dataplane/xen-block.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'hw/block') diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index aadca75644..0f200c5fb0 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -318,7 +318,9 @@ static void xen_block_complete_aio(void *opaque, int ret) } xen_block_release_request(request); - qemu_bh_schedule(dataplane->bh); + if (dataplane->more_work) { + qemu_bh_schedule(dataplane->bh); + } done: aio_context_release(dataplane->ctx); @@ -515,12 +517,13 @@ static int xen_block_get_request(XenBlockDataPlane *dataplane, */ #define IO_PLUG_THRESHOLD 1 -static void xen_block_handle_requests(XenBlockDataPlane *dataplane) +static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) { RING_IDX rc, rp; XenBlockRequest *request; int inflight_atstart = dataplane->requests_inflight; int batched = 0; + bool done_something = false; dataplane->more_work = 0; @@ -552,6 +555,7 @@ static void xen_block_handle_requests(XenBlockDataPlane *dataplane) } xen_block_get_request(dataplane, request, rc); dataplane->rings.common.req_cons = ++rc; + done_something = true; /* parse them */ if (xen_block_parse_request(request) != 0) { @@ -603,10 +607,7 @@ static void xen_block_handle_requests(XenBlockDataPlane *dataplane) blk_io_unplug(dataplane->blk); } - if (dataplane->more_work && - dataplane->requests_inflight < dataplane->max_requests) { - qemu_bh_schedule(dataplane->bh); - } + return done_something; } static void xen_block_dataplane_bh(void *opaque) @@ -618,11 +619,11 @@ static void xen_block_dataplane_bh(void *opaque) aio_context_release(dataplane->ctx); } -static void xen_block_dataplane_event(void *opaque) +static bool xen_block_dataplane_event(void *opaque) { XenBlockDataPlane *dataplane = opaque; - qemu_bh_schedule(dataplane->bh); + return xen_block_handle_requests(dataplane); } XenBlockDataPlane *xen_block_dataplane_create(XenDevice *xendev, -- cgit v1.2.3 From a3434a2d56aee3018f4a0f55c7e0f0cda11f3d9e Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Fri, 21 Jun 2019 11:54:41 +0100 Subject: xen: Import other xen/io/*.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Xen public header have been imported into QEMU (by f65eadb639 "xen: import ring.h from xen"), but there are other header that depends on ring.h which come from the system when building QEMU. This patch resolves the issue of having headers from the system importing a different copie of ring.h. This patch is prompt by the build issue described in the previous patch: 'Revert xen/io/ring.h of "Clean up a few header guard symbols"' ring.h and the new imported headers are moved to "include/hw/xen/interface" as those describe interfaces with a guest. The imported headers are cleaned up a bit while importing them: some part of the file that QEMU doesn't use are removed (description of how to make hypercall in grant_table.h have been removed). Other cleanup: - xen-mapcache.c and xen-legacy-backend.c don't need grant_table.h. - xenfb.c doesn't need event_channel.h. Signed-off-by: Anthony PERARD Reviewed-by: Daniel P. Berrangé Reviewed-by: Paul Durrant Message-Id: <20190621105441.3025-3-anthony.perard@citrix.com> --- hw/block/xen_blkif.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'hw/block') diff --git a/hw/block/xen_blkif.h b/hw/block/xen_blkif.h index a353693ea0..99733529c1 100644 --- a/hw/block/xen_blkif.h +++ b/hw/block/xen_blkif.h @@ -1,9 +1,8 @@ #ifndef XEN_BLKIF_H #define XEN_BLKIF_H -#include "hw/xen/io/ring.h" -#include -#include +#include "hw/xen/interface/io/blkif.h" +#include "hw/xen/interface/io/protocols.h" /* * Not a real protocol. Used to generate ring structs which contain -- cgit v1.2.3