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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

~Andrew

>
> 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.
>
> Thanks,
> Jean
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com




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