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

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

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.


Xen-devel mailing list



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