All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 0/4] new ring macros, 9pfs and pvcalls headers
@ 2017-03-31 19:15 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
  0 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2017-03-31 19:15 UTC (permalink / raw)
  To: xen-devel; +Cc: sstabellini, JBeulich

Hi all,

this patch series introduces a set of new ring macros to support rings
in the formats specified by the Xen 9pfs transport and PV Calls
protocol. It also introduces the Xen 9pfs and PV Calls protocols
headers.


Changes in v8:
- code style fixes in Makefile
- add back lost pvcall header file
- exit 1/ exit $$?
- exit C++ tests if $(CXX) is not available

Changes in v7:
- code style
- remove -include from prereq variable
- use only one prereq variable for both cxx and c99

Changes in v6:
- remove stray semicolons
- code style fix for return statements
- make the last element of DEFINE_XEN_FLEX_RING non a static inline
  function
- improve ring.h prereq comment
- remove mask_order
- use ring_size as parameter instead of ring_order
- fix indentation of parameters in ring.h
- improve order of parameters in ring.h
- introduce per header prereqs in xen/include/Makefile

Changes in v5:
- parenthesize uses of macro parameters in XEN_FLEX_RING_SIZE
- add grant_table.h to the list of prereqs in ring.h and remove the
  #include from ring.h
- #include grant_table.h in 9pfs.h and pvcalls.h
- remove PAGE_SHIFT definition, define XEN_PAGE_SHIFT instead
- code style fixes
- remove struct xen_9pfs_header definition
- don't add extra -include cstring to all c++ tests, only when needed
- add headers99.chk to .gitignore 

Changes in v4:
- include ../grant_table.h in ring.h
- add a comment about required declarations on top of ring.h
- add a patch to introduce a C99 headers check
- add -include string.h to the existing C++ headers check
- add 9pfs and pvcalls to the C99 headers check

Changes in v3:
- fix commit message
- add newlines after read/write_packet functions
- reorder DEFINE_XEN_FLEX_RING_AND_INTF and DEFINE_XEN_FLEX_RING

Changes in v2:
- replace __attribute__((packed)) with #pragma pack
- remove XEN_9PFS_RING_ORDER, the 9pfs ring order should be dynamic
- add editor configuration blocks
- add links to the specs


Stefano Stabellini (4):
      ring.h: introduce macros to handle monodirectional rings with multiple req sizes
      xen: introduce a C99 headers check
      Introduce the Xen 9pfs transport header
      Introduce the pvcalls header

 .gitignore                      |   3 +-
 xen/include/Makefile            |  33 +++++----
 xen/include/public/io/9pfs.h    |  49 +++++++++++++
 xen/include/public/io/pvcalls.h | 153 ++++++++++++++++++++++++++++++++++++++++
 xen/include/public/io/ring.h    | 151 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 375 insertions(+), 14 deletions(-)
 create mode 100644 xen/include/public/io/9pfs.h
 create mode 100644 xen/include/public/io/pvcalls.h


Cheers,

Stefano

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
  2017-03-31 19:15 [PATCH v8 0/4] new ring macros, 9pfs and pvcalls headers Stefano Stabellini
