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

["Michael S. Tsirkin" <mst@redhat.com>]

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

> > 
> > >      - 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?

-- 
MST