* [PATCH v2 01/19] xen/blkfront: switch blkfront to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 02/19] xen/netfront: switch netfront " Juergen Gross
` (17 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-block, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
Roger Pau Monné,
Jens Axboe
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/block/xen-blkfront.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 003056d4f7f5..7f35e30e626a 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -229,8 +229,6 @@ static unsigned int nr_minors;
static unsigned long *minors;
static DEFINE_SPINLOCK(minor_lock);
-#define GRANT_INVALID_REF 0
-
#define PARTS_PER_DISK 16
#define PARTS_PER_EXT_DISK 256
@@ -321,7 +319,7 @@ static int fill_grant_buffer(struct blkfront_ring_info *rinfo, int num)
gnt_list_entry->page = granted_page;
}
- gnt_list_entry->gref = GRANT_INVALID_REF;
+ gnt_list_entry->gref = INVALID_GRANT_REF;
list_add(&gnt_list_entry->node, &rinfo->grants);
i++;
}
@@ -350,7 +348,7 @@ static struct grant *get_free_grant(struct blkfront_ring_info *rinfo)
node);
list_del(&gnt_list_entry->node);
- if (gnt_list_entry->gref != GRANT_INVALID_REF)
+ if (gnt_list_entry->gref != INVALID_GRANT_REF)
rinfo->persistent_gnts_c--;
return gnt_list_entry;
@@ -372,7 +370,7 @@ static struct grant *get_grant(grant_ref_t *gref_head,
struct grant *gnt_list_entry = get_free_grant(rinfo);
struct blkfront_info *info = rinfo->dev_info;
- if (gnt_list_entry->gref != GRANT_INVALID_REF)
+ if (gnt_list_entry->gref != INVALID_GRANT_REF)
return gnt_list_entry;
/* Assign a gref to this page */
@@ -396,7 +394,7 @@ static struct grant *get_indirect_grant(grant_ref_t *gref_head,
struct grant *gnt_list_entry = get_free_grant(rinfo);
struct blkfront_info *info = rinfo->dev_info;
- if (gnt_list_entry->gref != GRANT_INVALID_REF)
+ if (gnt_list_entry->gref != INVALID_GRANT_REF)
return gnt_list_entry;
/* Assign a gref to this page */
@@ -1221,7 +1219,7 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
list_for_each_entry_safe(persistent_gnt, n,
&rinfo->grants, node) {
list_del(&persistent_gnt->node);
- if (persistent_gnt->gref != GRANT_INVALID_REF) {
+ if (persistent_gnt->gref != INVALID_GRANT_REF) {
gnttab_end_foreign_access(persistent_gnt->gref,
0UL);
rinfo->persistent_gnts_c--;
@@ -1283,9 +1281,9 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
/* Free resources associated with old device channel. */
for (i = 0; i < info->nr_ring_pages; i++) {
- if (rinfo->ring_ref[i] != GRANT_INVALID_REF) {
+ if (rinfo->ring_ref[i] != INVALID_GRANT_REF) {
gnttab_end_foreign_access(rinfo->ring_ref[i], 0);
- rinfo->ring_ref[i] = GRANT_INVALID_REF;
+ rinfo->ring_ref[i] = INVALID_GRANT_REF;
}
}
free_pages_exact(rinfo->ring.sring,
@@ -1475,7 +1473,7 @@ static int blkif_completion(unsigned long *id,
* to the tail of the list, so it will not be picked
* again unless we run out of persistent grants.
*/
- s->grants_used[i]->gref = GRANT_INVALID_REF;
+ s->grants_used[i]->gref = INVALID_GRANT_REF;
list_add_tail(&s->grants_used[i]->node, &rinfo->grants);
}
}
@@ -1500,7 +1498,7 @@ static int blkif_completion(unsigned long *id,
indirect_page = s->indirect_grants[i]->page;
list_add(&indirect_page->lru, &rinfo->indirect_pages);
}
- s->indirect_grants[i]->gref = GRANT_INVALID_REF;
+ s->indirect_grants[i]->gref = INVALID_GRANT_REF;
list_add_tail(&s->indirect_grants[i]->node, &rinfo->grants);
}
}
@@ -1687,7 +1685,7 @@ static int setup_blkring(struct xenbus_device *dev,
grant_ref_t gref[XENBUS_MAX_RING_GRANTS];
for (i = 0; i < info->nr_ring_pages; i++)
- rinfo->ring_ref[i] = GRANT_INVALID_REF;
+ rinfo->ring_ref[i] = INVALID_GRANT_REF;
sring = alloc_pages_exact(ring_size, GFP_NOIO);
if (!sring) {
@@ -2544,13 +2542,13 @@ static void purge_persistent_grants(struct blkfront_info *info)
list_for_each_entry_safe(gnt_list_entry, tmp, &rinfo->grants,
node) {
- if (gnt_list_entry->gref == GRANT_INVALID_REF ||
+ if (gnt_list_entry->gref == INVALID_GRANT_REF ||
!gnttab_try_end_foreign_access(gnt_list_entry->gref))
continue;
list_del(&gnt_list_entry->node);
rinfo->persistent_gnts_c--;
- gnt_list_entry->gref = GRANT_INVALID_REF;
+ gnt_list_entry->gref = INVALID_GRANT_REF;
list_add_tail(&gnt_list_entry->node, &grants);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 02/19] xen/netfront: switch netfront to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
2022-04-28 8:27 ` [PATCH v2 01/19] xen/blkfront: switch blkfront to use INVALID_GRANT_REF Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 03/19] xen/scsifront: remove unused GRANT_INVALID_REF definition Juergen Gross
` (16 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, netdev, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
David S. Miller, Jakub Kicinski, Paolo Abeni
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/net/xen-netfront.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e2b4a1893a13..af3d3de7d9fa 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -78,8 +78,6 @@ struct netfront_cb {
#define RX_COPY_THRESHOLD 256
-#define GRANT_INVALID_REF 0
-
#define NET_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, XEN_PAGE_SIZE)
#define NET_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, XEN_PAGE_SIZE)
@@ -224,7 +222,7 @@ static grant_ref_t xennet_get_rx_ref(struct netfront_queue *queue,
{
int i = xennet_rxidx(ri);
grant_ref_t ref = queue->grant_rx_ref[i];
- queue->grant_rx_ref[i] = GRANT_INVALID_REF;
+ queue->grant_rx_ref[i] = INVALID_GRANT_REF;
return ref;
}
@@ -432,7 +430,7 @@ static bool xennet_tx_buf_gc(struct netfront_queue *queue)
}
gnttab_release_grant_reference(
&queue->gref_tx_head, queue->grant_tx_ref[id]);
- queue->grant_tx_ref[id] = GRANT_INVALID_REF;
+ queue->grant_tx_ref[id] = INVALID_GRANT_REF;
queue->grant_tx_page[id] = NULL;
add_id_to_list(&queue->tx_skb_freelist, queue->tx_link, id);
dev_kfree_skb_irq(skb);
@@ -1021,7 +1019,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
* the backend driver. In future this should flag the bad
* situation to the system controller to reboot the backend.
*/
- if (ref == GRANT_INVALID_REF) {
+ if (ref == INVALID_GRANT_REF) {
if (net_ratelimit())
dev_warn(dev, "Bad rx response id %d.\n",
rx->id);
@@ -1390,7 +1388,7 @@ static void xennet_release_tx_bufs(struct netfront_queue *queue)
gnttab_end_foreign_access(queue->grant_tx_ref[i],
(unsigned long)page_address(queue->grant_tx_page[i]));
queue->grant_tx_page[i] = NULL;
- queue->grant_tx_ref[i] = GRANT_INVALID_REF;
+ queue->grant_tx_ref[i] = INVALID_GRANT_REF;
add_id_to_list(&queue->tx_skb_freelist, queue->tx_link, i);
dev_kfree_skb_irq(skb);
}
@@ -1411,7 +1409,7 @@ static void xennet_release_rx_bufs(struct netfront_queue *queue)
continue;
ref = queue->grant_rx_ref[id];
- if (ref == GRANT_INVALID_REF)
+ if (ref == INVALID_GRANT_REF)
continue;
page = skb_frag_page(&skb_shinfo(skb)->frags[0]);
@@ -1422,7 +1420,7 @@ static void xennet_release_rx_bufs(struct netfront_queue *queue)
get_page(page);
gnttab_end_foreign_access(ref,
(unsigned long)page_address(page));
- queue->grant_rx_ref[id] = GRANT_INVALID_REF;
+ queue->grant_rx_ref[id] = INVALID_GRANT_REF;
kfree_skb(skb);
}
@@ -1761,7 +1759,7 @@ static int netfront_probe(struct xenbus_device *dev,
static void xennet_end_access(int ref, void *page)
{
/* This frees the page as a side-effect */
- if (ref != GRANT_INVALID_REF)
+ if (ref != INVALID_GRANT_REF)
gnttab_end_foreign_access(ref, (unsigned long)page);
}
@@ -1798,8 +1796,8 @@ static void xennet_disconnect_backend(struct netfront_info *info)
xennet_end_access(queue->tx_ring_ref, queue->tx.sring);
xennet_end_access(queue->rx_ring_ref, queue->rx.sring);
- queue->tx_ring_ref = GRANT_INVALID_REF;
- queue->rx_ring_ref = GRANT_INVALID_REF;
+ queue->tx_ring_ref = INVALID_GRANT_REF;
+ queue->rx_ring_ref = INVALID_GRANT_REF;
queue->tx.sring = NULL;
queue->rx.sring = NULL;
@@ -1927,8 +1925,8 @@ static int setup_netfront(struct xenbus_device *dev,
grant_ref_t gref;
int err;
- queue->tx_ring_ref = GRANT_INVALID_REF;
- queue->rx_ring_ref = GRANT_INVALID_REF;
+ queue->tx_ring_ref = INVALID_GRANT_REF;
+ queue->rx_ring_ref = INVALID_GRANT_REF;
queue->rx.sring = NULL;
queue->tx.sring = NULL;
@@ -1978,17 +1976,17 @@ static int setup_netfront(struct xenbus_device *dev,
* granted pages because backend is not accessing it at this point.
*/
fail:
- if (queue->rx_ring_ref != GRANT_INVALID_REF) {
+ if (queue->rx_ring_ref != INVALID_GRANT_REF) {
gnttab_end_foreign_access(queue->rx_ring_ref,
(unsigned long)rxs);
- queue->rx_ring_ref = GRANT_INVALID_REF;
+ queue->rx_ring_ref = INVALID_GRANT_REF;
} else {
free_page((unsigned long)rxs);
}
- if (queue->tx_ring_ref != GRANT_INVALID_REF) {
+ if (queue->tx_ring_ref != INVALID_GRANT_REF) {
gnttab_end_foreign_access(queue->tx_ring_ref,
(unsigned long)txs);
- queue->tx_ring_ref = GRANT_INVALID_REF;
+ queue->tx_ring_ref = INVALID_GRANT_REF;
} else {
free_page((unsigned long)txs);
}
@@ -2020,7 +2018,7 @@ static int xennet_init_queue(struct netfront_queue *queue)
queue->tx_pend_queue = TX_LINK_NONE;
for (i = 0; i < NET_TX_RING_SIZE; i++) {
queue->tx_link[i] = i + 1;
- queue->grant_tx_ref[i] = GRANT_INVALID_REF;
+ queue->grant_tx_ref[i] = INVALID_GRANT_REF;
queue->grant_tx_page[i] = NULL;
}
queue->tx_link[NET_TX_RING_SIZE - 1] = TX_LINK_NONE;
@@ -2028,7 +2026,7 @@ static int xennet_init_queue(struct netfront_queue *queue)
/* Clear out rx_skbs */
for (i = 0; i < NET_RX_RING_SIZE; i++) {
queue->rx_skbs[i] = NULL;
- queue->grant_rx_ref[i] = GRANT_INVALID_REF;
+ queue->grant_rx_ref[i] = INVALID_GRANT_REF;
}
/* A grant for every tx ring slot */
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 03/19] xen/scsifront: remove unused GRANT_INVALID_REF definition
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
2022-04-28 8:27 ` [PATCH v2 01/19] xen/blkfront: switch blkfront to use INVALID_GRANT_REF Juergen Gross
2022-04-28 8:27 ` [PATCH v2 02/19] xen/netfront: switch netfront " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 04/19] xen/usb: switch xen-hcd to use INVALID_GRANT_REF Juergen Gross
` (15 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-scsi, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
James E.J. Bottomley, Martin K. Petersen
GRANT_INVALID_REF isn't used in scsifront, so remove it.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/scsi/xen-scsifront.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 56173beecbc6..4c55e479fc36 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -58,9 +58,6 @@
#include <asm/xen/hypervisor.h>
-
-#define GRANT_INVALID_REF 0
-
#define VSCSIFRONT_OP_ADD_LUN 1
#define VSCSIFRONT_OP_DEL_LUN 2
#define VSCSIFRONT_OP_READD_LUN 3
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 04/19] xen/usb: switch xen-hcd to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (2 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 03/19] xen/scsifront: remove unused GRANT_INVALID_REF definition Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 05/19] xen/drm: switch xen_drm_front " Juergen Gross
` (14 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-usb, linux-kernel; +Cc: Juergen Gross, Greg Kroah-Hartman
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/host/xen-hcd.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xen-hcd.c b/drivers/usb/host/xen-hcd.c
index 3e487baf8422..9cbc7c2dab02 100644
--- a/drivers/usb/host/xen-hcd.c
+++ b/drivers/usb/host/xen-hcd.c
@@ -87,8 +87,6 @@ struct xenhcd_info {
bool error;
};
-#define GRANT_INVALID_REF 0
-
#define XENHCD_RING_JIFFIES (HZ/200)
#define XENHCD_SCAN_JIFFIES 1
@@ -1100,17 +1098,17 @@ static void xenhcd_destroy_rings(struct xenhcd_info *info)
unbind_from_irqhandler(info->irq, info);
info->irq = 0;
- if (info->urb_ring_ref != GRANT_INVALID_REF) {
+ if (info->urb_ring_ref != INVALID_GRANT_REF) {
gnttab_end_foreign_access(info->urb_ring_ref,
(unsigned long)info->urb_ring.sring);
- info->urb_ring_ref = GRANT_INVALID_REF;
+ info->urb_ring_ref = INVALID_GRANT_REF;
}
info->urb_ring.sring = NULL;
- if (info->conn_ring_ref != GRANT_INVALID_REF) {
+ if (info->conn_ring_ref != INVALID_GRANT_REF) {
gnttab_end_foreign_access(info->conn_ring_ref,
(unsigned long)info->conn_ring.sring);
- info->conn_ring_ref = GRANT_INVALID_REF;
+ info->conn_ring_ref = INVALID_GRANT_REF;
}
info->conn_ring.sring = NULL;
}
@@ -1123,8 +1121,8 @@ static int xenhcd_setup_rings(struct xenbus_device *dev,
grant_ref_t gref;
int err;
- info->urb_ring_ref = GRANT_INVALID_REF;
- info->conn_ring_ref = GRANT_INVALID_REF;
+ info->urb_ring_ref = INVALID_GRANT_REF;
+ info->conn_ring_ref = INVALID_GRANT_REF;
urb_sring = (struct xenusb_urb_sring *)get_zeroed_page(
GFP_NOIO | __GFP_HIGH);
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 05/19] xen/drm: switch xen_drm_front to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (3 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 04/19] xen/usb: switch xen-hcd to use INVALID_GRANT_REF Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 06/19] xen/sound: switch xen_snd_front " Juergen Gross
` (13 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, dri-devel, linux-kernel
Cc: Juergen Gross, Oleksandr Andrushchenko, David Airlie, Daniel Vetter
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) {
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 06/19] xen/sound: switch xen_snd_front to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (4 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 05/19] xen/drm: switch xen_drm_front " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 07/19] xen/dmabuf: switch gntdev-dmabuf " Juergen Gross
` (12 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Oleksandr Andrushchenko, Jaroslav Kysela,
Takashi Iwai, alsa-devel
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
sound/xen/xen_snd_front_evtchnl.c | 4 ++--
sound/xen/xen_snd_front_evtchnl.h | 9 ---------
2 files changed, 2 insertions(+), 11 deletions(-)
diff --git a/sound/xen/xen_snd_front_evtchnl.c b/sound/xen/xen_snd_front_evtchnl.c
index ecbc294fc59a..3e21369c8216 100644
--- a/sound/xen/xen_snd_front_evtchnl.c
+++ b/sound/xen/xen_snd_front_evtchnl.c
@@ -167,7 +167,7 @@ static void evtchnl_free(struct xen_snd_front_info *front_info,
xenbus_free_evtchn(front_info->xb_dev, channel->port);
/* End access and free the page. */
- if (channel->gref != GRANT_INVALID_REF)
+ if (channel->gref != INVALID_GRANT_REF)
gnttab_end_foreign_access(channel->gref, page);
else
free_page(page);
@@ -207,7 +207,7 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
channel->index = index;
channel->front_info = front_info;
channel->state = EVTCHNL_STATE_DISCONNECTED;
- channel->gref = GRANT_INVALID_REF;
+ channel->gref = INVALID_GRANT_REF;
page = get_zeroed_page(GFP_KERNEL);
if (!page) {
ret = -ENOMEM;
diff --git a/sound/xen/xen_snd_front_evtchnl.h b/sound/xen/xen_snd_front_evtchnl.h
index cbe51fd1ec15..3675fba70564 100644
--- a/sound/xen/xen_snd_front_evtchnl.h
+++ b/sound/xen/xen_snd_front_evtchnl.h
@@ -15,15 +15,6 @@
struct xen_snd_front_info;
-#ifndef GRANT_INVALID_REF
-/*
- * FIXME: 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
-
/* Timeout in ms to wait for backend to respond. */
#define VSND_WAIT_BACK_MS 3000
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 07/19] xen/dmabuf: switch gntdev-dmabuf to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (5 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 06/19] xen/sound: switch xen_snd_front " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 08/19] xen/shbuf: switch xen-front-pgdir-shbuf " Juergen Gross
` (11 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/gntdev-dmabuf.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/xen/gntdev-dmabuf.c b/drivers/xen/gntdev-dmabuf.c
index d5bfd7b867fc..91073b4e4a20 100644
--- a/drivers/xen/gntdev-dmabuf.c
+++ b/drivers/xen/gntdev-dmabuf.c
@@ -24,15 +24,6 @@
MODULE_IMPORT_NS(DMA_BUF);
-#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 driver,
- * because of the fact it is already in use/reserved by the PV console.
- */
-#define GRANT_INVALID_REF 0
-#endif
-
struct gntdev_dmabuf {
struct gntdev_dmabuf_priv *priv;
struct dma_buf *dmabuf;
@@ -532,7 +523,7 @@ static void dmabuf_imp_end_foreign_access(u32 *refs, int count)
int i;
for (i = 0; i < count; i++)
- if (refs[i] != GRANT_INVALID_REF)
+ if (refs[i] != INVALID_GRANT_REF)
gnttab_end_foreign_access(refs[i], 0UL);
}
@@ -567,7 +558,7 @@ static struct gntdev_dmabuf *dmabuf_imp_alloc_storage(int count)
gntdev_dmabuf->nr_pages = count;
for (i = 0; i < count; i++)
- gntdev_dmabuf->u.imp.refs[i] = GRANT_INVALID_REF;
+ gntdev_dmabuf->u.imp.refs[i] = INVALID_GRANT_REF;
return gntdev_dmabuf;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 08/19] xen/shbuf: switch xen-front-pgdir-shbuf to use INVALID_GRANT_REF
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (6 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 07/19] xen/dmabuf: switch gntdev-dmabuf " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
[not found] ` <CAPD2p-nisRgMOzy+w2jx5ULfZTyv4MqtG0wkV9jNn3wNg415sQ@mail.gmail.com>
2022-04-28 8:27 ` [PATCH v2 09/19] xen: update ring.h Juergen Gross
` (10 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini
Instead of using a private macro for an invalid grant reference use
the common one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/xen-front-pgdir-shbuf.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/drivers/xen/xen-front-pgdir-shbuf.c b/drivers/xen/xen-front-pgdir-shbuf.c
index a959dee21134..fa2921d4fbfc 100644
--- a/drivers/xen/xen-front-pgdir-shbuf.c
+++ b/drivers/xen/xen-front-pgdir-shbuf.c
@@ -21,15 +21,6 @@
#include <xen/xen-front-pgdir-shbuf.h>
-#ifndef GRANT_INVALID_REF
-/*
- * FIXME: 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
-
/**
* This structure represents the structure of a shared page
* that contains grant references to the pages of the shared
@@ -83,7 +74,7 @@ grant_ref_t
xen_front_pgdir_shbuf_get_dir_start(struct xen_front_pgdir_shbuf *buf)
{
if (!buf->grefs)
- return GRANT_INVALID_REF;
+ return INVALID_GRANT_REF;
return buf->grefs[0];
}
@@ -142,7 +133,7 @@ void xen_front_pgdir_shbuf_free(struct xen_front_pgdir_shbuf *buf)
int i;
for (i = 0; i < buf->num_grefs; i++)
- if (buf->grefs[i] != GRANT_INVALID_REF)
+ if (buf->grefs[i] != INVALID_GRANT_REF)
gnttab_end_foreign_access(buf->grefs[i], 0UL);
}
kfree(buf->grefs);
@@ -355,7 +346,7 @@ static void backend_fill_page_dir(struct xen_front_pgdir_shbuf *buf)
}
/* Last page must say there is no more pages. */
page_dir = (struct xen_page_directory *)ptr;
- page_dir->gref_dir_next_page = GRANT_INVALID_REF;
+ page_dir->gref_dir_next_page = INVALID_GRANT_REF;
}
/**
@@ -384,7 +375,7 @@ static void guest_fill_page_dir(struct xen_front_pgdir_shbuf *buf)
if (grefs_left <= XEN_NUM_GREFS_PER_PAGE) {
to_copy = grefs_left;
- page_dir->gref_dir_next_page = GRANT_INVALID_REF;
+ page_dir->gref_dir_next_page = INVALID_GRANT_REF;
} else {
to_copy = XEN_NUM_GREFS_PER_PAGE;
page_dir->gref_dir_next_page = buf->grefs[i + 1];
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 09/19] xen: update ring.h
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (7 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 08/19] xen/shbuf: switch xen-front-pgdir-shbuf " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 10/19] xen/xenbus: add xenbus_setup_ring() service function Juergen Gross
` (9 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, netdev, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
David S. Miller, Jakub Kicinski, Paolo Abeni
Update include/xen/interface/io/ring.h to its newest version.
Switch the two improper use cases of RING_HAS_UNCONSUMED_RESPONSES() to
XEN_RING_NR_UNCONSUMED_RESPONSES() in order to avoid the nasty
XEN_RING_HAS_UNCONSUMED_IS_BOOL #define.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
drivers/net/xen-netfront.c | 4 ++--
include/xen/interface/io/ring.h | 19 ++++++++++++++-----
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index af3d3de7d9fa..966bee2a6902 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -866,7 +866,7 @@ static void xennet_set_rx_rsp_cons(struct netfront_queue *queue, RING_IDX val)
spin_lock_irqsave(&queue->rx_cons_lock, flags);
queue->rx.rsp_cons = val;
- queue->rx_rsp_unconsumed = RING_HAS_UNCONSUMED_RESPONSES(&queue->rx);
+ queue->rx_rsp_unconsumed = XEN_RING_NR_UNCONSUMED_RESPONSES(&queue->rx);
spin_unlock_irqrestore(&queue->rx_cons_lock, flags);
}
@@ -1498,7 +1498,7 @@ static bool xennet_handle_rx(struct netfront_queue *queue, unsigned int *eoi)
return false;
spin_lock_irqsave(&queue->rx_cons_lock, flags);
- work_queued = RING_HAS_UNCONSUMED_RESPONSES(&queue->rx);
+ work_queued = XEN_RING_NR_UNCONSUMED_RESPONSES(&queue->rx);
if (work_queued > queue->rx_rsp_unconsumed) {
queue->rx_rsp_unconsumed = work_queued;
*eoi = 0;
diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 2470ec45ebb2..ba4c4274b714 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -72,9 +72,8 @@ typedef unsigned int RING_IDX;
* of the shared memory area (PAGE_SIZE, for instance). To initialise
* the front half:
*
- * mytag_front_ring_t front_ring;
- * SHARED_RING_INIT((mytag_sring_t *)shared_page);
- * FRONT_RING_INIT(&front_ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
+ * mytag_front_ring_t ring;
+ * XEN_FRONT_RING_INIT(&ring, (mytag_sring_t *)shared_page, PAGE_SIZE);
*
* Initializing the back follows similarly (note that only the front
* initializes the shared ring):
@@ -146,6 +145,11 @@ struct __name##_back_ring { \
#define FRONT_RING_INIT(_r, _s, __size) FRONT_RING_ATTACH(_r, _s, 0, __size)
+#define XEN_FRONT_RING_INIT(r, s, size) do { \
+ SHARED_RING_INIT(s); \
+ FRONT_RING_INIT(r, s, size); \
+} while (0)
+
#define BACK_RING_ATTACH(_r, _s, _i, __size) do { \
(_r)->rsp_prod_pvt = (_i); \
(_r)->req_cons = (_i); \
@@ -170,16 +174,21 @@ struct __name##_back_ring { \
(RING_FREE_REQUESTS(_r) == 0)
/* Test if there are outstanding messages to be processed on a ring. */
-#define RING_HAS_UNCONSUMED_RESPONSES(_r) \
+#define XEN_RING_NR_UNCONSUMED_RESPONSES(_r) \
((_r)->sring->rsp_prod - (_r)->rsp_cons)
-#define RING_HAS_UNCONSUMED_REQUESTS(_r) ({ \
+#define XEN_RING_NR_UNCONSUMED_REQUESTS(_r) ({ \
unsigned int req = (_r)->sring->req_prod - (_r)->req_cons; \
unsigned int rsp = RING_SIZE(_r) - \
((_r)->req_cons - (_r)->rsp_prod_pvt); \
req < rsp ? req : rsp; \
})
+#define RING_HAS_UNCONSUMED_RESPONSES(_r) \
+ (!!XEN_RING_NR_UNCONSUMED_RESPONSES(_r))
+#define RING_HAS_UNCONSUMED_REQUESTS(_r) \
+ (!!XEN_RING_NR_UNCONSUMED_REQUESTS(_r))
+
/* Direct access to individual ring elements, by index. */
#define RING_GET_REQUEST(_r, _idx) \
(&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 10/19] xen/xenbus: add xenbus_setup_ring() service function
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (8 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 09/19] xen: update ring.h Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 11/19] xen/blkfront: use xenbus_setup_ring() and xenbus_teardown_ring() Juergen Gross
` (8 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini
Most PV device frontends share very similar code for setting up shared
ring buffers:
- allocate page(s)
- init the ring admin data
- give the backend access to the ring via grants
Tearing down the ring requires similar actions in all frontends again:
- remove grants
- free the page(s)
Provide service functions xenbus_setup_ring() and xenbus_teardown_ring()
for that purpose.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/xen/xenbus/xenbus_client.c | 69 ++++++++++++++++++++++++++++++
include/xen/xenbus.h | 4 ++
2 files changed, 73 insertions(+)
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index df6890681231..1a2e0d94ccd1 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -407,6 +407,75 @@ int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
}
EXPORT_SYMBOL_GPL(xenbus_grant_ring);
+/*
+ * xenbus_setup_ring
+ * @dev: xenbus device
+ * @vaddr: pointer to starting virtual address of the ring
+ * @nr_pages: number of pages to be granted
+ * @grefs: grant reference array to be filled in
+ *
+ * Allocate physically contiguous pages for a shared ring buffer and grant it
+ * to the peer of the given device. The ring buffer is initially filled with
+ * zeroes. The virtual address of the ring is stored at @vaddr and the
+ * grant references are stored in the @grefs array. In case of error @vaddr
+ * will be set to NULL and @grefs will be filled with INVALID_GRANT_REF.
+ */
+int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
+ unsigned int nr_pages, grant_ref_t *grefs)
+{
+ unsigned long ring_size = nr_pages * XEN_PAGE_SIZE;
+ unsigned int i;
+ int ret;
+
+ *vaddr = alloc_pages_exact(ring_size, gfp | __GFP_ZERO);
+ if (!*vaddr) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ ret = xenbus_grant_ring(dev, *vaddr, nr_pages, grefs);
+ if (ret)
+ goto err;
+
+ return 0;
+
+ err:
+ if (*vaddr)
+ free_pages_exact(*vaddr, ring_size);
+ for (i = 0; i < nr_pages; i++)
+ grefs[i] = INVALID_GRANT_REF;
+ *vaddr = NULL;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(xenbus_setup_ring);
+
+/*
+ * xenbus_teardown_ring
+ * @vaddr: starting virtual address of the ring
+ * @nr_pages: number of pages
+ * @grefs: grant reference array
+ *
+ * Remove grants for the shared ring buffer and free the associated memory.
+ * On return the grant reference array is filled with INVALID_GRANT_REF.
+ */
+void xenbus_teardown_ring(void **vaddr, unsigned int nr_pages,
+ grant_ref_t *grefs)
+{
+ unsigned int i;
+
+ for (i = 0; i < nr_pages; i++) {
+ if (grefs[i] != INVALID_GRANT_REF) {
+ gnttab_end_foreign_access(grefs[i], 0);
+ grefs[i] = INVALID_GRANT_REF;
+ }
+ }
+
+ if (*vaddr)
+ free_pages_exact(*vaddr, nr_pages * XEN_PAGE_SIZE);
+ *vaddr = NULL;
+}
+EXPORT_SYMBOL_GPL(xenbus_teardown_ring);
/**
* Allocate an event channel for the given xenbus_device, assigning the newly
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index b13eb86395e0..b533b4adc835 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -226,6 +226,10 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
unsigned int nr_pages, grant_ref_t *grefs);
+int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
+ unsigned int nr_pages, grant_ref_t *grefs);
+void xenbus_teardown_ring(void **vaddr, unsigned int nr_pages,
+ grant_ref_t *grefs);
int xenbus_map_ring_valloc(struct xenbus_device *dev, grant_ref_t *gnt_refs,
unsigned int nr_grefs, void **vaddr);
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 11/19] xen/blkfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (9 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 10/19] xen/xenbus: add xenbus_setup_ring() service function Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 12/19] xen/netfront: " Juergen Gross
` (7 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-block, linux-kernel
Cc: Juergen Gross, Roger Pau Monné,
Boris Ostrovsky, Stefano Stabellini, Jens Axboe
Simplify blkfront's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/block/xen-blkfront.c | 37 ++++++++----------------------------
1 file changed, 8 insertions(+), 29 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 7f35e30e626a..bd7b34f29193 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1280,15 +1280,8 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
flush_work(&rinfo->work);
/* Free resources associated with old device channel. */
- for (i = 0; i < info->nr_ring_pages; i++) {
- if (rinfo->ring_ref[i] != INVALID_GRANT_REF) {
- gnttab_end_foreign_access(rinfo->ring_ref[i], 0);
- rinfo->ring_ref[i] = INVALID_GRANT_REF;
- }
- }
- free_pages_exact(rinfo->ring.sring,
- info->nr_ring_pages * XEN_PAGE_SIZE);
- rinfo->ring.sring = NULL;
+ xenbus_teardown_ring((void **)&rinfo->ring.sring, info->nr_ring_pages,
+ rinfo->ring_ref);
if (rinfo->irq)
unbind_from_irqhandler(rinfo->irq, rinfo);
@@ -1679,30 +1672,16 @@ static int setup_blkring(struct xenbus_device *dev,
struct blkfront_ring_info *rinfo)
{
struct blkif_sring *sring;
- int err, i;
+ int err;
struct blkfront_info *info = rinfo->dev_info;
unsigned long ring_size = info->nr_ring_pages * XEN_PAGE_SIZE;
- grant_ref_t gref[XENBUS_MAX_RING_GRANTS];
-
- for (i = 0; i < info->nr_ring_pages; i++)
- rinfo->ring_ref[i] = INVALID_GRANT_REF;
- sring = alloc_pages_exact(ring_size, GFP_NOIO);
- if (!sring) {
- xenbus_dev_fatal(dev, -ENOMEM, "allocating shared ring");
- return -ENOMEM;
- }
- SHARED_RING_INIT(sring);
- FRONT_RING_INIT(&rinfo->ring, sring, ring_size);
-
- err = xenbus_grant_ring(dev, rinfo->ring.sring, info->nr_ring_pages, gref);
- if (err < 0) {
- free_pages_exact(sring, ring_size);
- rinfo->ring.sring = NULL;
+ err = xenbus_setup_ring(dev, GFP_NOIO, (void **)&sring,
+ info->nr_ring_pages, rinfo->ring_ref);
+ if (err)
goto fail;
- }
- for (i = 0; i < info->nr_ring_pages; i++)
- rinfo->ring_ref[i] = gref[i];
+
+ XEN_FRONT_RING_INIT(&rinfo->ring, sring, ring_size);
err = xenbus_alloc_evtchn(dev, &rinfo->evtchn);
if (err)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 12/19] xen/netfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (10 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 11/19] xen/blkfront: use xenbus_setup_ring() and xenbus_teardown_ring() Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 13/19] xen/tpmfront: " Juergen Gross
` (6 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, netdev, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
David S. Miller, Jakub Kicinski, Paolo Abeni
Simplify netfront's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/net/xen-netfront.c | 53 +++++++++-----------------------------
1 file changed, 12 insertions(+), 41 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 966bee2a6902..65ab907aca5a 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1921,8 +1921,7 @@ static int setup_netfront(struct xenbus_device *dev,
struct netfront_queue *queue, unsigned int feature_split_evtchn)
{
struct xen_netif_tx_sring *txs;
- struct xen_netif_rx_sring *rxs = NULL;
- grant_ref_t gref;
+ struct xen_netif_rx_sring *rxs;
int err;
queue->tx_ring_ref = INVALID_GRANT_REF;
@@ -1930,33 +1929,19 @@ static int setup_netfront(struct xenbus_device *dev,
queue->rx.sring = NULL;
queue->tx.sring = NULL;
- txs = (struct xen_netif_tx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH);
- if (!txs) {
- err = -ENOMEM;
- xenbus_dev_fatal(dev, err, "allocating tx ring page");
+ err = xenbus_setup_ring(dev, GFP_NOIO | __GFP_HIGH, (void **)&txs,
+ 1, &queue->tx_ring_ref);
+ if (err)
goto fail;
- }
- SHARED_RING_INIT(txs);
- FRONT_RING_INIT(&queue->tx, txs, XEN_PAGE_SIZE);
- err = xenbus_grant_ring(dev, txs, 1, &gref);
- if (err < 0)
- goto fail;
- queue->tx_ring_ref = gref;
+ XEN_FRONT_RING_INIT(&queue->tx, txs, XEN_PAGE_SIZE);
- rxs = (struct xen_netif_rx_sring *)get_zeroed_page(GFP_NOIO | __GFP_HIGH);
- if (!rxs) {
- err = -ENOMEM;
- xenbus_dev_fatal(dev, err, "allocating rx ring page");
+ err = xenbus_setup_ring(dev, GFP_NOIO | __GFP_HIGH, (void **)&rxs,
+ 1, &queue->rx_ring_ref);
+ if (err)
goto fail;
- }
- SHARED_RING_INIT(rxs);
- FRONT_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);
- err = xenbus_grant_ring(dev, rxs, 1, &gref);
- if (err < 0)
- goto fail;
- queue->rx_ring_ref = gref;
+ XEN_FRONT_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);
if (feature_split_evtchn)
err = setup_netfront_split(queue);
@@ -1972,24 +1957,10 @@ static int setup_netfront(struct xenbus_device *dev,
return 0;
- /* If we fail to setup netfront, it is safe to just revoke access to
- * granted pages because backend is not accessing it at this point.
- */
fail:
- if (queue->rx_ring_ref != INVALID_GRANT_REF) {
- gnttab_end_foreign_access(queue->rx_ring_ref,
- (unsigned long)rxs);
- queue->rx_ring_ref = INVALID_GRANT_REF;
- } else {
- free_page((unsigned long)rxs);
- }
- if (queue->tx_ring_ref != INVALID_GRANT_REF) {
- gnttab_end_foreign_access(queue->tx_ring_ref,
- (unsigned long)txs);
- queue->tx_ring_ref = INVALID_GRANT_REF;
- } else {
- free_page((unsigned long)txs);
- }
+ xenbus_teardown_ring((void **)&queue->rx.sring, 1, &queue->rx_ring_ref);
+ xenbus_teardown_ring((void **)&queue->tx.sring, 1, &queue->tx_ring_ref);
+
return err;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 13/19] xen/tpmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (11 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 12/19] xen/netfront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 14/19] xen/drmfront: " Juergen Gross
` (5 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-integrity, linux-kernel
Cc: Juergen Gross, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe
Simplify tpmfront's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/char/tpm/xen-tpmfront.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 69df04ae2401..379291826261 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -253,20 +253,12 @@ static int setup_ring(struct xenbus_device *dev, struct tpm_private *priv)
struct xenbus_transaction xbt;
const char *message = NULL;
int rv;
- grant_ref_t gref;
- priv->shr = (void *)__get_free_page(GFP_KERNEL|__GFP_ZERO);
- if (!priv->shr) {
- xenbus_dev_fatal(dev, -ENOMEM, "allocating shared ring");
- return -ENOMEM;
- }
-
- rv = xenbus_grant_ring(dev, priv->shr, 1, &gref);
+ rv = xenbus_setup_ring(dev, GFP_KERNEL, (void **)&priv->shr, 1,
+ &priv->ring_ref);
if (rv < 0)
return rv;
- priv->ring_ref = gref;
-
rv = xenbus_alloc_evtchn(dev, &priv->evtchn);
if (rv)
return rv;
@@ -331,11 +323,7 @@ static void ring_free(struct tpm_private *priv)
if (!priv)
return;
- if (priv->ring_ref)
- gnttab_end_foreign_access(priv->ring_ref,
- (unsigned long)priv->shr);
- else
- free_page((unsigned long)priv->shr);
+ xenbus_teardown_ring((void **)&priv->shr, 1, &priv->ring_ref);
if (priv->irq)
unbind_from_irqhandler(priv->irq, priv);
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 14/19] xen/drmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (12 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 13/19] xen/tpmfront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-29 16:10 ` Oleksandr
2022-04-28 8:27 ` [PATCH v2 15/19] xen/pcifront: " Juergen Gross
` (4 subsequent siblings)
18 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, dri-devel, linux-kernel
Cc: Juergen Gross, Oleksandr Andrushchenko, David Airlie, Daniel Vetter
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)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 14/19] xen/drmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 ` [PATCH v2 14/19] xen/drmfront: " Juergen Gross
@ 2022-04-29 16:10 ` Oleksandr
0 siblings, 0 replies; 29+ messages in thread
From: Oleksandr @ 2022-04-29 16:10 UTC (permalink / raw)
To: Juergen Gross, xen-devel, dri-devel, linux-kernel
Cc: Oleksandr Andrushchenko, David Airlie, Daniel Vetter
On 28.04.22 11:27, Juergen Gross wrote:
Hello Juergen, all
> Simplify drmfront's ring creation and removal via xenbus_setup_ring()
> and xenbus_teardown_ring().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
I am not familiar with DRM bits of this driver, but a little bit
familiar with Xen bits this patch only touches and I have environment to
test.
Xen specific changes looks good to me. Also I didn't see any specific to
this series issues when testing virtulized display driver except one I
have already pointed out in PATCH v2 08/19.
root@salvator-x-h3-4x2g-xt-domu:~# dmesg | grep drm
[ 0.158190] [drm] Registering XEN PV vdispl
[ 0.159620] [drm] Connector device/vdispl/0/0: resolution 1920x1080
[ 0.159888] [drm] Have 1 connector(s)
[ 0.289069] [drm] Creating Xen PV DRM Display Unit
[ 0.289873] [drm] Initialized xendrm-du 1.0.0 20180221 for vdispl-0
on minor 0
[ 0.289918] [drm] Initialized xendrm-du 1.0.0 20180221 on minor 0
root@generic-armv8-xt-dom0:~# xenstore-ls -f | grep vdispl
/local/domain/1/backend/vdispl = ""
/local/domain/1/backend/vdispl/2 = ""
/local/domain/1/backend/vdispl/2/0 = ""
/local/domain/1/backend/vdispl/2/0/frontend =
"/local/domain/2/device/vdispl/0"
/local/domain/1/backend/vdispl/2/0/frontend-id = "2"
/local/domain/1/backend/vdispl/2/0/online = "1"
/local/domain/1/backend/vdispl/2/0/state = "4"
/local/domain/2/device/vdispl = ""
/local/domain/2/device/vdispl/0 = ""
/local/domain/2/device/vdispl/0/backend =
"/local/domain/1/backend/vdispl/2/0"
/local/domain/2/device/vdispl/0/backend-id = "1"
/local/domain/2/device/vdispl/0/state = "4"
/local/domain/2/device/vdispl/0/be-alloc = "0"
/local/domain/2/device/vdispl/0/0 = ""
/local/domain/2/device/vdispl/0/0/resolution = "1920x1080"
/local/domain/2/device/vdispl/0/0/unique-id = "HDMI-A-1"
/local/domain/2/device/vdispl/0/0/req-ring-ref = "8"
/local/domain/2/device/vdispl/0/0/req-event-channel = "7"
/local/domain/2/device/vdispl/0/0/evt-ring-ref = "9"
/local/domain/2/device/vdispl/0/0/evt-event-channel = "8"
/libxl/2/device/vdispl = ""
/libxl/2/device/vdispl/0 = ""
/libxl/2/device/vdispl/0/frontend = "/local/domain/2/device/vdispl/0"
/libxl/2/device/vdispl/0/backend = "/local/domain/1/backend/vdispl/2/0"
/libxl/2/device/vdispl/0/frontend-id = "2"
/libxl/2/device/vdispl/0/online = "1"
/libxl/2/device/vdispl/0/state = "1"
It worth mentioning I noticed one issue, but I believe it is not related
to your series.
root@salvator-x-h3-4x2g-xt-domu:~# modetest -M xendrm-du -s 31:1920x1080
[ 62.431887] ------------[ cut here ]------------
[ 62.431940] WARNING: CPU: 0 PID: 244 at
drivers/gpu/drm/drm_gem.c:1055 drm_gem_mmap_obj+0x16c/0x180
[ 62.432000] Modules linked in:
[ 62.432025] CPU: 0 PID: 244 Comm: modetest Tainted: G W
5.18.0-rc4-yocto-standard-00096-g936342d8fae2 #1
[ 62.432067] Hardware name: XENVM-4.17 (DT)
[ 62.432089] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 62.432126] pc : drm_gem_mmap_obj+0x16c/0x180
[ 62.432153] lr : drm_gem_mmap_obj+0x78/0x180
[ 62.432178] sp : ffff800009da3bb0
[ 62.432196] x29: ffff800009da3bb0 x28: 0000000000000008 x27:
ffff0001c3a56780
[ 62.432237] x26: ffff0001c3a56f00 x25: 00000000000007e9 x24:
ffff0001c23dec00
[ 62.432279] x23: ffff0001c0c98000 x22: ffff0001c2162b80 x21:
0000000000000000
[ 62.432320] x20: ffff0001c3a56780 x19: ffff0001c23dea00 x18:
0000000000000001
[ 62.432355] x17: 0000000000000000 x16: 0000000000000000 x15:
000000000003603c
[ 62.432394] x14: 0000000000000000 x13: 0000000000000000 x12:
0000000000000000
[ 62.432430] x11: 0000000000100000 x10: 0000ffff88071000 x9 :
ffff0001c0f17e70
[ 62.432470] x8 : ffff8001f65ce000 x7 : 0000000000000001 x6 :
ffff0001c388a000
[ 62.432505] x5 : ffff800009da3a10 x4 : 0000000000000090 x3 :
0000000010046400
[ 62.432539] x2 : 00000000000007e9 x1 : 9b0023a536f4f400 x0 :
00000000100000fb
[ 62.432579] Call trace:
[ 62.432593] drm_gem_mmap_obj+0x16c/0x180
[ 62.432617] drm_gem_mmap+0x128/0x228
[ 62.432641] mmap_region+0x384/0x5a0
[ 62.432667] do_mmap+0x354/0x4f0
[ 62.432687] vm_mmap_pgoff+0xdc/0x108
[ 62.432710] ksys_mmap_pgoff+0x1b8/0x208
[ 62.432734] __arm64_sys_mmap+0x30/0x48
[ 62.432760] invoke_syscall+0x44/0x108
[ 62.432783] el0_svc_common.constprop.0+0xcc/0xf0
[ 62.432811] do_el0_svc+0x24/0x88
[ 62.432831] el0_svc+0x2c/0x88
[ 62.432855] el0t_64_sync_handler+0xb0/0xb8
[ 62.432875] el0t_64_sync+0x18c/0x190
[ 62.432898] ---[ end trace 0000000000000000 ]---
setting mode 1920x1080-60.00Hz@XR24 on connectors 31, crtc 34
Although we see that WARNING, the application still works. Looking into
the code, I assume that problem is that frontend doesn't set the
VM_DONTEXPAND flag in mmap callback.
This diff fixes an issue in my environment:
index 5a5bf4e..e31554d 100644
--- a/drivers/gpu/drm/xen/xen_drm_front_gem.c
+++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c
@@ -71,7 +71,7 @@ static int xen_drm_front_gem_object_mmap(struct
drm_gem_object *gem_obj,
* the whole buffer.
*/
vma->vm_flags &= ~VM_PFNMAP;
- vma->vm_flags |= VM_MIXEDMAP;
+ vma->vm_flags |= VM_MIXEDMAP | VM_DONTEXPAND;
vma->vm_pgoff = 0;
/*
I am not 100% sure whether it is a proper fix, so I would kindly ask DRM
folks to confirm. I will be able to send a formal patch then.
> ---
> 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)
--
Regards,
Oleksandr Tyshchenko
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 15/19] xen/pcifront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (13 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 14/19] xen/drmfront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 16/19] xen/scsifront: " Juergen Gross
` (3 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-pci, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini, Bjorn Helgaas
Simplify pcifront's shared page creation and removal via
xenbus_setup_ring() and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/pci/xen-pcifront.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 3edc1565a27c..689271c4245c 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -709,9 +709,8 @@ static struct pcifront_device *alloc_pdev(struct xenbus_device *xdev)
if (pdev == NULL)
goto out;
- pdev->sh_info =
- (struct xen_pci_sharedinfo *)__get_free_page(GFP_KERNEL);
- if (pdev->sh_info == NULL) {
+ if (xenbus_setup_ring(xdev, GFP_KERNEL, (void **)&pdev->sh_info, 1,
+ &pdev->gnt_ref)) {
kfree(pdev);
pdev = NULL;
goto out;
@@ -729,7 +728,6 @@ static struct pcifront_device *alloc_pdev(struct xenbus_device *xdev)
spin_lock_init(&pdev->sh_info_lock);
pdev->evtchn = INVALID_EVTCHN;
- pdev->gnt_ref = INVALID_GRANT_REF;
pdev->irq = -1;
INIT_WORK(&pdev->op_work, pcifront_do_aer);
@@ -754,11 +752,7 @@ static void free_pdev(struct pcifront_device *pdev)
if (pdev->evtchn != INVALID_EVTCHN)
xenbus_free_evtchn(pdev->xdev, pdev->evtchn);
- if (pdev->gnt_ref != INVALID_GRANT_REF)
- gnttab_end_foreign_access(pdev->gnt_ref,
- (unsigned long)pdev->sh_info);
- else
- free_page((unsigned long)pdev->sh_info);
+ xenbus_teardown_ring((void **)&pdev->sh_info, 1, &pdev->gnt_ref);
dev_set_drvdata(&pdev->xdev->dev, NULL);
@@ -769,13 +763,6 @@ static int pcifront_publish_info(struct pcifront_device *pdev)
{
int err = 0;
struct xenbus_transaction trans;
- grant_ref_t gref;
-
- err = xenbus_grant_ring(pdev->xdev, pdev->sh_info, 1, &gref);
- if (err < 0)
- goto out;
-
- pdev->gnt_ref = gref;
err = xenbus_alloc_evtchn(pdev->xdev, &pdev->evtchn);
if (err)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 16/19] xen/scsifront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (14 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 15/19] xen/pcifront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 17/19] xen/usbfront: " Juergen Gross
` (2 subsequent siblings)
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-scsi, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini,
James E.J. Bottomley, Martin K. Petersen
Simplify scsifront's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/scsi/xen-scsifront.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index 4c55e479fc36..51afc66e839d 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -798,27 +798,15 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info)
{
struct xenbus_device *dev = info->dev;
struct vscsiif_sring *sring;
- grant_ref_t gref;
- int err = -ENOMEM;
+ int err;
/***** Frontend to Backend ring start *****/
- sring = (struct vscsiif_sring *)__get_free_page(GFP_KERNEL);
- if (!sring) {
- xenbus_dev_fatal(dev, err,
- "fail to allocate shared ring (Front to Back)");
+ err = xenbus_setup_ring(dev, GFP_KERNEL, (void **)&sring, 1,
+ &info->ring_ref);
+ if (err)
return err;
- }
- SHARED_RING_INIT(sring);
- FRONT_RING_INIT(&info->ring, sring, PAGE_SIZE);
- err = xenbus_grant_ring(dev, sring, 1, &gref);
- if (err < 0) {
- free_page((unsigned long)sring);
- xenbus_dev_fatal(dev, err,
- "fail to grant shared ring (Front to Back)");
- return err;
- }
- info->ring_ref = gref;
+ XEN_FRONT_RING_INIT(&info->ring, sring, PAGE_SIZE);
err = xenbus_alloc_evtchn(dev, &info->evtchn);
if (err) {
@@ -847,8 +835,7 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info)
free_irq:
unbind_from_irqhandler(info->irq, info);
free_gnttab:
- gnttab_end_foreign_access(info->ring_ref,
- (unsigned long)info->ring.sring);
+ xenbus_teardown_ring((void **)&sring, 1, &info->ring_ref);
return err;
}
@@ -856,8 +843,7 @@ static int scsifront_alloc_ring(struct vscsifrnt_info *info)
static void scsifront_free_ring(struct vscsifrnt_info *info)
{
unbind_from_irqhandler(info->irq, info);
- gnttab_end_foreign_access(info->ring_ref,
- (unsigned long)info->ring.sring);
+ xenbus_teardown_ring((void **)&info->ring.sring, 1, &info->ring_ref);
}
static int scsifront_init_ring(struct vscsifrnt_info *info)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 17/19] xen/usbfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (15 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 16/19] xen/scsifront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-28 8:27 ` [PATCH v2 18/19] xen/sndfront: " Juergen Gross
2022-04-28 8:27 ` [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring() Juergen Gross
18 siblings, 0 replies; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-usb, linux-kernel; +Cc: Juergen Gross, Greg Kroah-Hartman
Simplify xen-hcd's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/usb/host/xen-hcd.c | 61 ++++++++++----------------------------
1 file changed, 15 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/host/xen-hcd.c b/drivers/usb/host/xen-hcd.c
index 9cbc7c2dab02..de1b09158318 100644
--- a/drivers/usb/host/xen-hcd.c
+++ b/drivers/usb/host/xen-hcd.c
@@ -1098,19 +1098,10 @@ static void xenhcd_destroy_rings(struct xenhcd_info *info)
unbind_from_irqhandler(info->irq, info);
info->irq = 0;
- if (info->urb_ring_ref != INVALID_GRANT_REF) {
- gnttab_end_foreign_access(info->urb_ring_ref,
- (unsigned long)info->urb_ring.sring);
- info->urb_ring_ref = INVALID_GRANT_REF;
- }
- info->urb_ring.sring = NULL;
-
- if (info->conn_ring_ref != INVALID_GRANT_REF) {
- gnttab_end_foreign_access(info->conn_ring_ref,
- (unsigned long)info->conn_ring.sring);
- info->conn_ring_ref = INVALID_GRANT_REF;
- }
- info->conn_ring.sring = NULL;
+ xenbus_teardown_ring((void **)&info->urb_ring.sring, 1,
+ &info->urb_ring_ref);
+ xenbus_teardown_ring((void **)&info->conn_ring.sring, 1,
+ &info->conn_ring_ref);
}
static int xenhcd_setup_rings(struct xenbus_device *dev,
@@ -1118,46 +1109,24 @@ static int xenhcd_setup_rings(struct xenbus_device *dev,
{
struct xenusb_urb_sring *urb_sring;
struct xenusb_conn_sring *conn_sring;
- grant_ref_t gref;
int err;
- info->urb_ring_ref = INVALID_GRANT_REF;
info->conn_ring_ref = INVALID_GRANT_REF;
-
- urb_sring = (struct xenusb_urb_sring *)get_zeroed_page(
- GFP_NOIO | __GFP_HIGH);
- if (!urb_sring) {
- xenbus_dev_fatal(dev, -ENOMEM, "allocating urb ring");
- return -ENOMEM;
- }
- SHARED_RING_INIT(urb_sring);
- FRONT_RING_INIT(&info->urb_ring, urb_sring, PAGE_SIZE);
-
- err = xenbus_grant_ring(dev, urb_sring, 1, &gref);
- if (err < 0) {
- free_page((unsigned long)urb_sring);
- info->urb_ring.sring = NULL;
- goto fail;
- }
- info->urb_ring_ref = gref;
-
- conn_sring = (struct xenusb_conn_sring *)get_zeroed_page(
- GFP_NOIO | __GFP_HIGH);
- if (!conn_sring) {
- xenbus_dev_fatal(dev, -ENOMEM, "allocating conn ring");
- err = -ENOMEM;
- goto fail;
+ err = xenbus_setup_ring(dev, GFP_NOIO | __GFP_HIGH,
+ (void **)&urb_sring, 1, &info->urb_ring_ref);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "allocating urb ring");
+ return err;
}
- SHARED_RING_INIT(conn_sring);
- FRONT_RING_INIT(&info->conn_ring, conn_sring, PAGE_SIZE);
+ XEN_FRONT_RING_INIT(&info->urb_ring, urb_sring, PAGE_SIZE);
- err = xenbus_grant_ring(dev, conn_sring, 1, &gref);
- if (err < 0) {
- free_page((unsigned long)conn_sring);
- info->conn_ring.sring = NULL;
+ err = xenbus_setup_ring(dev, GFP_NOIO | __GFP_HIGH,
+ (void **)&conn_sring, 1, &info->conn_ring_ref);
+ if (err) {
+ xenbus_dev_fatal(dev, err, "allocating conn ring");
goto fail;
}
- info->conn_ring_ref = gref;
+ XEN_FRONT_RING_INIT(&info->conn_ring, conn_sring, PAGE_SIZE);
err = xenbus_alloc_evtchn(dev, &info->evtchn);
if (err) {
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH v2 18/19] xen/sndfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (16 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 17/19] xen/usbfront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-29 18:07 ` Oleksandr
2022-04-28 8:27 ` [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring() Juergen Gross
18 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Oleksandr Andrushchenko, Jaroslav Kysela,
Takashi Iwai, alsa-devel
Simplify sndfront's ring creation and removal via xenbus_setup_ring()
and xenbus_teardown_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
sound/xen/xen_snd_front_evtchnl.c | 44 +++++++------------------------
1 file changed, 10 insertions(+), 34 deletions(-)
diff --git a/sound/xen/xen_snd_front_evtchnl.c b/sound/xen/xen_snd_front_evtchnl.c
index 3e21369c8216..26d1b3987887 100644
--- a/sound/xen/xen_snd_front_evtchnl.c
+++ b/sound/xen/xen_snd_front_evtchnl.c
@@ -143,12 +143,12 @@ void xen_snd_front_evtchnl_flush(struct xen_snd_front_evtchnl *channel)
static void evtchnl_free(struct xen_snd_front_info *front_info,
struct xen_snd_front_evtchnl *channel)
{
- unsigned long page = 0;
+ void *page = NULL;
if (channel->type == EVTCHNL_TYPE_REQ)
- page = (unsigned long)channel->u.req.ring.sring;
+ page = channel->u.req.ring.sring;
else if (channel->type == EVTCHNL_TYPE_EVT)
- page = (unsigned long)channel->u.evt.page;
+ page = channel->u.evt.page;
if (!page)
return;
@@ -167,10 +167,7 @@ static void evtchnl_free(struct xen_snd_front_info *front_info,
xenbus_free_evtchn(front_info->xb_dev, channel->port);
/* End access and free the page. */
- if (channel->gref != INVALID_GRANT_REF)
- gnttab_end_foreign_access(channel->gref, page);
- else
- free_page(page);
+ xenbus_teardown_ring(&page, 1, &channel->gref);
memset(channel, 0, sizeof(*channel));
}
@@ -196,8 +193,7 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
enum xen_snd_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;
char *handler_name = NULL;
int ret;
@@ -207,12 +203,9 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
channel->index = index;
channel->front_info = front_info;
channel->state = EVTCHNL_STATE_DISCONNECTED;
- channel->gref = INVALID_GRANT_REF;
- page = get_zeroed_page(GFP_KERNEL);
- if (!page) {
- ret = -ENOMEM;
+ ret = xenbus_setup_ring(xb_dev, GFP_KERNEL, &page, 1, &channel->gref);
+ if (ret)
goto fail;
- }
handler_name = kasprintf(GFP_KERNEL, "%s-%s", XENSND_DRIVER_NAME,
type == EVTCHNL_TYPE_REQ ?
@@ -226,33 +219,18 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
mutex_init(&channel->ring_io_lock);
if (type == EVTCHNL_TYPE_REQ) {
- struct xen_sndif_sring *sring = (struct xen_sndif_sring *)page;
+ struct xen_sndif_sring *sring = page;
init_completion(&channel->u.req.completion);
mutex_init(&channel->u.req.req_io_lock);
- SHARED_RING_INIT(sring);
- FRONT_RING_INIT(&channel->u.req.ring, sring, XEN_PAGE_SIZE);
-
- ret = xenbus_grant_ring(xb_dev, sring, 1, &gref);
- if (ret < 0) {
- channel->u.req.ring.sring = NULL;
- goto fail;
- }
+ XEN_FRONT_RING_INIT(&channel->u.req.ring, sring, XEN_PAGE_SIZE);
handler = evtchnl_interrupt_req;
} else {
- ret = gnttab_grant_foreign_access(xb_dev->otherend_id,
- virt_to_gfn((void *)page), 0);
- if (ret < 0)
- goto fail;
-
- channel->u.evt.page = (struct xensnd_event_page *)page;
- gref = ret;
+ channel->u.evt.page = page;
handler = evtchnl_interrupt_evt;
}
- channel->gref = gref;
-
ret = xenbus_alloc_evtchn(xb_dev, &channel->port);
if (ret < 0)
goto fail;
@@ -279,8 +257,6 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
return 0;
fail:
- if (page)
- free_page(page);
kfree(handler_name);
dev_err(&xb_dev->dev, "Failed to allocate ring: %d\n", ret);
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 18/19] xen/sndfront: use xenbus_setup_ring() and xenbus_teardown_ring()
2022-04-28 8:27 ` [PATCH v2 18/19] xen/sndfront: " Juergen Gross
@ 2022-04-29 18:07 ` Oleksandr
0 siblings, 0 replies; 29+ messages in thread
From: Oleksandr @ 2022-04-29 18:07 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel
Cc: Oleksandr Andrushchenko, Jaroslav Kysela, Takashi Iwai, alsa-devel
On 28.04.22 11:27, Juergen Gross wrote:
Hello Juergen, all
> Simplify sndfront's ring creation and removal via xenbus_setup_ring()
> and xenbus_teardown_ring().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
I am not familiar with SOUND bits of this driver, but a little bit
familiar with Xen bits this patch only touches and I have environment to
test.
Xen specific changes looks good to me. Also I didn't see any issues when
testing virtulized sound driver with current series except one I have
already pointed out in PATCH v2 08/19.
root@salvator-x-h3-4x2g-xt-domu:~# dmesg | grep vsnd
[ 0.432181] Initialising Xen vsnd frontend driver
root@salvator-x-h3-4x2g-xt-domu:~# aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: vsnd [], device 0: dev1 [Virtual card PCM]
Subdevices: 1/1
Subdevice #0: subdevice #0
root@generic-armv8-xt-dom0:~# xenstore-ls -f | grep vsnd
/local/domain/1/backend/vsnd = ""
/local/domain/1/backend/vsnd/6 = ""
/local/domain/1/backend/vsnd/6/0 = ""
/local/domain/1/backend/vsnd/6/0/frontend = "/local/domain/6/device/vsnd/0"
/local/domain/1/backend/vsnd/6/0/frontend-id = "6"
/local/domain/1/backend/vsnd/6/0/online = "1"
/local/domain/1/backend/vsnd/6/0/state = "4"
/local/domain/6/device/vsnd = ""
/local/domain/6/device/vsnd/0 = ""
/local/domain/6/device/vsnd/0/backend = "/local/domain/1/backend/vsnd/6/0"
/local/domain/6/device/vsnd/0/backend-id = "1"
/local/domain/6/device/vsnd/0/state = "4"
/local/domain/6/device/vsnd/0/long-name = "Virtual sound card"
/local/domain/6/device/vsnd/0/short-name = "VCard"
/local/domain/6/device/vsnd/0/sample-rates =
"8000,11025,16000,22050,32000,44100,48000"
/local/domain/6/device/vsnd/0/sample-formats = "s16_le"
/local/domain/6/device/vsnd/0/buffer-size = "65536"
/local/domain/6/device/vsnd/0/0 = ""
/local/domain/6/device/vsnd/0/0/name = "dev1"
/local/domain/6/device/vsnd/0/0/0 = ""
/local/domain/6/device/vsnd/0/0/0/unique-id = "pulse"
/local/domain/6/device/vsnd/0/0/0/type = "p"
/local/domain/6/device/vsnd/0/0/0/ring-ref = "2070"
/local/domain/6/device/vsnd/0/0/0/event-channel = "18"
/local/domain/6/device/vsnd/0/0/0/evt-ring-ref = "2071"
/local/domain/6/device/vsnd/0/0/0/evt-event-channel = "19"
/libxl/6/device/vsnd = ""
/libxl/6/device/vsnd/0 = ""
/libxl/6/device/vsnd/0/frontend = "/local/domain/6/device/vsnd/0"
/libxl/6/device/vsnd/0/backend = "/local/domain/1/backend/vsnd/6/0"
/libxl/6/device/vsnd/0/frontend-id = "6"
/libxl/6/device/vsnd/0/online = "1"
/libxl/6/device/vsnd/0/state = "1"
> ---
> sound/xen/xen_snd_front_evtchnl.c | 44 +++++++------------------------
> 1 file changed, 10 insertions(+), 34 deletions(-)
>
> diff --git a/sound/xen/xen_snd_front_evtchnl.c b/sound/xen/xen_snd_front_evtchnl.c
> index 3e21369c8216..26d1b3987887 100644
> --- a/sound/xen/xen_snd_front_evtchnl.c
> +++ b/sound/xen/xen_snd_front_evtchnl.c
> @@ -143,12 +143,12 @@ void xen_snd_front_evtchnl_flush(struct xen_snd_front_evtchnl *channel)
> static void evtchnl_free(struct xen_snd_front_info *front_info,
> struct xen_snd_front_evtchnl *channel)
> {
> - unsigned long page = 0;
> + void *page = NULL;
>
> if (channel->type == EVTCHNL_TYPE_REQ)
> - page = (unsigned long)channel->u.req.ring.sring;
> + page = channel->u.req.ring.sring;
> else if (channel->type == EVTCHNL_TYPE_EVT)
> - page = (unsigned long)channel->u.evt.page;
> + page = channel->u.evt.page;
>
> if (!page)
> return;
> @@ -167,10 +167,7 @@ static void evtchnl_free(struct xen_snd_front_info *front_info,
> xenbus_free_evtchn(front_info->xb_dev, channel->port);
>
> /* End access and free the page. */
> - if (channel->gref != INVALID_GRANT_REF)
> - gnttab_end_foreign_access(channel->gref, page);
> - else
> - free_page(page);
> + xenbus_teardown_ring(&page, 1, &channel->gref);
>
> memset(channel, 0, sizeof(*channel));
> }
> @@ -196,8 +193,7 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
> enum xen_snd_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;
> char *handler_name = NULL;
> int ret;
> @@ -207,12 +203,9 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
> channel->index = index;
> channel->front_info = front_info;
> channel->state = EVTCHNL_STATE_DISCONNECTED;
> - channel->gref = INVALID_GRANT_REF;
> - page = get_zeroed_page(GFP_KERNEL);
> - if (!page) {
> - ret = -ENOMEM;
> + ret = xenbus_setup_ring(xb_dev, GFP_KERNEL, &page, 1, &channel->gref);
> + if (ret)
> goto fail;
> - }
>
> handler_name = kasprintf(GFP_KERNEL, "%s-%s", XENSND_DRIVER_NAME,
> type == EVTCHNL_TYPE_REQ ?
> @@ -226,33 +219,18 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
> mutex_init(&channel->ring_io_lock);
>
> if (type == EVTCHNL_TYPE_REQ) {
> - struct xen_sndif_sring *sring = (struct xen_sndif_sring *)page;
> + struct xen_sndif_sring *sring = page;
>
> init_completion(&channel->u.req.completion);
> mutex_init(&channel->u.req.req_io_lock);
> - SHARED_RING_INIT(sring);
> - FRONT_RING_INIT(&channel->u.req.ring, sring, XEN_PAGE_SIZE);
> -
> - ret = xenbus_grant_ring(xb_dev, sring, 1, &gref);
> - if (ret < 0) {
> - channel->u.req.ring.sring = NULL;
> - goto fail;
> - }
> + XEN_FRONT_RING_INIT(&channel->u.req.ring, sring, XEN_PAGE_SIZE);
>
> handler = evtchnl_interrupt_req;
> } else {
> - ret = gnttab_grant_foreign_access(xb_dev->otherend_id,
> - virt_to_gfn((void *)page), 0);
> - if (ret < 0)
> - goto fail;
> -
> - channel->u.evt.page = (struct xensnd_event_page *)page;
> - gref = ret;
> + channel->u.evt.page = page;
> handler = evtchnl_interrupt_evt;
> }
>
> - channel->gref = gref;
> -
> ret = xenbus_alloc_evtchn(xb_dev, &channel->port);
> if (ret < 0)
> goto fail;
> @@ -279,8 +257,6 @@ static int evtchnl_alloc(struct xen_snd_front_info *front_info, int index,
> return 0;
>
> fail:
> - if (page)
> - free_page(page);
> kfree(handler_name);
> dev_err(&xb_dev->dev, "Failed to allocate ring: %d\n", ret);
> return ret;
--
Regards,
Oleksandr Tyshchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring()
2022-04-28 8:27 [PATCH v2 00/19] xen: simplify frontend side ring setup Juergen Gross
` (17 preceding siblings ...)
2022-04-28 8:27 ` [PATCH v2 18/19] xen/sndfront: " Juergen Gross
@ 2022-04-28 8:27 ` Juergen Gross
2022-04-29 15:10 ` Oleksandr
18 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-04-28 8:27 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Juergen Gross, Boris Ostrovsky, Stefano Stabellini
There is no external user of xenbus_grant_ring() left, so merge it into
the only caller xenbus_setup_ring().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- make error message more precise (Andrew Cooper)
---
drivers/xen/xenbus/xenbus_client.c | 65 +++++++++---------------------
include/xen/xenbus.h | 2 -
2 files changed, 19 insertions(+), 48 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 1a2e0d94ccd1..d6fdd2d209d3 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -363,50 +363,6 @@ static void xenbus_switch_fatal(struct xenbus_device *dev, int depth, int err,
__xenbus_switch_state(dev, XenbusStateClosing, 1);
}
-/**
- * xenbus_grant_ring
- * @dev: xenbus device
- * @vaddr: starting virtual address of the ring
- * @nr_pages: number of pages to be granted
- * @grefs: grant reference array to be filled in
- *
- * Grant access to the given @vaddr to the peer of the given device.
- * Then fill in @grefs with grant references. Return 0 on success, or
- * -errno on error. On error, the device will switch to
- * XenbusStateClosing, and the error will be saved in the store.
- */
-int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
- unsigned int nr_pages, grant_ref_t *grefs)
-{
- int err;
- unsigned int i;
- grant_ref_t gref_head;
-
- err = gnttab_alloc_grant_references(nr_pages, &gref_head);
- if (err) {
- xenbus_dev_fatal(dev, err, "granting access to ring page");
- return err;
- }
-
- for (i = 0; i < nr_pages; i++) {
- unsigned long gfn;
-
- if (is_vmalloc_addr(vaddr))
- gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr));
- else
- gfn = virt_to_gfn(vaddr);
-
- grefs[i] = gnttab_claim_grant_reference(&gref_head);
- gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
- gfn, 0);
-
- vaddr = vaddr + XEN_PAGE_SIZE;
- }
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(xenbus_grant_ring);
-
/*
* xenbus_setup_ring
* @dev: xenbus device
@@ -424,6 +380,7 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
unsigned int nr_pages, grant_ref_t *grefs)
{
unsigned long ring_size = nr_pages * XEN_PAGE_SIZE;
+ grant_ref_t gref_head;
unsigned int i;
int ret;
@@ -433,9 +390,25 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
goto err;
}
- ret = xenbus_grant_ring(dev, *vaddr, nr_pages, grefs);
- if (ret)
+ ret = gnttab_alloc_grant_references(nr_pages, &gref_head);
+ if (ret) {
+ xenbus_dev_fatal(dev, ret, "granting access to %u ring pages",
+ nr_pages);
goto err;
+ }
+
+ for (i = 0; i < nr_pages; i++) {
+ unsigned long gfn;
+
+ if (is_vmalloc_addr(*vaddr))
+ gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr[i]));
+ else
+ gfn = virt_to_gfn(vaddr[i]);
+
+ grefs[i] = gnttab_claim_grant_reference(&gref_head);
+ gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
+ gfn, 0);
+ }
return 0;
diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index b533b4adc835..eaa932b99d8a 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -224,8 +224,6 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
const char *pathfmt, ...);
int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
-int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
- unsigned int nr_pages, grant_ref_t *grefs);
int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
unsigned int nr_pages, grant_ref_t *grefs);
void xenbus_teardown_ring(void **vaddr, unsigned int nr_pages,
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring()
2022-04-28 8:27 ` [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring() Juergen Gross
@ 2022-04-29 15:10 ` Oleksandr
2022-05-02 13:30 ` Juergen Gross
0 siblings, 1 reply; 29+ messages in thread
From: Oleksandr @ 2022-04-29 15:10 UTC (permalink / raw)
To: Juergen Gross, xen-devel, linux-kernel
Cc: Boris Ostrovsky, Stefano Stabellini
On 28.04.22 11:27, Juergen Gross wrote:
Hello Juergen
> There is no external user of xenbus_grant_ring() left, so merge it into
> the only caller xenbus_setup_ring().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2:
> - make error message more precise (Andrew Cooper)
> ---
> drivers/xen/xenbus/xenbus_client.c | 65 +++++++++---------------------
> include/xen/xenbus.h | 2 -
> 2 files changed, 19 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
> index 1a2e0d94ccd1..d6fdd2d209d3 100644
> --- a/drivers/xen/xenbus/xenbus_client.c
> +++ b/drivers/xen/xenbus/xenbus_client.c
> @@ -363,50 +363,6 @@ static void xenbus_switch_fatal(struct xenbus_device *dev, int depth, int err,
> __xenbus_switch_state(dev, XenbusStateClosing, 1);
> }
>
> -/**
> - * xenbus_grant_ring
> - * @dev: xenbus device
> - * @vaddr: starting virtual address of the ring
> - * @nr_pages: number of pages to be granted
> - * @grefs: grant reference array to be filled in
> - *
> - * Grant access to the given @vaddr to the peer of the given device.
> - * Then fill in @grefs with grant references. Return 0 on success, or
> - * -errno on error. On error, the device will switch to
> - * XenbusStateClosing, and the error will be saved in the store.
> - */
> -int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
> - unsigned int nr_pages, grant_ref_t *grefs)
> -{
> - int err;
> - unsigned int i;
> - grant_ref_t gref_head;
> -
> - err = gnttab_alloc_grant_references(nr_pages, &gref_head);
> - if (err) {
> - xenbus_dev_fatal(dev, err, "granting access to ring page");
> - return err;
> - }
> -
> - for (i = 0; i < nr_pages; i++) {
> - unsigned long gfn;
> -
> - if (is_vmalloc_addr(vaddr))
> - gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr));
> - else
> - gfn = virt_to_gfn(vaddr);
> -
> - grefs[i] = gnttab_claim_grant_reference(&gref_head);
> - gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
> - gfn, 0);
> -
> - vaddr = vaddr + XEN_PAGE_SIZE;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(xenbus_grant_ring);
> -
> /*
> * xenbus_setup_ring
> * @dev: xenbus device
> @@ -424,6 +380,7 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
> unsigned int nr_pages, grant_ref_t *grefs)
> {
> unsigned long ring_size = nr_pages * XEN_PAGE_SIZE;
> + grant_ref_t gref_head;
> unsigned int i;
> int ret;
>
> @@ -433,9 +390,25 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
> goto err;
> }
>
> - ret = xenbus_grant_ring(dev, *vaddr, nr_pages, grefs);
> - if (ret)
> + ret = gnttab_alloc_grant_references(nr_pages, &gref_head);
> + if (ret) {
> + xenbus_dev_fatal(dev, ret, "granting access to %u ring pages",
> + nr_pages);
> goto err;
> + }
> +
> + for (i = 0; i < nr_pages; i++) {
> + unsigned long gfn;
> +
> + if (is_vmalloc_addr(*vaddr))
> + gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr[i]));
> + else
> + gfn = virt_to_gfn(vaddr[i]);
> +
> + grefs[i] = gnttab_claim_grant_reference(&gref_head);
gnttab_claim_grant_reference() can return error if no free grant
reference remains.
I understand this patch only moves the code, but probably it would be
better to add a missing check here (and likely rollback already
processed grants if any?).
> + gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
> + gfn, 0);
> + }
>
> return 0;
>
> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
> index b533b4adc835..eaa932b99d8a 100644
> --- a/include/xen/xenbus.h
> +++ b/include/xen/xenbus.h
> @@ -224,8 +224,6 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
> const char *pathfmt, ...);
>
> int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
> -int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
> - unsigned int nr_pages, grant_ref_t *grefs);
> int xenbus_setup_ring(struct xenbus_device *dev, gfp_t gfp, void **vaddr,
> unsigned int nr_pages, grant_ref_t *grefs);
> void xenbus_teardown_ring(void **vaddr, unsigned int nr_pages,
--
Regards,
Oleksandr Tyshchenko
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring()
2022-04-29 15:10 ` Oleksandr
@ 2022-05-02 13:30 ` Juergen Gross
2022-05-02 14:12 ` Oleksandr
0 siblings, 1 reply; 29+ messages in thread
From: Juergen Gross @ 2022-05-02 13:30 UTC (permalink / raw)
To: Oleksandr, xen-devel, linux-kernel; +Cc: Boris Ostrovsky, Stefano Stabellini
[-- Attachment #1.1.1: Type: text/plain, Size: 4426 bytes --]
On 29.04.22 17:10, Oleksandr wrote:
>
> On 28.04.22 11:27, Juergen Gross wrote:
>
>
> Hello Juergen
>
>
>> There is no external user of xenbus_grant_ring() left, so merge it into
>> the only caller xenbus_setup_ring().
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - make error message more precise (Andrew Cooper)
>> ---
>> drivers/xen/xenbus/xenbus_client.c | 65 +++++++++---------------------
>> include/xen/xenbus.h | 2 -
>> 2 files changed, 19 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/xen/xenbus/xenbus_client.c
>> b/drivers/xen/xenbus/xenbus_client.c
>> index 1a2e0d94ccd1..d6fdd2d209d3 100644
>> --- a/drivers/xen/xenbus/xenbus_client.c
>> +++ b/drivers/xen/xenbus/xenbus_client.c
>> @@ -363,50 +363,6 @@ static void xenbus_switch_fatal(struct xenbus_device
>> *dev, int depth, int err,
>> __xenbus_switch_state(dev, XenbusStateClosing, 1);
>> }
>> -/**
>> - * xenbus_grant_ring
>> - * @dev: xenbus device
>> - * @vaddr: starting virtual address of the ring
>> - * @nr_pages: number of pages to be granted
>> - * @grefs: grant reference array to be filled in
>> - *
>> - * Grant access to the given @vaddr to the peer of the given device.
>> - * Then fill in @grefs with grant references. Return 0 on success, or
>> - * -errno on error. On error, the device will switch to
>> - * XenbusStateClosing, and the error will be saved in the store.
>> - */
>> -int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
>> - unsigned int nr_pages, grant_ref_t *grefs)
>> -{
>> - int err;
>> - unsigned int i;
>> - grant_ref_t gref_head;
>> -
>> - err = gnttab_alloc_grant_references(nr_pages, &gref_head);
>> - if (err) {
>> - xenbus_dev_fatal(dev, err, "granting access to ring page");
>> - return err;
>> - }
>> -
>> - for (i = 0; i < nr_pages; i++) {
>> - unsigned long gfn;
>> -
>> - if (is_vmalloc_addr(vaddr))
>> - gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr));
>> - else
>> - gfn = virt_to_gfn(vaddr);
>> -
>> - grefs[i] = gnttab_claim_grant_reference(&gref_head);
>> - gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
>> - gfn, 0);
>> -
>> - vaddr = vaddr + XEN_PAGE_SIZE;
>> - }
>> -
>> - return 0;
>> -}
>> -EXPORT_SYMBOL_GPL(xenbus_grant_ring);
>> -
>> /*
>> * xenbus_setup_ring
>> * @dev: xenbus device
>> @@ -424,6 +380,7 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t
>> gfp, void **vaddr,
>> unsigned int nr_pages, grant_ref_t *grefs)
>> {
>> unsigned long ring_size = nr_pages * XEN_PAGE_SIZE;
>> + grant_ref_t gref_head;
>> unsigned int i;
>> int ret;
>> @@ -433,9 +390,25 @@ int xenbus_setup_ring(struct xenbus_device *dev, gfp_t
>> gfp, void **vaddr,
>> goto err;
>> }
>> - ret = xenbus_grant_ring(dev, *vaddr, nr_pages, grefs);
>> - if (ret)
>> + ret = gnttab_alloc_grant_references(nr_pages, &gref_head);
>> + if (ret) {
>> + xenbus_dev_fatal(dev, ret, "granting access to %u ring pages",
>> + nr_pages);
>> goto err;
>> + }
>> +
>> + for (i = 0; i < nr_pages; i++) {
>> + unsigned long gfn;
>> +
>> + if (is_vmalloc_addr(*vaddr))
>> + gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr[i]));
>> + else
>> + gfn = virt_to_gfn(vaddr[i]);
>> +
>> + grefs[i] = gnttab_claim_grant_reference(&gref_head);
>
> gnttab_claim_grant_reference() can return error if no free grant reference remains.
This can happen only in case gnttab_alloc_grant_references() didn't
allocate enough grants but told us it succeeded doing so.
> I understand this patch only moves the code, but probably it would be better to
> add a missing check here (and likely rollback already processed grants if any?).
I don't think this is needed, as this would be a clear bug in the code.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 19/19] xen/xenbus: eliminate xenbus_grant_ring()
2022-05-02 13:30 ` Juergen Gross
@ 2022-05-02 14:12 ` Oleksandr
0 siblings, 0 replies; 29+ messages in thread
From: Oleksandr @ 2022-05-02 14:12 UTC (permalink / raw)
To: Juergen Gross
Cc: xen-devel, linux-kernel, Boris Ostrovsky, Stefano Stabellini
On 02.05.22 16:30, Juergen Gross wrote:
Hello Juergen
> On 29.04.22 17:10, Oleksandr wrote:
>>
>> On 28.04.22 11:27, Juergen Gross wrote:
>>
>>
>> Hello Juergen
>>
>>
>>> There is no external user of xenbus_grant_ring() left, so merge it into
>>> the only caller xenbus_setup_ring().
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>> V2:
>>> - make error message more precise (Andrew Cooper)
>>> ---
>>> drivers/xen/xenbus/xenbus_client.c | 65
>>> +++++++++---------------------
>>> include/xen/xenbus.h | 2 -
>>> 2 files changed, 19 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/xen/xenbus/xenbus_client.c
>>> b/drivers/xen/xenbus/xenbus_client.c
>>> index 1a2e0d94ccd1..d6fdd2d209d3 100644
>>> --- a/drivers/xen/xenbus/xenbus_client.c
>>> +++ b/drivers/xen/xenbus/xenbus_client.c
>>> @@ -363,50 +363,6 @@ static void xenbus_switch_fatal(struct
>>> xenbus_device *dev, int depth, int err,
>>> __xenbus_switch_state(dev, XenbusStateClosing, 1);
>>> }
>>> -/**
>>> - * xenbus_grant_ring
>>> - * @dev: xenbus device
>>> - * @vaddr: starting virtual address of the ring
>>> - * @nr_pages: number of pages to be granted
>>> - * @grefs: grant reference array to be filled in
>>> - *
>>> - * Grant access to the given @vaddr to the peer of the given device.
>>> - * Then fill in @grefs with grant references. Return 0 on success, or
>>> - * -errno on error. On error, the device will switch to
>>> - * XenbusStateClosing, and the error will be saved in the store.
>>> - */
>>> -int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
>>> - unsigned int nr_pages, grant_ref_t *grefs)
>>> -{
>>> - int err;
>>> - unsigned int i;
>>> - grant_ref_t gref_head;
>>> -
>>> - err = gnttab_alloc_grant_references(nr_pages, &gref_head);
>>> - if (err) {
>>> - xenbus_dev_fatal(dev, err, "granting access to ring page");
>>> - return err;
>>> - }
>>> -
>>> - for (i = 0; i < nr_pages; i++) {
>>> - unsigned long gfn;
>>> -
>>> - if (is_vmalloc_addr(vaddr))
>>> - gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr));
>>> - else
>>> - gfn = virt_to_gfn(vaddr);
>>> -
>>> - grefs[i] = gnttab_claim_grant_reference(&gref_head);
>>> - gnttab_grant_foreign_access_ref(grefs[i], dev->otherend_id,
>>> - gfn, 0);
>>> -
>>> - vaddr = vaddr + XEN_PAGE_SIZE;
>>> - }
>>> -
>>> - return 0;
>>> -}
>>> -EXPORT_SYMBOL_GPL(xenbus_grant_ring);
>>> -
>>> /*
>>> * xenbus_setup_ring
>>> * @dev: xenbus device
>>> @@ -424,6 +380,7 @@ int xenbus_setup_ring(struct xenbus_device *dev,
>>> gfp_t gfp, void **vaddr,
>>> unsigned int nr_pages, grant_ref_t *grefs)
>>> {
>>> unsigned long ring_size = nr_pages * XEN_PAGE_SIZE;
>>> + grant_ref_t gref_head;
>>> unsigned int i;
>>> int ret;
>>> @@ -433,9 +390,25 @@ int xenbus_setup_ring(struct xenbus_device
>>> *dev, gfp_t gfp, void **vaddr,
>>> goto err;
>>> }
>>> - ret = xenbus_grant_ring(dev, *vaddr, nr_pages, grefs);
>>> - if (ret)
>>> + ret = gnttab_alloc_grant_references(nr_pages, &gref_head);
>>> + if (ret) {
>>> + xenbus_dev_fatal(dev, ret, "granting access to %u ring pages",
>>> + nr_pages);
>>> goto err;
>>> + }
>>> +
>>> + for (i = 0; i < nr_pages; i++) {
>>> + unsigned long gfn;
>>> +
>>> + if (is_vmalloc_addr(*vaddr))
>>> + gfn = pfn_to_gfn(vmalloc_to_pfn(vaddr[i]));
>>> + else
>>> + gfn = virt_to_gfn(vaddr[i]);
>>> +
>>> + grefs[i] = gnttab_claim_grant_reference(&gref_head);
>>
>> gnttab_claim_grant_reference() can return error if no free grant
>> reference remains.
>
> This can happen only in case gnttab_alloc_grant_references() didn't
> allocate enough grants but told us it succeeded doing so.
>
>> I understand this patch only moves the code, but probably it would be
>> better to add a missing check here (and likely rollback already
>> processed grants if any?).
>
> I don't think this is needed, as this would be a clear bug in the code.
I would put WARN_ON_ONCE if ref is an error value then like xen-netfront
does. Either way, with or without it you can add my:
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
>
>
> Juergen
--
Regards,
Oleksandr Tyshchenko
^ permalink raw reply [flat|nested] 29+ messages in thread