RE: [virtio-dev] [PATCH v2] virtio: Improve queue_reset polarity to match to default reset state

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, April 27, 2022 11:44 AM
> 
> On Wed, Apr 27, 2022 at 03:39:40PM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Wednesday, April 27, 2022 11:30 AM
> > >
> > > On Wed, Apr 27, 2022 at 02:51:36PM +0000, Parav Pandit wrote:
> > > >
> > > > > From: Jason Wang <jasowang@redhat.com>
> > > > > Sent: Wednesday, April 27, 2022 7:30 AM
> > > > >
> > > > > On Wed, Apr 27, 2022 at 6:26 PM Parav Pandit <parav@nvidia.com>
> wrote:
> > > > > >
> > > > > > example flow:
> > > > > > a) 0,0 -> device init time value
> > > > > > b) 1,0 -> vq is enabled by driver and working
> > > > >
> > > > > Did you see my reply in V1? What's the reason for using write to
> > > > > clear behavior that is different from the device status?
> > > > >
> > > > > We can simply make this as 1, 1 here and let the driver write to
> > > > > 0 to reset the virtqueue.
> > > > >
> > > > > And if we do this, the queue_enable and queue_reset are always
> > > > > the same, then we can simply reuse queue_enable.
> > > > >
> > > > Yes, I know we can make this work using new feature bit + single
> > > queue_enable register.
> > > > I replied that in v0 to Michael.
> > >
> > > A bigger question in my eyes is that down the road we might want to
> > > be able to stop the ring without having it lose state.
> > > The natural interface for that seems to be writing 0 to queue enable.
> > Why queue_enable and not queue_reset?
> 
> If what to disable ring without reset then writing into reset seems
> unintuitive.
> 
True. I assume you want to start the queue again later, hence the stop/start.
Make sense.

> > to me this interface is unlikely performant and useful for such case.
> > When we want to pause/stop the VQ and query the state we need
> performant scheme, that can even work in a batch for all the VQs.
> > At that point programming 64 registers to pause/stop VQ without losing
> state and querying its indices etc won't be scalable with register interface.
> > I imagine a AQ (likely) or some other interface.
> >
> > >
> > > > I was not sure how drastic that would be at this point in the spec
> > > > release cycle
> > > that Michael highlighted.
> > > > Hence, I proposed a minimal change fix to queue_reset register given
> timeline.
> > >
> > > Well if accepted this proposal is going to delay the release anyway.
> > > If we are doing a new feature then that can love alongside the one
> > > that is already in the spec.
> > I didn't quite understand your point.
> 
> I understood your "given timeline" to mean "to avoid delays in 1.2 release".
Yes.

> My point is any material change will mean a delay at this time.
But this is so basic.
It's hard to gaze at this spec for coming years and the code to see,
Hey sometimes 0 means disabled, sometime 0 means still enabled, sometime 1 means enabled, and sometimes 1 means now disabled...
And maintain those weird code in device side and extra state bits burning some expensive chip resource.

Is removing from 1.2 is equal delay to get is fixed in 1.3?
If yes, I make humble request to fix this and have errata.
Some of the professional standard bodies release the spec and short after that errata/ratification follows the release that resolve such small issues.
May be time for virtio spec to take this opportunity now and be bit agile on it.
Your call. :)