Re: [virtio-comment] Re: [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Aug 03 2021, Max Gurtovoy <mgurtovoy@nvidia.com> wrote:

> On 8/3/2021 9:51 AM, Cornelia Huck wrote:
>> On Tue, Aug 03 2021, Jason Wang <jasowang@redhat.com> wrote:
>>
>>> å 2021/8/3 äå2:28, Cornelia Huck åé:
>>>> On Mon, Aug 02 2021, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>>>
>>>>> On Mon, Aug 02, 2021 at 07:03:11PM +0300, Max Gurtovoy wrote:
>>>>>> There is no much bits left in the generic feature field for all the features
>>>>>> we would like to add.
>>>>>> I mentioned only 5-6 in the above example and it will bring us to bit 46
>>>>>> already.
>>>>>>
>>>>>> please think of 5-10 years from today.
>>>>>>
>>>>> IIUC nothing prevents adding more once we exhaust 64 bits. IMHO it's actually
>>>>> pretty important to make sure the feature negotiation works well
>>>>> and covers relevant usecases. If we have limitations preventing that
>>>>> I'd like to at least try to fix that not replacing feature negotiation with
>>>>> something else.
>>>> I recall that we had a discussion about that years ago when we
>>>> introduced VERSION_1; we explicitly agreed that we can extend features
>>>> beyond 64 bit once we need it. (A quick search did not turn up that mail
>>>> exchange, though.)
>>>>
>>> E.g PCI transport has feature_select.
>>>
>>>   ÂÂÂÂÂÂÂ le32 device_feature_select;ÂÂÂÂ /* read-write */
>>>   ÂÂÂÂÂÂÂ le32 device_feature;ÂÂÂÂÂÂÂÂÂÂÂ /* read-only for driver */
>>>   ÂÂÂÂÂÂÂ le32 driver_feature_select;ÂÂÂÂ /* read-write */
>>>   ÂÂÂÂÂÂÂ le32 driver_feature;ÂÂÂÂÂÂÂÂÂÂÂ /* read-write */
>>>
>>> Technically it can support 32*32 different features.
>> MMIO uses a similar scheme; CCW uses
>>
>> struct virtio_feature_desc {
>>         le32 features;
>>         u8 index;
>> };
>>
>> so there's plenty of room to spare.
>
> Feature negotiation can be done in admin q level. You don't want this 
> branches in the spec per each transport or device type.

This has nothing to do with transports. We only wanted to demonstrate
that all of the transports can be extended, so feature bits are not a
scarce resource.

>
> We have an admin command to get all device admin q features and we can 
> create a command to set all the driver supported.
>
> Again, I don't see the point of doing that. If a device is supporting 
> command_A, the driver should be able to use it. Telling the device "hi, 
> I'm going to use command_A in the future" is redundant.
>
> The device knows already how to handle it.

If the driver negotiates a certain feature, it can mean that the device
can make certain optimizations.

That's not a particularily exotic pattern.