RE: [virtio-dev] RE: [PATCH v5 0/7] Introduce device group and device management

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, July 6, 2022 7:00 AM

> > Reader and writer via different channel is really a hack.
> > It still doesn't help to have efficient hw.
> >
> > What is the technical reason to use feature bits which has strong relationships
> with device initialization sequence?
> 
> The reason is it's a reasonably well understood mechanism, already addressing
> questions like lifecycle, dependencies, extensibility.  For example, one might
> reasonably ask what happens if driver uses commands to change number of
> MSI-X vectors, then disables the capability bit related to relevant commands?
> Should device forget the allocated vectors? There's no longer a way to change
> the allocation...
> If the set of allowed commands are locked in place until reset then it's clearer.
> 
I can't think of a real use case why a driver would do something like this.
But say its done, it means that driver is not interested of assigning the MSI-X resource anymore from the AQ.
The PF driver did something nasty that it should well behave.
All configurations done stays as-is.

The first line of virtio spec introducing virtqueue is : " The mechanism for bulk data transport on virtio devices is pretentiously called a virtqueue".

This is no different than doing queue_reset on the AQ, what happens on the resource configuration done via AQ before reset?
What happens to packets and IO sent via virtio before the queue reset?
If virtqueue is destroyed and recreate.. things happened before cannot be reverted.
Same behavior when capabilities are switched off...
Device won't allow any new configuration.

This is matter of wording it...

This is important; I will get this text added in the v6.

> Again I am sure the specific question can be answered. But I would really just like
> to see the admin queue proposal get finalized as quickly as possible, this means
> making compromizes to reuse as much of existing functionality as possible. And
> we can add extensions down the road.
> 
We can add new capability command later, but there is no point duplicating the feature bit at that point.

> > The proposed bits are AQ functionality bits and not per_say device feature
> bits?
> 
> Could not parse this sentence.
> 
Meaning capabilities negotiated is about what all different config will happen via AQ.

> > This enables to build device in lot more flexible (and frankly simple way).
> > Looking at sw guest driver being simple to use existing limited 64-bits is just
> one part of it.
> 
> I'm not sure what does this refers to. What was suggested using e.g. feature bits
> 64 to 127, not stealing some of the 64 ones in use.
> All existing transports support doing that without adding any new transport
> specific interfaces.
> 
> Yes adding commands to tweak features might be benefitial.  I just suggest
> splitting that out simply because this might or might not make sense for all
> features.  For example, a related common request is modifying feature bits and
> generally some config space on the device side to facilitate migration across
> devices with varying features.
> Should that be included in the proposal then?
> 
> > And when AQ exists, most of the plumbing comes for free.
> > So I fail to see the simplicity benefit of overloading feature bits for things that
> has no relation to device init sequence.
> 
> Well most of feature bits are not *directly* tied to device init sequence.
Sure, but that was past. When feature bits introduced unrelated to device init sequence, at that time there was no AQ in the same spec update.
So it might have been fine then.

> 
> The similicity just comes from reusing an existing mechanism so people do not
> need to learn a new one.
> 
I think the device efficiency weights more than driver code learning about AQ specific bits in generic device init sequence.

> Again I don't know what to do with this. I feel if it's put up for vote in the current
> form it's likely to fail. I propose cutting out as much as possible as a first step, so
> we can make progress. Specifically the MSI-X commands are clearly PF specific
> so there's no concern about VF memory use at all.  We can worry about other
> types of command down the road when it becomes relevant.

Since feature bits proposes are limited to PF, I agree that current short cut is fine to place in 64-127 feature bits.

When/if similar functionality is needed at scale for the VF or SIOV devices, placing them in 64-127 bits area weight way less for sake of "people familiarity to feature bits".
Do you agree?