Re: [PATCH V2 1/6] virtio: introduce virtqueue state

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 11/6/2023 5:45 PM, Michael S. Tsirkin wrote:
> On Mon, Nov 06, 2023 at 05:42:10PM +0800, Zhu, Lingshan wrote:
> > On 11/6/2023 5:35 PM, Michael S. Tsirkin wrote:
> > > On Fri, Nov 03, 2023 at 10:49:42PM +0800, Zhu, Lingshan wrote:
> > > >           +When SUSPEND is set, the device MUST record the Available State of every enabled splited virtqueue
> > > >           +in \field{Available State} field,
> > > >           +and correspondingly restore the Available State of every enabled splited virtqueue
> > > >           +from \field{Available State} field when DRIVER_OK is set.
> > > >           +
> > > >           +The device SHOULD reset \field{Available State} field upon a device reset.
> > > >
> > > >       At this point I have no idea
> > > >       - how can a state of a virtqueue at a random time be represented
> > > >         by a 16 bit integer
> > > >
> > > > not sure what is a random time, this is to request the device to reset
> > > > its avail state, for example, it is "le16 queue_avail_state" in virtio-pci
> > > > common cfg. Resetting this so the device will not recover from a wrong value of
> > > > the last run.
> > > You simply never bother to say what is "Available State" and what
> > > does it mean to restore it.  Not to mention words like "splited"
> > > which just adds to the confusion.
> > It says:
> > +The available state field is two bytes of virtqueue state that is used by
> > +the device to read the next available buffer. It is presented in the
> > following format:
> >
> > Do you want me to add more descriptions?
> maybe start with an example
I think they are already in the spec, I can add:
see also "2.7.6 The Virtqueue Available Ring" and "2.7.13.1 Placing 
Buffers Into The Descriptor Table"
> > > >       - if it's not at a random time then why do you even need an integer -
> > > >         synchronize queue to memory and then all state is in memory
> > > >
> > > > Not sure what is a sync queue, but for example, "le16 queue_avail_state" for
> > > > PCI transport exists in a cap.
> > > I just point out that normally a lot of ring state is in memory.
> > > So you need to be much more specific about how you are augmenting that.
> > > For example, if buffers are used exactly in order for a split ring
> > > then used index seems to be exactly the same as last available index
> > > you describe - it's a free running counter. OTOH if they are not
> > > used in order then I don't see how is a single index sufficient to
> > > describe which ones have been used and which not.
> > I am not sure I get it.
> >
> > Used idx(not like packed vq, no over-writing descriptors) and other states
> > are in guest memory, so migrated with guest migration.
> yes and so? why is that not enough and what is this available state then?
The spec has illustrated how available index work and has given an 
example(see above cited sections)
And this patch even has given a more clear description for it.

Other states are in guest memory and migrated with guest memory.
>