Re: [virtio-dev] [PATCH v3] content: enhance device requirements for feature bits

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

On Fri, Jun 15, 2018 at 02:42:58PM +0200, Halil Pasic wrote:
> 
> 
> On 06/15/2018 02:19 PM, Michael S. Tsirkin wrote:
> > On Fri, Jun 15, 2018 at 02:10:11PM +0200, Halil Pasic wrote:
> > > 
> > > 
> > > On 06/11/2018 09:56 AM, Tiwei Bie wrote:
> > > > Suggested-by: Michael S. Tsirkin <mst@redhat.com>
> > > > Signed-off-by: Tiwei Bie <tiwei.bie@intel.com>
> > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/14
> > > > ---
> > > > v2:
> > > > - Refine the wording (Cornelia);
> > > > 
> > > > v3:
> > > > - Refine the wording (MST);
> > > > 
> > > >    content.tex | 7 +++++++
> > > >    1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/content.tex b/content.tex
> > > > index f996fad..3c7d67d 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -125,6 +125,13 @@ which was not offered.  The device SHOULD accept any valid subset
> > > >    of features the driver accepts, otherwise it MUST fail to set the
> > > >    FEATURES_OK \field{device status} bit when the driver writes it.
> > > > +If a device has successfully negotiated a set of features
> > > > +at least once (by accepting the FEATURES_OK \field{device
> > > > +status} bit during device initialization), then it SHOULD
> > > > +NOT fail re-negotiation of the same set of features after
> > > > +a device or system reset.  Failure to do so would interfere
> > > > +with resuming from suspend and error recovery.
> > > > +
> > > 
> > > 
> > > Sorry people but I don't get it. I mean it is kind of reasonable
> > > to assume that with a given device and a given driver (given, i.e.
> > > nothing changes) the two will always negotiate the same features
> > > (including the extremal case where the negotiation fails).
> > > 
> > > Either the device or a driver rolling a dice to make feature negotiation
> > > more fun seems quite unreasonable. So I assume this is not what we are
> > > bothering to soft prohibit here.
> > > 
> > > So the interesting scenario seems to be when stuff changes. When
> > > migrating the implementation of the device could change. Or something
> > > changes regarding the resources used to provide the virtual device.
> > > 
> > > But then, if the device really can not support the set of features
> > > it used to be able, I guess the SHOULD does not take effect (I guess
> > > that is the difference compared to MUST).
> > > 
> > > Bottom line is: I tried to figure out what is this about, but I failed.
> > > I've read https://github.com/oasis-tcs/virtio-spec/issues/14 too but
> > > it did not click. I would appreciate some assistance.
> > 
> > It's exactly what it says. Let's say you negotiated a feature and then
> > device sets NEED_RESET.  Driver must now reset the device and put it
> > back in the same state it had before the reset, then resubmit
> > requests that were available but never used.
> > 
> > What if any of the features changed? Device suddenly
> > needs to check for requests which do not match the
> > features.
> > 
> > Suspend is similar: guests tend to assume hardware does not change
> > across suspend/resume, any changes tend to make resume fail.
> > 
> 
> Thank you very much! But it still does not answer why would a device
> want to do that (fail to negotiate a feature that it was able to
> negotiate before). So I'm still in the dark about what are we trading
> for what.

It would be a mis-configured device.  For example QEMU does not migrate
the device features so if you misconfigure QEMU with different flags on
source and destination (not a supported configuration), features might
seem to change from guest POV.

> Is there somewhere a patch that fixes such a bug? Maybe that would
> help me understand what can be done at the device to avoid the
> problem.
> 
> Regards,
> Halil
> 
> 
> > > 
> > > >    \subsection{Legacy Interface: A Note on Feature
> > > >    Bits}\label{sec:Basic Facilities of a Virtio Device / Feature
> > > >    Bits / Legacy Interface: A Note on Feature Bits}
> > > > 
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > 

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org