[PATCH v5 0/7] Xen transport for 9pfs frontend driver

[PATCH v5 0/7] Xen transport for 9pfs frontend driver

[Stefano Stabellini]

Hi all,

This patch series implements a new transport for 9pfs, aimed at Xen
systems.

The transport is based on a traditional Xen frontend and backend drivers
pair. This patch series implements the frontend, which typically runs in
a regular unprivileged guest.

I also sent a series that implements the backend in userspace in QEMU,
which typically runs in Dom0 (but could also run in a another guest).

The frontend complies to the Xen transport for 9pfs specification
version 1, available here:

https://xenbits.xen.org/docs/unstable/misc/9pfs.html


Changes in v5:
- test priv->tag instead of ret
- run checkpatch.pl against the whole series, fix all issues
- set intf->ring_order appropriately
- use shorter link to 9pfs spec

Changes in v4:
- code style improvements
- use xenbus_read_unsigned when possible
- do not leak "versions"
- introduce BUILD_BUG_ON
- introduce rwlock to protect the xen_9pfs_devs list
- add review-by

Changes in v3:
- add full copyright header to trans_xen.c
- rename ring->ring to ring->data
- handle gnttab_grant_foreign_access errors
- remove ring->bytes
- wrap long lines
- add reviewed-by

Changes in v2:
- use XEN_PAGE_SHIFT instead of PAGE_SHIFT
- remove unnecessary initializations
- fix error paths
- fix memory allocations for 64K kernels
- simplify p9_xen_create and p9_xen_close
- use virt_XXX barriers
- set status = REQ_STATUS_ERROR inside the p9_xen_response loop
- add in-code comments


Stefano Stabellini (7):
      xen: import new ring macros in ring.h
      xen: introduce the header file for the Xen 9pfs transport protocol
      xen/9pfs: introduce Xen 9pfs transport driver
      xen/9pfs: connect to the backend
      xen/9pfs: send requests to the backend
      xen/9pfs: receive responses
      xen/9pfs: build 9pfs Xen transport driver

 include/xen/interface/io/9pfs.h |  42 ++++
 include/xen/interface/io/ring.h | 131 ++++++++++
 net/9p/Kconfig                  |   8 +
 net/9p/Makefile                 |   4 +
 net/9p/trans_xen.c              | 541 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 726 insertions(+)
 create mode 100644 include/xen/interface/io/9pfs.h
 create mode 100644 net/9p/trans_xen.c

Cheers,

Stefano

[PATCH v5 0/7] Xen transport for 9pfs frontend driver

[Stefano Stabellini]

Hi all,

This patch series implements a new transport for 9pfs, aimed at Xen
systems.

The transport is based on a traditional Xen frontend and backend drivers
pair. This patch series implements the frontend, which typically runs in
a regular unprivileged guest.

I also sent a series that implements the backend in userspace in QEMU,
which typically runs in Dom0 (but could also run in a another guest).

The frontend complies to the Xen transport for 9pfs specification
version 1, available here:

https://xenbits.xen.org/docs/unstable/misc/9pfs.html


Changes in v5:
- test priv->tag instead of ret
- run checkpatch.pl against the whole series, fix all issues
- set intf->ring_order appropriately
- use shorter link to 9pfs spec

Changes in v4:
- code style improvements
- use xenbus_read_unsigned when possible
- do not leak "versions"
- introduce BUILD_BUG_ON
- introduce rwlock to protect the xen_9pfs_devs list
- add review-by

Changes in v3:
- add full copyright header to trans_xen.c
- rename ring->ring to ring->data
- handle gnttab_grant_foreign_access errors
- remove ring->bytes
- wrap long lines
- add reviewed-by

Changes in v2:
- use XEN_PAGE_SHIFT instead of PAGE_SHIFT
- remove unnecessary initializations
- fix error paths
- fix memory allocations for 64K kernels
- simplify p9_xen_create and p9_xen_close
- use virt_XXX barriers
- set status = REQ_STATUS_ERROR inside the p9_xen_response loop
- add in-code comments