@ 2017-03-31 19:15 ` Stefano Stabellini
  2017-03-31 19:15   ` [PATCH v8 2/4] xen: introduce a C99 headers check Stefano Stabellini
                     ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Stefano Stabellini @ 2017-03-31 19:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, sstabellini, JBeulich

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()).
+ */
+
+#define XEN_PAGE_SHIFT 12
+#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) {                               \
+        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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v8 2/4] xen: introduce a C99 headers check
  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   ` Stefano Stabellini
  2017-04-03  8:15     ` Jan Beulich
  2017-03-31 19:15   ` [PATCH v8 3/4] Introduce the Xen 9pfs transport header Stefano Stabellini
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2017-03-31 19:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, sstabellini, JBeulich

Introduce a C99 headers check, for non-ANSI compliant headers: 9pfs.h
and pvcalls.h.

In addition to the usual -include stdint.h, also add -include string.h
to the C99 check to get the declaration of memcpy and size_t.

For the same reason, also add -include cstring to the C++ check when
necessary.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: JBeulich@suse.com
CC: konrad.wilk@oracle.com
---
 .gitignore           |  3 +--
 xen/include/Makefile | 30 ++++++++++++++++++------------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/.gitignore b/.gitignore
index 443b12a..0265c1e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -273,8 +273,7 @@ xen/arch/*/efi/boot.c
 xen/arch/*/efi/compat.c
 xen/arch/*/efi/efi.h
 xen/arch/*/efi/runtime.c
-xen/include/headers.chk
-xen/include/headers++.chk
+xen/include/headers*.chk
 xen/include/asm
 xen/include/asm-*/asm-offsets.h
 xen/include/asm-x86/cpuid-autogen.h
diff --git a/xen/include/Makefile b/xen/include/Makefile
index aca7f20..fd57ce4 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -90,11 +90,12 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) Makefile
 
 ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
 
-all: headers.chk headers++.chk
+all: headers.chk headers99.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h, $(PUBLIC_HEADERS))
+PUBLIC_C99_HEADERS :=
+PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
 
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	for i in $(filter %.h,$^); do \
@@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	done >$@.new
 	mv $@.new $@
 
+headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
+	rm -f $@.new
+	$(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
+	    -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
+	    -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)
+	mv $@.new $@
+
 headers++.chk: $(PUBLIC_HEADERS) Makefile
-	if $(CXX) -v >/dev/null 2>&1; then \
-	    for i in $(filter %.h,$^); do \
-	        echo '#include "'$$i'"' \
-	        | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
-	          -include stdint.h -include public/xen.h -S -o /dev/null - \
-	        || exit 1; \
-	        echo $$i; \
-	    done ; \
-	fi >$@.new
+	rm -f $@.new
+	$(CXX) -v >/dev/null 2>&1 || exit 0;                               \
+	$(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
+	    |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
+	    -include stdint.h -include public/xen.h                        \
+	    $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
+	    || exit $$?; echo $(i) >> $@.new;)
 	mv $@.new $@
 
 endif
@@ -128,5 +134,5 @@ all: $(BASEDIR)/include/asm-x86/cpuid-autogen.h
 endif
 
 clean::
-	rm -rf compat headers.chk headers++.chk
+	rm -rf compat headers*.chk
 	rm -f $(BASEDIR)/include/asm-x86/cpuid-autogen.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] 13+ messages in thread

* [PATCH v8 3/4] Introduce the Xen 9pfs transport header
  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-03-31 19:15   ` 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:49   ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
  3 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2017-03-31 19:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, sstabellini, JBeulich

Define the ring according to the protocol specification, using the new
DEFINE_XEN_FLEX_RING_AND_INTF macro.

Add the header to the C99 check.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: JBeulich@suse.com
CC: konrad.wilk@oracle.com
---
 xen/include/Makefile         |  4 +++-
 xen/include/public/io/9pfs.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 xen/include/public/io/9pfs.h

diff --git a/xen/include/Makefile b/xen/include/Makefile
index fd57ce4..a241444 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -94,9 +94,11 @@ all: headers.chk headers99.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_C99_HEADERS :=
+PUBLIC_C99_HEADERS := public/io/9pfs.h
 PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
 
+public/io/9pfs.h-prereq := string
+
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	for i in $(filter %.h,$^); do \
 	    $(CC) -x c -ansi -Wall -Werror -include stdint.h \
