[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v9 14/16] VIRTIO_F_NOTIFICATION_DATA: extra data to devices
On Wed, Mar 07, 2018 at 03:49:41PM +0100, Cornelia Huck wrote: > On Wed, 7 Mar 2018 16:09:33 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Wed, Mar 07, 2018 at 12:11:58PM +0100, Cornelia Huck wrote: > > > On Thu, 1 Mar 2018 01:31:37 +0200 > > > "Michael S. Tsirkin" <mst@redhat.com> 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? > > > > > @@ -2340,12 +2455,22 @@ GPR & Input Value & Output Value \\ > > > > \hline > > > > 2 & Subchannel ID & Host Cookie \\ > > > > \hline > > > > - 3 & Virtqueue number & \\ > > > > + 3 & Notification data & \\ > > > > \hline > > > > 4 & Host Cookie & \\ > > > > \hline > > > > \end{tabular} > > > > > > > > +When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, > > > > +the \field{Notification data} includes the Virtqueue number. > > > > + > > > > +When VIRTIO_F_NOTIFICATION_DATA has been negotiated, > > > > +the value has the following format: > > > > +\lstinputlisting{notifications.c} > > > > > > And we probably want to make this be explicitly. > > > > Are you sure? > > I looked at s390 code and it just uses VQ index in native > > endian-ness, so I kept this consistent. > > Native endian is always be on s390. The registers for the hypercall are > therefore always be. Oh I thought there's ppc LE now. > > > > diff --git a/notifications.c b/notifications.c > > > > new file mode 100644 > > > > index 0000000..2ae96d4 > > > > --- /dev/null > > > > +++ b/notifications.c > > > > @@ -0,0 +1,3 @@ > > > > +u32 vqn : 16, > > > > + next_off : 15, > > > > + next_wrap : 1; > > > > > > I'm wondering how useful the u32 notation is here. > > > > It says vqn in low 16 bits of a 32 bit counter, so e.g. > > on LE system byte 0 and on BE system byte 3. > > Compare to > > > > be32 vqn : 16, > > next_off : 15, > > next_wrap : 1; > > > > where we say it's in low 16 bits of a 32 bit BE integer, > > so byte 3. > > I was referring to the endianness: AFAICS, pci and mmio want le, ccw > wants be. If we want to be explicit, notifications.c does not make much > sense IMO. Will split up.
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]