Stefano Stabellini (7):
      xen: import new ring macros in ring.h
      xen: introduce the header file for the Xen 9pfs transport protocol
      xen/9pfs: introduce Xen 9pfs transport driver
      xen/9pfs: connect to the backend
      xen/9pfs: send requests to the backend
      xen/9pfs: receive responses
      xen/9pfs: build 9pfs Xen transport driver

 include/xen/interface/io/9pfs.h |  42 ++++
 include/xen/interface/io/ring.h | 131 ++++++++++
 net/9p/Kconfig                  |   8 +
 net/9p/Makefile                 |   4 +
 net/9p/trans_xen.c              | 541 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 726 insertions(+)
 create mode 100644 include/xen/interface/io/9pfs.h
 create mode 100644 net/9p/trans_xen.c

Cheers,

Stefano

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 1/7] xen: import new ring macros in ring.h

[Stefano Stabellini]

Sync the ring.h file with upstream Xen, to introduce the new ring macros.
They will be used by the Xen transport for 9pfs.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: konrad.wilk@oracle.com
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
CC: groug@kaod.org

---
NB: The new macros have not been committed to Xen yet. Do not apply this
patch until they do.
---
---
 include/xen/interface/io/ring.h | 131 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/include/xen/interface/io/ring.h b/include/xen/interface/io/ring.h
index 21f4fbd..500e68d 100644
--- a/include/xen/interface/io/ring.h
+++ b/include/xen/interface/io/ring.h
@@ -283,4 +283,135 @@ struct __name##_back_ring {						\
     (_work_to_do) = RING_HAS_UNCONSUMED_RESPONSES(_r);			\
 } while (0)
 
