Re: [PATCH] content: document console vq detach buffer

[Pankaj Gupta <pagupta@redhat.com>]

> 
> On Tue, Aug 13, 2019 at 06:21:33PM +0530, Pankaj Gupta wrote:
> > This patch documents console special case where vq buffers
> > are deleted at port hotunplug time. This behavior is different in
> > other devices where vq buffers are deleted at device unplug time.
> > 
> > Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> > ---
> >  content.tex | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index ee0d7c9..33e8ccc 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -497,6 +497,8 @@ \section{Device Cleanup}\label{sec:General
> > Initialization And Device Operation /
> >  of a live virtqueue.
> >  
> >  Thus a driver MUST ensure a virtqueue isn't live (by device reset) before
> >  removing exposed buffers.
> > +Console has a special property that when port is detached virtqueue is
> > considered stopped, device
> > +must not use any buffers, and it is legal to take buffers out of the
> > device.
> >  
> >  \chapter{Virtio Transport Options}\label{sec:Virtio Transport Options}
> 
> I don't think that's enough, or a good way to do it.
> 
> The assumption that once exposed buffers stay exposed until used
> is spread out in lots of places in the spec.

oh... I did not notice the other places.

> 	E.g.
> 		2.6.6.1
> 		Driver Requirements: The Virtqueue Available Ring
> 		A driver MUST NOT decrement the available idx on a virtqueue (ie. there is
> 		no way to âunexposeâ buffers).
> 
> 	And in the packed ring part, e.g.
> 
> 	A device MUST NOT use a descriptor unless it observes the VIRTQ_DESC_F_AVAIL
> 	bit in its flags
> 	being changed (e.g. as compared to the initial zero value). A device MUST
> 	NOT change a descriptor after
> 	changing itâs the VIRTQ_DESC_F_USED bit in its flags.
> 	2.7.16
> 	Driver Requirements: The Virtqueue Descriptor Table
> 	A driver MUST NOT change a descriptor unless it observes the
> 	VIRTQ_DESC_F_USED bit in its flags being
> 	changed. A driver MUST NOT change a descriptor after changing the
> 	VIRTQ_DESC_F_AVAIL bit in its flags.
> 
> So we need to document how exactly to revert added buffers in all
> these places. I propose adding special sections, explaining
> that some devices allow taking back available buffers.

Sure.

> 
> We need to see how this interacts with other places in the spec:
> e.g. if we do allow taking back buffers, what happens with notifications
> such as when using event index? Are they resent when buffer is
> re-added?

o.k. Will check this.

> 
> 
> I also worry that since the spec said this can't happen, some
> hypervisors might implement a policy that will crash if the guest
> violates this rule.  Seems unlikely since Linux violated the rule for a
> while, but I'd suggest that you take a look at least at some open-source
> hypervisors to see what is going on. 

Good point. I will check.

An alternative is a feature flag -
> if we do that I would actually advocate for a generic feature that
> allows stopping queues at any time, then restarting it. I think it would
> be handy generally for things like enabling/disabling XDP.

o.k. Good idea. Generic queue stopping will require additional work
e.g draining the queue and might require certain device specific operation.
Will look at this.
  

Thank you for the suggestions.

Best regards,
Pankaj


> 
> 
> 
> > --
> > 2.21.0
>