Many Xen PV frontends share similar code for setting up a ring page (allocating and granting access for the backend) and for tearing it down.
Create new service functions doing all needed steps in one go.
This requires all frontends to use a common value for an invalid grant reference in order to make the functions idempotent.
Changes in V3: - new patches 1 and 2, comments addressed
Changes in V2: - new patch 9 and related changes in patches 10-18
Juergen Gross (21): xen: update grant_table.h xen/grant-table: never put a reserved grant on the free list xen/blkfront: switch blkfront to use INVALID_GRANT_REF xen/netfront: switch netfront to use INVALID_GRANT_REF xen/scsifront: remove unused GRANT_INVALID_REF definition xen/usb: switch xen-hcd to use INVALID_GRANT_REF xen/drm: switch xen_drm_front to use INVALID_GRANT_REF xen/sound: switch xen_snd_front to use INVALID_GRANT_REF xen/dmabuf: switch gntdev-dmabuf to use INVALID_GRANT_REF xen/shbuf: switch xen-front-pgdir-shbuf to use INVALID_GRANT_REF xen: update ring.h xen/xenbus: add xenbus_setup_ring() service function xen/blkfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/netfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/tpmfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/drmfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/pcifront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/scsifront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/usbfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/sndfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/xenbus: eliminate xenbus_grant_ring()
drivers/block/xen-blkfront.c | 57 +++---- drivers/char/tpm/xen-tpmfront.c | 18 +-- drivers/gpu/drm/xen/xen_drm_front.h | 9 -- drivers/gpu/drm/xen/xen_drm_front_evtchnl.c | 43 ++---- drivers/net/xen-netfront.c | 85 ++++------- drivers/pci/xen-pcifront.c | 19 +-- drivers/scsi/xen-scsifront.c | 31 +--- drivers/usb/host/xen-hcd.c | 65 ++------ drivers/xen/gntdev-dmabuf.c | 13 +- drivers/xen/grant-table.c | 12 +- drivers/xen/xen-front-pgdir-shbuf.c | 18 +-- drivers/xen/xenbus/xenbus_client.c | 82 +++++++--- include/xen/grant_table.h | 2 - include/xen/interface/grant_table.h | 161 ++++++++++++-------- include/xen/interface/io/ring.h | 19 ++- include/xen/xenbus.h | 4 +- sound/xen/xen_snd_front_evtchnl.c | 44 ++---- sound/xen/xen_snd_front_evtchnl.h | 9 -- 18 files changed, 287 insertions(+), 404 deletions(-)
Instead of using a private macro for an invalid grant reference use the common one.
Signed-off-by: Juergen Gross jgross@suse.com --- drivers/gpu/drm/xen/xen_drm_front.h | 9 --------- drivers/gpu/drm/xen/xen_drm_front_evtchnl.c | 4 ++-- 2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.h b/drivers/gpu/drm/xen/xen_drm_front.h index cefafe859aba..a987c78abe41 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.h +++ b/drivers/gpu/drm/xen/xen_drm_front.h @@ -80,15 +80,6 @@ struct drm_pending_vblank_event; /* timeout in ms to wait for backend to respond */ #define XEN_DRM_FRONT_WAIT_BACK_MS 3000
-#ifndef GRANT_INVALID_REF -/* - * Note on usage of grant reference 0 as invalid grant reference: - * grant reference 0 is valid, but never exposed to a PV driver, - * because of the fact it is already in use/reserved by the PV console. - */ -#define GRANT_INVALID_REF 0 -#endif - struct xen_drm_front_info { struct xenbus_device *xb_dev; struct xen_drm_front_drm_info *drm_info; diff --git a/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c b/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c index 08b526eeec16..4006568b9e32 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c +++ b/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c @@ -147,7 +147,7 @@ static void evtchnl_free(struct xen_drm_front_info *front_info, xenbus_free_evtchn(front_info->xb_dev, evtchnl->port);
/* end access and free the page */ - if (evtchnl->gref != GRANT_INVALID_REF) + if (evtchnl->gref != INVALID_GRANT_REF) gnttab_end_foreign_access(evtchnl->gref, page);
memset(evtchnl, 0, sizeof(*evtchnl)); @@ -168,7 +168,7 @@ static int evtchnl_alloc(struct xen_drm_front_info *front_info, int index, evtchnl->index = index; evtchnl->front_info = front_info; evtchnl->state = EVTCHNL_STATE_DISCONNECTED; - evtchnl->gref = GRANT_INVALID_REF; + evtchnl->gref = INVALID_GRANT_REF;
page = get_zeroed_page(GFP_NOIO | __GFP_HIGH); if (!page) {
Simplify drmfront's ring creation and removal via xenbus_setup_ring() and xenbus_teardown_ring().
Signed-off-by: Juergen Gross jgross@suse.com --- drivers/gpu/drm/xen/xen_drm_front_evtchnl.c | 43 ++++++--------------- 1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c b/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c index 4006568b9e32..e52afd792346 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c +++ b/drivers/gpu/drm/xen/xen_drm_front_evtchnl.c @@ -123,12 +123,12 @@ static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id) static void evtchnl_free(struct xen_drm_front_info *front_info, struct xen_drm_front_evtchnl *evtchnl) { - unsigned long page = 0; + void *page = NULL;
if (evtchnl->type == EVTCHNL_TYPE_REQ) - page = (unsigned long)evtchnl->u.req.ring.sring; + page = evtchnl->u.req.ring.sring; else if (evtchnl->type == EVTCHNL_TYPE_EVT) - page = (unsigned long)evtchnl->u.evt.page; + page = evtchnl->u.evt.page; if (!page) return;
@@ -147,8 +147,7 @@ static void evtchnl_free(struct xen_drm_front_info *front_info, xenbus_free_evtchn(front_info->xb_dev, evtchnl->port);
/* end access and free the page */ - if (evtchnl->gref != INVALID_GRANT_REF) - gnttab_end_foreign_access(evtchnl->gref, page); + xenbus_teardown_ring(&page, 1, &evtchnl->gref);
memset(evtchnl, 0, sizeof(*evtchnl)); } @@ -158,8 +157,7 @@ static int evtchnl_alloc(struct xen_drm_front_info *front_info, int index, enum xen_drm_front_evtchnl_type type) { struct xenbus_device *xb_dev = front_info->xb_dev; - unsigned long page; - grant_ref_t gref; + void *page; irq_handler_t handler; int ret;
@@ -168,44 +166,25 @@ static int evtchnl_alloc(struct xen_drm_front_info *front_info, int index, evtchnl->index = index; evtchnl->front_info = front_info; evtchnl->state = EVTCHNL_STATE_DISCONNECTED; - evtchnl->gref = INVALID_GRANT_REF;
- page = get_zeroed_page(GFP_NOIO | __GFP_HIGH); - if (!page) { - ret = -ENOMEM; + ret = xenbus_setup_ring(xb_dev, GFP_NOIO | __GFP_HIGH, &page, + 1, &evtchnl->gref); + if (ret) goto fail; - }
if (type == EVTCHNL_TYPE_REQ) { struct xen_displif_sring *sring;
init_completion(&evtchnl->u.req.completion); mutex_init(&evtchnl->u.req.req_io_lock); - sring = (struct xen_displif_sring *)page; - SHARED_RING_INIT(sring); - FRONT_RING_INIT(&evtchnl->u.req.ring, sring, XEN_PAGE_SIZE); - - ret = xenbus_grant_ring(xb_dev, sring, 1, &gref); - if (ret < 0) { - evtchnl->u.req.ring.sring = NULL; - free_page(page); - goto fail; - } + sring = page; + XEN_FRONT_RING_INIT(&evtchnl->u.req.ring, sring, XEN_PAGE_SIZE);
handler = evtchnl_interrupt_ctrl; } else { - ret = gnttab_grant_foreign_access(xb_dev->otherend_id, - virt_to_gfn((void *)page), 0); - if (ret < 0) { - free_page(page); - goto fail; - } - - evtchnl->u.evt.page = (struct xendispl_event_page *)page; - gref = ret; + evtchnl->u.evt.page = page; handler = evtchnl_interrupt_evt; } - evtchnl->gref = gref;
ret = xenbus_alloc_evtchn(xb_dev, &evtchnl->port); if (ret < 0)
On 5/5/22 4:16 AM, Juergen Gross wrote:
Many Xen PV frontends share similar code for setting up a ring page (allocating and granting access for the backend) and for tearing it down.
Create new service functions doing all needed steps in one go.
This requires all frontends to use a common value for an invalid grant reference in order to make the functions idempotent.
Changes in V3:
- new patches 1 and 2, comments addressed
Changes in V2:
- new patch 9 and related changes in patches 10-18
For the patches that I was explicitly copied on:
Reviewed-by: Boris Ostrovsky boris.ostrovsky@oracle.com
On 05.05.22 11:16, Juergen Gross wrote:
Hello Juergen.
Many Xen PV frontends share similar code for setting up a ring page (allocating and granting access for the backend) and for tearing it down.
Create new service functions doing all needed steps in one go.
This requires all frontends to use a common value for an invalid grant reference in order to make the functions idempotent.
Changes in V3:
- new patches 1 and 2, comments addressed
Changes in V2:
- new patch 9 and related changes in patches 10-18
Juergen Gross (21): xen: update grant_table.h xen/grant-table: never put a reserved grant on the free list xen/blkfront: switch blkfront to use INVALID_GRANT_REF xen/netfront: switch netfront to use INVALID_GRANT_REF xen/scsifront: remove unused GRANT_INVALID_REF definition xen/usb: switch xen-hcd to use INVALID_GRANT_REF xen/drm: switch xen_drm_front to use INVALID_GRANT_REF xen/sound: switch xen_snd_front to use INVALID_GRANT_REF xen/dmabuf: switch gntdev-dmabuf to use INVALID_GRANT_REF xen/shbuf: switch xen-front-pgdir-shbuf to use INVALID_GRANT_REF xen: update ring.h xen/xenbus: add xenbus_setup_ring() service function xen/blkfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/netfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/tpmfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/drmfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/pcifront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/scsifront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/usbfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/sndfront: use xenbus_setup_ring() and xenbus_teardown_ring() xen/xenbus: eliminate xenbus_grant_ring()
For the patches that touch PV display (#07, #16), PV sound (#08, #20) and shared buffer framework used by both frontends (#10):
Reviewed-by: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com
Also I didn't see any issues with these frontends while testing on Arm64 based board. So, you can also add:
[Arm64 only] Tested-by: Oleksandr Tyshchenko oleksandr_tyshchenko@epam.com
Thanks!
drivers/block/xen-blkfront.c | 57 +++---- drivers/char/tpm/xen-tpmfront.c | 18 +-- drivers/gpu/drm/xen/xen_drm_front.h | 9 -- drivers/gpu/drm/xen/xen_drm_front_evtchnl.c | 43 ++---- drivers/net/xen-netfront.c | 85 ++++------- drivers/pci/xen-pcifront.c | 19 +-- drivers/scsi/xen-scsifront.c | 31 +--- drivers/usb/host/xen-hcd.c | 65 ++------ drivers/xen/gntdev-dmabuf.c | 13 +- drivers/xen/grant-table.c | 12 +- drivers/xen/xen-front-pgdir-shbuf.c | 18 +-- drivers/xen/xenbus/xenbus_client.c | 82 +++++++--- include/xen/grant_table.h | 2 - include/xen/interface/grant_table.h | 161 ++++++++++++-------- include/xen/interface/io/ring.h | 19 ++- include/xen/xenbus.h | 4 +- sound/xen/xen_snd_front_evtchnl.c | 44 ++---- sound/xen/xen_snd_front_evtchnl.h | 9 -- 18 files changed, 287 insertions(+), 404 deletions(-)
dri-devel@lists.freedesktop.org
-
Boris Ostrovsky -
Juergen Gross -
Oleksandr