diff --git a/xen/include/public/io/9pfs.h b/xen/include/public/io/9pfs.h
new file mode 100644
index 0000000..4bfd5d4
--- /dev/null
+++ b/xen/include/public/io/9pfs.h
@@ -0,0 +1,49 @@
+/*
+ * 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 "../grant_table.h"
+#include "ring.h"
+
+/*
+ * See docs/misc/9pfs.markdown in xen.git for the full specification:
+ * https://xenbits.xen.org/docs/unstable/misc/9pfs.html
+ */
+DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v8 4/4] Introduce the pvcalls header
  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-03-31 19:15   ` [PATCH v8 3/4] Introduce the Xen 9pfs transport header Stefano Stabellini
@ 2017-03-31 19:15   ` Stefano Stabellini
  2017-04-04 19:50     ` Konrad Rzeszutek Wilk
  2017-04-04 19:49   ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
  3 siblings, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2017-03-31 19:15 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini, sstabellini, JBeulich

Define the ring and request and response structs according to the
specification. Use the new DEFINE_XEN_FLEX_RING macro.

Add the header to the C99 check.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
CC: JBeulich@suse.com
CC: konrad.wilk@oracle.com
---
 xen/include/Makefile            |   3 +-
 xen/include/public/io/pvcalls.h | 153 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 155 insertions(+), 1 deletion(-)
 create mode 100644 xen/include/public/io/pvcalls.h

diff --git a/xen/include/Makefile b/xen/include/Makefile
index a241444..56b389e 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -94,10 +94,11 @@ all: headers.chk headers99.chk headers++.chk
 
 PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
 
-PUBLIC_C99_HEADERS := public/io/9pfs.h
+PUBLIC_C99_HEADERS := public/io/9pfs.h public/io/pvcalls.h
 PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
 
 public/io/9pfs.h-prereq := string
+public/io/pvcalls.h-prereq := string
 
 headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 	for i in $(filter %.h,$^); do \
diff --git a/xen/include/public/io/pvcalls.h b/xen/include/public/io/pvcalls.h
new file mode 100644
index 0000000..cb81712
--- /dev/null
+++ b/xen/include/public/io/pvcalls.h
@@ -0,0 +1,153 @@
+/*
+ * 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 "../grant_table.h"
+#include "ring.h"
+
+/*
+ * See docs/misc/pvcalls.markdown in xen.git for the full specification:
+ * https://xenbits.xen.org/docs/unstable/misc/pvcalls.html
+ */
+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
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.1


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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 2/4] xen: introduce a C99 headers check
  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-05  0:12       ` Stefano Stabellini
  0 siblings, 2 replies; 13+ messages in thread
From: Jan Beulich @ 2017-04-03  8:15 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel

>>> On 31.03.17 at 21:15, <sstabellini@kernel.org> wrote:
> Introduce a C99 headers check, for non-ANSI compliant headers: 9pfs.h
> and pvcalls.h.
> 
> In addition to the usual -include stdint.h, also add -include string.h
> to the C99 check to get the declaration of memcpy and size_t.
> 
> For the same reason, also add -include cstring to the C++ check when
> necessary.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: JBeulich@suse.com 
> CC: konrad.wilk@oracle.com 
> ---
>  .gitignore           |  3 +--
>  xen/include/Makefile | 30 ++++++++++++++++++------------
>  2 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 443b12a..0265c1e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -273,8 +273,7 @@ xen/arch/*/efi/boot.c
>  xen/arch/*/efi/compat.c
>  xen/arch/*/efi/efi.h
>  xen/arch/*/efi/runtime.c
> -xen/include/headers.chk
> -xen/include/headers++.chk
> +xen/include/headers*.chk
>  xen/include/asm
>  xen/include/asm-*/asm-offsets.h
>  xen/include/asm-x86/cpuid-autogen.h
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index aca7f20..fd57ce4 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -90,11 +90,12 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) 
> Makefile
>  
>  ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>  
> -all: headers.chk headers++.chk
> +all: headers.chk headers99.chk headers++.chk
>  
>  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard 
> public/*.h public/*/*.h) $(public-y))
>  
> -PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> public/%hvm/save.h, $(PUBLIC_HEADERS))
> +PUBLIC_C99_HEADERS :=
> +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
>  
>  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>  	for i in $(filter %.h,$^); do \
> @@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>  	done >$@.new
>  	mv $@.new $@
>  
> +headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
> +	rm -f $@.new
> +	$(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
> +	    -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
> +	    -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)

I would have wished that you formatted this along the lines of
the C++ rule below (|| first on its line, aligned with the beginning
of the command). But anyway - I can live with it here, but ...

