From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Stefano Stabellini <stefano@aporeto.com>,
xen-devel@lists.xenproject.org, JBeulich@suse.com
Subject: Re: [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
Date: Tue, 4 Apr 2017 15:49:00 -0400 [thread overview]
Message-ID: <20170404194900.GB29986@char.us.oracle.com> (raw)
In-Reply-To: <1490987719-25452-1-git-send-email-sstabellini@kernel.org>
On Fri, Mar 31, 2017 at 12:15:16PM -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->4095: in |
> | ref[0] |
> |-------------------|
> | @4096->8191: 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
> CC: JBeulich@suse.com
> ---
> xen/include/public/io/ring.h | 151 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 151 insertions(+)
>
> diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h
> index 801c0da..7b7794c 100644
> --- a/xen/include/public/io/ring.h
> +++ b/xen/include/public/io/ring.h
> @@ -27,6 +27,21 @@
> #ifndef __XEN_PUBLIC_IO_RING_H__
> #define __XEN_PUBLIC_IO_RING_H__
>
> +/*
> + * When #include'ing this header, you need to provide the following
> + * declaration upfront:
> + * - standard integers types (uint8_t, uint16_t, etc)
> + * They are provided by stdint.h of the standard headers.
> + *
> + * In addition, if you intend to use the FLEX macros, you also need to
> + * provide the following, before invoking the FLEX macros:
> + * - size_t
> + * - memcpy
> + * - grant_ref_t
> + * These declarations are provided by string.h of the standard headers,
> + * and grant_table.h from the Xen public headers.
> + */
> +
> #include "../xen-compat.h"
>
> #if __XEN_INTERFACE_VERSION__ < 0x00030208
> @@ -313,6 +328,142 @@ 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()).
s/ring_size/XEN_FLEX_RING_SIZE/ ?
> + */
> +
> +#define XEN_PAGE_SHIFT 12
Perhaps a comment explaining why you have that? It looks very architecture
dependent?
Or perhaps you could do with
#ifndef XEN_PAGE_SHIFT
/*
* Hypercall interface only supports only 4Kb page unless the architecture
* overrides it.
*/
#define XEN_PAGE_SHIFT 12
#endif
?
> +#define XEN_FLEX_RING_SIZE(order) \
> + (1UL << ((order) + XEN_PAGE_SHIFT - 1))
> +
> +#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 unsigned char *name##_get_ring_ptr(unsigned char *buf, \
> + RING_IDX idx, \
> + RING_IDX ring_size) \
> +{ \
> + return buf + name##_mask(idx, ring_size); \
> +} \
> + \
> +static inline void name##_read_packet(void *opaque, \
> + const unsigned char *buf, \
> + size_t size, \
> + RING_IDX masked_prod, \
> + RING_IDX *masked_cons, \
> + RING_IDX ring_size) \
> +{ \
> + if (*masked_cons < masked_prod || \
> + size <= ring_size - *masked_cons) { \
'size <= ..' is not aligned here while it is in _write_packet?
Besides that it looks fine to me.
> + 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, \
> + const void *opaque, \
> + size_t size, \
> + RING_IDX *masked_prod, \
> + RING_IDX masked_cons, \
> + RING_IDX ring_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); \
> +} \
> + \
> +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; \
> +} \
> + \
> +struct name##_data { \
> + unsigned char *in; /* half of the allocation */ \
> + unsigned char *out; /* half of the allocation */ \
> +}
> +
> +#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)
> +
> #endif /* __XEN_PUBLIC_IO_RING_H__ */
>
> /*
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
next prev parent reply other threads:[~2017-04-04 19:49 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-31 19:15 [PATCH v8 0/4] new ring macros, 9pfs and pvcalls headers Stefano Stabellini
2017-03-31 19:15 ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
2017-03-31 19:15 ` [PATCH v8 2/4] xen: introduce a C99 headers check Stefano Stabellini
2017-04-03 8:15 ` Jan Beulich
2017-04-03 18:25 ` Stefano Stabellini
2017-04-04 7:59 ` Jan Beulich
2017-04-05 0:12 ` Stefano Stabellini
2017-03-31 19:15 ` [PATCH v8 3/4] Introduce the Xen 9pfs transport header Stefano Stabellini
2017-04-04 19:50 ` Konrad Rzeszutek Wilk
2017-03-31 19:15 ` [PATCH v8 4/4] Introduce the pvcalls header Stefano Stabellini
2017-04-04 19:50 ` Konrad Rzeszutek Wilk
2017-04-04 19:49 ` Konrad Rzeszutek Wilk [this message]
2017-04-04 23:47 ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170404194900.GB29986@char.us.oracle.com \
--to=konrad.wilk@oracle.com \
--cc=JBeulich@suse.com \
--cc=sstabellini@kernel.org \
--cc=stefano@aporeto.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).