Hi Jan,
On 20/04/2020 09:04, Jan Beulich wrote:
On 19.04.2020 12:49, Julien Grall wrote:
--- a/docs/misc/pvcalls.pandoc
+++ b/docs/misc/pvcalls.pandoc
@@ -246,9 +246,7 @@ The format is defined as follows:
uint32_t domain;
uint32_t type;
uint32_t protocol;
- #ifdef CONFIG_X86_32
uint8_t pad[4];
- #endif
} socket;
struct xen_pvcalls_connect {
uint64_t id;
@@ -257,16 +255,12 @@ The format is defined as follows:
uint32_t flags;
grant_ref_t ref;
uint32_t evtchn;
- #ifdef CONFIG_X86_32
uint8_t pad[4];
- #endif
} connect;
struct xen_pvcalls_release {
uint64_t id;
uint8_t reuse;
- #ifdef CONFIG_X86_32
uint8_t pad[7];
- #endif
} release;
struct xen_pvcalls_bind {
uint64_t id;
@@ -276,9 +270,7 @@ The format is defined as follows:
struct xen_pvcalls_listen {
uint64_t id;
uint32_t backlog;
- #ifdef CONFIG_X86_32
uint8_t pad[4];
- #endif
} listen;
struct xen_pvcalls_accept {
uint64_t id;
I wonder on what grounds these #ifdef-s had been there - they're
plain wrong with the types used in the public header.
--- a/xen/include/public/io/pvcalls.h
+++ b/xen/include/public/io/pvcalls.h
@@ -65,6 +65,7 @@ struct xen_pvcalls_request {
uint32_t domain;
uint32_t type;
uint32_t protocol;
+ uint8_t pad[4];
} socket;
struct xen_pvcalls_connect {
uint64_t id;
@@ -73,10 +74,12 @@ struct xen_pvcalls_request {
uint32_t flags;
grant_ref_t ref;
uint32_t evtchn;
+ uint8_t pad[4];
} connect;
struct xen_pvcalls_release {
uint64_t id;
uint8_t reuse;
+ uint8_t pad[7];
} release;
struct xen_pvcalls_bind {
uint64_t id;
@@ -86,6 +89,7 @@ struct xen_pvcalls_request {
struct xen_pvcalls_listen {
uint64_t id;
uint32_t backlog;
+ uint8_t pad[4];
} listen;
I'm afraid we can't change these in such a way - your additions
change sizeof() for the respective sub-structures on 32-bit x86,
and hence this is not a backwards compatible adjustment.
This is a bit confusing, each structure contain a 64-bit field so I
would have thought it the structure would be 8-byte aligned (as on
32-bit Arm). But looking at the spec, a uint64_t will only aligned to
4-byte.
However, I am not sure why sizeof() matters here. I understand the value
would be different, but AFAICT, this is not used as part of the protocol.
IIUC the request should always be 56-bytes, so at worse you will read
unknown bytes. Those bytes are at the end of the structure, so it should
not matter.
The
best I can think of right now that we could do is make the
difference explicit, by putting the padding fields inside
#ifndef __i386__. But of course this is awkward at least when
thinking about a 32-bit / 64-bit pair of communication ends on
an x86-64 host.
I don't think this is necessary because of the way a request has been
defined.
Cheers,
--
Julien Grall
