diff mbox

[v5,4/7] xen/9pfs: connect to the backend

Message ID 1490032788-23471-4-git-send-email-sstabellini@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Stefano Stabellini March 20, 2017, 5:59 p.m. UTC
Implement functions to handle the xenbus handshake. Upon connection,
allocate the rings according to the protocol specification.

Initialize a work_struct and a wait_queue. The work_struct will be used
to schedule work upon receiving an event channel notification from the
backend. The wait_queue will be used to wait when the ring is full and
we need to send a new request.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: groug@kaod.org
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
CC: Eric Van Hensbergen <ericvh@gmail.com>
CC: Ron Minnich <rminnich@sandia.gov>
CC: Latchesar Ionkov <lucho@ionkov.net>
CC: v9fs-developer@lists.sourceforge.net
---
 net/9p/trans_xen.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 273 insertions(+)

Comments

Juergen Gross March 21, 2017, 8:11 a.m. UTC | #1
On 20/03/17 18:59, Stefano Stabellini wrote:
> Implement functions to handle the xenbus handshake. Upon connection,
> allocate the rings according to the protocol specification.
> 
> Initialize a work_struct and a wait_queue. The work_struct will be used
> to schedule work upon receiving an event channel notification from the
> backend. The wait_queue will be used to wait when the ring is full and
> we need to send a new request.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: groug@kaod.org
> CC: boris.ostrovsky@oracle.com
> CC: jgross@suse.com
> CC: Eric Van Hensbergen <ericvh@gmail.com>
> CC: Ron Minnich <rminnich@sandia.gov>
> CC: Latchesar Ionkov <lucho@ionkov.net>
> CC: v9fs-developer@lists.sourceforge.net
> ---
>  net/9p/trans_xen.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 273 insertions(+)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index 3d07260..5279ae0 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
...
> +static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
> +					 struct xen_9pfs_dataring *ring)
> +{
> +	int i = 0;
> +	int ret = -ENOMEM;
> +	void *bytes = NULL;
> +
> +	init_waitqueue_head(&ring->wq);
> +	spin_lock_init(&ring->lock);
> +	INIT_WORK(&ring->work, p9_xen_response);
> +
> +	ring->intf = (struct xen_9pfs_data_intf *)get_zeroed_page(GFP_KERNEL);
> +	if (!ring->intf)
> +		return ret;
> +	ret = gnttab_grant_foreign_access(dev->otherend_id,
> +					  virt_to_gfn(ring->intf), 0);
> +	if (ret < 0)
> +		goto out;
> +	ring->ref = ret;
> +	bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
> +			XEN_9PFS_RING_ORDER - (PAGE_SHIFT - XEN_PAGE_SHIFT));
> +	if (!bytes)
> +		goto out;

ret is wrong here.

> +	for (; i < (1 << XEN_9PFS_RING_ORDER); i++) {
> +		ret = gnttab_grant_foreign_access(
> +				dev->otherend_id, virt_to_gfn(bytes) + i, 0);
> +		if (ret < 0)
> +			goto out;
> +		ring->intf->ref[i] = ret;
> +	}
> +	ring->intf->ring_order = XEN_9PFS_RING_ORDER;
> +	ring->data.in = bytes;
> +	ring->data.out = bytes + XEN_9PFS_RING_SIZE;
> +
> +	ret = xenbus_alloc_evtchn(dev, &ring->evtchn);
> +	if (ret)
> +		goto out;
> +	ring->irq = bind_evtchn_to_irqhandler(ring->evtchn,
> +					      xen_9pfs_front_event_handler,
> +					      0, "xen_9pfs-frontend", ring);
> +	if (ring->irq < 0) {
> +		xenbus_free_evtchn(dev, ring->evtchn);
> +		ret = ring->irq;
> +		goto out;
> +	}
>  	return 0;


What about:

if (ring->irq >= 0)
	return 0;

xenbus_free_evtchn(dev, ring->evtchn);
ret = ring->irq;

