Re: [PATCH v6 1/5] Introduce device group

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

On Wed, Aug 03, 2022 at 12:44:38PM +0800, Jason Wang wrote:
> On Tue, Aug 2, 2022 at 9:42 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > I feel some of my latest review opened some questions that I don't have
> > good answers for and might have felt a bit rambling.
> > So to focus the discussion:
> >
> > On Sun, Jul 31, 2022 at 06:43:50PM +0300, Max Gurtovoy wrote:
> > > +A device can be a member of one or more device groups.
> >
> > Presumably this is so we can e.g. create subfunctions inside a VF.
> 
> Then VF should have its own transport virtqueue. And subfunctions need
> to be created there. If we don't all thing in PF, we may end up with
> nesting issue when assign VF to the guest.
> > A VF now is a member of a SRIOV and SIOV type groups and we
> > can use type to distinguish between these.
> >
> > We should probably be explicit that each of these groups has to
> > have a distinct group type then.
> >
> > And this raises the question: different types have different
> > capabilities. So let's say admin queue is used to both
> > control features for SRIOV VFs and to create SIOV SFs.
> 
> I don't get how the admin queue can be used to control VF features
> considering VF has its capabilities. (SR-IOV lacks the ability to
> provision a single VF).

Well look at latest proposal, last patch controls VF features from PF.

> > I guess we'll have a feature bit to say "command to create
> > SIOV SFs is supported" but how do we say that this command
> > is only supported for VFs not SFs?
> 
> I think we should first answer if having VF and SF to be dealt with a
> single type of virtqueue is a good idea. They have something in common
> but they distinguish each other:
> 
> - SF requires per virtual device lifecycle management
> - SF requires a transport other than PCI
> - SF requires more mediation in the software layer for presenting a
> virtual device
> 
> Using a single type of virtqueue may end up with complex design.
> Having a dedicated queue for SF might be a better choice.

And dedicated feature bits for commands thereof?  For example, I imagine
we could have commands to control the MAC of the group member. That is
the same for SF and VF, yes? How do we avoid duplication for that?

> >
> > Do we just make features list a superset of what is supported and simply
> > say in the spec which commands are legal with which group types?
> >
> >
> > Jason Cornelia what do you think?
> 
> It looks to me it would be much more simpler if we use separated
> virtqueues for SRIOV and SIOV.
> 
> Thanks

Then is it still helpful that we have the generic group type concept?
I was hoping it will work so the same command can be used for VFs
and SFs.


> >
> >
> >
> > > +\item Self type (group identifier = 0) - this group has only one device in the group. Each virtio device is a member of at least one device group, the Self type group.
> >
> > Presumably, this is here so we can send commands that refer to the
> > device itself as opposed to a group member (e.g. to
> > PF as opposed to VF). Is that right?
> >
> > It's handy but again the problem here is, this refers to
> > device as part of which group? Let's just drop this type?
> >
> >
> > --
> > MST
> >