+
+/*
+ * DEFINE_XEN_FLEX_RING_AND_INTF defines two monodirectional rings and
+ * functions to check if there is data on the ring, and to read and
+ * write to them.
+ *
+ * DEFINE_XEN_FLEX_RING is similar to DEFINE_XEN_FLEX_RING_AND_INTF, but
+ * does not define the indexes page. As different protocols can have
+ * extensions to the basic format, this macro allow them to define their
+ * own struct.
+ *
+ * XEN_FLEX_RING_SIZE
+ *   Convenience macro to calculate the size of one of the two rings
+ *   from the overall order.
+ *
+ * $NAME_mask
+ *   Function to apply the size mask to an index, to reduce the index
+ *   within the range [0-size].
+ *
+ * $NAME_read_packet
+ *   Function to read data from the ring. The amount of data to read is
+ *   specified by the "size" argument.
+ *
+ * $NAME_write_packet
+ *   Function to write data to the ring. The amount of data to write is
+ *   specified by the "size" argument.
+ *
+ * $NAME_get_ring_ptr
+ *   Convenience function that returns a pointer to read/write to the
+ *   ring at the right location.
+ *
+ * $NAME_data_intf
+ *   Indexes page, shared between frontend and backend. It also
+ *   contains the array of grant refs.
+ *
+ * $NAME_queued
+ *   Function to calculate how many bytes are currently on the ring,
+ *   ready to be read. It can also be used to calculate how much free
+ *   space is currently on the ring (ring_size - $NAME_queued()).
+ */
+#define XEN_FLEX_RING_SIZE(order)                                             \
+    (1UL << (order + XEN_PAGE_SHIFT - 1))
+
+#define DEFINE_XEN_FLEX_RING_AND_INTF(name)                                   \
+struct name##_data_intf {                                                     \
+    RING_IDX in_cons, in_prod;                                                \
+                                                                              \
+    uint8_t pad1[56];                                                         \
+                                                                              \
+    RING_IDX out_cons, out_prod;                                              \
+                                                                              \
+    uint8_t pad2[56];                                                         \
+                                                                              \
+    RING_IDX ring_order;                                                      \
+    grant_ref_t ref[];                                                        \
+};                                                                            \
+DEFINE_XEN_FLEX_RING(name);
+
+#define DEFINE_XEN_FLEX_RING(name)                                            \
+static inline RING_IDX name##_mask(RING_IDX idx, RING_IDX ring_size)          \
+{                                                                             \
+    return (idx & (ring_size - 1));                                           \
+}                                                                             \
+                                                                              \
+static inline RING_IDX name##_mask_order(RING_IDX idx, RING_IDX ring_order)   \
+{                                                                             \
+    return (idx & (XEN_FLEX_RING_SIZE(ring_order) - 1));                      \
+}                                                                             \
+                                                                              \
+static inline unsigned char* name##_get_ring_ptr(unsigned char *buf,          \
+                                                 RING_IDX idx,                \
+                                                 RING_IDX ring_order)         \
+{                                                                             \
+    return buf + name##_mask_order(idx, ring_order);                          \
+}                                                                             \
+                                                                              \
+static inline void name##_read_packet(const unsigned char *buf,               \
+        RING_IDX masked_prod, RING_IDX *masked_cons,                          \
+        RING_IDX ring_size, void *opaque, size_t size) {                      \
+    if (*masked_cons < masked_prod ||                                         \
+            size <= ring_size - *masked_cons) {                               \
+        memcpy(opaque, buf + *masked_cons, size);                             \
+    } else {                                                                  \
+        memcpy(opaque, buf + *masked_cons, ring_size - *masked_cons);         \
+        memcpy((unsigned char *)opaque + ring_size - *masked_cons, buf,       \
+                size - (ring_size - *masked_cons));                           \
+    }                                                                         \
+    *masked_cons = name##_mask(*masked_cons + size, ring_size);               \
+}                                                                             \
+                                                                              \
+static inline void name##_write_packet(unsigned char *buf,                    \
+        RING_IDX *masked_prod, RING_IDX masked_cons,                          \
+        RING_IDX ring_size, const void *opaque, size_t size) {                \
+    if (*masked_prod < masked_cons ||                                         \
+        size <= ring_size - *masked_prod) {                                   \
+        memcpy(buf + *masked_prod, opaque, size);                             \
+    } else {                                                                  \
+        memcpy(buf + *masked_prod, opaque, ring_size - *masked_prod);         \
+        memcpy(buf, (unsigned char *)opaque + (ring_size - *masked_prod),     \
+                size - (ring_size - *masked_prod));                           \
+    }                                                                         \
+    *masked_prod = name##_mask(*masked_prod + size, ring_size);               \
+}                                                                             \
+                                                                              \
+struct name##_data {                                                          \
+    unsigned char *in; /* half of the allocation */                           \
+    unsigned char *out; /* half of the allocation */                          \
+};                                                                            \
+                                                                              \
+                                                                              \
+static inline RING_IDX name##_queued(RING_IDX prod,                           \
+        RING_IDX cons, RING_IDX ring_size)                                    \
+{                                                                             \
+    RING_IDX size;                                                            \
+                                                                              \
+    if (prod == cons)                                                         \
+        return 0;                                                             \
+                                                                              \
+    prod = name##_mask(prod, ring_size);                                      \
+    cons = name##_mask(cons, ring_size);                                      \
+                                                                              \
+    if (prod == cons)                                                         \
+        return ring_size;                                                     \
+                                                                              \
+    if (prod > cons)                                                          \
+        size = prod - cons;                                                   \
+    else                                                                      \
+        size = ring_size - (cons - prod);                                     \
+    return size;                                                              \
+};
+
 #endif /* __XEN_PUBLIC_IO_RING_H__ */
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 2/7] xen: introduce the header file for the Xen 9pfs transport protocol

[Stefano Stabellini]

