Re: [Xen-devel] [PATCH] blkif: add indirect descriptors interface to public headers

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

Paul Durrant <Paul.Durrant@xxxxxxxxxx> wrote:
>> -----Original Message-----
>> From: Ian Campbell
>> Sent: 13 November 2013 09:27
>> To: Paul Durrant
>> Cc: Konrad Rzeszutek Wilk; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir
>(Xen.org);
>> Jan Beulich; Roger Pau Monne
>> Subject: Re: [Xen-devel] [PATCH] blkif: add indirect descriptors
>interface to
>> public headers
>> 
>> On Tue, 2013-11-12 at 15:16 +0000, Paul Durrant wrote:
>> > > -----Original Message-----
>> > > From: Ian Campbell
>> > > Sent: 12 November 2013 14:29
>> > > To: Konrad Rzeszutek Wilk
>> > > Cc: Paul Durrant; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org);
>Jan
>> Beulich;
>> > > Roger Pau Monne
>> > > Subject: Re: [Xen-devel] [PATCH] blkif: add indirect descriptors
>interface
>> to
>> > > public headers
>> > >
>> > > On Tue, 2013-11-12 at 09:22 -0500, Konrad Rzeszutek Wilk wrote:
>> > >
>> > > > > > +struct blkif_request_indirect {
>> > > > > > +    uint8_t        operation;    /* BLKIF_OP_INDIRECT     
>              */
>> > > > > > +    uint8_t        indirect_op;  /* BLKIF_OP_{READ/WRITE} 
>              */
>> > > > > > +    uint16_t       nr_segments;  /* number of segments    
>              */
>> > > > >
>> > > > > This is going to be a problem. What alignment boundary are
>you
>> > > > expecting the next field to start on? AFAIK 32-bit gcc will
>4-byte
>> > > > align it, 32-bit MSVC will 8-byte align it.
>> > > > >
>> > > >
>> > > > Oh no. I thought that the Linux one had this set correctly, ah
>it did:
>> > > >
>> > > >
>> > > > struct blkif_request_indirect {
>> > > > [...]
>> > > > } __attribute__((__packed__));
>> > >
>> > > That attribute packed isn't allowed in the public interface
>headers.
>> > >
>> > > Since compilers do differ in their packing, and guests may be
>using
>> > > various pragmas, it might be useful to write down that for x86
>these
>> > > headers are to be treated as using the <WHATEVER> ABI (gcc? Some
>Intel
>> > > standard?).
>> > >
>> >
>> > Can we go for types aligned on their size then rather than gcc
>brokenness.
>> 
>> We should go for some existing well defined ABI spec not make up our
>> own.
>> 
>> In effect the x86 ABI has historically been de-facto specified as the
>> gcc ABI.
>> 
>
>Since the linux headers seem to hardcode the x64 ABI for this struct,
>do we need to support an x86 variant? After all there's no backwards
>compatibility issue here.

It was merged in 3.11 and now 3.12 has been released. Also it also in FreeBSD. 
So there is backwards computability with that to think of.  Or did you mean 32 
vs 64 size and offset? That should be the same.
>
>   Paul
>
>> >
>> >   Paul
>> >
>> > > For ARM we reference the specific standard[0]. It is up to the
>guest OS
>> > > to make sure that it's version of the headers lay things out
>following
>> > > that standard (NB Linux blkif.h is currently buggy on ARM in this
>> > > regard, Julien has the details).
>> > >
>> > > Ian.
>> > >
>> > > [0]
>> > >
>http://xenbits.xen.org/docs/unstable/hypercall/arm/include,public,arch-
>> > > arm.h.html#incontents_arm_abi
>> >
>> 



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