diff mbox series

[09/18] xen/xenbus: add xenbus_setup_ring() service function

Message ID 20220420150942.31235-10-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series xen: simplify frontend side ring setup | expand

Commit Message

Jürgen Groß April 20, 2022, 3:09 p.m. UTC
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(+)

Comments

Boris Ostrovsky April 20, 2022, 6:44 p.m. UTC | #1
On 4/20/22 11:09 AM, Juergen Gross wrote:
>   
> +/*
> + * 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;
> +}


We can create a wrapper around this function that will also call SHARED_RING_INIT() and FRONT_RING_INIT(). A bunch of drivers do that.


-boris
Jürgen Groß April 21, 2022, 7:57 a.m. UTC | #2
On 20.04.22 20:44, Boris Ostrovsky wrote:
> 
> On 4/20/22 11:09 AM, Juergen Gross wrote:
>> +/*
>> + * 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;
>> +}
> 
> 
> We can create a wrapper around this function that will also call 
> SHARED_RING_INIT() and FRONT_RING_INIT(). A bunch of drivers do that.

This wrapper would need to be a macro, so I decided not to do that.

It would make sense to merge the call of SHARED_RING_INIT() into
FRONT_RING_INIT() (or better, have a new macro combining the two),
though, as there is no use case of FRONT_RING_INIT() without a
call of SHARED_RING_INIT() directly before it.


Juergen
diff mbox series

Patch

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);