It uses the new ring.h macros to declare rings and interfaces.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: konrad.wilk@oracle.com
CC: boris.ostrovsky@oracle.com
CC: jgross@suse.com
CC: groug@kaod.org
---
 include/xen/interface/io/9pfs.h | 42 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 include/xen/interface/io/9pfs.h

diff --git a/include/xen/interface/io/9pfs.h b/include/xen/interface/io/9pfs.h
new file mode 100644
index 0000000..088345d
--- /dev/null
+++ b/include/xen/interface/io/9pfs.h
@@ -0,0 +1,42 @@
+/*
+ * 9pfs.h -- Xen 9PFS transport
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (C) 2017 Stefano Stabellini <stefano@aporeto.com>
+ */
+
+#ifndef __XEN_PUBLIC_IO_9PFS_H__
+#define __XEN_PUBLIC_IO_9PFS_H__
+
+#include "xen/interface/io/ring.h"
+
+/*
+ * See docs/misc/9pfs.markdown in xen.git for the full specification:
+ * https://xenbits.xen.org/docs/unstable/misc/9pfs.html
+ */
+struct xen_9pfs_header {
+	uint32_t size;
+	uint8_t id;
+	uint16_t tag;
+} __packed;
+
+DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
+
+#endif
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 3/7] xen/9pfs: introduce Xen 9pfs transport driver

[Stefano Stabellini]

Introduce the Xen 9pfs transport driver: add struct xenbus_driver to
register as a xenbus driver and add struct p9_trans_module to register
as v9fs driver.