> +	mv $@.new $@
> +
>  headers++.chk: $(PUBLIC_HEADERS) Makefile
> -	if $(CXX) -v >/dev/null 2>&1; then \
> -	    for i in $(filter %.h,$^); do \
> -	        echo '#include "'$$i'"' \
> -	        | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
> -	          -include stdint.h -include public/xen.h -S -o /dev/null - \
> -	        || exit 1; \
> -	        echo $$i; \
> -	    done ; \
> -	fi >$@.new
> +	rm -f $@.new
> +	$(CXX) -v >/dev/null 2>&1 || exit 0;                               \
> +	$(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
> +	    |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
> +	    -include stdint.h -include public/xen.h                        \
> +	    $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
> +	    || exit $$?; echo $(i) >> $@.new;)

... indentation still doesn't match how it was originally (including,
as mentioned above as well, aligning the start of the command
with | and || ) and there's a blank missing after | . Of course I'm
fine with you fixing this upon commit, if no other need arises for
a v9, so on that basis with those adjustments
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 2/4] xen: introduce a C99 headers check
  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
  1 sibling, 1 reply; 13+ messages in thread
From: Stefano Stabellini @ 2017-04-03 18:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini

On Mon, 3 Apr 2017, Jan Beulich wrote:
> >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> wrote:
> > Introduce a C99 headers check, for non-ANSI compliant headers: 9pfs.h
> > and pvcalls.h.
> > 
> > In addition to the usual -include stdint.h, also add -include string.h
> > to the C99 check to get the declaration of memcpy and size_t.
> > 
> > For the same reason, also add -include cstring to the C++ check when
> > necessary.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: JBeulich@suse.com 
> > CC: konrad.wilk@oracle.com 
> > ---
> >  .gitignore           |  3 +--
> >  xen/include/Makefile | 30 ++++++++++++++++++------------
> >  2 files changed, 19 insertions(+), 14 deletions(-)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 443b12a..0265c1e 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -273,8 +273,7 @@ xen/arch/*/efi/boot.c
> >  xen/arch/*/efi/compat.c
> >  xen/arch/*/efi/efi.h
> >  xen/arch/*/efi/runtime.c
> > -xen/include/headers.chk
> > -xen/include/headers++.chk
> > +xen/include/headers*.chk
> >  xen/include/asm
> >  xen/include/asm-*/asm-offsets.h
> >  xen/include/asm-x86/cpuid-autogen.h
> > diff --git a/xen/include/Makefile b/xen/include/Makefile
> > index aca7f20..fd57ce4 100644
> > --- a/xen/include/Makefile
> > +++ b/xen/include/Makefile
> > @@ -90,11 +90,12 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) 
> > Makefile
> >  
> >  ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
> >  
> > -all: headers.chk headers++.chk
> > +all: headers.chk headers99.chk headers++.chk
> >  
> >  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard 
> > public/*.h public/*/*.h) $(public-y))
> >  
> > -PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> > public/%hvm/save.h, $(PUBLIC_HEADERS))
> > +PUBLIC_C99_HEADERS :=
> > +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> > public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
> >  
> >  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
> >  	for i in $(filter %.h,$^); do \
> > @@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
> >  	done >$@.new
> >  	mv $@.new $@
> >  
> > +headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
> > +	rm -f $@.new
> > +	$(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
> > +	    -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
> > +	    -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)
> 
> I would have wished that you formatted this along the lines of
> the C++ rule below (|| first on its line, aligned with the beginning
> of the command). But anyway - I can live with it here, but ...
> 
> > +	mv $@.new $@
> > +
> >  headers++.chk: $(PUBLIC_HEADERS) Makefile
> > -	if $(CXX) -v >/dev/null 2>&1; then \
> > -	    for i in $(filter %.h,$^); do \
> > -	        echo '#include "'$$i'"' \
> > -	        | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
> > -	          -include stdint.h -include public/xen.h -S -o /dev/null - \
> > -	        || exit 1; \
> > -	        echo $$i; \
> > -	    done ; \
> > -	fi >$@.new
> > +	rm -f $@.new
> > +	$(CXX) -v >/dev/null 2>&1 || exit 0;                               \
> > +	$(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
> > +	    |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
> > +	    -include stdint.h -include public/xen.h                        \
> > +	    $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
> > +	    || exit $$?; echo $(i) >> $@.new;)
> 
> ... indentation still doesn't match how it was originally (including,
> as mentioned above as well, aligning the start of the command
> with | and || ) and there's a blank missing after | . Of course I'm
> fine with you fixing this upon commit, if no other need arises for
> a v9, so on that basis with those adjustments
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thank you, I can do that! Are you OK also with the other patches, patch
#1 in particular?

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 2/4] xen: introduce a C99 headers check
  2017-04-03 18:25       ` Stefano Stabellini
@ 2017-04-04  7:59         ` Jan Beulich
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Beulich @ 2017-04-04  7:59 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel

