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

[Halil Pasic <pasic@linux.ibm.com>]

On 06/15/2018 03:38 PM, Michael S. Tsirkin wrote:
> 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.
> 

Do you mean set (or rather restrict) what QEMU calls the host_features?

AFAIR there is no reset right after the migration. But yes if then there
is a reset and another migration. After a lots of thinking, it seems you
speak about the scenario I described in the answer to Tiwei Bie. But
there I also say that this statement you add here is not good enough for
that. Still puzzled.

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


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