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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.

> 
>   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