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

[Max Gurtovoy <mgurtovoy@nvidia.com>]

On 8/3/2021 11:55 AM, Cornelia Huck wrote:
> 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.

It's also doesn't allow forward compatibility.

Do you really want to add SW for each new HW cap ?

We're having difficulties because of this non fwd-compatible handshake.