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

[Jean Guyader <jean.guyader@xxxxxxxxx>]

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.

Thanks,
Jean

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