> +
> +out:
> +	if (!bytes) {

Really? Shouldn't this be "if (bytes)" ?

> +		for (i--; i >= 0; i--)
> +			gnttab_end_foreign_access(ring->intf->ref[i], 0, 0);
> +		free_pages((unsigned long)bytes,
> +			   XEN_9PFS_RING_ORDER -
> +			   (PAGE_SHIFT - XEN_PAGE_SHIFT));
> +	}
> +	gnttab_end_foreign_access(ring->ref, 0, 0);
> +	free_page((unsigned long)ring->intf);
> +	return ret;
>  }


Juergen
Stefano Stabellini March 21, 2017, 6:21 p.m. UTC | #2
On Tue, 21 Mar 2017, Juergen Gross wrote:
> On 20/03/17 18:59, Stefano Stabellini wrote:
> > Implement functions to handle the xenbus handshake. Upon connection,
> > allocate the rings according to the protocol specification.
> > 
> > Initialize a work_struct and a wait_queue. The work_struct will be used
> > to schedule work upon receiving an event channel notification from the
> > backend. The wait_queue will be used to wait when the ring is full and
> > we need to send a new request.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: groug@kaod.org
> > CC: boris.ostrovsky@oracle.com
> > CC: jgross@suse.com
> > CC: Eric Van Hensbergen <ericvh@gmail.com>
> > CC: Ron Minnich <rminnich@sandia.gov>
> > CC: Latchesar Ionkov <lucho@ionkov.net>
> > CC: v9fs-developer@lists.sourceforge.net
> > ---
> >  net/9p/trans_xen.c | 273 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 273 insertions(+)
> > 
> > diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> > index 3d07260..5279ae0 100644
> > --- a/net/9p/trans_xen.c
> > +++ b/net/9p/trans_xen.c
> ...
> > +static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
> > +					 struct xen_9pfs_dataring *ring)
> > +{
> > +	int i = 0;
> > +	int ret = -ENOMEM;
> > +	void *bytes = NULL;
> > +
> > +	init_waitqueue_head(&ring->wq);
> > +	spin_lock_init(&ring->lock);
> > +	INIT_WORK(&ring->work, p9_xen_response);
> > +
> > +	ring->intf = (struct xen_9pfs_data_intf *)get_zeroed_page(GFP_KERNEL);
> > +	if (!ring->intf)
> > +		return ret;
> > +	ret = gnttab_grant_foreign_access(dev->otherend_id,
> > +					  virt_to_gfn(ring->intf), 0);
> > +	if (ret < 0)
> > +		goto out;
> > +	ring->ref = ret;
> > +	bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
> > +			XEN_9PFS_RING_ORDER - (PAGE_SHIFT - XEN_PAGE_SHIFT));
> > +	if (!bytes)
> > +		goto out;
> 
> ret is wrong here.

I'll fix


> > +	for (; i < (1 << XEN_9PFS_RING_ORDER); i++) {
> > +		ret = gnttab_grant_foreign_access(
> > +				dev->otherend_id, virt_to_gfn(bytes) + i, 0);
> > +		if (ret < 0)
> > +			goto out;
> > +		ring->intf->ref[i] = ret;
> > +	}
> > +	ring->intf->ring_order = XEN_9PFS_RING_ORDER;
> > +	ring->data.in = bytes;
> > +	ring->data.out = bytes + XEN_9PFS_RING_SIZE;
> > +
> > +	ret = xenbus_alloc_evtchn(dev, &ring->evtchn);
> > +	if (ret)
> > +		goto out;
> > +	ring->irq = bind_evtchn_to_irqhandler(ring->evtchn,
> > +					      xen_9pfs_front_event_handler,
> > +					      0, "xen_9pfs-frontend", ring);
> > +	if (ring->irq < 0) {
> > +		xenbus_free_evtchn(dev, ring->evtchn);
> > +		ret = ring->irq;
> > +		goto out;
> > +	}
> >  	return 0;
> 
> 
> What about:
> 
> if (ring->irq >= 0)
> 	return 0;
> 
> xenbus_free_evtchn(dev, ring->evtchn);
> ret = ring->irq;

