On Wed, Mar 07, 2018 at 05:14:27PM +0100, Cornelia Huck wrote:
> On Wed, 7 Mar 2018 17:05:24 +0100
> Halil Pasic <> wrote:
>
> > On 03/07/2018 03:49 PM, Cornelia Huck wrote:
> > >>>> +When VIRTIO_F_NOTIFICATION_DATA has been negotiated,
> > >>>> +the driver notifies the device by writing the following
> > >>>> +32-bit value to the Queue Notify address:
> > >>>> +\begin{lstlisting}
> > >>>> +le32 vqn : 16,
> > >>>> + next_off : 15,
> > >>>> + next_wrap : 1;
> > >>> Don't we want to write this as
> > >>>
> > >>> le32 vqn : 16;
> > >>> le32 next_off :15;
> > >>> le32 next_wrap : 1;
> > >>>
> > >>> ?
> > >> Same thing in C, but would be more confusing IMHO since it will be up to
> > >> the reader to figure out which fields comprise the 32 bit integer.
> > > It looked weird to me. Other opinions?
> > >
> >
> > Regarding the c11 standard the two are equivalent. Thus it does not
> > matter to me which notation is used. AFAIK bit-fields are only defined
> > in the context of structs (and/or unions), so I assumed that. Putting a
> > struct around it would be much better IMHO.
> >
> > I don't agree with Michael's argument about 'which fields comprise the
> > 32 bit integer', as IMHO it does not make sense in terms of c11.
> >
> > Consider
> >
> > struct A {
> > uint32_t a:30, b:1, c:2:, d:8;
> > };
> >
> > I think, in this particular case the notation ain't very helpful in
> > figuring out what comprises what. For that reason, if I really need
> > to choose, I would side with Connie on this one.
> >
> > But there is another, more significant problem IMHO. The guarantees
> > provided by the C language (c11) regarding the resulting memory layout
> > are not sufficient to reason about it like Michael's comment and
> > the bit's of the draft imply. To know the memory layout we need the
> > ABI specification for the given platform on top of the C standard.
> >
> > So if the bit fields are about in memory layout, I find the stuff
> > problematic. If however we use bit-fields only to define how arithmetic
> > works, then we are fine.
>
> I'm not sure we should go down the C standard rabbit hole. People have
> gotten lost in there.
+1
In particular virtio already uses C-like syntax for structure
without regard to what the C standard says. It's just pseudo-code,
nothing to be hang about.
> If the clarification of what we mean by this notation (patch 1 + the
> update sent later) is not enough,
Halil, can you pls say whether it's enough?
> I'd rather prefer us to add a
> clarifying sentence/diagram/... there. I was mainly bothered by the
> change to the definition in this patch...
Here's what we are trying to say (for example):
+the value A stored in the low 15 bit of a 16 bit
+integer and the value B stored in the high bit of the 16 bit
+integer, the integer in turn using the big-endian byte order.
Thus we can write:
be16 A : 15,
B : 1;
which maps to:
be -> big endian
16 -> 16 bit integer
A : 15 - low 15 bits
B : 1 - following 1 bit
Why not repeat be16 twice? Well first of all why repeat information
twice? Second this notation lets us list many integer fields
as we might have in the structure:
be16 A : 15,
B : 1;
be16 C;
And it also ties to the existing notation for full integers.
Any suggestions on how to do it better?
Please provide an example based on the above.
--
MST