All functions are empty stubs for now.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: groug@kaod.org
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 | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100644 net/9p/trans_xen.c

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
new file mode 100644
index 0000000..3d07260
--- /dev/null
+++ b/net/9p/trans_xen.c
@@ -0,0 +1,125 @@
+/*
+ * linux/fs/9p/trans_xen
+ *
+ * Xen transport layer.
+ *
+ * Copyright (C) 2017 by Stefano Stabellini <stefano@aporeto.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation; or, when distributed
+ * separately from the Linux kernel or incorporated into other
+ * software packages, subject to the following license:
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this source file (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy, modify,
+ * merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include <xen/events.h>
+#include <xen/grant_table.h>
+#include <xen/xen.h>
+#include <xen/xenbus.h>
+#include <xen/interface/io/9pfs.h>
+
+#include <linux/module.h>
+#include <net/9p/9p.h>
+#include <net/9p/client.h>
+#include <net/9p/transport.h>
+
+static int p9_xen_cancel(struct p9_client *client, struct p9_req_t *req)
+{
+	return 0;
+}
+
+static int p9_xen_create(struct p9_client *client, const char *addr, char *args)
+{
+	return 0;
+}
+
+static void p9_xen_close(struct p9_client *client)
+{
+}
+
+static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
+{
+	return 0;
+}
+
+static struct p9_trans_module p9_xen_trans = {
+	.name = "xen",
+	.maxsize = 1 << (XEN_9PFS_RING_ORDER + XEN_PAGE_SHIFT),
+	.def = 1,
+	.create = p9_xen_create,
+	.close = p9_xen_close,
+	.request = p9_xen_request,
+	.cancel = p9_xen_cancel,
+	.owner = THIS_MODULE,
+};
+
+static const struct xenbus_device_id xen_9pfs_front_ids[] = {
+	{ "9pfs" },
+	{ "" }
+};
+
+static int xen_9pfs_front_remove(struct xenbus_device *dev)
+{
+	return 0;
+}
+
+static int xen_9pfs_front_probe(struct xenbus_device *dev,
+				const struct xenbus_device_id *id)
+{
+	return 0;
+}
+
+static int xen_9pfs_front_resume(struct xenbus_device *dev)
+{
+	return 0;
+}
+
+static void xen_9pfs_front_changed(struct xenbus_device *dev,
+				   enum xenbus_state backend_state)
+{
+}
+
+static struct xenbus_driver xen_9pfs_front_driver = {
+	.ids = xen_9pfs_front_ids,
+	.probe = xen_9pfs_front_probe,
+	.remove = xen_9pfs_front_remove,
+	.resume = xen_9pfs_front_resume,
+	.otherend_changed = xen_9pfs_front_changed,
+};
+
+int p9_trans_xen_init(void)
+{
+	if (!xen_domain())
+		return -ENODEV;
+
+	pr_info("Initialising Xen transport for 9pfs\n");
+
+	v9fs_register_trans(&p9_xen_trans);
+	return xenbus_register_frontend(&xen_9pfs_front_driver);
+}
+module_init(p9_trans_xen_init);
+
+void p9_trans_xen_exit(void)
+{
+	v9fs_unregister_trans(&p9_xen_trans);
+	return xenbus_unregister_driver(&xen_9pfs_front_driver);
+}
+module_exit(p9_trans_xen_exit);
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Stefano Stabellini]

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
@@ -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 = {
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 5/7] xen/9pfs: send requests to the backend

[Stefano Stabellini]

Implement struct p9_trans_module create and close functions by looking
at the available Xen 9pfs frontend-backend connections. We don't expect
many frontend-backend connections, thus walking a list is OK.

Send requests to the backend by copying each request to one of the
available rings (each frontend-backend connection comes with multiple
rings). Handle the ring and notifications following the 9pfs
specification. If there are not enough free bytes on the ring for the
request, wait on the wait_queue: the backend will send a notification
after consuming more requests.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: groug@kaod.org
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 | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 89 insertions(+), 2 deletions(-)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index 5279ae0..e51edac 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -77,22 +77,109 @@ struct xen_9pfs_front_priv {
 static LIST_HEAD(xen_9pfs_devs);
 static DEFINE_RWLOCK(xen_9pfs_lock);
 
+/* We don't currently allow canceling of requests */
 static int p9_xen_cancel(struct p9_client *client, struct p9_req_t *req)
 {
-	return 0;
+	return 1;
 }
 
 static int p9_xen_create(struct p9_client *client, const char *addr, char *args)
 {
-	return 0;
+	struct xen_9pfs_front_priv *priv;
+
+	read_lock(&xen_9pfs_lock);
+	list_for_each_entry(priv, &xen_9pfs_devs, list) {
+		if (!strcmp(priv->tag, addr)) {
+			priv->client = client;
+			read_unlock(&xen_9pfs_lock);
+			return 0;
+		}
+	}
+	read_unlock(&xen_9pfs_lock);
+	return -EINVAL;
 }
 
 static void p9_xen_close(struct p9_client *client)
 {
+	struct xen_9pfs_front_priv *priv;
+
+	read_lock(&xen_9pfs_lock);
+	list_for_each_entry(priv, &xen_9pfs_devs, list) {
+		if (priv->client == client) {
+			priv->client = NULL;
+			read_unlock(&xen_9pfs_lock);
+			return;
+		}
+	}
+	read_unlock(&xen_9pfs_lock);
+}
+
+static int p9_xen_write_todo(struct xen_9pfs_dataring *ring, RING_IDX size)
+{
+	RING_IDX cons, prod;
+
+	cons = ring->intf->out_cons;
+	prod = ring->intf->out_prod;
+	virt_mb();
+
+	if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
+						 XEN_9PFS_RING_SIZE) >= size)
+		return 1;
+	else
+		return 0;
 }
 
 static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 {
+	struct xen_9pfs_front_priv *priv = NULL;
+	RING_IDX cons, prod, masked_cons, masked_prod;
+	unsigned long flags;
+	u32 size = p9_req->tc->size;
+	struct xen_9pfs_dataring *ring;
+	int num;
+
+	read_lock(&xen_9pfs_lock);
+	list_for_each_entry(priv, &xen_9pfs_devs, list) {
+		if (priv->client == client)
+			break;
+	}
+	read_unlock(&xen_9pfs_lock);
+	if (!priv || priv->client != client)
+		return -EINVAL;
+
+	num = p9_req->tc->tag % priv->num_rings;
+	ring = &priv->rings[num];
+
+again:
+	while (wait_event_interruptible(ring->wq,
+					p9_xen_write_todo(ring, size) > 0) != 0)
+		;
+
+	spin_lock_irqsave(&ring->lock, flags);
+	cons = ring->intf->out_cons;
+	prod = ring->intf->out_prod;
+	virt_mb();
+
+	if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
+						 XEN_9PFS_RING_SIZE) < size) {
+		spin_unlock_irqrestore(&ring->lock, flags);
+		goto again;
+	}
+
+	masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
+	masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
+
+	xen_9pfs_write_packet(ring->data.out,
+			      &masked_prod, masked_cons,
+			      XEN_9PFS_RING_SIZE, p9_req->tc->sdata, size);
+
+	p9_req->status = REQ_STATUS_SENT;
+	virt_wmb();			/* write ring before updating pointer */
+	prod += size;
+	ring->intf->out_prod = prod;
+	spin_unlock_irqrestore(&ring->lock, flags);
+	notify_remote_via_irq(ring->irq);
+
 	return 0;
 }
 
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 6/7] xen/9pfs: receive responses

