[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH 1/1] ccw: ccw payload description
On Tue, Jan 21, 2014 at 12:30:21PM +0100, Cornelia Huck wrote: > On Tue, 21 Jan 2014 10:53:49 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Jan 21, 2014 at 10:37:26AM +1030, Rusty Russell wrote: > > > Cornelia Huck <cornelia.huck@de.ibm.com> writes: > > > > On Mon, 20 Jan 2014 11:17:17 +0200 > > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > >> On Mon, Jan 20, 2014 at 02:28:00PM +1030, Rusty Russell wrote: > > > >> > We'll just have to live with the lack of padding, and let's try not to > > > >> > add any more packed-requiring structures. If we deprecate > > > >> > virtio-balloon, the rest will be contained in CCW. > > > >> > > > > >> > Cheers, > > > >> > Rusty. > > > >> > > > >> For CCW, how about my idea of adding optional padding explicitly? > > > >> In the compatibility section, we can explicitly specify the > > > >> the length. > > > >> Drivers can use offsetof(padding) and avoid packed attribute. > > > > > > > > Since Rusty does not like the tables, this looks like a good idea. > > > > > > Surely making the padding optional makes it more complex, for no real > > > gain? It's not that much simpler in practice, is it? > > > > > > This patch just removes the __attribute__((packed)) where it's useless, > > > and notes explicitly where it's required: > > (...) > > > I was trying to avoid the non-portable __attribute__((packed)) > > replacing it with portable offsetof(..., padding) > > everywhere. It only has to be optional for the > > transitional devices. But if Thomas is fine with > > __attribute__((packed)) now, that's fine with me too. > > > > I do think we need a general comment explaining the > > packed directive here. > > > > E.g. > > > > Many device and driver in-memory structure layouts > > are documented using the C struct syntax. > > In this case no extra padding is assumed within the structure. > > To stress this, cases where common C compilers are known to insert extra > > padding within structures are tagged using the GNU C __attribute__((packed)) > > syntax. > > Do we want to add a general statement regarding our usage of C structs? > > Basically, we can't predict if and where a compiler will insert > padding, and different compilers on different architectures might pad > in different ways. So I'd propose to state that all our C structs are > assumed to be packed with no extra padding appended, even if we take > care to introduce no new structures where a common C compiler would > insert padding. What does this add to what I wrote above? > (We may want to specify our le32, ... usage in the same paragraph.) u8 etc too - these are not standard types. I would make it a separate paragraph though. -- MST
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]