>>> On 03.04.17 at 20:25, <sstabellini@kernel.org> wrote:
> On Mon, 3 Apr 2017, Jan Beulich wrote:
>> >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> wrote:
>> > @@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>> >  	done >$@.new
>> >  	mv $@.new $@
>> >  
>> > +headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
>> > +	rm -f $@.new
>> > +	$(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
>> > +	    -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
>> > +	    -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)
>> 
>> I would have wished that you formatted this along the lines of
>> the C++ rule below (|| first on its line, aligned with the beginning
>> of the command). But anyway - I can live with it here, but ...
>> 
>> > +	mv $@.new $@
>> > +
>> >  headers++.chk: $(PUBLIC_HEADERS) Makefile
>> > -	if $(CXX) -v >/dev/null 2>&1; then \
>> > -	    for i in $(filter %.h,$^); do \
>> > -	        echo '#include "'$$i'"' \
>> > -	        | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
>> > -	          -include stdint.h -include public/xen.h -S -o /dev/null - \
>> > -	        || exit 1; \
>> > -	        echo $$i; \
>> > -	    done ; \
>> > -	fi >$@.new
>> > +	rm -f $@.new
>> > +	$(CXX) -v >/dev/null 2>&1 || exit 0;                               \
>> > +	$(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
>> > +	    |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
>> > +	    -include stdint.h -include public/xen.h                        \
>> > +	    $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
>> > +	    || exit $$?; echo $(i) >> $@.new;)
>> 
>> ... indentation still doesn't match how it was originally (including,
>> as mentioned above as well, aligning the start of the command
>> with | and || ) and there's a blank missing after | . Of course I'm
>> fine with you fixing this upon commit, if no other need arises for
>> a v9, so on that basis with those adjustments
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Thank you, I can do that! Are you OK also with the other patches, patch
> #1 in particular?

I have no further comments on them, but I can't ack them, and
I don't feel like giving my R-b for them.

Jan


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
  2017-03-31 19:15 ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Stefano Stabellini
                     ` (2 preceding siblings ...)
  2017-03-31 19:15   ` [PATCH v8 4/4] Introduce the pvcalls header Stefano Stabellini
@ 2017-04-04 19:49   ` Konrad Rzeszutek Wilk
  2017-04-04 23:47     ` Stefano Stabellini
  3 siblings, 1 reply; 13+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-04-04 19:49 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel, JBeulich

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 3/4] Introduce the Xen 9pfs transport header
  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
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-04-04 19:50 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel, JBeulich

On Fri, Mar 31, 2017 at 12:15:18PM -0700, Stefano Stabellini wrote:
> Define the ring according to the protocol specification, using the new
> DEFINE_XEN_FLEX_RING_AND_INTF macro.
> 
> Add the header to the C99 check.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: JBeulich@suse.com
> CC: konrad.wilk@oracle.com

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