[Stefano Stabellini]

Upon receiving a notification from the backend, schedule the
p9_xen_response work_struct. p9_xen_response checks if any responses are
available, if so, it reads them one by one, calling p9_client_cb to send
them up to the 9p layer (p9_client_cb completes the request). Handle the
ring following the Xen 9pfs specification.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: groug@kaod.org
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 | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
index e51edac..4ffe250 100644
--- a/net/9p/trans_xen.c
+++ b/net/9p/trans_xen.c
@@ -185,6 +185,62 @@ static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
 
 static void p9_xen_response(struct work_struct *work)
 {
+	struct xen_9pfs_front_priv *priv;
+	struct xen_9pfs_dataring *ring;
+	RING_IDX cons, prod, masked_cons, masked_prod;
+	struct xen_9pfs_header h;
+	struct p9_req_t *req;
+	int status;
+
+	ring = container_of(work, struct xen_9pfs_dataring, work);
+	priv = ring->priv;
+
+	while (1) {
+		cons = ring->intf->in_cons;
+		prod = ring->intf->in_prod;
+		virt_rmb();
+
+		if (xen_9pfs_queued(prod, cons, XEN_9PFS_RING_SIZE) <
+		    sizeof(h)) {
+			notify_remote_via_irq(ring->irq);
+			return;
+		}
+
+		masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
+		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
+
+		/* First, read just the header */
+		xen_9pfs_read_packet(ring->data.in,
+				     masked_prod, &masked_cons,
+				     XEN_9PFS_RING_SIZE, &h, sizeof(h));
+
+		req = p9_tag_lookup(priv->client, h.tag);
+		if (!req || req->status != REQ_STATUS_SENT) {
+			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
+			cons += h.size;
+			virt_mb();
+			ring->intf->in_cons = cons;
+			continue;
+		}
+
+		memcpy(req->rc, &h, sizeof(h));
+		req->rc->offset = 0;
+
+		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
+		/* Then, read the whole packet (including the header) */
+		xen_9pfs_read_packet(ring->data.in,
+				     masked_prod, &masked_cons,
+				     XEN_9PFS_RING_SIZE, req->rc->sdata, h.size);
+
+		virt_mb();
+		cons += h.size;
+		ring->intf->in_cons = cons;
+
+		status = (req->status != REQ_STATUS_ERROR) ?
+			REQ_STATUS_RCVD : REQ_STATUS_ERROR;
+
+		p9_client_cb(priv->client, req, status);
+	}
 }
 
 static irqreturn_t xen_9pfs_front_event_handler(int irq, void *r)
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v5 7/7] xen/9pfs: build 9pfs Xen transport driver

