[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] pvcalls: Document correctly and explicitely the padding for all arches



Sorry I forgot to CC the maintainers on the e-mail.

I will resent it.

On 16/05/2020 11:19, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>

The documentation of pvcalls suggests there is padding for 32-bit x86
at the end of most the structure. However, they are not described in
in the public header.

Because of that all the structures would be 32-bit aligned and not
64-bit aligned for 32-bit x86.

For all the other architectures supported (Arm and 64-bit x86), the
structure are aligned to 64-bit because they contain uint64_t field.
Therefore all the structures contain implicit padding.

The paddings are now corrected for 32-bit x86 and written explicitly for
all the architectures.

While the structure size between 32-bit and 64-bit x86 is different, it
shouldn't cause any incompatibility between a 32-bit and 64-bit
frontend/backend because the commands are always 56 bits and the padding
are at the end of the structure.

As an aside, the padding sadly cannot be mandated to be 0 as they are
already present. So it is not going to be possible to use the padding
for extending a command in the future.

Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>

---
     Changes in v2:
         - It is not possible to use the same padding for 32-bit x86 and
         all the other supported architectures.
---
  docs/misc/pvcalls.pandoc        | 18 ++++++++++--------
  xen/include/public/io/pvcalls.h | 14 ++++++++++++++
  2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/docs/misc/pvcalls.pandoc b/docs/misc/pvcalls.pandoc
index 665dad556c39..c25412868f5d 100644
--- a/docs/misc/pvcalls.pandoc
+++ b/docs/misc/pvcalls.pandoc
@@ -246,9 +246,9 @@ The format is defined as follows:
                        uint32_t domain;
                        uint32_t type;
                        uint32_t protocol;
-                       #ifdef CONFIG_X86_32
+                       #ifndef CONFIG_X86_32
                        uint8_t pad[4];
-                       #endif
+                       #endif
                } socket;
                struct xen_pvcalls_connect {
                        uint64_t id;
@@ -257,16 +257,18 @@ The format is defined as follows:
                        uint32_t flags;
                        grant_ref_t ref;
                        uint32_t evtchn;
-                       #ifdef CONFIG_X86_32
+                       #ifndef CONFIG_X86_32
                        uint8_t pad[4];
-                       #endif
+                       #endif
                } connect;
                struct xen_pvcalls_release {
                        uint64_t id;
                        uint8_t reuse;
-                       #ifdef CONFIG_X86_32
+                       #ifndef CONFIG_X86_32
                        uint8_t pad[7];
-                       #endif
+                       #else
+                       uint8_t pad[3];
+                       #endif
                } release;
                struct xen_pvcalls_bind {
                        uint64_t id;
@@ -276,9 +278,9 @@ The format is defined as follows:
                struct xen_pvcalls_listen {
                        uint64_t id;
                        uint32_t backlog;
-                       #ifdef CONFIG_X86_32
+                       #ifndef CONFIG_X86_32
                        uint8_t pad[4];
-                       #endif
+                       #endif
                } listen;
                struct xen_pvcalls_accept {
                        uint64_t id;
diff --git a/xen/include/public/io/pvcalls.h b/xen/include/public/io/pvcalls.h
index cb8171275c13..590c5e9e41aa 100644
--- a/xen/include/public/io/pvcalls.h
+++ b/xen/include/public/io/pvcalls.h
@@ -65,6 +65,9 @@ struct xen_pvcalls_request {
              uint32_t domain;
              uint32_t type;
              uint32_t protocol;
+#ifndef CONFIG_X86_32
+            uint8_t pad[4];
+#endif
          } socket;
          struct xen_pvcalls_connect {
              uint64_t id;
@@ -73,10 +76,18 @@ struct xen_pvcalls_request {
              uint32_t flags;
              grant_ref_t ref;
              uint32_t evtchn;
+#ifndef CONFIG_X86_32
+            uint8_t pad[4];
+#endif
          } connect;
          struct xen_pvcalls_release {
              uint64_t id;
              uint8_t reuse;
+#ifndef CONFIG_X86_32
+            uint8_t pad[7];
+#else
+            uint8_t pad[3];
+#endif
          } release;
          struct xen_pvcalls_bind {
              uint64_t id;
@@ -86,6 +97,9 @@ struct xen_pvcalls_request {
          struct xen_pvcalls_listen {
              uint64_t id;
              uint32_t backlog;
+#ifndef CONFIG_X86_32
+            uint8_t pad[4];
+#endif
          } listen;
          struct xen_pvcalls_accept {
              uint64_t id;


--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.