* [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
@ 2017-03-14 22:18 Stefano Stabellini
2017-03-14 22:18 ` [PATCH 2/3] Introduce the Xen 9pfs transport header Stefano Stabellini
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-14 22:18 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, sstabellini
This patch introduces macros, structs and functions to handle rings in
the format described by docs/misc/pvcalls.markdown and
docs/misc/9pfs.markdown. The index page (struct __name##_data_intf)
contains the indexes and the grant refs to setup two rings.
Indexes page
+----------------------+
|@0 $NAME_data_intf: |
|@76: ring_order = 1 |
|@80: ref[0]+ |
|@84: ref[1]+ |
| | |
| | |
+----------------------+
|
v (data ring)
+-------+-----------+
| @0->4098: in |
| ref[0] |
|-------------------|
| @4099->8196: out |
| ref[1] |
+-------------------+
$NAME_read_packet and $NAME_write_packet are provided to read or write
any data struct from/to the ring. In pvcalls, they are unused. In xen
9pfs, they are used to read or write the 9pfs header. In other protocols
they could be used to read/write the whole request structure. See
docs/misc/9pfs.markdown:Ring Usage to learn how to check how much data
is on the ring, and how to handle notifications.
There is a ring_size parameter to most functions so that protocols using
these macros don't have to have a statically defined ring order at build
time. In pvcalls for example, each new ring could have a different
order.
These macros don't help you share the indexes page or the event channels
needed for notifications. You can do that with other out of band
mechanisms, such as xenstore or another ring.
It is not possible to use a macro to define another macro with a
variable name. For this reason, this patch introduces static inline
functions instead, that are not C89 compliant. Additionally, the macro
defines a struct with a variable sized array, which is also not C89
compliant.
Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: konrad.wilk@oracle.com
---
Changes in v4:
- remove packet_t, use void* and size instead
Changes in v3:
- mention C89 compliance breakages
- constify parameters
- use unsigned chars for buffers
- add two macros, one doesn't define the struct
Changes in v2:
- fix typo
- remove leading underscores from names
- use UL
- do not parenthesize parameters
- code readability improvements
Give a look at the following branch to see how they are used with
pvcalls and xen-9pfs (the drivers are still work in progress):
git://git.kernel.org/pub/scm/linux/kernel/git/sstabellini/xen.git 9pfs-async-v7
---
---
xen/include/public/io/ring.h | 131 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h
index 801c0da..8ac9ca3 100644
--- a/xen/include/public/io/ring.h
+++ b/xen/include/public/io/ring.h
@@ -313,6 +313,137 @@ typedef struct __name##_back_ring __name##_back_ring_t
(_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 + 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] Introduce the Xen 9pfs transport header
2017-03-14 22:18 [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
@ 2017-03-14 22:18 ` Stefano Stabellini
2017-03-17 15:03 ` Konrad Rzeszutek Wilk
2017-03-14 22:18 ` [PATCH 3/3] Introduce the pvcalls header Stefano Stabellini
2017-03-17 15:04 ` [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
2 siblings, 1 reply; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-14 22:18 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, sstabellini
Define the ring according to the protocol specification, using the new
DEFINE_XEN_FLEX_RING_AND_INTF macro.
Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: konrad.wilk@oracle.com
---
xen/include/public/io/9pfs.h | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
create mode 100644 xen/include/public/io/9pfs.h
diff --git a/xen/include/public/io/9pfs.h b/xen/include/public/io/9pfs.h
new file mode 100644
index 0000000..b38ee66
--- /dev/null
+++ b/xen/include/public/io/9pfs.h
@@ -0,0 +1,42 @@
+/*
+ * 9pfs.h -- Xen 9PFS transport
+ *
+ * Refer to docs/misc/9pfs.markdown for the specification
+ *
+ * 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 "ring.h"
+
+struct xen_9pfs_header {
+ uint32_t size;
+ uint8_t id;
+ uint16_t tag;
+} __attribute__((packed));
+
+#define XEN_9PFS_RING_ORDER 6
+#define XEN_9PFS_RING_SIZE XEN_FLEX_RING_SIZE(XEN_9PFS_RING_ORDER)
+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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] Introduce the pvcalls header
2017-03-14 22:18 [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
2017-03-14 22:18 ` [PATCH 2/3] Introduce the Xen 9pfs transport header Stefano Stabellini
@ 2017-03-14 22:18 ` Stefano Stabellini
2017-03-17 15:00 ` Konrad Rzeszutek Wilk
2017-03-17 15:04 ` [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
2 siblings, 1 reply; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-14 22:18 UTC (permalink / raw)
To: xen-devel; +Cc: Stefano Stabellini, sstabellini
Define the ring and request and response structs according to the
specification. Use the new DEFINE_XEN_FLEX_RING macro.
Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: konrad.wilk@oracle.com
---
xen/include/public/io/pvcalls.h | 138 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
create mode 100644 xen/include/public/io/pvcalls.h
diff --git a/xen/include/public/io/pvcalls.h b/xen/include/public/io/pvcalls.h
new file mode 100644
index 0000000..074f52f
--- /dev/null
+++ b/xen/include/public/io/pvcalls.h
@@ -0,0 +1,138 @@
+/*
+ * pvcalls.h -- Xen PV Calls Protocol
+ *
+ * Refer to docs/misc/pvcalls.markdown for the specification
+ *
+ * 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_PVCALLS_H__
+#define __XEN_PUBLIC_IO_PVCALLS_H__
+
+#include "ring.h"
+
+struct pvcalls_data_intf {
+ RING_IDX in_cons, in_prod, in_error;
+
+ uint8_t pad1[52];
+
+ RING_IDX out_cons, out_prod, out_error;
+
+ uint8_t pad2[52];
+
+ RING_IDX ring_order;
+ grant_ref_t ref[];
+};
+DEFINE_XEN_FLEX_RING(pvcalls);
+
+#define PVCALLS_SOCKET 0
+#define PVCALLS_CONNECT 1
+#define PVCALLS_RELEASE 2
+#define PVCALLS_BIND 3
+#define PVCALLS_LISTEN 4
+#define PVCALLS_ACCEPT 5
+#define PVCALLS_POLL 6
+
+struct xen_pvcalls_request {
+ uint32_t req_id; /* private to guest, echoed in response */
+ uint32_t cmd; /* command to execute */
+ union {
+ struct xen_pvcalls_socket {
+ uint64_t id;
+ uint32_t domain;
+ uint32_t type;
+ uint32_t protocol;
+ } socket;
+ struct xen_pvcalls_connect {
+ uint64_t id;
+ uint8_t addr[28];
+ uint32_t len;
+ uint32_t flags;
+ grant_ref_t ref;
+ uint32_t evtchn;
+ } connect;
+ struct xen_pvcalls_release {
+ uint64_t id;
+ uint8_t reuse;
+ } release;
+ struct xen_pvcalls_bind {
+ uint64_t id;
+ uint8_t addr[28];
+ uint32_t len;
+ } bind;
+ struct xen_pvcalls_listen {
+ uint64_t id;
+ uint32_t backlog;
+ } listen;
+ struct xen_pvcalls_accept {
+ uint64_t id;
+ uint64_t id_new;
+ grant_ref_t ref;
+ uint32_t evtchn;
+ } accept;
+ struct xen_pvcalls_poll {
+ uint64_t id;
+ } poll;
+ /* dummy member to force sizeof(struct xen_pvcalls_request)
+ * to match across archs */
+ struct xen_pvcalls_dummy {
+ uint8_t dummy[56];
+ } dummy;
+ } u;
+};
+
+struct xen_pvcalls_response {
+ uint32_t req_id;
+ uint32_t cmd;
+ int32_t ret;
+ uint32_t pad;
+ union {
+ struct _xen_pvcalls_socket {
+ uint64_t id;
+ } socket;
+ struct _xen_pvcalls_connect {
+ uint64_t id;
+ } connect;
+ struct _xen_pvcalls_release {
+ uint64_t id;
+ } release;
+ struct _xen_pvcalls_bind {
+ uint64_t id;
+ } bind;
+ struct _xen_pvcalls_listen {
+ uint64_t id;
+ } listen;
+ struct _xen_pvcalls_accept {
+ uint64_t id;
+ } accept;
+ struct _xen_pvcalls_poll {
+ uint64_t id;
+ } poll;
+ struct _xen_pvcalls_dummy {
+ uint8_t dummy[8];
+ } dummy;
+ } u;
+};
+
+DEFINE_RING_TYPES(xen_pvcalls, struct xen_pvcalls_request,
+ struct xen_pvcalls_response);
+
+#endif
--
1.9.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] Introduce the pvcalls header
2017-03-14 22:18 ` [PATCH 3/3] Introduce the pvcalls header Stefano Stabellini
@ 2017-03-17 15:00 ` Konrad Rzeszutek Wilk
2017-03-17 21:09 ` Stefano Stabellini
0 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-03-17 15:00 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel
On Tue, Mar 14, 2017 at 03:18:36PM -0700, Stefano Stabellini wrote:
> Define the ring and request and response structs according to the
> specification. Use the new DEFINE_XEN_FLEX_RING macro.
Don't want to copy-n-paste some of the XenBus entries? Or at least
point to them (the docs)?
May I also suggest you add the editor block at the end of the file?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] Introduce the Xen 9pfs transport header
2017-03-14 22:18 ` [PATCH 2/3] Introduce the Xen 9pfs transport header Stefano Stabellini
@ 2017-03-17 15:03 ` Konrad Rzeszutek Wilk
2017-03-17 22:04 ` Stefano Stabellini
0 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-03-17 15:03 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel
On Tue, Mar 14, 2017 at 03:18:35PM -0700, Stefano Stabellini wrote:
> Define the ring according to the protocol specification, using the new
> DEFINE_XEN_FLEX_RING_AND_INTF macro.
>
There is a bit of 9pfs code being posted. Is this patch still
up-to-date with that? I am going to assume yes, in which case
see below
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: konrad.wilk@oracle.com
> ---
> xen/include/public/io/9pfs.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
> create mode 100644 xen/include/public/io/9pfs.h
>
> diff --git a/xen/include/public/io/9pfs.h b/xen/include/public/io/9pfs.h
> new file mode 100644
> index 0000000..b38ee66
> --- /dev/null
> +++ b/xen/include/public/io/9pfs.h
> @@ -0,0 +1,42 @@
> +/*
> + * 9pfs.h -- Xen 9PFS transport
> + *
> + * Refer to docs/misc/9pfs.markdown for the specification
> + *
> + * 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 "ring.h"
> +
> +struct xen_9pfs_header {
> + uint32_t size;
> + uint8_t id;
> + uint16_t tag;
> +} __attribute__((packed));
I think the Xen headers are not to have __packed__ on the them.
Perhaps you can make this work by using #pragma?
> +
> +#define XEN_9PFS_RING_ORDER 6
Perhaps a bit details? The specs mentions the max and min but .. this
6 value is rather arbitrary?
> +#define XEN_9PFS_RING_SIZE XEN_FLEX_RING_SIZE(XEN_9PFS_RING_ORDER)
> +DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
> +
> +#endif
Pls add the bottom of the patch the editor configuration block.
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
2017-03-14 22:18 [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
2017-03-14 22:18 ` [PATCH 2/3] Introduce the Xen 9pfs transport header Stefano Stabellini
2017-03-14 22:18 ` [PATCH 3/3] Introduce the pvcalls header Stefano Stabellini
@ 2017-03-17 15:04 ` Konrad Rzeszutek Wilk
2017-03-17 21:00 ` Stefano Stabellini
2 siblings, 1 reply; 9+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-03-17 15:04 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel
On Tue, Mar 14, 2017 at 03:18:34PM -0700, Stefano Stabellini wrote:
> This patch introduces macros, structs and functions to handle rings in
> the format described by docs/misc/pvcalls.markdown and
> docs/misc/9pfs.markdown. The index page (struct __name##_data_intf)
> contains the indexes and the grant refs to setup two rings.
>
> Indexes page
> +----------------------+
> |@0 $NAME_data_intf: |
> |@76: ring_order = 1 |
> |@80: ref[0]+ |
> |@84: ref[1]+ |
> | | |
> | | |
> +----------------------+
> |
> v (data ring)
> +-------+-----------+
> | @0->4098: in |
> | ref[0] |
> |-------------------|
> | @4099->8196: out |
> | ref[1] |
> +-------------------+
>
> $NAME_read_packet and $NAME_write_packet are provided to read or write
> any data struct from/to the ring. In pvcalls, they are unused. In xen
> 9pfs, they are used to read or write the 9pfs header. In other protocols
> they could be used to read/write the whole request structure. See
> docs/misc/9pfs.markdown:Ring Usage to learn how to check how much data
> is on the ring, and how to handle notifications.
>
> There is a ring_size parameter to most functions so that protocols using
> these macros don't have to have a statically defined ring order at build
> time. In pvcalls for example, each new ring could have a different
> order.
>
> These macros don't help you share the indexes page or the event channels
> needed for notifications. You can do that with other out of band
> mechanisms, such as xenstore or another ring.
>
> It is not possible to use a macro to define another macro with a
> variable name. For this reason, this patch introduces static inline
> functions instead, that are not C89 compliant. Additionally, the macro
> defines a struct with a variable sized array, which is also not C89
> compliant.
>
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: konrad.wilk@oracle.com
>
> ---
> Changes in v4:
> - remove packet_t, use void* and size instead
There seems to be quite a bit of lively discussion on the implementation
part. Are there any changes to this file based on that discussion?
Thanks.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
2017-03-17 15:04 ` [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
@ 2017-03-17 21:00 ` Stefano Stabellini
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-17 21:00 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini
On Fri, 17 Mar 2017, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 14, 2017 at 03:18:34PM -0700, Stefano Stabellini wrote:
> > This patch introduces macros, structs and functions to handle rings in
> > the format described by docs/misc/pvcalls.markdown and
> > docs/misc/9pfs.markdown. The index page (struct __name##_data_intf)
> > contains the indexes and the grant refs to setup two rings.
> >
> > Indexes page
> > +----------------------+
> > |@0 $NAME_data_intf: |
> > |@76: ring_order = 1 |
> > |@80: ref[0]+ |
> > |@84: ref[1]+ |
> > | | |
> > | | |
> > +----------------------+
> > |
> > v (data ring)
> > +-------+-----------+
> > | @0->4098: in |
> > | ref[0] |
> > |-------------------|
> > | @4099->8196: out |
> > | ref[1] |
> > +-------------------+
> >
> > $NAME_read_packet and $NAME_write_packet are provided to read or write
> > any data struct from/to the ring. In pvcalls, they are unused. In xen
> > 9pfs, they are used to read or write the 9pfs header. In other protocols
> > they could be used to read/write the whole request structure. See
> > docs/misc/9pfs.markdown:Ring Usage to learn how to check how much data
> > is on the ring, and how to handle notifications.
> >
> > There is a ring_size parameter to most functions so that protocols using
> > these macros don't have to have a statically defined ring order at build
> > time. In pvcalls for example, each new ring could have a different
> > order.
> >
> > These macros don't help you share the indexes page or the event channels
> > needed for notifications. You can do that with other out of band
> > mechanisms, such as xenstore or another ring.
> >
> > It is not possible to use a macro to define another macro with a
> > variable name. For this reason, this patch introduces static inline
> > functions instead, that are not C89 compliant. Additionally, the macro
> > defines a struct with a variable sized array, which is also not C89
> > compliant.
> >
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: konrad.wilk@oracle.com
> >
> > ---
> > Changes in v4:
> > - remove packet_t, use void* and size instead
>
> There seems to be quite a bit of lively discussion on the implementation
> part. Are there any changes to this file based on that discussion?
No changes, or requests for changes, to the macros so far. But I made
clear that these macros need to be accepted in xen.git first, before
they can be used elsewhere.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] Introduce the pvcalls header
2017-03-17 15:00 ` Konrad Rzeszutek Wilk
@ 2017-03-17 21:09 ` Stefano Stabellini
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-17 21:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini
On Fri, 17 Mar 2017, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 14, 2017 at 03:18:36PM -0700, Stefano Stabellini wrote:
> > Define the ring and request and response structs according to the
> > specification. Use the new DEFINE_XEN_FLEX_RING macro.
>
> Don't want to copy-n-paste some of the XenBus entries? Or at least
> point to them (the docs)?
I'll add a link to the doc as you suggested in the Linux thread
> May I also suggest you add the editor block at the end of the file?
I'll do
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] Introduce the Xen 9pfs transport header
2017-03-17 15:03 ` Konrad Rzeszutek Wilk
@ 2017-03-17 22:04 ` Stefano Stabellini
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Stabellini @ 2017-03-17 22:04 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini
On Fri, 17 Mar 2017, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 14, 2017 at 03:18:35PM -0700, Stefano Stabellini wrote:
> > Define the ring according to the protocol specification, using the new
> > DEFINE_XEN_FLEX_RING_AND_INTF macro.
> >
>
> There is a bit of 9pfs code being posted. Is this patch still
> up-to-date with that? I am going to assume yes, in which case
> see below
Yes, it is. The only change so far is that QEMU requested to introduce a
common struct instead of xen_9pfs_header, as it is used by virtio too.
It makes sense. And it makes sense for the Xen header to keep the
definition of struct xen_9pfs_header, I think (unless
__attribute__((packed)) creates too much trouble).
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: konrad.wilk@oracle.com
> > ---
> > xen/include/public/io/9pfs.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> > create mode 100644 xen/include/public/io/9pfs.h
> >
> > diff --git a/xen/include/public/io/9pfs.h b/xen/include/public/io/9pfs.h
> > new file mode 100644
> > index 0000000..b38ee66
> > --- /dev/null
> > +++ b/xen/include/public/io/9pfs.h
> > @@ -0,0 +1,42 @@
> > +/*
> > + * 9pfs.h -- Xen 9PFS transport
> > + *
> > + * Refer to docs/misc/9pfs.markdown for the specification
> > + *
> > + * 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 "ring.h"
> > +
> > +struct xen_9pfs_header {
> > + uint32_t size;
> > + uint8_t id;
> > + uint16_t tag;
> > +} __attribute__((packed));
>
> I think the Xen headers are not to have __packed__ on the them.
> Perhaps you can make this work by using #pragma?
Something like:
#pragma pack(push)
#pragma pack(1)
struct xen_9pfs_header {
uint32_t size;
uint8_t id;
uint16_t tag;
};
#pragma pack(pop)
?
Ugly, but I can do that.
> > +
> > +#define XEN_9PFS_RING_ORDER 6
>
>
> Perhaps a bit details? The specs mentions the max and min but .. this
> 6 value is rather arbitrary?
Yes, it is arbitrary (although it is chosen based on benchmarks and
common sense). I introduced it for simplicity so that we don't actually
have to handle rings of multiple sizes in the backend. I'll remove it
and make the order dynamic, it's not difficult actually.
> > +#define XEN_9PFS_RING_SIZE XEN_FLEX_RING_SIZE(XEN_9PFS_RING_ORDER)
> > +DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
> > +
> > +#endif
>
> Pls add the bottom of the patch the editor configuration block.
OK
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-03-17 22:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-14 22:18 [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
2017-03-14 22:18 ` [PATCH 2/3] Introduce the Xen 9pfs transport header Stefano Stabellini
2017-03-17 15:03 ` Konrad Rzeszutek Wilk
2017-03-17 22:04 ` Stefano Stabellini
2017-03-14 22:18 ` [PATCH 3/3] Introduce the pvcalls header Stefano Stabellini
2017-03-17 15:00 ` Konrad Rzeszutek Wilk
2017-03-17 21:09 ` Stefano Stabellini
2017-03-17 15:04 ` [PATCH 1/3] [RESEND v4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
2017-03-17 21:00 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.