OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: [PATCH v5 3/7] Introduce new destination type for admin commands


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Sunday, May 15, 2022 11:09 AM
> 
> > ---
> >  admin.tex | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/admin.tex b/admin.tex
> > index 6725daa..f816c3b 100644
> > --- a/admin.tex
> > +++ b/admin.tex
> > @@ -11,11 +11,13 @@ \section{Administration command
> set}\label{sec:Basic Facilities of a Virtio Devi
> >          le16 command;
> >          /*
> >           * 0 - self
> > -         * 1 - 65535 are reserved
> > +         * 1 - other virtio device (identified by vdev_id) in the same device
> group
> > +         * 2 - 65535 are reserved
> >           */
> >          le16 dst_type;
> > +        le64 vdev_id;
> 
> Alignment problems. Proposal:
> 
> vdev_id
> dst_type
> command

I remember that this has come up internal review as well.
Though it is certainly good to naturally align, I don't think we have alignment problem spec wise based on below snippet of the spec [1].
It was kind of counter intuitive to see vdev_id before seeing what the actual command is.
That way current layout made more sense.

There was some internal version or external, do not recall, where vdev_id was optional or was union.

But I think if we always going have vdev_id, it can be naturally aligned like your above proposal.

[1] "Structure Specifications
Many device and driver in-memory structure layouts are documented using the C struct syntax. All structures
are assumed to be without additional padding. 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."


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]