OK


> > +
> > +out:
> > +	if (!bytes) {
> 
> Really? Shouldn't this be "if (bytes)" ?

Wops. I'll fix.


> > +		for (i--; i >= 0; i--)
> > +			gnttab_end_foreign_access(ring->intf->ref[i], 0, 0);
> > +		free_pages((unsigned long)bytes,
> > +			   XEN_9PFS_RING_ORDER -
> > +			   (PAGE_SHIFT - XEN_PAGE_SHIFT));
> > +	}
> > +	gnttab_end_foreign_access(ring->ref, 0, 0);
> > +	free_page((unsigned long)ring->intf);
> > +	return ret;
> >  }
> 
> 
> Juergen
>
diff mbox

Patch

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 3d07260..5279ae0 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -37,10 +37,46 @@ 
 #include <xen/interface/io/9pfs.h>
 
 #include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/rwlock.h>
 #include <net/9p/9p.h>
 #include <net/9p/client.h>
 #include <net/9p/transport.h>
 
+#define XEN_9PFS_NUM_RINGS 2
+#define XEN_9PFS_RING_ORDER 6
+#define XEN_9PFS_RING_SIZE  XEN_FLEX_RING_SIZE(XEN_9PFS_RING_ORDER)
+
+/* One per ring, more than one per 9pfs share */
+struct xen_9pfs_dataring {
+	struct xen_9pfs_front_priv *priv;
+
+	struct xen_9pfs_data_intf *intf;
+	grant_ref_t ref;
+	int evtchn;
+	int irq;
+	/* protect a ring from concurrent accesses */
+	spinlock_t lock;
+
+	struct xen_9pfs_data data;
+	wait_queue_head_t wq;
+	struct work_struct work;
+};
+
+/* One per 9pfs share */
+struct xen_9pfs_front_priv {
+	struct list_head list;
+	struct xenbus_device *dev;
+	char *tag;
+	struct p9_client *client;
+
+	int num_rings;
+	struct xen_9pfs_dataring *rings;
+};
+
+static LIST_HEAD(xen_9pfs_devs);
+static DEFINE_RWLOCK(xen_9pfs_lock);
+
 static int p9_xen_cancel(struct p9_client *client, struct p9_req_t *req)
 {
 	return 0;
@@ -60,6 +96,25 @@  static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 	return 0;
 }
 
+static void p9_xen_response(struct work_struct *work)
+{
+}
+
+static irqreturn_t xen_9pfs_front_event_handler(int irq, void *r)
+{
+	struct xen_9pfs_dataring *ring = r;
+
+	if (!ring || !ring->priv->client) {
+		/* ignore spurious interrupt */
+		return IRQ_HANDLED;
+	}
+
+	wake_up_interruptible(&ring->wq);
+	schedule_work(&ring->work);
+
+	return IRQ_HANDLED;
+}
+
 static struct p9_trans_module p9_xen_trans = {
 	.name = "xen",
 	.maxsize = 1 << (XEN_9PFS_RING_ORDER + XEN_PAGE_SHIFT),
@@ -76,25 +131,243 @@  static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 	{ "" }
 };
 
