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

Re: [Xen-devel] [PATCH 4/5] xen: Add V4V implementation



On 19/07/12 11:06, Jean Guyader wrote:
On 19 July 2012 10:54, Attilio Rao<attilio.rao@xxxxxxxxxx>  wrote:
On 19/07/12 10:58, Jean Guyader wrote:
On 19 July 2012 10:34, Andrew Cooper<andrew.cooper3@xxxxxxxxxx>   wrote:

On 18/07/12 21:09, Jean Guyader wrote:

On 29 June 2012 11:36, Jan Beulich<JBeulich@xxxxxxxx>   wrote:

On 29.06.12 at 12:03, Jean Guyader<jean.guyader@xxxxxxxxx>   wrote:

On 29 June 2012 09:33, Jan Beulich<JBeulich@xxxxxxxx>   wrote:

On 28.06.12 at 18:26, Jean Guyader<jean.guyader@xxxxxxxxxx>
wrote:

+typedef struct v4v_ring_id
+{
+    struct v4v_addr addr;
+    domid_t partner;
+} V4V_PACKED v4v_ring_id_t;
+

This structure is really the one that cause trouble. domid_t is 16b
and v4v_addr_t is used
inside v4v_ring_t. I would like the structure to remind as close as we
can from the original version
as we already versions in the field. Having explicit padding will make
all the structures different
which will make much harder to write a driver that will support the
two versions of the API.

Oh, I see, "partner" would end up on a different offset if the
packed attribute was removed from v4v_addr_t. But that
could still be solved by making this type a union:

typedef union v4v_ring_id
{
      struct v4v_addr addr;
      struct {
          uint32_t port;
          domid_t domain;
          domid_t partner;
      } full;
} v4v_ring_id_t;

That would guarantee binary compatibility. And you could even
achieve source compatibility for gcc users by making the naming
of the second structure conditional upon __GNUC__ being
undefined (or adding a second instance of the same, just
unnamed structure within a respective #ifdef - that would make
it possible to write code that can be compiled by both gcc and
non-gcc, yet existing gcc-only code would need changing).


Also most all the consumer of those headers will have to rewrite the
structure anyway, for instance
the Linux kernel have it's own naming convention, macros definitions
which are different, etc..

Such can usually be done via scripts, so having a fully defined
public header is still worthwhile.


Hi,

I've been working on this and it work for most of it apart from one
case.
Let's take this structure:

struct a
{
      uint64_t a;
      uint32_t b;

      uint32_t _pad0;

      uint16_t c;
      uint16_t d;
      uint32_t e;
      uint32_t f;
      uint32_t g;
      uint8_t  h[32];
      uint8_t  q[0];
};

Manually padding so the alignment is the same on 32 and 64 bit is the
only way to do this in the public headers, which cant have gcc'isms for
compatibility reasons with other compilers.


The problem isn't with the individual fields (they are all correctly
aligned) it is
the the overall structure size which is 64 even so offset of q is 60
(and sizeof q
should be 0).

I think there is no way around it. The structure I have should be
aligned on 64b anyway.



Can you use gcc __attribute__((aligned(64))) for this? Or we try to avoid
gcc-ism at all?

I'm trying to avoid any compiler specific statement. I'll added some
manual padding
to have it explicitly aligned to 64b.

Ideally though you can have an intermediate header where you workout compiler specific code (in particular if you are almost sure your code will always be likely compiled with gcc).
So, you can have an header which does the following:

#ifdef __GNUC__
#define _compiler_align(x) __attribute__((__aligned__(x)))
#elif __INTEL_COMPILER
#define _compiler_align(x) foo
#else
#define _compiler_align(x)
#endif

Probabily for something like that you want to use some header that is known to be included in a lot of files.

Attilio

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


 


Rackspace

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