[Stefano Stabellini]

This patch adds a Kconfig option and Makefile support for building the
9pfs Xen driver.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
Reviewed-by: Juergen Gross <jgross@suse.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/Kconfig  | 8 ++++++++
 net/9p/Makefile | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/net/9p/Kconfig b/net/9p/Kconfig
index a75174a..3f286f1 100644
--- a/net/9p/Kconfig
+++ b/net/9p/Kconfig
@@ -22,6 +22,14 @@ config NET_9P_VIRTIO
 	  This builds support for a transports between
 	  guest partitions and a host partition.
 
+config NET_9P_XEN
+	depends on XEN
+	tristate "9P Xen Transport"
+	help
+	  This builds support for a transport for 9pfs between
+	  two Xen domains.
+
+
 config NET_9P_RDMA
 	depends on INET && INFINIBAND && INFINIBAND_ADDR_TRANS
 	tristate "9P RDMA Transport (Experimental)"
diff --git a/net/9p/Makefile b/net/9p/Makefile
index a0874cc..697ea7c 100644
--- a/net/9p/Makefile
+++ b/net/9p/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_NET_9P) := 9pnet.o
+obj-$(CONFIG_NET_9P_XEN) += 9pnet_xen.o
 obj-$(CONFIG_NET_9P_VIRTIO) += 9pnet_virtio.o
 obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o
 
@@ -14,5 +15,8 @@ obj-$(CONFIG_NET_9P_RDMA) += 9pnet_rdma.o
 9pnet_virtio-objs := \
 	trans_virtio.o \
 
+9pnet_xen-objs := \
+	trans_xen.o \
+
 9pnet_rdma-objs := \
 	trans_rdma.o \
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v5 3/7] xen/9pfs: introduce Xen 9pfs transport driver

[Juergen Gross]

On 20/03/17 18:59, Stefano Stabellini wrote:
> Introduce the Xen 9pfs transport driver: add struct xenbus_driver to
> register as a xenbus driver and add struct p9_trans_module to register
> as v9fs driver.
> 
> All functions are empty stubs for now.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: groug@kaod.org
> 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

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Juergen Gross]

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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v5 5/7] xen/9pfs: send requests to the backend

[Juergen Gross]

On 20/03/17 18:59, Stefano Stabellini wrote:
> Implement struct p9_trans_module create and close functions by looking
> at the available Xen 9pfs frontend-backend connections. We don't expect
> many frontend-backend connections, thus walking a list is OK.
> 
> Send requests to the backend by copying each request to one of the
> available rings (each frontend-backend connection comes with multiple
> rings). Handle the ring and notifications following the 9pfs
> specification. If there are not enough free bytes on the ring for the
> request, wait on the wait_queue: the backend will send a notification
> after consuming more requests.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: groug@kaod.org
> 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 | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 89 insertions(+), 2 deletions(-)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index 5279ae0..e51edac 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c

> +static int p9_xen_write_todo(struct xen_9pfs_dataring *ring, RING_IDX size)
> +{
> +	RING_IDX cons, prod;
> +
> +	cons = ring->intf->out_cons;
> +	prod = ring->intf->out_prod;
> +	virt_mb();
> +
> +	if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
> +						 XEN_9PFS_RING_SIZE) >= size)
> +		return 1;
> +	else
> +		return 0;
>  }

Make return type of function bool and use:

return XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
                                            XEN_9PFS_RING_SIZE) >= size;

and further below ...