> ---
>  xen/include/Makefile         |  4 +++-
>  xen/include/public/io/9pfs.h | 49 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/public/io/9pfs.h
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index fd57ce4..a241444 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -94,9 +94,11 @@ all: headers.chk headers99.chk headers++.chk
>  
>  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
>  
> -PUBLIC_C99_HEADERS :=
> +PUBLIC_C99_HEADERS := public/io/9pfs.h
>  PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
>  
> +public/io/9pfs.h-prereq := string
> +
>  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>  	for i in $(filter %.h,$^); do \
>  	    $(CC) -x c -ansi -Wall -Werror -include stdint.h \
> diff --git a/xen/include/public/io/9pfs.h b/xen/include/public/io/9pfs.h
> new file mode 100644
> index 0000000..4bfd5d4
> --- /dev/null
> +++ b/xen/include/public/io/9pfs.h
> @@ -0,0 +1,49 @@
> +/*
> + * 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 "../grant_table.h"
> +#include "ring.h"
> +
> +/*
> + * See docs/misc/9pfs.markdown in xen.git for the full specification:
> + * https://xenbits.xen.org/docs/unstable/misc/9pfs.html
> + */
> +DEFINE_XEN_FLEX_RING_AND_INTF(xen_9pfs);
> +
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 1.9.1
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 4/4] Introduce the pvcalls header
  2017-03-31 19:15   ` [PATCH v8 4/4] Introduce the pvcalls header Stefano Stabellini
@ 2017-04-04 19:50     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 13+ messages in thread
From: Konrad Rzeszutek Wilk @ 2017-04-04 19:50 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Stefano Stabellini, xen-devel, JBeulich

On Fri, Mar 31, 2017 at 12:15:19PM -0700, Stefano Stabellini wrote:
> Define the ring and request and response structs according to the
> specification. Use the new DEFINE_XEN_FLEX_RING macro.
> 
> Add the header to the C99 check.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> CC: JBeulich@suse.com
> CC: konrad.wilk@oracle.com

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/include/Makefile            |   3 +-
>  xen/include/public/io/pvcalls.h | 153 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 155 insertions(+), 1 deletion(-)
>  create mode 100644 xen/include/public/io/pvcalls.h
> 
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index a241444..56b389e 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -94,10 +94,11 @@ all: headers.chk headers99.chk headers++.chk
>  
>  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
>  
> -PUBLIC_C99_HEADERS := public/io/9pfs.h
> +PUBLIC_C99_HEADERS := public/io/9pfs.h public/io/pvcalls.h
>  PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
>  
>  public/io/9pfs.h-prereq := string
> +public/io/pvcalls.h-prereq := string
>  
>  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>  	for i in $(filter %.h,$^); do \
> diff --git a/xen/include/public/io/pvcalls.h b/xen/include/public/io/pvcalls.h
> new file mode 100644
> index 0000000..cb81712
> --- /dev/null
> +++ b/xen/include/public/io/pvcalls.h
> @@ -0,0 +1,153 @@
> +/*
> + * 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 "../grant_table.h"
> +#include "ring.h"
> +
> +/*
> + * See docs/misc/pvcalls.markdown in xen.git for the full specification:
> + * https://xenbits.xen.org/docs/unstable/misc/pvcalls.html
> + */
> +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
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 1.9.1
> 

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes
  2017-04-04 19:49   ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
@ 2017-04-04 23:47     ` Stefano Stabellini
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2017-04-04 23:47 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Stefano Stabellini, xen-devel, Stefano Stabellini, JBeulich

On Tue, 4 Apr 2017, Konrad Rzeszutek Wilk wrote:
> 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/ ?

OK, I'll write (XEN_FLEX_RING_SIZE() - $NAME_queued()).
I think that also adding all the parameters would be too wordy.


> > + */
> > +
> > +#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.
>  */

I don't think there are any architecture overrides actually. On ARM is
always defined as 4K, no matter the choice of page granularity made by
the operating system.


> #define XEN_PAGE_SHIFT 12
> #endif
> 
> ?

I'll add the #ifndef to avoid possible collisions and add this comment:

  The PAGE_SIZE (thus, also PAGE_SHIFT) for ring protocols and hypercall
  interfaces is always 4K, regardless of the architecture and page
  granularity chosen by operating systems.


> > +#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.

I'll align


> > +        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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v8 2/4] xen: introduce a C99 headers check
  2017-04-03  8:15     ` Jan Beulich
  2017-04-03 18:25       ` Stefano Stabellini
