From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 3 Aug 2022 02:10:54 -0400 From: "Michael S. Tsirkin" Subject: Re: [PATCH v6 1/5] Introduce device group Message-ID: <20220803020125-mutt-send-email-mst@kernel.org> References: <20220731154354.15698-1-mgurtovoy@nvidia.com> <20220731154354.15698-2-mgurtovoy@nvidia.com> <20220802092302-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline To: Jason Wang Cc: Max Gurtovoy , virtio-comment@lists.oasis-open.org, Cornelia Huck , Virtio-Dev , Oren Duer , Parav Pandit , Shahaf Shuler , Ariel Adam , virtio@lists.oasis-open.org, eperezma List-ID: 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 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 > >