+static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
+{
+	int i, j;
+
+	write_lock(&xen_9pfs_lock);
+	list_del(&priv->list);
+	write_unlock(&xen_9pfs_lock);
+
+	for (i = 0; i < priv->num_rings; i++) {
+		if (!priv->rings[i].intf)
+			break;
+		if (priv->rings[i].irq > 0)
+			unbind_from_irqhandler(priv->rings[i].irq, priv->dev);
+		if (priv->rings[i].data.in) {
+			for (j = 0; j < (1 << XEN_9PFS_RING_ORDER); j++) {
+				grant_ref_t ref;
+
+				ref = priv->rings[i].intf->ref[j];
+				gnttab_end_foreign_access(ref, 0, 0);
+			}
+			free_pages((unsigned long)priv->rings[i].data.in,
+				   XEN_9PFS_RING_ORDER -
+				   (PAGE_SHIFT - XEN_PAGE_SHIFT));
+		}
+		gnttab_end_foreign_access(priv->rings[i].ref, 0, 0);
+		free_page((unsigned long)priv->rings[i].intf);
+	}
+	kfree(priv->rings);
+	kfree(priv->tag);
+	kfree(priv);
+}
+
 static int xen_9pfs_front_remove(struct xenbus_device *dev)
 {
+	struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
+
+	dev_set_drvdata(&dev->dev, NULL);
+	xen_9pfs_front_free(priv);
+	return 0;
+}
+
+static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev,
+					 struct xen_9pfs_dataring *ring)
+{
+	int i = 0;
+	int ret = -ENOMEM;
+	void *bytes = NULL;
+
+	init_waitqueue_head(&ring->wq);
+	spin_lock_init(&ring->lock);
+	INIT_WORK(&ring->work, p9_xen_response);
+
+	ring->intf = (struct xen_9pfs_data_intf *)get_zeroed_page(GFP_KERNEL);
+	if (!ring->intf)
+		return ret;
+	ret = gnttab_grant_foreign_access(dev->otherend_id,
+					  virt_to_gfn(ring->intf), 0);
+	if (ret < 0)
+		goto out;
+	ring->ref = ret;
+	bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+			XEN_9PFS_RING_ORDER - (PAGE_SHIFT - XEN_PAGE_SHIFT));
+	if (!bytes)
+		goto out;
+	for (; i < (1 << XEN_9PFS_RING_ORDER); i++) {
+		ret = gnttab_grant_foreign_access(
+				dev->otherend_id, virt_to_gfn(bytes) + i, 0);
+		if (ret < 0)
+			goto out;
+		ring->intf->ref[i] = ret;
+	}
+	ring->intf->ring_order = XEN_9PFS_RING_ORDER;
+	ring->data.in = bytes;
+	ring->data.out = bytes + XEN_9PFS_RING_SIZE;
+
+	ret = xenbus_alloc_evtchn(dev, &ring->evtchn);
+	if (ret)
+		goto out;
+	ring->irq = bind_evtchn_to_irqhandler(ring->evtchn,
+					      xen_9pfs_front_event_handler,
+					      0, "xen_9pfs-frontend", ring);
+	if (ring->irq < 0) {
+		xenbus_free_evtchn(dev, ring->evtchn);
+		ret = ring->irq;
+		goto out;
+	}
 	return 0;
