From b5e397a79ec874d9739e096f5e27a912d857a0ec Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Fri, 22 Sep 2017 14:07:24 +0200 Subject: xen: add a global indicator for grant copy being available The Xen qdisk backend needs to test whether grant copy operations is available in the kernel. Unfortunately this collides with using xengnttab_set_max_grants() on some kernels as this operation has to be the first one after opening the gnttab device. In order to solve this problem test for the availability of grant copy in xen_be_init() opening the gnttab device just for that purpose and closing it again afterwards. Advertise the availability via a global flag and use that flag in the qdisk backend. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Signed-off-by: Stefano Stabellini --- hw/block/xen_disk.c | 18 ++++++------------ hw/xen/xen_backend.c | 11 +++++++++++ include/hw/xen/xen_backend.h | 1 + 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 536e2ee..62506e3 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -121,9 +121,6 @@ struct XenBlkDev { unsigned int persistent_gnt_count; unsigned int max_grants; - /* Grant copy */ - gboolean feature_grant_copy; - /* qemu block driver */ DriveInfo *dinfo; BlockBackend *blk; @@ -616,7 +613,7 @@ static void qemu_aio_complete(void *opaque, int ret) return; } - if (ioreq->blkdev->feature_grant_copy) { + if (xen_feature_grant_copy) { switch (ioreq->req.operation) { case BLKIF_OP_READ: /* in case of failure ioreq->aio_errors is increased */ @@ -638,7 +635,7 @@ static void qemu_aio_complete(void *opaque, int ret) } ioreq->status = ioreq->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY; - if (!ioreq->blkdev->feature_grant_copy) { + if (!xen_feature_grant_copy) { ioreq_unmap(ioreq); } ioreq_finish(ioreq); @@ -698,7 +695,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) { struct XenBlkDev *blkdev = ioreq->blkdev; - if (ioreq->blkdev->feature_grant_copy) { + 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) && @@ -750,7 +747,7 @@ static int ioreq_runio_qemu_aio(struct ioreq *ioreq) } default: /* unknown operation (shouldn't happen -- parse catches this) */ - if (!ioreq->blkdev->feature_grant_copy) { + if (!xen_feature_grant_copy) { ioreq_unmap(ioreq); } goto err; @@ -1010,18 +1007,15 @@ static int blk_init(struct XenDevice *xendev) blkdev->file_blk = BLOCK_SIZE; - blkdev->feature_grant_copy = - (xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0); - xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n", - blkdev->feature_grant_copy ? "enabled" : "disabled"); + 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", - !blkdev->feature_grant_copy); + !xen_feature_grant_copy); xenstore_write_be_int(&blkdev->xendev, "info", info); xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order", diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index c46cbb0..0f849a2 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -44,6 +44,7 @@ BusState *xen_sysbus; /* public */ struct xs_handle *xenstore = NULL; const char *xen_protocol; +bool xen_feature_grant_copy; /* private */ static int debug; @@ -519,6 +520,8 @@ void xenstore_update_fe(char *watch, struct XenDevice *xendev) int xen_be_init(void) { + xengnttab_handle *gnttabdev; + xenstore = xs_daemon_open(); if (!xenstore) { xen_pv_printf(NULL, 0, "can't connect to xenstored\n"); @@ -532,6 +535,14 @@ int xen_be_init(void) goto err; } + gnttabdev = xengnttab_open(NULL, 0); + if (gnttabdev != NULL) { + if (xengnttab_grant_copy(gnttabdev, 0, NULL) == 0) { + xen_feature_grant_copy = true; + } + xengnttab_close(gnttabdev); + } + xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); qdev_init_nofail(xen_sysdev); xen_sysbus = qbus_create(TYPE_XENSYSBUS, DEVICE(xen_sysdev), "xen-sysbus"); diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h index 8a6fbcb..3a27692 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -16,6 +16,7 @@ /* 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; -- cgit v1.1 From e38c3e86dfffe3cc806d051b9deeab31836b196a Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Fri, 22 Sep 2017 14:07:25 +0200 Subject: xen: dont try setting max grants multiple times Trying to call xengnttab_set_max_grants() with the same file handle might fail on some kernels, as this operation is allowed only once. This is a problem for the qdisk backend as blk_connect() can be called multiple times for a domain, e.g. in case grub-xen is being used to boot it. So instead of letting the generic backend code open the gnttab device do it in blk_connect() and close it again in blk_disconnect. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Signed-off-by: Stefano Stabellini --- hw/block/xen_disk.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 62506e3..e431bd8 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -1220,6 +1220,12 @@ 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)); @@ -1327,6 +1333,11 @@ 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) @@ -1334,9 +1345,7 @@ static int blk_free(struct XenDevice *xendev) struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev); struct ioreq *ioreq; - if (blkdev->blk || blkdev->sring) { - blk_disconnect(xendev); - } + blk_disconnect(xendev); while (!QLIST_EMPTY(&blkdev->freelist)) { ioreq = QLIST_FIRST(&blkdev->freelist); @@ -1363,7 +1372,6 @@ static void blk_event(struct XenDevice *xendev) struct XenDevOps xen_blkdev_ops = { .size = sizeof(struct XenBlkDev), - .flags = DEVOPS_FLAG_NEED_GNTDEV, .alloc = blk_alloc, .init = blk_init, .initialise = blk_connect, -- cgit v1.1 From 7cdcca725b6bfc96634c15e3f74ae4b148cf9c40 Mon Sep 17 00:00:00 2001 From: Ross Lagerwall Date: Wed, 11 Oct 2017 16:52:03 +0100 Subject: xen: Log errno rather than return value xen_modified_memory() sets errno to communicate what went wrong so log this rather than the return value which is not interesting. Signed-off-by: Ross Lagerwall Acked-by: Anthony PERARD Signed-off-by: Stefano Stabellini --- hw/i386/xen/xen-hvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index d9ccd5d..8028bed 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -1446,7 +1446,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length) if (rc) { fprintf(stderr, "%s failed for "RAM_ADDR_FMT" ("RAM_ADDR_FMT"): %i, %s\n", - __func__, start, nb_pages, rc, strerror(-rc)); + __func__, start, nb_pages, errno, strerror(errno)); } } } -- cgit v1.1