|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] blkif: add indirect descriptors interface to public headers
On Tue, Nov 12, 2013 at 02:29:01PM +0000, Ian Campbell wrote:
> 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.
Right. I had hit that at some point with putting in a u64 and seeing
it mushroom up while if I did two u32 it padded it nicely.
Perhaps that is what we should do? The Linux struct has the same size
whether it be 64-bit or 32-bit.
>
> 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?).
Or perhaps just mention the size of the structure and offset?
>
> 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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |