Re: [PATCH v11 02/10] admin: introduce device group and related concepts

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

On Wed, Apr 05, 2023 at 11:42:24PM -0400, Parav Pandit wrote:
> 
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > Each device group has a type. For now, define one initial group type:
> > 
> > SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given
> > PCI SR-IOV physical function (PF). This group may contain zero or more
> > virtio devices according to NumVFs configured.
> > 
> > Each device within a group has a unique identifier. This identifier
> > is the group member identifier.
> > 
> > Note: one can argue both ways whether the new device group handling
> > functionality (this and following patches) is closer
> > to a new device type or a new transport type.
> > 
> From this point onward you can add,
> --- line. So that it doesnt go to the commit log.

I can but motivation for design choices is a good thing to have
in the commit log.

> > However, I expect that we will add more features in the near future. To
> > facilitate this as much as possible of the text is located in the new
> > admin chapter.
> > 
> > I did my best to minimize transport-specific text.
> > 
> > There's a bit of duplication with 0x1 repeated twice and
> > no special section for group type identifiers.
> > I think it's ok to defer adding these until we have more group
> > types.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > changes in v11:
> > 	dropped Max's S.O.B.
> > 	dropped an unmatched ) as reported by Parav
> > 	document that member id is 1 to numvfs
> > 	document that vf enable must be set (will also be reflected in
> > 		the conformance section)
> > 	document that we deferred generalizing text as requested by Parav -
> > 	I think we can do it later
> > 	minor wording fixes to address comments by David
> > 	add concepts of owner and member driver. unused currently
> > 	but will come in handy later, as suggested by Stefan
> > ---
> >   admin.tex   | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   content.tex |  2 ++
> >   2 files changed, 65 insertions(+)
> >   create mode 100644 admin.tex
> > 
> > diff --git a/admin.tex b/admin.tex
> > new file mode 100644
> > index 0000000..04d5498
> > --- /dev/null
> > +++ b/admin.tex
> > @@ -0,0 +1,63 @@
> > +\section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device groups}
> > +
> > +It is occasionally useful to have a device control a group of
> > +other devices. Terminology used in such cases:
> > +
> Only one case so, singular "such case:".
> 
> > +\begin{description}
> > +\item[Device group]
> > +        or just group, includes zero or more devices.
> > +\item[Owner device]
> > +        or owner, the device controlling the group.
> > +\item[Member device]
> > +        a device within a group. The owner device itself is not
> is a device within a group.
> So that the sentence completes like above two items.
> 
> > +	a member of the group.
> > +\item[Member identifier]
> > +        each member has this identifier, unique within the group
> > +	and used to address it through the owner device.
> to form the complete sentence like rest of the items writing it as below:
> 
> uniquely identifies a member within a group and is used to address it
> through the owner device.
> 
> > +\item[Group type identifier]
> > +	specifies what kind of member devices there are in a
> > +	group, how the member identifier is interpreted
> > +	and what kind of control the owner has.
> > +	A given owner can control multiple groups
> > +	of different types but only a single group of a given type,
> > +	thus the type and the owner together identify the group.
> > +	\footnote{Even though some group types only support
> > +			specific transports, group type identifiers
> > +			are global rather than transport-specific -
> > +			we don't expect a flood of new group types.}
> As as spec, better to avoid "we .."
> How about,
> too many new group types are not expected.?
> > +\end{description}
> > +
> > +\begin{note}
> > +Each device only has a single driver, thus for the purposes of
> > +this section, "the driver" is usually unambiguous and refers to
> > +the driver of the owner device.  When there's ambiguity, "owner
> > +driver" refers to the driver of the owner device, while "member
> > +driver" refers to the driver of a member device.
> > +\end{note}
> > +
> > +The following group types, and their identifiers, are currently specified:
> > +\begin{description}
> > +\item[SR-IOV group type (0x1)]
> > +This device group has a PCI Single Root I/O Virtualization
> > +(SR-IOV) physical function (PF) device as the owner and includes
> > +all its SR-IOV virtual functions (VFs) as members (see
> > +\hyperref[intro:PCIe]{[PCIe]}).
> > +
> > +The PF device itself is not a member of the group.
> > +
> > +The group type identifier for this group is 0x1.
> > +
> > +A member identifier for this group can have a value from 0x1 to
> > +\field{NumVFs} as specified in the
> > +SR-IOV Extended Capability of the owner device
> > +and equals the SR-IOV VF number of the member device;
> > +the group only exists when the \field{VF Enable} bit
> > +in the SR-IOV Control Register within the
> > +SR-IOV Extended Capability of the owner device is set
> > +(see \hyperref[intro:PCIe]{[PCIe]}).
> > +
> > +Both owner and member devices for this group type use the Virtio
> > +PCI transport (see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus}).
> > +\end{description}
> > +
> > +
> > diff --git a/content.tex b/content.tex
> > index 8098988..04592fb 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -493,6 +493,8 @@ \section{Exporting Objects}\label{sec:Basic Facilities of a Virtio Device / Expo
> >   types. It is RECOMMENDED that devices generate version 4
> >   UUIDs as specified by \hyperref[intro:rfc4122]{[RFC4122]}.
> > +\input{admin.tex}
> > +
> >   \chapter{General Initialization And Device Operation}\label{sec:General Initialization And Device Operation}
> >   We start with an overview of device initialization, then expand on the
> 
> Apart from above rewording,
> 
> Reviewed-by: Parav Pandit <parav@nvidia.com>