Re: [virtio-comment] Re: [PATCH] virtio-pci: introduce VIRITO_F_QUEUE_STATE

[Cornelia Huck <cohuck@redhat.com>]

On Wed, 17 Mar 2021 11:43:53 +0800
Jason Wang <jasowang@redhat.com> wrote:

> å 2021/3/16 äå7:06, Cornelia Huck åé:
> > On Tue, 16 Mar 2021 10:53:37 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >  
> >> å 2021/3/15 äå11:24, Cornelia Huck åé:  
> >>> On Mon, 15 Mar 2021 10:58:46 +0800
> >>> Jason Wang <jasowang@redhat.com> wrote:

> >>>>    \drivernormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> >>>>    
> >>>>    The driver MUST NOT write to \field{device_feature}, \field{num_queues}, \field{config_generation}, \field{queue_notify_off} or \field{queue_notify_data}.
> >>>> @@ -981,6 +1009,13 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> >>>>    
> >>>>    The driver MUST NOT write a 0 to \field{queue_enable}.
> >>>>    
> >>>> +If VIRTIO_F_QUEUE_STATE has been negotiated, a driver SHOULD set the
> >>>> +state of each virtqueue through \field{queue_state} before setting the
> >>>> +DRIVER_OK \field{device status} bit and SHOULD NOT write to
> >>>> +\field{queue_state} after setting the DRIVER_OK \field{device status}
> >>>> +bit. If a driver want to get the virtqueue state, it MUST first reset
> >>>> +the device then read state from \field{queue_state}.  
> >>> What should the driver do with a 'fresh' device? Does it need to start
> >>> out with a reset, read the (zero) state, and then write it back?  
> >>
> >> If 'fresh' means a normal probe procedure, in this case we don't need to
> >> get the virtqueue state. What we need is to set a proper state. For
> >> split virtqueue, the driver should write 0 (as last_avail_idx). For
> >> packed virtqueue, the driver shoudl write:
> >>
> >> {.last_avail_idx = 0, .last_avail_wrap_counter=1, .used_idx=0,
> >> used_wrap_counter=1}.  
> > Yes, that was what I had been thinking of.
> >
> > So, I think the driver won't get a useful state if it reads the state
> > after reset for a previously unused device (the device statement only
> > says that it MUST NOT clear the state after reset)?  
> 
> 
> Yes.
> 
> 
> > Do we need to add a
> > sentence that a driver needs to do that initial setup of the state for
> > a device it has not used previously?  
> 
> 
> I'm not sure whether we need to treat the device which has not been used 
> separatedly. Technically, when VIRTIO_F_QUEUE_STATE is neogitated, 
> driver can teach the device to start from non 0 index (though it was not 
> the way how Linux did right now).

Basically, the driver needs to be aware that there simply will not be
any usable state, and it needs to skip the getting and instead set some
initial values it deems reasonable. If that's obvious, I don't think we
need to spell it out.