>  
>  static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
>  {
> +	struct xen_9pfs_front_priv *priv = NULL;
> +	RING_IDX cons, prod, masked_cons, masked_prod;
> +	unsigned long flags;
> +	u32 size = p9_req->tc->size;
> +	struct xen_9pfs_dataring *ring;
> +	int num;
> +
> +	read_lock(&xen_9pfs_lock);
> +	list_for_each_entry(priv, &xen_9pfs_devs, list) {
> +		if (priv->client == client)
> +			break;
> +	}
> +	read_unlock(&xen_9pfs_lock);
> +	if (!priv || priv->client != client)
> +		return -EINVAL;
> +
> +	num = p9_req->tc->tag % priv->num_rings;
> +	ring = &priv->rings[num];
> +
> +again:
> +	while (wait_event_interruptible(ring->wq,
> +					p9_xen_write_todo(ring, size) > 0) != 0)

... you can omit the "> 0".


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v5 6/7] xen/9pfs: receive responses

[Juergen Gross]

On 20/03/17 18:59, Stefano Stabellini wrote:
> Upon receiving a notification from the backend, schedule the
> p9_xen_response work_struct. p9_xen_response checks if any responses are
> available, if so, it reads them one by one, calling p9_client_cb to send
> them up to the 9p layer (p9_client_cb completes the request). Handle the
> ring following the Xen 9pfs specification.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: groug@kaod.org
> 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

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v5 5/7] xen/9pfs: send requests to the backend

[Stefano Stabellini]

On Tue, 21 Mar 2017, Juergen Gross wrote:
> On 20/03/17 18:59, Stefano Stabellini wrote:
> > Implement struct p9_trans_module create and close functions by looking
> > at the available Xen 9pfs frontend-backend connections. We don't expect
> > many frontend-backend connections, thus walking a list is OK.
> > 
> > Send requests to the backend by copying each request to one of the
> > available rings (each frontend-backend connection comes with multiple
> > rings). Handle the ring and notifications following the 9pfs
> > specification. If there are not enough free bytes on the ring for the
> > request, wait on the wait_queue: the backend will send a notification
> > after consuming more requests.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > CC: groug@kaod.org
> > 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 | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 89 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> > index 5279ae0..e51edac 100644
> > --- a/net/9p/trans_xen.c
> > +++ b/net/9p/trans_xen.c
> 
> > +static int p9_xen_write_todo(struct xen_9pfs_dataring *ring, RING_IDX size)
> > +{
> > +	RING_IDX cons, prod;
> > +
> > +	cons = ring->intf->out_cons;
> > +	prod = ring->intf->out_prod;
> > +	virt_mb();
> > +
> > +	if (XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
> > +						 XEN_9PFS_RING_SIZE) >= size)
> > +		return 1;
> > +	else
> > +		return 0;
> >  }
> 
> Make return type of function bool and use:
> 
> return XEN_9PFS_RING_SIZE - xen_9pfs_queued(prod, cons,
>                                             XEN_9PFS_RING_SIZE) >= size;
> 
> and further below ...

Good idea


> >  
> >  static int p9_xen_request(struct p9_client *client, struct p9_req_t *p9_req)
> >  {
> > +	struct xen_9pfs_front_priv *priv = NULL;
> > +	RING_IDX cons, prod, masked_cons, masked_prod;
> > +	unsigned long flags;
> > +	u32 size = p9_req->tc->size;
> > +	struct xen_9pfs_dataring *ring;
> > +	int num;
> > +
> > +	read_lock(&xen_9pfs_lock);
> > +	list_for_each_entry(priv, &xen_9pfs_devs, list) {
> > +		if (priv->client == client)
> > +			break;
> > +	}
> > +	read_unlock(&xen_9pfs_lock);
> > +	if (!priv || priv->client != client)
> > +		return -EINVAL;
> > +
> > +	num = p9_req->tc->tag % priv->num_rings;
> > +	ring = &priv->rings[num];
> > +
> > +again:
> > +	while (wait_event_interruptible(ring->wq,
> > +					p9_xen_write_todo(ring, size) > 0) != 0)
> 
> ... you can omit the "> 0".

I'll do that

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Stefano Stabellini]

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
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel