From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 4/5] xen: Add V4V implementation Date: Thu, 19 Jul 2012 11:42:48 +0100 Message-ID: <5007E4A8.6000807@citrix.com> References: <1340900786-21802-1-git-send-email-jean.guyader@citrix.com> <1340900786-21802-5-git-send-email-jean.guyader@citrix.com> <4FED8467020000780008CC68@nat28.tlf.novell.com> <4FEDA13A020000780008CD19@nat28.tlf.novell.com> <5007D4A3.8090908@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jean Guyader Cc: "Jean Guyader (3P)" , Jan Beulich , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 19/07/12 10:58, Jean Guyader wrote: > On 19 July 2012 10:34, Andrew Cooper wrote: >> On 18/07/12 21:09, Jean Guyader wrote: >>> On 29 June 2012 11:36, Jan Beulich wrote: >>>>>>> On 29.06.12 at 12:03, Jean Guyader wrote: >>>>> On 29 June 2012 09:33, Jan Beulich wrote: >>>>>>>>> On 28.06.12 at 18:26, Jean Guyader 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. > > Thanks, > Jean Ah yes - silly me. I understand your problem now struct b { uint64_t a; uint32_t b; uint16_t c; uint16_t d; uint32_t e; uint32_t f; uint32_t g; uint8_t h[32]; union { uint8_t q[0]; uint32_t _pad; } u; }; This works for me on gcc and gives identical sizeof and offsetof results on both 32 and 64bit. -- Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer T: +44 (0)1223 225 900, http://www.citrix.com