+
+out:
+	if (!bytes) {
+		for (i--; i >= 0; i--)
+			gnttab_end_foreign_access(ring->intf->ref[i], 0, 0);
+		free_pages((unsigned long)bytes,
+			   XEN_9PFS_RING_ORDER -
+			   (PAGE_SHIFT - XEN_PAGE_SHIFT));
+	}
+	gnttab_end_foreign_access(ring->ref, 0, 0);
+	free_page((unsigned long)ring->intf);
+	return ret;
 }
 
 static int xen_9pfs_front_probe(struct xenbus_device *dev,
 				const struct xenbus_device_id *id)
 {
+	int ret, i;
+	struct xenbus_transaction xbt;
+	struct xen_9pfs_front_priv *priv = NULL;
+	char *versions;
+	unsigned int max_rings, max_ring_order, len;
+
+	versions = xenbus_read(XBT_NIL, dev->otherend, "versions", &len);
+	if (!len)
+		return -EINVAL;
+	if (strcmp(versions, "1")) {
+		kfree(versions);
+		return -EINVAL;
+	}
+	kfree(versions);
+	max_rings = xenbus_read_unsigned(dev->otherend, "max-rings", 0);
+	if (max_rings < XEN_9PFS_NUM_RINGS)
+		return -EINVAL;
+	max_ring_order = xenbus_read_unsigned(dev->otherend,
+					      "max-ring-page-order", 0);
+	if (max_ring_order < XEN_9PFS_RING_ORDER)
+		return -EINVAL;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->num_rings = XEN_9PFS_NUM_RINGS;
+	priv->rings = kcalloc(priv->num_rings, sizeof(*priv->rings),
+			      GFP_KERNEL);
+	if (!priv->rings) {
+		kfree(priv);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < priv->num_rings; i++) {
+		priv->rings[i].priv = priv;
+		ret = xen_9pfs_front_alloc_dataring(dev, &priv->rings[i]);
+		if (ret < 0)
+			goto error;
+	}
+
+ again:
+	ret = xenbus_transaction_start(&xbt);
+	if (ret) {
+		xenbus_dev_fatal(dev, ret, "starting transaction");
+		goto error;
+	}
+	ret = xenbus_printf(xbt, dev->nodename, "version", "%u", 1);
+	if (ret)
+		goto error_xenbus;
+	ret = xenbus_printf(xbt, dev->nodename, "num-rings", "%u",
+			    priv->num_rings);
+	if (ret)
+		goto error_xenbus;
+	for (i = 0; i < priv->num_rings; i++) {
+		char str[16];
+
+		BUILD_BUG_ON(XEN_9PFS_NUM_RINGS > 9);
+		sprintf(str, "ring-ref%u", i);
+		ret = xenbus_printf(xbt, dev->nodename, str, "%d",
+				    priv->rings[i].ref);
+		if (ret)
+			goto error_xenbus;
+
+		sprintf(str, "event-channel-%u", i);
+		ret = xenbus_printf(xbt, dev->nodename, str, "%u",
+				    priv->rings[i].evtchn);
+		if (ret)
+			goto error_xenbus;
+	}
+	priv->tag = xenbus_read(xbt, dev->nodename, "tag", NULL);
+	if (!priv->tag) {
+		ret = -EINVAL;
+		goto error_xenbus;
+	}
+	ret = xenbus_transaction_end(xbt, 0);
+	if (ret) {
+		if (ret == -EAGAIN)
+			goto again;
+		xenbus_dev_fatal(dev, ret, "completing transaction");
+		goto error;
+	}
+
+	write_lock(&xen_9pfs_lock);
+	list_add_tail(&priv->list, &xen_9pfs_devs);
+	write_unlock(&xen_9pfs_lock);
+	dev_set_drvdata(&dev->dev, priv);
+	xenbus_switch_state(dev, XenbusStateInitialised);
+
 	return 0;
+
+ error_xenbus:
+	xenbus_transaction_end(xbt, 1);
+	xenbus_dev_fatal(dev, ret, "writing xenstore");
+ error:
+	dev_set_drvdata(&dev->dev, NULL);
+	xen_9pfs_front_free(priv);
+	return ret;
 }
 
 static int xen_9pfs_front_resume(struct xenbus_device *dev)
 {
+	dev_warn(&dev->dev, "suspsend/resume unsupported\n");
 	return 0;
 }
 
 static void xen_9pfs_front_changed(struct xenbus_device *dev,
 				   enum xenbus_state backend_state)
 {
+	switch (backend_state) {
+	case XenbusStateReconfiguring:
+	case XenbusStateReconfigured:
+	case XenbusStateInitialising:
+	case XenbusStateInitialised:
+	case XenbusStateUnknown:
+		break;
+
+	case XenbusStateInitWait:
+		break;
+
+	case XenbusStateConnected:
+		xenbus_switch_state(dev, XenbusStateConnected);
+		break;
+
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
+	case XenbusStateClosing:
+		xenbus_frontend_closed(dev);
+		break;
+	}
 }
 
 static struct xenbus_driver xen_9pfs_front_driver = {