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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.07.12 at 22:09, Jean Guyader <jean.guyader@xxxxxxxxx> 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;
>     uint16_t c;
>     uint16_t d;
>     uint32_t e;
>     uint32_t f;
>     uint32_t g;
>     uint8_t  h[32];
>     uint8_t  q[0];
> };
> 
> On 32b with and without packing sizeof the struct a is 60 but on 64b the 
> size of
> the struct a is 64 without packing and 60 with packing.
> However offset of q is 60 is all the case below.
> 
> One option would be to replace in the code sizeof "struct q" with
> offset of "q" be
> I think there is probably something better that could be done.

Not sure what you're trying to tell me here, but I'd like to note
that sizeof() is mostly meaningless on a structure with a trailing
[0] member anyway, so all you're after is that all offsetof()-s
are identical, which you say they are.

Jan


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