Re: [virtio-comment] Re: [PATCH 0/2] Selective queue enabling

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

On Tue, Jun 13, 2023 at 09:09:05PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, June 13, 2023 3:55 PM
> > 
> > I do not get how.  Let's make sure we are talking about the same thing.
> > This is what I am saying:
> > 
> > Start operating:
> > 
> > - program queue address, size
> > - queue_enable
> > 
> > Now we change our mind about queue size or address. So:
> > 
> > - queue_reset
> 
> Why queue_reset is needed here?
> I don't see spec saying that queue_enable must be done only once for a given Q before DRIVER_OK.

I see you found it.

> > - program another queue address, size
> > 
> > Finally we are decided, let's start operating:
> > 
> > - DRIVER_OK
> > 
> > 
> > I don't see how it's possible with just queue_enable.
> > 
> > 
> > >
> > > It is not explicitly mentioned in the spec that one can setup the queue using
> > queue reset instead of queue enable.
> > > As Jason mentioned it is implementation specific, one device supports it and
> > one doesn't.
> > > Hence, it will break on those devices which doesnt support it.
> > >
> > 
> > Setup? No, and spec explicitly says to setup one has to use queue_enable. But
> > this is not what we discussed with Stefano here.
> > 
> The above sequence you described is not well documented.

It's the standard sequence for queue reset, is it not?

For example:

	Virtqueue reset is divided into two parts. The driver first resets a queue and
	can afterwards optionally re-enable it.


it seems clear that queue has to be enabled before being reset.



> > > Therefore, I would like to add it to the spec.
> > > "queue_reset register MUST be accessed by the driver only after device has
> > reached the DRIVER_OK stage."
> > 
> > I don't think we can add MUST requirements after the proposal is in the
> > released spec. If device has stricter requirements than the spec then it's not
> > compliant.
> >
> Lets resolve above question first about how many times one can use queue_enable before DRIVER_OK for a specific VQ.
> Without this either way driver and device combination is broken when queue_reset is used before DRIVER_OK.

I'm not sure what's broken.


If VIRTIO_F_RING_RESET has been negotiated, after the driver writes 1 to
\field{queue_reset} to reset the queue, the driver MUST NOT consider queue
reset to be complete until it reads back 0 in \field{queue_reset}. The driver
MAY re-enable the queue by writing 1 to \field{queue_enable} after ensuring
that other virtqueue fields have been set up correctly. The driver MAY set
driver-writeable queue configuration values to different values than those that
were used before the queue reset.


So the limitation is that it happens after FEATURES_OK.


-- 
MST