On Wed, Apr 05, 2023 at 01:20:04PM +0000, Parav Pandit wrote: > > > > From: Michael S. Tsirkin <m...@redhat.com> > > Sent: Wednesday, April 5, 2023 1:22 AM > > > > This is not much of an improvement. > > > > On Wed, Apr 05, 2023 at 04:06:50AM +0300, Parav Pandit wrote: > > > Currently queue_notify_data register indicates the device internal > > > queue notification identifier. This register is meaningful only when > > > feature bit VIRTIO_F_NOTIF_CONFIG_DATA is negotiated. > > > > > > However, above register name often get confusing association with very > > > similar feature bit VIRTIO_F_NOTIFICATION_DATA. > > > > > > When VIRTIO_F_NOTIFICATION_DATA feature bit is negotiated, > > > notification really involves sending additional queue progress related > > > information (not queue identifier). > > > > > > Hence > > > 1. to avoid any misunderstanding and association of queue_notify_data > > > with similar name VIRTIO_F_NOTIFICATION_DATA, > > > > > > and > > > 2. to reflect that queue_notify_data is the actual device internal vq > > > identifier, rename it to queue_notify_id. > > > > > > Reflect vq identifier in the driver notification structure by renaming > > > ambiguous vqn to vq_notify_id. > > > > > > The driver notification section assumes that queue notification > > > contains vq index always. CONFIG_DATA feature bit introduction missed > > > to update the driver notification section. Hence, correct it. > > > > > > Signed-off-by: Parav Pandit <pa...@nvidia.com> > > > > > > --- > > > Some side notes: > > > renaming vqn to vqnd is even more confusing because data is really the > > > queue identifier. > > > > Clear to whom? Why do you think so? Marvell who pushed this feature said > > they stick some kind of constant value there which matches what their > > hardware expects. Sounds like a valid way to use this. > Spec currently has described it as "identifier".
Nope, it gives an identifier as one example of a legal value. > Snippet: > "for example an internal virtqueue identifier, > or an internal offset related to the virtqueue number" > > > So no, not an identifier, and in any case "vq identifier" is a really > > general and > > useful term, I would rather not burn it up on a baroque feature that almost > > no > > one sets - for almost everyone else this is simply vq index. > > > > If you really insist on renaming this away from "vqn", maybe > > vq_index_config_data will do, and we can add a comment /* Either vq index or > > vq config data, previously named vqn */ > > > Vq_index_config_data also aligns with the crazy feature bit name. > So I will use that. :) It's just standard thing you get if you overload fields. And this is data path we could not just add fields easily. -- MST --------------------------------------------------------------------- To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org