@ 2017-04-05  0:12       ` Stefano Stabellini
  1 sibling, 0 replies; 13+ messages in thread
From: Stefano Stabellini @ 2017-04-05  0:12 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Stefano Stabellini, xen-devel, Stefano Stabellini

On Mon, 3 Apr 2017, Jan Beulich wrote:
> >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> wrote:
> > Introduce a C99 headers check, for non-ANSI compliant headers: 9pfs.h
> > and pvcalls.h.
> > 
> > In addition to the usual -include stdint.h, also add -include string.h
> > to the C99 check to get the declaration of memcpy and size_t.
> > 
> > For the same reason, also add -include cstring to the C++ check when
> > necessary.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> > CC: JBeulich@suse.com 
> > CC: konrad.wilk@oracle.com 
> > ---
> >  .gitignore           |  3 +--
> >  xen/include/Makefile | 30 ++++++++++++++++++------------
> >  2 files changed, 19 insertions(+), 14 deletions(-)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 443b12a..0265c1e 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -273,8 +273,7 @@ xen/arch/*/efi/boot.c
> >  xen/arch/*/efi/compat.c
> >  xen/arch/*/efi/efi.h
> >  xen/arch/*/efi/runtime.c
> > -xen/include/headers.chk
> > -xen/include/headers++.chk
> > +xen/include/headers*.chk
> >  xen/include/asm
> >  xen/include/asm-*/asm-offsets.h
> >  xen/include/asm-x86/cpuid-autogen.h
> > diff --git a/xen/include/Makefile b/xen/include/Makefile
> > index aca7f20..fd57ce4 100644
> > --- a/xen/include/Makefile
> > +++ b/xen/include/Makefile
> > @@ -90,11 +90,12 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) 
> > Makefile
> >  
> >  ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
> >  
> > -all: headers.chk headers++.chk
> > +all: headers.chk headers99.chk headers++.chk
> >  
> >  PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard 
> > public/*.h public/*/*.h) $(public-y))
> >  
> > -PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> > public/%hvm/save.h, $(PUBLIC_HEADERS))
> > +PUBLIC_C99_HEADERS :=
> > +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> > public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
> >  
> >  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
> >  	for i in $(filter %.h,$^); do \
> > @@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
> >  	done >$@.new
> >  	mv $@.new $@
> >  
> > +headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
> > +	rm -f $@.new
> > +	$(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
> > +	    -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
> > +	    -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)
> 
> I would have wished that you formatted this along the lines of
> the C++ rule below (|| first on its line, aligned with the beginning
> of the command). But anyway - I can live with it here, but ...
> 
> > +	mv $@.new $@
> > +
> >  headers++.chk: $(PUBLIC_HEADERS) Makefile
> > -	if $(CXX) -v >/dev/null 2>&1; then \
> > -	    for i in $(filter %.h,$^); do \
> > -	        echo '#include "'$$i'"' \
> > -	        | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
> > -	          -include stdint.h -include public/xen.h -S -o /dev/null - \
> > -	        || exit 1; \
> > -	        echo $$i; \
> > -	    done ; \
> > -	fi >$@.new
> > +	rm -f $@.new
> > +	$(CXX) -v >/dev/null 2>&1 || exit 0;                               \
> > +	$(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
> > +	    |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
> > +	    -include stdint.h -include public/xen.h                        \
> > +	    $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
> > +	    || exit $$?; echo $(i) >> $@.new;)
> 
> ... indentation still doesn't match how it was originally (including,
> as mentioned above as well, aligning the start of the command
> with | and || ) and there's a blank missing after | . Of course I'm
> fine with you fixing this upon commit, if no other need arises for
> a v9, so on that basis with those adjustments
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Turns out that I have a small fix to patch #1 to do, so I'll resend the
series once again with your comments and reviewed-by. I hope I
interpreted your alignment asks correctly.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-04-05  0:12 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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   ` [PATCH v8 1/4] ring.h: introduce macros to handle monodirectional rings with multiple req sizes Konrad Rzeszutek Wilk
2017-04-04 23:47     ` 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.