|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] xenbus_client: extend interface to suppurt multi-page ring
On Sat, Nov 9, 2013 at 4:30 AM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> @@ -357,17 +365,39 @@ static void xenbus_switch_fatal(struct xenbus_device
> *dev, int depth, int err,
> /**
> * xenbus_grant_ring
> * @dev: xenbus device
> - * @ring_mfn: mfn of ring to grant
> -
> - * Grant access to the given @ring_mfn to the peer of the given device.
> Return
> - * 0 on success, or -errno on error. On error, the device will switch to
> + * @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, unsigned long ring_mfn)
> +int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
> + unsigned int nr_pages, grant_ref_t *grefs)
> {
> - int err = gnttab_grant_foreign_access(dev->otherend_id, ring_mfn, 0);
> - if (err < 0)
> - xenbus_dev_fatal(dev, err, "granting access to ring page");
> + int err;
> + int i;
> +
> + for (i = 0; i < nr_pages; i++) {
> + unsigned long addr = (unsigned long)vaddr +
> + (PAGE_SIZE * i);
> + err = gnttab_grant_foreign_access(dev->otherend_id,
> + virt_to_mfn(addr), 0);
> + if (err < 0) {
> + xenbus_dev_fatal(dev, err,
> + "granting access to ring page");
> + goto fail;
> + }
> + grefs[i] = err;
> + }
> +
> + return 0;
> +
> +fail:
> + for (; i >= 0; i--)
There's an off-by-one here, causing gnttab_end_foreign_access_ref to
be called on the first uninitialized (at least in this function) gref.
> + gnttab_end_foreign_access_ref(grefs[i], 0);
> return err;
> }
> EXPORT_SYMBOL_GPL(xenbus_grant_ring);
> @@ -448,62 +478,131 @@ EXPORT_SYMBOL_GPL(xenbus_free_evtchn);
> /**
> * xenbus_map_ring_valloc
> * @dev: xenbus device
> - * @gnt_ref: grant reference
> + * @gnt_refs: grant reference array
> + * @nr_grefs: number of grant references
> * @vaddr: pointer to address to be filled out by mapping
> *
> - * Based on Rusty Russell's skeleton driver's map_page.
> - * Map a page of memory into this domain from another domain's grant table.
> - * xenbus_map_ring_valloc allocates a page of virtual address space, maps the
> - * page to that address, and sets *vaddr to that address.
> - * Returns 0 on success, and GNTST_* (see
> xen/include/interface/grant_table.h)
> - * or -ENOMEM on error. If an error is returned, device will switch to
> + * Map @nr_grefs pages of memory into this domain from another
> + * domain's grant table. xenbus_map_ring_valloc allocates @nr_grefs
> + * pages of virtual address space, maps the pages to that address, and
> + * sets *vaddr to that address. Returns 0 on success, and GNTST_*
> + * (see xen/include/interface/grant_table.h) or -ENOMEM / -EINVAL on
> + * error. If an error is returned, device will switch to
> * XenbusStateClosing and the error message will be saved in XenStore.
> */
> -int xenbus_map_ring_valloc(struct xenbus_device *dev, int gnt_ref, void
> **vaddr)
> +int xenbus_map_ring_valloc(struct xenbus_device *dev, grant_ref_t *gnt_refs,
> + unsigned int nr_grefs, void **vaddr)
> {
> - return ring_ops->map(dev, gnt_ref, vaddr);
> + return ring_ops->map(dev, gnt_refs, nr_grefs, vaddr);
> }
> EXPORT_SYMBOL_GPL(xenbus_map_ring_valloc);
>
> +/* N.B. sizeof(phys_addr_t) doesn't always equal to sizeof(unsigned
> + * long), e.g. 32-on-64. Caller is responsible for preparing the
> + * right array to feed into this function */
> +static int __xenbus_map_ring(struct xenbus_device *dev,
> + grant_ref_t *gnt_refs,
> + unsigned int nr_grefs,
> + grant_handle_t *handles,
> + phys_addr_t *addrs,
> + unsigned int flags,
> + bool *leaked)
> +{
> + struct gnttab_map_grant_ref map[XENBUS_MAX_RING_PAGES];
> + struct gnttab_unmap_grant_ref unmap[XENBUS_MAX_RING_PAGES];
> + int i, j;
> + int err = GNTST_okay;
> +
> + if (nr_grefs > XENBUS_MAX_RING_PAGES)
> + return -EINVAL;
> +
> + for (i = 0; i < nr_grefs; i++) {
> + memset(&map[i], 0, sizeof(map[i]));
> + gnttab_set_map_op(&map[i], addrs[i], flags, gnt_refs[i],
> + dev->otherend_id);
> + handles[i] = INVALID_GRANT_HANDLE;
> + }
> +
> + gnttab_batch_map(map, i);
> +
> + for (i = 0; i < nr_grefs; i++) {
> + if (map[i].status != GNTST_okay) {
> + err = map[i].status;
> + xenbus_dev_fatal(dev, map[i].status,
> + "mapping in shared page %d from
> domain %d",
> + gnt_refs[i], dev->otherend_id);
> + goto fail;
> + } else
> + handles[i] = map[i].handle;
> + }
> +
> + return GNTST_okay;
> +
> + fail:
> + for (i = j = 0; i < nr_grefs; i++) {
> + memset(&unmap[i], 0, sizeof(unmap[i]));
I think you mean unmap[j], not unmap[i] (both times)? If so, you might
as well only do this in the following if block, not out here
unconditionally.
> + if (handles[i] != INVALID_GRANT_HANDLE) {
> + gnttab_set_unmap_op(&unmap[j], (phys_addr_t)addrs[i],
> + GNTMAP_host_map, handles[i]);
> + j++;
> + }
> + }
> +
> + if (HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, j))
> + BUG();
> +
> + *leaked = false;
> + for (i = 0; i < j; i++) {
> + if (unmap[i].status != GNTST_okay) {
> + *leaked = true;
> + break;
> + }
> + }
> +
> + return err;
> +
Extra blank line?
- Matthew
> +}
> +
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |