[virtio-dev] [PATCH v11 00/10] Introduce device group and device management

[virtio-dev] [PATCH v11 00/10] Introduce device group and device management

[Michael S. Tsirkin]

Change log:

since v10:
	addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, Max
	Cornelia, do you want to pick up 10/10 separately?

since v9:
	addressed comments by Parav, Max, Cornelia, David and Zhu Lingshan:
		added link to errno header from Linux
		rename _MEM to _MEMBER
		admin vq num is zero based
		clarify who sends commands where
		minor english tweaks
		clarify command length
		specify interaction with sriov capability
		correct commit log - NumVFs can be 0

	i could not decide what should happen when VFs are
	disabled. for now did not specify.

since v8:
	addressed comments by Cornelia - as we agreed on list
	
since v7:
	make high level error codes match linux, with virtio specific codes
		in a separate field
	renamed _ACCEPT to _USE since that's what it does
	clarified forward compatibility and non pci transports
	support multiple admin vqs
	conformance statements
	lots of changes all over the place to I changed author from Max
	to myself. Don't need to take credit but also don't want
	to blame Max for my mistakes.

since v6:

	- removed some extentions intended for future use.
	  We'll do them when we get there.

	- brought back command list query from v5 in a simplified form -
	  it's here to address the case where a single parent
	  can address multiple groups, such as PF addressing
	  transport vq and sriov vfs.

	- attempt to make terminology more formal.
	In particular a term for whoever controls the group.
 	I am still going back
	and forth between "parent" and "owner" - owner might
	be better after all since it will work if we ever
	have a self group. For now it's parent.

TODO (maybe?) - probably ok to defer until this part is upstream:

	Add "all members" member id.

	Add commands for MSI, feature discovery.

	Add commands for transport vq.


My intent is to try and support both SR-IOV and SIOV
usecases with the same structure and maybe even the same
VQ.

For example, it might make sense to split creating/destroying
SIOV devices from the transport passing data from the guest - the
driver would then not negotiate VIRTIO_F_SR_IOV (which
then means auto-provisioning).

More ideas for use-cases:
virtio VF features query and configuration space provisioning
virtio VF resource (queues, msix vectors count) provisioning


Future directions (shouldn't block this patch)
- aborting commands - left for later. or is vq reset enough?
- should we rename structures from admin to group admin?
  left as is for now just in case we soon get more admin things which are
  not group administration. easy to change later.


Michael S. Tsirkin (10):
  virtio: document forward compatibility guarantees
  admin: introduce device group and related concepts
  admin: introduce group administration commands
  admin: introduce virtio admin virtqueues
  pci: add admin vq registers to virtio over pci
  mmio: document ADMIN_VQ as reserved
  ccw: document ADMIN_VQ as reserved
  admin: command list discovery
  admin: conformance clauses
  ccw: document more reserved features

 admin.tex        | 586 +++++++++++++++++++++++++++++++++++++++++++++++
 content.tex      | 113 ++++++++-
 introduction.tex |   3 +
 3 files changed, 700 insertions(+), 2 deletions(-)
 create mode 100644 admin.tex

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 01/10] virtio: document forward compatibility guarantees

[Michael S. Tsirkin]

Feature negotiation forms the basis of forward compatibility
guarantees of virtio but has never been properly documented.
Do it now.

Suggested-by: Halil Pasic <pasic@linux.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---

Changes in v11:
	make explanation about not using any unspecified bits
	more detailed as suggested by Cornelia and David
---
 content.tex | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/content.tex b/content.tex
index 0e474dd..8098988 100644
--- a/content.tex
+++ b/content.tex
@@ -114,21 +114,65 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
 In particular, new fields in the device configuration space are
 indicated by offering a new feature bit.
 
+To keep the feature negotiation mechanism extensible, it is
+important that devices \em{do not} offer any feature bits that
+they would not be able to handle if the driver accepted them
+(even though drivers are not supposed to accept any unspecified,
+reserved, or unsupported features even if offered, according to
+the specification.) Likewise, it is important that drivers \em{do
+not} accept feature bits they do not know how to handle (even
+though devices are not supposed to offer any unspecified,
+reserved, or unsupported features in the first place,
+according to the specification.) The preferred
+way for handling reserved and unexpected features is that the
+driver ignores them.
+
+In particular, this is
+especially important for features limited to specific transports,
+as enabling these for more transports in future versions of the
+specification is highly likely to require changing the behaviour
+from drivers and devices.  Drivers and devices supporting
+multiple transports need to carefully maintain per-transport
+lists of allowed features.
+
 \drivernormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device / Feature Bits}
 The driver MUST NOT accept a feature which the device did not offer,
 and MUST NOT accept a feature which requires another feature which was
 not accepted.
 
+The driver MUST validate the feature bits offered by the device.
+The driver MUST ignore and MUST NOT accept any feature bit that is
+\begin{itemize}
+\item not described in this specification,
+\item marked as reserved,
+\item not supported for the specific transport,
+\item not defined for the device type.
+\end{itemize}
+
 The driver SHOULD go into backwards compatibility mode
 if the device does not offer a feature it understands, otherwise MUST
 set the FAILED \field{device status} bit and cease initialization.
 
+By contrast, the driver MUST NOT fail solely because a feature
+it does not understand has been offered by the device.
+
 \devicenormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device / Feature Bits}
 The device MUST NOT offer a feature which requires another feature
 which was not offered.  The device SHOULD accept any valid subset
 of features the driver accepts, otherwise it MUST fail to set the
 FEATURES_OK \field{device status} bit when the driver writes it.
 
+The device MUST NOT offer feature bits corresponding to features
+it would not support if accepted by the driver (even if the
+driver is prohibited from accepting the feature bits by the
+specification); for the sake of clarity, this refers to feature
+bits not described in this specification, reserved feature bits
+and feature bits reserved or not supported for the specific
+transport or the specific device type, but this does not preclude
+devices written to a future version of this specification from
+offering such feature bits should such a specification have a
+provision for devices to support the corresponding features.
+
 If a device has successfully negotiated a set of features
 at least once (by accepting the FEATURES_OK \field{device
 status} bit during device initialization), then it SHOULD
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 01/10] virtio: document forward compatibility guarantees

[Zhu, Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Feature negotiation forms the basis of forward compatibility
> guarantees of virtio but has never been properly documented.
> Do it now.
>
> Suggested-by: Halil Pasic <pasic@linux.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> ---
>
> Changes in v11:
> 	make explanation about not using any unspecified bits
> 	more detailed as suggested by Cornelia and David
> ---
>   content.tex | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 0e474dd..8098988 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -114,21 +114,65 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>   In particular, new fields in the device configuration space are
>   indicated by offering a new feature bit.
>   
> +To keep the feature negotiation mechanism extensible, it is
> +important that devices \em{do not} offer any feature bits that
> +they would not be able to handle if the driver accepted them
> +(even though drivers are not supposed to accept any unspecified,
> +reserved, or unsupported features even if offered, according to
> +the specification.) Likewise, it is important that drivers \em{do
> +not} accept feature bits they do not know how to handle (even
> +though devices are not supposed to offer any unspecified,
> +reserved, or unsupported features in the first place,
> +according to the specification.) The preferred
> +way for handling reserved and unexpected features is that the
> +driver ignores them.
> +
> +In particular, this is
> +especially important for features limited to specific transports,
> +as enabling these for more transports in future versions of the
> +specification is highly likely to require changing the behaviour
> +from drivers and devices.  Drivers and devices supporting
> +multiple transports need to carefully maintain per-transport
> +lists of allowed features.
> +
>   \drivernormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device / Feature Bits}
>   The driver MUST NOT accept a feature which the device did not offer,
>   and MUST NOT accept a feature which requires another feature which was
>   not accepted.
>   
> +The driver MUST validate the feature bits offered by the device.
> +The driver MUST ignore and MUST NOT accept any feature bit that is
> +\begin{itemize}
> +\item not described in this specification,
> +\item marked as reserved,
> +\item not supported for the specific transport,
> +\item not defined for the device type.
> +\end{itemize}
> +
>   The driver SHOULD go into backwards compatibility mode
>   if the device does not offer a feature it understands, otherwise MUST
>   set the FAILED \field{device status} bit and cease initialization.
>   
> +By contrast, the driver MUST NOT fail solely because a feature
> +it does not understand has been offered by the device.
> +
>   \devicenormative{\subsection}{Feature Bits}{Basic Facilities of a Virtio Device / Feature Bits}
>   The device MUST NOT offer a feature which requires another feature
>   which was not offered.  The device SHOULD accept any valid subset
>   of features the driver accepts, otherwise it MUST fail to set the
>   FEATURES_OK \field{device status} bit when the driver writes it.
>   
> +The device MUST NOT offer feature bits corresponding to features
> +it would not support if accepted by the driver (even if the
> +driver is prohibited from accepting the feature bits by the
> +specification); for the sake of clarity, this refers to feature
> +bits not described in this specification, reserved feature bits
> +and feature bits reserved or not supported for the specific
> +transport or the specific device type, but this does not preclude
> +devices written to a future version of this specification from
> +offering such feature bits should such a specification have a
> +provision for devices to support the corresponding features.
> +
>   If a device has successfully negotiated a set of features
>   at least once (by accepting the FEATURES_OK \field{device
>   status} bit during device initialization), then it SHOULD
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 02/10] admin: introduce device group and related concepts

[Michael S. Tsirkin]

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.

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:
+
+\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
+	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.
+\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.}
+\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
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1844 bytes --]

On Mon, Apr 03, 2023 at 11:03:09AM -0400, 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.
> 
> 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

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Parav Pandit]

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.

> 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>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Michael S. Tsirkin]

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>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v11 02/10] admin: introduce device group and related concepts

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, April 10, 2023 9:24 AM

> > 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.
> 
Yes. 
Righting it as
Usually, I see those design notes or rationale written as third person sentences.

Also, since those design choices are good to capture, can we start cover letter commit also, which usually give a complete or broad view of the design?

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] RE: [PATCH v11 02/10] admin: introduce device group and related concepts

[Michael S. Tsirkin]

On Mon, Apr 10, 2023 at 04:42:42PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, April 10, 2023 9:24 AM
> 
> > > 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.
> > 
> Yes. 
> Righting it as
> Usually, I see those design notes or rationale written as third person sentences.
> 
> Also, since those design choices are good to capture, can we start cover letter commit also, which usually give a complete or broad view of the design?

too much beaurocracy really imho.


> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [virtio] Re: [virtio-comment] RE: [PATCH v11 02/10] admin: introduce device group and related concepts

[Parav Pandit]

> From: virtio@lists.oasis-open.org <virtio@lists.oasis-open.org> On Behalf Of
> Michael S. Tsirkin
> 
> On Mon, Apr 10, 2023 at 04:42:42PM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Monday, April 10, 2023 9:24 AM
> >
> > > > 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.
> > >
> > Yes.
> > Righting it as
> > Usually, I see those design notes or rationale written as third person
> sentences.
> >
> > Also, since those design choices are good to capture, can we start cover letter
> commit also, which usually give a complete or broad view of the design?
> 
> too much beaurocracy really imho.
> 
Commit log is capturing the use case and other design aspects of the spec.
I don't see it as bureaucracy.

You didn't get the answer when _CONFIG_DATA question arises in past.
You also didn't get answer from other Intel engineer, which was unreachable, I forgot the issue.
Cover letter will capture those.

If we prefer to not have cover letter, please move these details in the individual patch.
Anyways you send out v12 so next work.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio] Re: [virtio-comment] RE: [PATCH v11 02/10] admin: introduce device group and related concepts

[Michael S. Tsirkin]

On Mon, Apr 24, 2023 at 04:45:27PM +0000, Parav Pandit wrote:
> 
> > From: virtio@lists.oasis-open.org <virtio@lists.oasis-open.org> On Behalf Of
> > Michael S. Tsirkin
> > 
> > On Mon, Apr 10, 2023 at 04:42:42PM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@redhat.com>
> > > > Sent: Monday, April 10, 2023 9:24 AM
> > >
> > > > > 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.
> > > >
> > > Yes.
> > > Righting it as
> > > Usually, I see those design notes or rationale written as third person
> > sentences.
> > >
> > > Also, since those design choices are good to capture, can we start cover letter
> > commit also, which usually give a complete or broad view of the design?
> > 
> > too much beaurocracy really imho.
> > 
> Commit log is capturing the use case and other design aspects of the spec.
> I don't see it as bureaucracy.

Motivation is what we have in the cover.  Considerations that went into
specific design are really hard to capture, and we have enough trouble
adding things to spec without adding extra requirements from
contributors to justify each decision.

> You didn't get the answer when _CONFIG_DATA question arises in past.
> You also didn't get answer from other Intel engineer, which was unreachable, I forgot the issue.
> Cover letter will capture those.
> 
> If we prefer to not have cover letter, please move these details in the individual patch.
> Anyways you send out v12 so next work.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

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

[Zhu, Lingshan]

On 4/3/2023 11:03 PM, 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.
>
> 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:
> +
> +\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
> +	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.
> +\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.}
> +\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
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 03/10] admin: introduce group administration commands

[Michael S. Tsirkin]

This introduces a general structure for group administration commands,
used to control device groups through their owner.

Following patches will introduce specific commands and an interface for
submitting these commands to the owner.

Note that the commands are focused on controlling device groups:
this is why group related fields are in the generic part of
the structure.
Without this the admin vq would become a "whatever" vq which does not do
anything specific at all, just a general transport like thing.
I feel going this way opens the design space to the point where
we no longer know what belongs in e.g. config space
what in the control q and what in the admin q.
As it is, whatever deals with groups is in the admin q; other
things not in the admin q.

There are specific exceptions such as query but that's an exception that
proves the rule ;)

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

changes since v10:
	explain the role of status vs status_qualifier, addresses
		multiple comments
	add more status values and appropriate qualifiers,
		as suggested by Parav
	clarify reserved command opcodes as suggested by Stefan
	better formatting for ranges, comment by Jiri
	make sure command-specific-data is a multiple of qword,
	so things are aligned, suggested by Jiri
	add Q_OK qualifier for success
---
 admin.tex        | 121 +++++++++++++++++++++++++++++++++++++++++++++++
 introduction.tex |   3 ++
 2 files changed, 124 insertions(+)

diff --git a/admin.tex b/admin.tex
index 04d5498..c869a1a 100644
--- a/admin.tex
+++ b/admin.tex
@@ -60,4 +60,125 @@ \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g
 PCI transport (see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus}).
 \end{description}
 
+\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
 
+The driver sends group administration commands to the owner device of
+a group to control member devices of the group.
+This mechanism can
+be used, for example, to configure a member device before it is
+initialized by its driver.
+\footnote{The term "administration" is intended to be interpreted
+widely to include any kind of control. See specific commands
+for detail.}
+
+All the group administration commands are of the following form:
+
+\begin{lstlisting}
+struct virtio_admin_cmd {
+        /* Device-readable part */
+        le16 opcode;
+        /*
+         * 1       - SR-IOV
+         * 2-65535 - reserved
+         */
+        le16 group_type;
+        /* unused, reserved for future extensions */
+        u8 reserved1[12];
+        le64 group_member_id;
+        le64 command_specific_data[];
+
+        /* Device-writable part */
+        le16 status;
+        le16 status_qualifier;
+        /* unused, reserved for future extensions */
+        u8 reserved2[4];
+        u8 command_specific_result[];
+};
+\end{lstlisting}
+
+For all commands, \field{opcode}, \field{group_type} and if
+necessary \field{group_member_id} and \field{command_specific_data} are
+set by the driver, and the owner device sets \field{status} and if
+needed \field{status_qualifier} and
+\field{command_specific_result}.
+
+Generally, any unused device-readable fields are set to zero by the driver
+and ignored by the device.  Any unused device-writeable fields are set to zero
+by the device and ignored by the driver.
+
+\field{opcode} specifies the command. The valid
+values for \field{opcode} can be found in the following table:
+
+\begin{tabular}{|l|l|}
+\hline
+opcode & Name & Command Description \\
+\hline \hline
+0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
+\hline
+\end{tabular}
+
+The \field{group_type} specifies the group type identifier.
+The \field{group_member_id} specifies the member identifier within the group.
+See section \ref{sec:Introduction / Terminology / Device group}
+for the definition of the group type identifier and group member
+identifier.
+
+The \field{status} describes the command result and possibly
+failure reason at an abstract level, this is appropriate for
+forwarding to applications. The \field{status_qualifier} describes
+failures at a low virtio specific level, as appropriate for debugging.
+The following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common \hyperref[intro:errno]{Linux error names and numbers}:
+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\hline
+11   & VIRTIO_ADMIN_STATUS_EAGAIN    & try again \\
+\hline
+12   & VIRTIO_ADMIN_STATUS_ENOMEM    & insufficient resources \\
+\hline
+22   & VIRTIO_ADMIN_STATUS_EINVAL    & invalid command \\
+\hline
+other   & -    & group administration command error  \\
+\hline
+\end{tabular}
+
+When \field{status} is VIRTIO_ADMIN_STATUS_OK, \field{status_qualifier}
+is reserved and set to zero by the device.
+
+The following table describes possible \field{status_qualifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_OK               & used with VIRTIO_ADMIN_STATUS_OK  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND  & command error: no additional information  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE   & unsupported or invalid \field{opcode}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline
+0x05   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER   & unsupported or invalid \field{group_member_id} \\
+\hline
+0x06   & VIRTIO_ADMIN_STATUS_Q_NORESOURCE       & out of internal resources: ok to retry \\
+\hline
+0x07   & VIRTIO_ADMIN_STATUS_Q_TRYAGAIN         & command blocks for too long: should retry \\
+\hline
+0x08-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
+
+Each command uses a different \field{command_specific_data} and
+\field{command_specific_result} structures and the length of
+\field{command_specific_data} and \field{command_specific_result}
+depends on these structures and is described separately or is
+implicit in the structure description.
diff --git a/introduction.tex b/introduction.tex
index 287c5fc..0d849a9 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -68,6 +68,9 @@ \section{Normative References}\label{sec:Normative References}
 	\phantomsection\label{intro:FUSE}\textbf{[FUSE]} &
 	Linux FUSE interface,
 	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/fuse.h}\\
+	\phantomsection\label{intro:errno}\textbf{[errno]} &
+	Linux error names and numbers,
+	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/uapi/asm-generic/errno-base.h}\\
         \phantomsection\label{intro:eMMC}\textbf{[eMMC]} &
         eMMC Electrical Standard (5.1), JESD84-B51,
         \newline\url{http://www.jedec.org/sites/default/files/docs/JESD84-B51.pdf}\\
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 03/10] admin: introduce group administration commands

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]

On Mon, Apr 03, 2023 at 11:03:16AM -0400, Michael S. Tsirkin wrote:
> This introduces a general structure for group administration commands,
> used to control device groups through their owner.
> 
> Following patches will introduce specific commands and an interface for
> submitting these commands to the owner.
> 
> Note that the commands are focused on controlling device groups:
> this is why group related fields are in the generic part of
> the structure.
> Without this the admin vq would become a "whatever" vq which does not do
> anything specific at all, just a general transport like thing.
> I feel going this way opens the design space to the point where
> we no longer know what belongs in e.g. config space
> what in the control q and what in the admin q.
> As it is, whatever deals with groups is in the admin q; other
> things not in the admin q.
> 
> There are specific exceptions such as query but that's an exception that
> proves the rule ;)
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> changes since v10:
> 	explain the role of status vs status_qualifier, addresses
> 		multiple comments
> 	add more status values and appropriate qualifiers,
> 		as suggested by Parav
> 	clarify reserved command opcodes as suggested by Stefan
> 	better formatting for ranges, comment by Jiri
> 	make sure command-specific-data is a multiple of qword,
> 	so things are aligned, suggested by Jiri
> 	add Q_OK qualifier for success
> ---
>  admin.tex        | 121 +++++++++++++++++++++++++++++++++++++++++++++++
>  introduction.tex |   3 ++
>  2 files changed, 124 insertions(+)

My thoughts on status/status_qualifier haven't changed, but it's a
question of taste:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [PATCH v11 03/10] admin: introduce group administration commands

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> This introduces a general structure for group administration commands,
> used to control device groups through their owner.
> 
> Following patches will introduce specific commands and an interface for
> submitting these commands to the owner.
> 
> Note that the commands are focused on controlling device groups:
> this is why group related fields are in the generic part of
> the structure.
> Without this the admin vq would become a "whatever" vq which does not do
The abbreviation vq for virtqueue looks good here :)

> anything specific at all, just a general transport like thing.
> I feel going this way opens the design space to the point where
> we no longer know what belongs in e.g. config space
> what in the control q and what in the admin q.
Better to refer them as control vq and admin vq like above.

> As it is, whatever deals with groups is in the admin q; other
admin vq.

> things not in the admin q.
admin vq.
> 
> There are specific exceptions such as query but that's an exception that
> proves the rule ;)
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> changes since v10:
> 	explain the role of status vs status_qualifier, addresses
> 		multiple comments
> 	add more status values and appropriate qualifiers,
> 		as suggested by Parav
> 	clarify reserved command opcodes as suggested by Stefan
> 	better formatting for ranges, comment by Jiri
> 	make sure command-specific-data is a multiple of qword,
> 	so things are aligned, suggested by Jiri
> 	add Q_OK qualifier for success
> ---
>   admin.tex        | 121 +++++++++++++++++++++++++++++++++++++++++++++++
>   introduction.tex |   3 ++
>   2 files changed, 124 insertions(+)
> 
> diff --git a/admin.tex b/admin.tex
> index 04d5498..c869a1a 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -60,4 +60,125 @@ \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g
>   PCI transport (see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus}).
>   \end{description}
>   
> +\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
>   
> +The driver sends group administration commands to the owner device of
> +a group to control member devices of the group.
> +This mechanism can
> +be used, for example, to configure a member device before it is
> +initialized by its driver.
> +\footnote{The term "administration" is intended to be interpreted
> +widely to include any kind of control. See specific commands
> +for detail.}
> +
> +All the group administration commands are of the following form:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd {
> +        /* Device-readable part */
> +        le16 opcode;
> +        /*
> +         * 1       - SR-IOV
> +         * 2-65535 - reserved
> +         */
> +        le16 group_type;
> +        /* unused, reserved for future extensions */
> +        u8 reserved1[12];
> +        le64 group_member_id;
> +        le64 command_specific_data[];
> +
> +        /* Device-writable part */
> +        le16 status;
> +        le16 status_qualifier;
> +        /* unused, reserved for future extensions */
> +        u8 reserved2[4];
> +        u8 command_specific_result[];
> +};
> +\end{lstlisting}
> +
> +For all commands, \field{opcode}, \field{group_type} and if
> +necessary \field{group_member_id} and \field{command_specific_data} are
> +set by the driver, and the owner device sets \field{status} and if
> +needed \field{status_qualifier} and
> +\field{command_specific_result}.
> +
> +Generally, any unused device-readable fields are set to zero by the driver
> +and ignored by the device.  Any unused device-writeable fields are set to zero
> +by the device and ignored by the driver.
> +
> +\field{opcode} specifies the command. The valid
> +values for \field{opcode} can be found in the following table:
> +
> +\begin{tabular}{|l|l|}
> +\hline
> +opcode & Name & Command Description \\
> +\hline \hline
> +0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> +\hline
> +0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
> +\hline
> +\end{tabular}
> +
> +The \field{group_type} specifies the group type identifier.
> +The \field{group_member_id} specifies the member identifier within the group.
> +See section \ref{sec:Introduction / Terminology / Device group}
> +for the definition of the group type identifier and group member
> +identifier.
> +
> +The \field{status} describes the command result and possibly
> +failure reason at an abstract level, this is appropriate for
> +forwarding to applications. The \field{status_qualifier} describes
> +failures at a low virtio specific level, as appropriate for debugging.
> +The following table describes possible \field{status} values;
> +to simplify common implementations, they are intentionally
> +matching common \hyperref[intro:errno]{Linux error names and numbers}:
> +
> +\begin{tabular}{|l|l|l|}
> +\hline
> +Status (decimal) & Name & Description \\
> +\hline \hline
> +00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
> +\hline
> +11   & VIRTIO_ADMIN_STATUS_EAGAIN    & try again \\
> +\hline
> +12   & VIRTIO_ADMIN_STATUS_ENOMEM    & insufficient resources \\
> +\hline
> +22   & VIRTIO_ADMIN_STATUS_EINVAL    & invalid command \\
> +\hline
> +other   & -    & group administration command error  \\
> +\hline
> +\end{tabular}
> +
> +When \field{status} is VIRTIO_ADMIN_STATUS_OK, \field{status_qualifier}
> +is reserved and set to zero by the device.
> +
> +The following table describes possible \field{status_qualifier} values:
> +\begin{tabular}{|l|l|l|}
> +\hline
> +Status & Name & Description \\
> +\hline \hline
> +0x00   & VIRTIO_ADMIN_STATUS_Q_OK               & used with VIRTIO_ADMIN_STATUS_OK  \\
> +\hline
> +0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND  & command error: no additional information  \\
> +\hline
> +0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE   & unsupported or invalid \field{opcode}  \\
> +\hline
> +0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
> +\hline
> +0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
> +\hline
> +0x05   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER   & unsupported or invalid \field{group_member_id} \\
> +\hline
> +0x06   & VIRTIO_ADMIN_STATUS_Q_NORESOURCE       & out of internal resources: ok to retry \\
> +\hline
> +0x07   & VIRTIO_ADMIN_STATUS_Q_TRYAGAIN         & command blocks for too long: should retry \\
> +\hline
> +0x08-0xFFFF   & -    & reserved for future use \\
> +\hline
> +\end{tabular}
> +
> +Each command uses a different \field{command_specific_data} and
> +\field{command_specific_result} structures and the length of
> +\field{command_specific_data} and \field{command_specific_result}
> +depends on these structures and is described separately or is
> +implicit in the structure description.
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..0d849a9 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -68,6 +68,9 @@ \section{Normative References}\label{sec:Normative References}
>   	\phantomsection\label{intro:FUSE}\textbf{[FUSE]} &
>   	Linux FUSE interface,
>   	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/fuse.h}\\
> +	\phantomsection\label{intro:errno}\textbf{[errno]} &
> +	Linux error names and numbers,
> +	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/uapi/asm-generic/errno-base.h}\\
Above link is broken because "uapi" is twice.
Please remove one "uapi/".

>           \phantomsection\label{intro:eMMC}\textbf{[eMMC]} &
>           eMMC Electrical Standard (5.1), JESD84-B51,
>           \newline\url{http://www.jedec.org/sites/default/files/docs/JESD84-B51.pdf}\\

With above url link fix,

Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 03/10] admin: introduce group administration commands

[Zhu, Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> This introduces a general structure for group administration commands,
> used to control device groups through their owner.
>
> Following patches will introduce specific commands and an interface for
> submitting these commands to the owner.
>
> Note that the commands are focused on controlling device groups:
> this is why group related fields are in the generic part of
> the structure.
> Without this the admin vq would become a "whatever" vq which does not do
> anything specific at all, just a general transport like thing.
> I feel going this way opens the design space to the point where
> we no longer know what belongs in e.g. config space
> what in the control q and what in the admin q.
> As it is, whatever deals with groups is in the admin q; other
> things not in the admin q.
>
> There are specific exceptions such as query but that's an exception that
> proves the rule ;)
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> changes since v10:
> 	explain the role of status vs status_qualifier, addresses
> 		multiple comments
> 	add more status values and appropriate qualifiers,
> 		as suggested by Parav
> 	clarify reserved command opcodes as suggested by Stefan
> 	better formatting for ranges, comment by Jiri
> 	make sure command-specific-data is a multiple of qword,
> 	so things are aligned, suggested by Jiri
> 	add Q_OK qualifier for success
> ---
>   admin.tex        | 121 +++++++++++++++++++++++++++++++++++++++++++++++
>   introduction.tex |   3 ++
>   2 files changed, 124 insertions(+)
>
> diff --git a/admin.tex b/admin.tex
> index 04d5498..c869a1a 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -60,4 +60,125 @@ \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g
>   PCI transport (see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus}).
>   \end{description}
>   
> +\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
>   
> +The driver sends group administration commands to the owner device of
> +a group to control member devices of the group.
> +This mechanism can
> +be used, for example, to configure a member device before it is
> +initialized by its driver.
> +\footnote{The term "administration" is intended to be interpreted
> +widely to include any kind of control. See specific commands
> +for detail.}
> +
> +All the group administration commands are of the following form:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd {
> +        /* Device-readable part */
> +        le16 opcode;
> +        /*
> +         * 1       - SR-IOV
> +         * 2-65535 - reserved
> +         */
> +        le16 group_type;
> +        /* unused, reserved for future extensions */
> +        u8 reserved1[12];
> +        le64 group_member_id;
> +        le64 command_specific_data[];
> +
> +        /* Device-writable part */
> +        le16 status;
> +        le16 status_qualifier;
> +        /* unused, reserved for future extensions */
> +        u8 reserved2[4];
> +        u8 command_specific_result[];
> +};
> +\end{lstlisting}
> +
> +For all commands, \field{opcode}, \field{group_type} and if
> +necessary \field{group_member_id} and \field{command_specific_data} are
> +set by the driver, and the owner device sets \field{status} and if
> +needed \field{status_qualifier} and
> +\field{command_specific_result}.
> +
> +Generally, any unused device-readable fields are set to zero by the driver
> +and ignored by the device.  Any unused device-writeable fields are set to zero
> +by the device and ignored by the driver.
> +
> +\field{opcode} specifies the command. The valid
> +values for \field{opcode} can be found in the following table:
> +
> +\begin{tabular}{|l|l|}
> +\hline
> +opcode & Name & Command Description \\
> +\hline \hline
> +0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> +\hline
> +0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
> +\hline
> +\end{tabular}
> +
> +The \field{group_type} specifies the group type identifier.
> +The \field{group_member_id} specifies the member identifier within the group.
> +See section \ref{sec:Introduction / Terminology / Device group}
> +for the definition of the group type identifier and group member
> +identifier.
> +
> +The \field{status} describes the command result and possibly
> +failure reason at an abstract level, this is appropriate for
> +forwarding to applications. The \field{status_qualifier} describes
> +failures at a low virtio specific level, as appropriate for debugging.
> +The following table describes possible \field{status} values;
> +to simplify common implementations, they are intentionally
> +matching common \hyperref[intro:errno]{Linux error names and numbers}:
> +
> +\begin{tabular}{|l|l|l|}
> +\hline
> +Status (decimal) & Name & Description \\
> +\hline \hline
> +00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
> +\hline
> +11   & VIRTIO_ADMIN_STATUS_EAGAIN    & try again \\
> +\hline
> +12   & VIRTIO_ADMIN_STATUS_ENOMEM    & insufficient resources \\
> +\hline
> +22   & VIRTIO_ADMIN_STATUS_EINVAL    & invalid command \\
> +\hline
> +other   & -    & group administration command error  \\
> +\hline
> +\end{tabular}
> +
> +When \field{status} is VIRTIO_ADMIN_STATUS_OK, \field{status_qualifier}
> +is reserved and set to zero by the device.
> +
> +The following table describes possible \field{status_qualifier} values:
> +\begin{tabular}{|l|l|l|}
> +\hline
> +Status & Name & Description \\
> +\hline \hline
> +0x00   & VIRTIO_ADMIN_STATUS_Q_OK               & used with VIRTIO_ADMIN_STATUS_OK  \\
> +\hline
> +0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND  & command error: no additional information  \\
> +\hline
> +0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE   & unsupported or invalid \field{opcode}  \\
> +\hline
> +0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
> +\hline
> +0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
> +\hline
> +0x05   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER   & unsupported or invalid \field{group_member_id} \\
> +\hline
> +0x06   & VIRTIO_ADMIN_STATUS_Q_NORESOURCE       & out of internal resources: ok to retry \\
> +\hline
> +0x07   & VIRTIO_ADMIN_STATUS_Q_TRYAGAIN         & command blocks for too long: should retry \\
> +\hline
> +0x08-0xFFFF   & -    & reserved for future use \\
> +\hline
> +\end{tabular}
> +
> +Each command uses a different \field{command_specific_data} and
> +\field{command_specific_result} structures and the length of
> +\field{command_specific_data} and \field{command_specific_result}
> +depends on these structures and is described separately or is
> +implicit in the structure description.
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..0d849a9 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -68,6 +68,9 @@ \section{Normative References}\label{sec:Normative References}
>   	\phantomsection\label{intro:FUSE}\textbf{[FUSE]} &
>   	Linux FUSE interface,
>   	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/fuse.h}\\
> +	\phantomsection\label{intro:errno}\textbf{[errno]} &
> +	Linux error names and numbers,
> +	\newline\url{https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/uapi/asm-generic/errno-base.h}\\
>           \phantomsection\label{intro:eMMC}\textbf{[eMMC]} &
>           eMMC Electrical Standard (5.1), JESD84-B51,
>           \newline\url{http://www.jedec.org/sites/default/files/docs/JESD84-B51.pdf}\\
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Michael S. Tsirkin]

The admin virtqueues will be the first interface used to issue admin commands.

Currently the virtio specification defines control virtqueue to manipulate
features and configuration of the device it operates on:
virtio-net, virtio-scsi, etc all have existing control virtqueues. However,
control virtqueue commands are device type specific, which makes it very
difficult to extend for device agnostic commands.

Keeping the device-specific virtqueue separate from the admin virtqueue
is simpler and has fewer potential problems. I don't think creating
common infrastructure for device-specific control virtqueues across
device types worthwhile or within the scope of this patch series.

To support this requirement in a more generic way, this patch introduces
a new admin virtqueue interface.
The admin virtqueue can be seen as the virtqueue analog to a transport.
The admin queue thus does nothing device type-specific (net, scsi, etc)
and instead focuses on transporting the admin commands.

We also support more than one admin virtqueue, for QoS and
scalability requirements.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
explain ordering of commands as suggested by Stefan
dropped Max's S.O.B
reword commit log as suggested by David
minor wording fixes suggested by David
---
 admin.tex   | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 content.tex |  7 +++--
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/admin.tex b/admin.tex
index c869a1a..f201bcd 100644
--- a/admin.tex
+++ b/admin.tex
@@ -182,3 +182,78 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \field{command_specific_data} and \field{command_specific_result}
 depends on these structures and is described separately or is
 implicit in the structure description.
+
+\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
+
+An administration virtqueue of an owner device is used to submit
+group administration commands. An owner device can have more
+than one administration virtqueue.
+
+If VIRTIO_F_ADMIN_VQ has been negotiated, an owner device exposes one
+or more adminstration virtqueues. The number and locations of the
+administration virtqueues are exposed by the owner device in a transport
+specific manner.
+
+The driver queues requests to an arbitrary administration
+virtqueue, and they are used by the device on that same
+virtqueue. It is the responsibility of the driver to ensure
+strict request ordering for commands, because they will be
+consumed with no order constraints.  For example, if consistency
+is required then the driver can wait for the processing of a
+first command by the device to be completed before submitting
+another command depending on the first one.
+
+Administration virtqueues are used as follows:
+\begin{itemize}
+\item The driver submits the command using the \field{struct virtio_admin_cmd}
+structure using a buffer consisting of two parts: a device-readable one followed by a
+device-writable one.
+\item the device-readable part includes fields from \field{opcode}
+through \field{command_specific_data}.
+\item the device-writeable buffer includes fields from \field{status}
+through \field{command_specific_result} inclusive.
+\end{itemize}
+
+For each command, this specification describes a distinct
+format structure used for \field{command_specific_data} and
+\field{command_specific_result}, the length of these fields
+depends on the command.
+
+However, to ensure forward compatibility
+\begin{itemize}
+\item drivers are allowed to submit buffers that are longer
+than the device expects
+(that is, longer than the length of
+\field{opcode} through \field{command_specific_data}).
+This allows the driver to maintain
+a single format structure even if some structure fields are
+unused by the device.
+\item drivers are allowed to submit buffers that are shorter
+than what the device expects
+(that is, shorter than the length of \field{status} through
+\field{command_specific_result}). This allows the device to maintain
+a single format structure even if some structure fields are
+unused by the driver.
+\end{itemize}
+
+The device compares the length of each part (device-readable and
+device-writeable) of the buffer as submitted by driver to what it
+expects and then silently truncates the structures to either the
+length submitted by the driver, or the length described in this
+specification, whichever is shorter.  The device silently ignores
+any data falling outside the shorter of the two lengths. Any
+missing fields are interpreted as set to zero.
+
+Similarly, the driver compares the used buffer length
+of the buffer to what it expects and then silently
+truncates the structure to the used buffer length.
+The driver silently ignores any data falling outside
+the used buffer length reported by the device.  Any missing
+fields are interpreted as set to zero.
+
+This simplifies driver and device implementations since the
+driver/device can simply maintain a single large structure (such
+as a C structure) for a command and its result. As new versions
+of the specification are designed, new fields can be added to the
+tail of a structure, with the driver/device using the full
+structure without concern for versioning.
diff --git a/content.tex b/content.tex
index 04592fb..2eb15fa 100644
--- a/content.tex
+++ b/content.tex
@@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
 \begin{description}
 \item[0 to 23, and 50 to 127] Feature bits for the specific device type
 
-\item[24 to 40] Feature bits reserved for extensions to the queue and
+\item[24 to 41] Feature bits reserved for extensions to the queue and
   feature negotiation mechanisms
 
-\item[41 to 49, and 128 and above] Feature bits reserved for future extensions.
+\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
 \end{description}
 
 \begin{note}
@@ -7684,6 +7684,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
   that the driver can reset a queue individually.
   See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}.
 
+  \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
+  administration virtqueues.
+
 \end{description}
 
 \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

On Mon, Apr 03, 2023 at 11:03:23AM -0400, Michael S. Tsirkin wrote:
> The admin virtqueues will be the first interface used to issue admin commands.
> 
> Currently the virtio specification defines control virtqueue to manipulate
> features and configuration of the device it operates on:
> virtio-net, virtio-scsi, etc all have existing control virtqueues. However,
> control virtqueue commands are device type specific, which makes it very
> difficult to extend for device agnostic commands.
> 
> Keeping the device-specific virtqueue separate from the admin virtqueue
> is simpler and has fewer potential problems. I don't think creating
> common infrastructure for device-specific control virtqueues across
> device types worthwhile or within the scope of this patch series.
> 
> To support this requirement in a more generic way, this patch introduces
> a new admin virtqueue interface.
> The admin virtqueue can be seen as the virtqueue analog to a transport.
> The admin queue thus does nothing device type-specific (net, scsi, etc)
> and instead focuses on transporting the admin commands.
> 
> We also support more than one admin virtqueue, for QoS and
> scalability requirements.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> explain ordering of commands as suggested by Stefan
> dropped Max's S.O.B
> reword commit log as suggested by David
> minor wording fixes suggested by David
> ---
>  admin.tex   | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  content.tex |  7 +++--
>  2 files changed, 80 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> The admin virtqueues will be the first interface used to issue admin commands.
> 
The admin virtqueue

> Currently the virtio specification defines control virtqueue to manipulate
> features and configuration of the device it operates on:
> virtio-net, virtio-scsi, etc all have existing control virtqueues. However,
> control virtqueue commands are device type specific, which makes it very
> difficult to extend for device agnostic commands.
> 
> Keeping the device-specific virtqueue separate from the admin virtqueue
> is simpler and has fewer potential problems. 

> I don't think creating
> common infrastructure for device-specific control virtqueues across
> device types worthwhile or within the scope of this patch series.
>  > To support this requirement in a more generic way, this patch introduces
> a new admin virtqueue interface.
> The admin virtqueue can be seen as the virtqueue analog to a transport.
> The admin queue thus does nothing device type-specific (net, scsi, etc)
> and instead focuses on transporting the admin commands.
> 
> We also support more than one admin virtqueue, for QoS and
> scalability requirements.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> explain ordering of commands as suggested by Stefan
> dropped Max's S.O.B
> reword commit log as suggested by David
> minor wording fixes suggested by David
> ---
>   admin.tex   | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   content.tex |  7 +++--
>   2 files changed, 80 insertions(+), 2 deletions(-)
> 
> diff --git a/admin.tex b/admin.tex
> index c869a1a..f201bcd 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -182,3 +182,78 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   \field{command_specific_data} and \field{command_specific_result}
>   depends on these structures and is described separately or is
>   implicit in the structure description.
> +
> +\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> +
> +An administration virtqueue of an owner device is used to submit
> +group administration commands. An owner device can have more
> +than one administration virtqueue.
> +
> +If VIRTIO_F_ADMIN_VQ has been negotiated, an owner device exposes one
> +or more adminstration virtqueues. The number and locations of the
> +administration virtqueues are exposed by the owner device in a transport
> +specific manner.
> +
> +The driver queues requests to an arbitrary administration
The driver enqueues
> +virtqueue, and they are used by the device on that same
> +virtqueue. It is the responsibility of the driver to ensure
> +strict request ordering for commands, because they will be
> +consumed with no order constraints.  For example, if consistency
> +is required then the driver can wait for the processing of a
> +first command by the device to be completed before submitting
> +another command depending on the first one.
> +
> +Administration virtqueues are used as follows:
> +\begin{itemize}
> +\item The driver submits the command using the \field{struct virtio_admin_cmd}
> +structure using a buffer consisting of two parts: a device-readable one followed by a
> +device-writable one.
> +\item the device-readable part includes fields from \field{opcode}
> +through \field{command_specific_data}.
> +\item the device-writeable buffer includes fields from \field{status}
> +through \field{command_specific_result} inclusive.
> +\end{itemize}
> +
> +For each command, this specification describes a distinct
> +format structure used for \field{command_specific_data} and
> +\field{command_specific_result}, the length of these fields
> +depends on the command.
> +
> +However, to ensure forward compatibility
> +\begin{itemize}
> +\item drivers are allowed to submit buffers that are longer
> +than the device expects
> +(that is, longer than the length of
> +\field{opcode} through \field{command_specific_data}).
> +This allows the driver to maintain
> +a single format structure even if some structure fields are
> +unused by the device.
> +\item drivers are allowed to submit buffers that are shorter
> +than what the device expects
> +(that is, shorter than the length of \field{status} through
> +\field{command_specific_result}). This allows the device to maintain
> +a single format structure even if some structure fields are
> +unused by the driver.
> +\end{itemize}
> +
> +The device compares the length of each part (device-readable and
> +device-writeable) of the buffer as submitted by driver to what it
> +expects and then silently truncates the structures to either the
> +length submitted by the driver, or the length described in this
> +specification, whichever is shorter.  The device silently ignores
> +any data falling outside the shorter of the two lengths. Any
> +missing fields are interpreted as set to zero.
> +
> +Similarly, the driver compares the used buffer length
> +of the buffer to what it expects and then silently
> +truncates the structure to the used buffer length.
> +The driver silently ignores any data falling outside
> +the used buffer length reported by the device.  Any missing
> +fields are interpreted as set to zero.
> +
> +This simplifies driver and device implementations since the
> +driver/device can simply maintain a single large structure (such
> +as a C structure) for a command and its result. As new versions
> +of the specification are designed, new fields can be added to the
> +tail of a structure, with the driver/device using the full
> +structure without concern for versioning.
> diff --git a/content.tex b/content.tex
> index 04592fb..2eb15fa 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>   \begin{description}
>   \item[0 to 23, and 50 to 127] Feature bits for the specific device type
>   
> -\item[24 to 40] Feature bits reserved for extensions to the queue and
> +\item[24 to 41] Feature bits reserved for extensions to the queue and
>     feature negotiation mechanisms
>   
> -\item[41 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
>   \end{description}
>   
>   \begin{note}
> @@ -7684,6 +7684,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>     that the driver can reset a queue individually.
>     See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}.
>   
> +  \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
> +  administration virtqueues.
> +
>   \end{description}
>   
>   \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}

Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Michael S. Tsirkin]

On Thu, Apr 06, 2023 at 12:08:03AM -0400, Parav Pandit wrote:
> 
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > The admin virtqueues will be the first interface used to issue admin commands.
> > 
> The admin virtqueue
> 
> > Currently the virtio specification defines control virtqueue to manipulate
> > features and configuration of the device it operates on:
> > virtio-net, virtio-scsi, etc all have existing control virtqueues. However,
> > control virtqueue commands are device type specific, which makes it very
> > difficult to extend for device agnostic commands.
> > 
> > Keeping the device-specific virtqueue separate from the admin virtqueue
> > is simpler and has fewer potential problems.
> 
> > I don't think creating
> > common infrastructure for device-specific control virtqueues across
> > device types worthwhile or within the scope of this patch series.
> >  > To support this requirement in a more generic way, this patch introduces
> > a new admin virtqueue interface.
> > The admin virtqueue can be seen as the virtqueue analog to a transport.
> > The admin queue thus does nothing device type-specific (net, scsi, etc)
> > and instead focuses on transporting the admin commands.
> > 
> > We also support more than one admin virtqueue, for QoS and
> > scalability requirements.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > explain ordering of commands as suggested by Stefan
> > dropped Max's S.O.B
> > reword commit log as suggested by David
> > minor wording fixes suggested by David
> > ---
> >   admin.tex   | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   content.tex |  7 +++--
> >   2 files changed, 80 insertions(+), 2 deletions(-)
> > 
> > diff --git a/admin.tex b/admin.tex
> > index c869a1a..f201bcd 100644
> > --- a/admin.tex
> > +++ b/admin.tex
> > @@ -182,3 +182,78 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> >   \field{command_specific_data} and \field{command_specific_result}
> >   depends on these structures and is described separately or is
> >   implicit in the structure description.
> > +
> > +\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> > +
> > +An administration virtqueue of an owner device is used to submit
> > +group administration commands. An owner device can have more
> > +than one administration virtqueue.
> > +
> > +If VIRTIO_F_ADMIN_VQ has been negotiated, an owner device exposes one
> > +or more adminstration virtqueues. The number and locations of the
> > +administration virtqueues are exposed by the owner device in a transport
> > +specific manner.
> > +
> > +The driver queues requests to an arbitrary administration
> The driver enqueues

Not sure why - these are synonims - and this variant is more popular:

[mst@tuck virtio]$ git grep 'The driver queues'
admin.tex:The driver queues requests to an arbitrary administration
content.tex:The driver queues requests to the virtqueues, and they are used by
content.tex:The driver queues requests to an arbitrary request queue, and
virtio-gpio.tex:\item The driver queues \field{struct virtio_gpio_request} and
virtio-i2c.tex:The driver queues requests to the virtqueue, and they are used by the
virtio-vsock.tex:The driver queues outgoing packets on the tx virtqueue and incoming packet
[mst@tuck virtio]$ git grep 'The driver enqueues'
virtio-fs.tex:The driver enqueues normal requests on an arbitrary request queue. High


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, April 10, 2023 9:23 AM

> Not sure why - these are synonims - and this variant is more popular:
> 
With my limited knowledge, when written for third person doing the adding entries to the queue, "enqueue" is suitable.

No strong opinion though.

> [mst@tuck virtio]$ git grep 'The driver queues'
> admin.tex:The driver queues requests to an arbitrary administration
> content.tex:The driver queues requests to the virtqueues, and they are used by
> content.tex:The driver queues requests to an arbitrary request queue, and
> virtio-gpio.tex:\item The driver queues \field{struct virtio_gpio_request} and
> virtio-i2c.tex:The driver queues requests to the virtqueue, and they are used by
> the virtio-vsock.tex:The driver queues outgoing packets on the tx virtqueue and
> incoming packet [mst@tuck virtio]$ git grep 'The driver enqueues'
> virtio-fs.tex:The driver enqueues normal requests on an arbitrary request
> queue. High


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Michael S. Tsirkin]

On Mon, Apr 10, 2023 at 02:22:20PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, April 10, 2023 9:23 AM
> 
> > Not sure why - these are synonims - and this variant is more popular:
> > 
> With my limited knowledge, when written for third person doing the adding entries to the queue, "enqueue" is suitable.

Look it up in a dictionary.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, April 10, 2023 4:05 PM
> > >
> > With my limited knowledge, when written for third person doing the adding
> entries to the queue, "enqueue" is suitable.
> 
> Look it up in a dictionary.

Queue is verb and noun both.
As opposed to that enqueue is not ambiguous being only verb.

When "driver queues" is read, it needs tiny more efforts to realize that queues here is verb and not plural driver queues.

So, without strong opinion, enqueue is preferred but it's up to you.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Michael S. Tsirkin]

On Tue, Apr 11, 2023 at 01:39:32PM +0000, Parav Pandit wrote:
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, April 10, 2023 4:05 PM
> > > >
> > > With my limited knowledge, when written for third person doing the adding
> > entries to the queue, "enqueue" is suitable.
> > 
> > Look it up in a dictionary.
> 
> Queue is verb and noun both.
> As opposed to that enqueue is not ambiguous being only verb.
> 
> When "driver queues" is read, it needs tiny more efforts to realize that queues here is verb and not plural driver queues.
> 
> So, without strong opinion, enqueue is preferred but it's up to you.

If you feel like this feel free to change all of text to enqueue.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] RE: [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Parav Pandit]

 From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, April 11, 2023 10:01 AM
> 
> On Tue, Apr 11, 2023 at 01:39:32PM +0000, Parav Pandit wrote:
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Monday, April 10, 2023 4:05 PM
> > > > >
> > > > With my limited knowledge, when written for third person doing the
> adding
> > > entries to the queue, "enqueue" is suitable.
> > >
> > > Look it up in a dictionary.
> >
> > Queue is verb and noun both.
> > As opposed to that enqueue is not ambiguous being only verb.
> >
> > When "driver queues" is read, it needs tiny more efforts to realize that
> queues here is verb and not plural driver queues.
> >
> > So, without strong opinion, enqueue is preferred but it's up to you.
> 
> If you feel like this feel free to change all of text to enqueue.

Ok. Thanks.
Will change. AQ series doesn't need to wait for this cleanup.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v11 04/10] admin: introduce virtio admin virtqueues

[Zhu Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> The admin virtqueues will be the first interface used to issue admin commands.
>
> Currently the virtio specification defines control virtqueue to manipulate
> features and configuration of the device it operates on:
> virtio-net, virtio-scsi, etc all have existing control virtqueues. However,
> control virtqueue commands are device type specific, which makes it very
> difficult to extend for device agnostic commands.
>
> Keeping the device-specific virtqueue separate from the admin virtqueue
> is simpler and has fewer potential problems. I don't think creating
> common infrastructure for device-specific control virtqueues across
> device types worthwhile or within the scope of this patch series.
>
> To support this requirement in a more generic way, this patch introduces
> a new admin virtqueue interface.
> The admin virtqueue can be seen as the virtqueue analog to a transport.
> The admin queue thus does nothing device type-specific (net, scsi, etc)
> and instead focuses on transporting the admin commands.
>
> We also support more than one admin virtqueue, for QoS and
> scalability requirements.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
> explain ordering of commands as suggested by Stefan
> dropped Max's S.O.B
> reword commit log as suggested by David
> minor wording fixes suggested by David
> ---
>   admin.tex   | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   content.tex |  7 +++--
>   2 files changed, 80 insertions(+), 2 deletions(-)
>
> diff --git a/admin.tex b/admin.tex
> index c869a1a..f201bcd 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -182,3 +182,78 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   \field{command_specific_data} and \field{command_specific_result}
>   depends on these structures and is described separately or is
>   implicit in the structure description.
> +
> +\section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> +
> +An administration virtqueue of an owner device is used to submit
> +group administration commands. An owner device can have more
> +than one administration virtqueue.
> +
> +If VIRTIO_F_ADMIN_VQ has been negotiated, an owner device exposes one
> +or more adminstration virtqueues. The number and locations of the
> +administration virtqueues are exposed by the owner device in a transport
> +specific manner.
> +
> +The driver queues requests to an arbitrary administration
> +virtqueue, and they are used by the device on that same
> +virtqueue. It is the responsibility of the driver to ensure
> +strict request ordering for commands, because they will be
> +consumed with no order constraints.  For example, if consistency
> +is required then the driver can wait for the processing of a
> +first command by the device to be completed before submitting
> +another command depending on the first one.
> +
> +Administration virtqueues are used as follows:
> +\begin{itemize}
> +\item The driver submits the command using the \field{struct virtio_admin_cmd}
> +structure using a buffer consisting of two parts: a device-readable one followed by a
> +device-writable one.
> +\item the device-readable part includes fields from \field{opcode}
> +through \field{command_specific_data}.
> +\item the device-writeable buffer includes fields from \field{status}
> +through \field{command_specific_result} inclusive.
> +\end{itemize}
> +
> +For each command, this specification describes a distinct
> +format structure used for \field{command_specific_data} and
> +\field{command_specific_result}, the length of these fields
> +depends on the command.
> +
> +However, to ensure forward compatibility
> +\begin{itemize}
> +\item drivers are allowed to submit buffers that are longer
> +than the device expects
> +(that is, longer than the length of
> +\field{opcode} through \field{command_specific_data}).
> +This allows the driver to maintain
> +a single format structure even if some structure fields are
> +unused by the device.
> +\item drivers are allowed to submit buffers that are shorter
> +than what the device expects
> +(that is, shorter than the length of \field{status} through
> +\field{command_specific_result}). This allows the device to maintain
> +a single format structure even if some structure fields are
> +unused by the driver.
> +\end{itemize}
> +
> +The device compares the length of each part (device-readable and
> +device-writeable) of the buffer as submitted by driver to what it
> +expects and then silently truncates the structures to either the
> +length submitted by the driver, or the length described in this
> +specification, whichever is shorter.  The device silently ignores
> +any data falling outside the shorter of the two lengths. Any
> +missing fields are interpreted as set to zero.
> +
> +Similarly, the driver compares the used buffer length
> +of the buffer to what it expects and then silently
> +truncates the structure to the used buffer length.
> +The driver silently ignores any data falling outside
> +the used buffer length reported by the device.  Any missing
> +fields are interpreted as set to zero.
> +
> +This simplifies driver and device implementations since the
> +driver/device can simply maintain a single large structure (such
> +as a C structure) for a command and its result. As new versions
> +of the specification are designed, new fields can be added to the
> +tail of a structure, with the driver/device using the full
> +structure without concern for versioning.
> diff --git a/content.tex b/content.tex
> index 04592fb..2eb15fa 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -99,10 +99,10 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B
>   \begin{description}
>   \item[0 to 23, and 50 to 127] Feature bits for the specific device type
>   
> -\item[24 to 40] Feature bits reserved for extensions to the queue and
> +\item[24 to 41] Feature bits reserved for extensions to the queue and
>     feature negotiation mechanisms
>   
> -\item[41 to 49, and 128 and above] Feature bits reserved for future extensions.
> +\item[42 to 49, and 128 and above] Feature bits reserved for future extensions.
>   \end{description}
>   
>   \begin{note}
> @@ -7684,6 +7684,9 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>     that the driver can reset a queue individually.
>     See \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}.
>   
> +  \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
> +  administration virtqueues.
> +
>   \end{description}
>   
>   \drivernormative{\section}{Reserved Feature Bits}{Reserved Feature Bits}
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 05/10] pci: add admin vq registers to virtio over pci

[Michael S. Tsirkin]

Add new registers to the PCI common configuration structure.

These registers will be used for querying the indices of the admin
virtqueues of the owner device. To configure, reset or enable the admin
virtqueues, the driver should follow existing queue configuration/setup
sequence.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
dropped Max's S.O.B
make queue_num not 0 based
---
 content.tex | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/content.tex b/content.tex
index 2eb15fa..5057df2 100644
--- a/content.tex
+++ b/content.tex
@@ -948,6 +948,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         le64 queue_device;              /* read-write */
         le16 queue_notify_data;         /* read-only for driver */
         le16 queue_reset;               /* read-write */
+
+        /* About the administration virtqueue. */
+        le16 admin_queue_index;         /* read-only for driver */
+        le16 admin_queue_num;         /* read-only for driver */
 };
 \end{lstlisting}
 
@@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
         This field exists only if VIRTIO_F_RING_RESET has been
         negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
+\item[\field{admin_queue_index}]
+        The device uses this to report the index of the first administration virtqueue.
+        This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
+\item[\field{admin_queue_num}]
+	The device uses this to report the number of the
+	supported administration virtqueues.
+	Virtqueues with index
+	between \field{admin_queue_index} and (\field{admin_queue_index} +
+	\field{admin_queue_num} - 1) inclusive serve as administration
+	virtqueues.
+	The value 0 indicates no supported administration virtqueues.
+	This field is valid only if VIRTIO_F_ADMIN_VQ has been
+	negotiated.
 \end{description}
 
 \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
@@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
 were used before the queue reset.
 (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
 
+If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
+configures any administration virtqueues, the driver MUST
+configure the administration virtqueues using the index
+in the range \field{admin_queue_index} to
+\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
+The driver MAY configure less administration virtqueues than
+supported by the device.
+
 \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
 
 The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
@@ -7686,6 +7711,15 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
   \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
   administration virtqueues.
+  At the moment this feature is only supported for devices using
+  \ref{sec:Virtio Transport Options / Virtio Over PCI
+	  Bus}~\nameref{sec:Virtio Transport Options / Virtio Over PCI Bus}
+	  as the transport and is reserved for future use for
+	  devices using other transports (see
+	  \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}
+	and
+	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
+	handling features reserved for future use.
 
 \end{description}
 
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 05/10] pci: add admin vq registers to virtio over pci

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2174 bytes --]

On Mon, Apr 03, 2023 at 11:03:29AM -0400, Michael S. Tsirkin wrote:
> @@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>          This field exists only if VIRTIO_F_RING_RESET has been
>          negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>  
> +\item[\field{admin_queue_index}]
> +        The device uses this to report the index of the first administration virtqueue.
> +        This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
> +\item[\field{admin_queue_num}]
> +	The device uses this to report the number of the
> +	supported administration virtqueues.
> +	Virtqueues with index
> +	between \field{admin_queue_index} and (\field{admin_queue_index} +
> +	\field{admin_queue_num} - 1) inclusive serve as administration
> +	virtqueues.
> +	The value 0 indicates no supported administration virtqueues.
> +	This field is valid only if VIRTIO_F_ADMIN_VQ has been
> +	negotiated.
>  \end{description}

Maybe add a device-normative statement that [admin_queue_index,
admin_queue_index + admin_queue_num) must be located after
device-specific virtqueues?

That would remind implementers that the device-specific virtqueue layout
needs to be followed and cannot be modified by the presence of
Administration Virtqueues.

>  \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> @@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>  were used before the queue reset.
>  (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>  
> +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
> +configures any administration virtqueues, the driver MUST
> +configure the administration virtqueues using the index
> +in the range \field{admin_queue_index} to
> +\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
> +The driver MAY configure less administration virtqueues than

s/less/fewer/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [PATCH v11 05/10] pci: add admin vq registers to virtio over pci

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> Add new registers to the PCI common configuration structure.
> 
> These registers will be used for querying the indices of the admin
> virtqueues of the owner device. To configure, reset or enable the admin
> virtqueues, the driver should follow existing queue configuration/setup
> sequence.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> dropped Max's S.O.B
> make queue_num not 0 based
> ---
>   content.tex | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 2eb15fa..5057df2 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -948,6 +948,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>           le64 queue_device;              /* read-write */
>           le16 queue_notify_data;         /* read-only for driver */
>           le16 queue_reset;               /* read-write */
> +
> +        /* About the administration virtqueue. */
> +        le16 admin_queue_index;         /* read-only for driver */
> +        le16 admin_queue_num;         /* read-only for driver */
>   };
>   \end{lstlisting}
>   
> @@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>           This field exists only if VIRTIO_F_RING_RESET has been
>           negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>   
> +\item[\field{admin_queue_index}]
> +        The device uses this to report the index of the first administration virtqueue.
> +        This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
> +\item[\field{admin_queue_num}]
> +	The device uses this to report the number of the
> +	supported administration virtqueues.
> +	Virtqueues with index
> +	between \field{admin_queue_index} and (\field{admin_queue_index} +
> +	\field{admin_queue_num} - 1) inclusive serve as administration
> +	virtqueues.
> +	The value 0 indicates no supported administration virtqueues.
no supported .. reads odd.
How about,
The value of 0 indicates that administration virtuques are unsupported.

> +	This field is valid only if VIRTIO_F_ADMIN_VQ has been
> +	negotiated.
>   \end{description}
>   
>   \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> @@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>   were used before the queue reset.
>   (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>   
> +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
> +configures any administration virtqueues, the driver MUST
> +configure the administration virtqueues using the index
> +in the range \field{admin_queue_index} to
> +\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
> +The driver MAY configure less administration virtqueues than
> +supported by the device.
> +
>   \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>   
>   The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
> @@ -7686,6 +7711,15 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>   
>     \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
>     administration virtqueues.
> +  At the moment this feature is only supported for devices using
> +  \ref{sec:Virtio Transport Options / Virtio Over PCI
> +	  Bus}~\nameref{sec:Virtio Transport Options / Virtio Over PCI Bus}
> +	  as the transport and is reserved for future use for
> +	  devices using other transports (see
> +	  \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}
> +	and
> +	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
> +	handling features reserved for future use.
>   
>   \end{description}
>   
Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Re: [PATCH v11 05/10] pci: add admin vq registers to virtio over pci

[Michael S. Tsirkin]

On Thu, Apr 06, 2023 at 12:14:32AM -0400, Parav Pandit wrote:
> 
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > Add new registers to the PCI common configuration structure.
> > 
> > These registers will be used for querying the indices of the admin
> > virtqueues of the owner device. To configure, reset or enable the admin
> > virtqueues, the driver should follow existing queue configuration/setup
> > sequence.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > dropped Max's S.O.B
> > make queue_num not 0 based
> > ---
> >   content.tex | 34 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index 2eb15fa..5057df2 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -948,6 +948,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> >           le64 queue_device;              /* read-write */
> >           le16 queue_notify_data;         /* read-only for driver */
> >           le16 queue_reset;               /* read-write */
> > +
> > +        /* About the administration virtqueue. */
> > +        le16 admin_queue_index;         /* read-only for driver */
> > +        le16 admin_queue_num;         /* read-only for driver */
> >   };
> >   \end{lstlisting}
> > @@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> >           This field exists only if VIRTIO_F_RING_RESET has been
> >           negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
> > +\item[\field{admin_queue_index}]
> > +        The device uses this to report the index of the first administration virtqueue.
> > +        This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
> > +\item[\field{admin_queue_num}]
> > +	The device uses this to report the number of the
> > +	supported administration virtqueues.
> > +	Virtqueues with index
> > +	between \field{admin_queue_index} and (\field{admin_queue_index} +
> > +	\field{admin_queue_num} - 1) inclusive serve as administration
> > +	virtqueues.
> > +	The value 0 indicates no supported administration virtqueues.
> no supported .. reads odd.
> How about,
> The value of 0 indicates that administration virtuques are unsupported.

I went back and forth on this but I feel "no queues" is clearer
in the end, and not odd: zero number of queues means no queues.

> > +	This field is valid only if VIRTIO_F_ADMIN_VQ has been
> > +	negotiated.
> >   \end{description}
> >   \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> > @@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
> >   were used before the queue reset.
> >   (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
> > +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
> > +configures any administration virtqueues, the driver MUST
> > +configure the administration virtqueues using the index
> > +in the range \field{admin_queue_index} to
> > +\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
> > +The driver MAY configure less administration virtqueues than
> > +supported by the device.
> > +
> >   \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
> >   The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
> > @@ -7686,6 +7711,15 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >     \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
> >     administration virtqueues.
> > +  At the moment this feature is only supported for devices using
> > +  \ref{sec:Virtio Transport Options / Virtio Over PCI
> > +	  Bus}~\nameref{sec:Virtio Transport Options / Virtio Over PCI Bus}
> > +	  as the transport and is reserved for future use for
> > +	  devices using other transports (see
> > +	  \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}
> > +	and
> > +	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
> > +	handling features reserved for future use.
> >   \end{description}
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> 
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio] [PATCH v11 05/10] pci: add admin vq registers to virtio over pci

[Zhu, Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Add new registers to the PCI common configuration structure.
>
> These registers will be used for querying the indices of the admin
> virtqueues of the owner device. To configure, reset or enable the admin
> virtqueues, the driver should follow existing queue configuration/setup
> sequence.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
> dropped Max's S.O.B
> make queue_num not 0 based
> ---
>   content.tex | 34 ++++++++++++++++++++++++++++++++++
>   1 file changed, 34 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 2eb15fa..5057df2 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -948,6 +948,10 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>           le64 queue_device;              /* read-write */
>           le16 queue_notify_data;         /* read-only for driver */
>           le16 queue_reset;               /* read-write */
> +
> +        /* About the administration virtqueue. */
> +        le16 admin_queue_index;         /* read-only for driver */
> +        le16 admin_queue_num;         /* read-only for driver */
>   };
>   \end{lstlisting}
>   
> @@ -1033,6 +1037,19 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>           This field exists only if VIRTIO_F_RING_RESET has been
>           negotiated. (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>   
> +\item[\field{admin_queue_index}]
> +        The device uses this to report the index of the first administration virtqueue.
> +        This field is valid only if VIRTIO_F_ADMIN_VQ has been negotiated.
> +\item[\field{admin_queue_num}]
> +	The device uses this to report the number of the
> +	supported administration virtqueues.
> +	Virtqueues with index
> +	between \field{admin_queue_index} and (\field{admin_queue_index} +
> +	\field{admin_queue_num} - 1) inclusive serve as administration
> +	virtqueues.
> +	The value 0 indicates no supported administration virtqueues.
> +	This field is valid only if VIRTIO_F_ADMIN_VQ has been
> +	negotiated.
>   \end{description}
>   
>   \devicenormative{\paragraph}{Common configuration structure layout}{Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Common configuration structure layout}
> @@ -1119,6 +1136,14 @@ \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport
>   were used before the queue reset.
>   (see \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset}).
>   
> +If VIRTIO_F_ADMIN_VQ has been negotiated, and if the driver
> +configures any administration virtqueues, the driver MUST
> +configure the administration virtqueues using the index
> +in the range \field{admin_queue_index} to
> +\field{admin_queue_index} + \field{admin_queue_num} - 1 inclusive.
> +The driver MAY configure less administration virtqueues than
> +supported by the device.
> +
>   \subsubsection{Notification structure layout}\label{sec:Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability}
>   
>   The notification location is found using the VIRTIO_PCI_CAP_NOTIFY_CFG
> @@ -7686,6 +7711,15 @@ \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>   
>     \item[VIRTIO_F_ADMIN_VQ(41)] This feature indicates that the device exposes one or more
>     administration virtqueues.
> +  At the moment this feature is only supported for devices using
> +  \ref{sec:Virtio Transport Options / Virtio Over PCI
> +	  Bus}~\nameref{sec:Virtio Transport Options / Virtio Over PCI Bus}
> +	  as the transport and is reserved for future use for
> +	  devices using other transports (see
> +	  \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}
> +	and
> +	\ref{devicenormative:Basic Facilities of a Virtio Device / Feature Bits} for
> +	handling features reserved for future use.
>   
>   \end{description}
>   
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 06/10] mmio: document ADMIN_VQ as reserved

[Michael S. Tsirkin]

Adding relevant registers needs more work and it's not
clear what the use-case will be as currently only
the PCI transport is supported. But let's keep the
door open on this.
We already say it's reserved in a central place, but it
does not hurt to remind implementers to mask it.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Parav Pandit <parav@nvidia.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 content.tex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/content.tex b/content.tex
index 5057df2..f7446bf 100644
--- a/content.tex
+++ b/content.tex
@@ -2364,6 +2364,18 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
 
 Notification mechanisms did not change.
 
+\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio Over MMIO / Features reserved for future use}
+
+At this time, devices and drivers utilizing Virtio Over MMIO
+do not support the following features:
+\begin{itemize}
+
+\item VIRTIO_F_ADMIN_VQ
+
+\end{itemize}
+
+These features are reserved for future use.
+
 \section{Virtio Over Channel I/O}\label{sec:Virtio Transport Options / Virtio Over Channel I/O}
 
 S/390 based virtual machines support neither PCI nor MMIO, so a
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 06/10] mmio: document ADMIN_VQ as reserved

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> Adding relevant registers needs more work and it's not
> clear what the use-case will be as currently only
> the PCI transport is supported. But let's keep the
> door open on this.
> We already say it's reserved in a central place, but it
> does not hurt to remind implementers to mask it.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 5057df2..f7446bf 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2364,6 +2364,18 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
>   
>   Notification mechanisms did not change.
>   
> +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio Over MMIO / Features reserved for future use}
> +
> +At this time, devices and drivers utilizing Virtio Over MMIO
No need to mention, "At this time".
It is always spec version to version.
And "current time" of the spec is visible in its version.
Please remove "At this time".

> +do not support the following features:
> +\begin{itemize}
> +
> +\item VIRTIO_F_ADMIN_VQ
> +
> +\end{itemize}
> +
> +These features are reserved for future use.
> +
Above line should be moved up before listing the features.
or should be written as
s/These/Above

like, Above listed features are reserved for future use.

>   \section{Virtio Over Channel I/O}\label{sec:Virtio Transport Options / Virtio Over Channel I/O}
>   
>   S/390 based virtual machines support neither PCI nor MMIO, so a

Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Re: [PATCH v11 06/10] mmio: document ADMIN_VQ as reserved

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 11:54:04AM -0400, Parav Pandit wrote:
> 
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > Adding relevant registers needs more work and it's not
> > clear what the use-case will be as currently only
> > the PCI transport is supported. But let's keep the
> > door open on this.
> > We already say it's reserved in a central place, but it
> > does not hurt to remind implementers to mask it.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Parav Pandit <parav@nvidia.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   content.tex | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index 5057df2..f7446bf 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -2364,6 +2364,18 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
> >   Notification mechanisms did not change.
> > +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio Over MMIO / Features reserved for future use}
> > +
> > +At this time, devices and drivers utilizing Virtio Over MMIO
> No need to mention, "At this time".
> It is always spec version to version.
> And "current time" of the spec is visible in its version.
> Please remove "At this time".
> 
> > +do not support the following features:
> > +\begin{itemize}
> > +
> > +\item VIRTIO_F_ADMIN_VQ
> > +
> > +\end{itemize}
> > +
> > +These features are reserved for future use.
> > +
> Above line should be moved up before listing the features.
> or should be written as
> s/These/Above
> 
> like, Above listed features are reserved for future use.

I don't feel it matters. And
we were asked to reduce the use of "below" and "above" in new text
as disrespectful to people using screen readers and such.

> >   \section{Virtio Over Channel I/O}\label{sec:Virtio Transport Options / Virtio Over Channel I/O}
> >   S/390 based virtual machines support neither PCI nor MMIO, so a
> 
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> 
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH v11 06/10] mmio: document ADMIN_VQ as reserved

[Zhu Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Adding relevant registers needs more work and it's not
> clear what the use-case will be as currently only
> the PCI transport is supported. But let's keep the
> door open on this.
> We already say it's reserved in a central place, but it
> does not hurt to remind implementers to mask it.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 5057df2..f7446bf 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2364,6 +2364,18 @@ \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M
>   
>   Notification mechanisms did not change.
>   
> +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio Over MMIO / Features reserved for future use}
> +
> +At this time, devices and drivers utilizing Virtio Over MMIO
> +do not support the following features:
> +\begin{itemize}
> +
> +\item VIRTIO_F_ADMIN_VQ
> +
> +\end{itemize}
> +
> +These features are reserved for future use.
> +
>   \section{Virtio Over Channel I/O}\label{sec:Virtio Transport Options / Virtio Over Channel I/O}
>   
>   S/390 based virtual machines support neither PCI nor MMIO, so a
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 07/10] ccw: document ADMIN_VQ as reserved

[Michael S. Tsirkin]

Adding relevant registers needs more work and it's not
clear what the use-case will be as currently only
the PCI transport is supported. But let's keep the
door open on this.
We already say it's reserved in a central place, but it
does not hurt to remind implementers to mask it.

Note: there are more features to add to this list.
Will be done later with a patch on top.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 content.tex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/content.tex b/content.tex
index f7446bf..1213d48 100644
--- a/content.tex
+++ b/content.tex
@@ -2978,6 +2978,18 @@ \subsubsection{Resetting Devices}\label{sec:Virtio Transport Options / Virtio ov
 MAY also choose to verify reset completion by reading \field{device status} via
 CCW_CMD_READ_STATUS and checking whether it is 0 afterwards.
 
+\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio over channel I/O / Features reserved for future use}
+
+At this time, devices and drivers utilizing Virtio over channel I/O
+do not support the following features:
+\begin{itemize}
+
+\item VIRTIO_F_ADMIN_VQ
+
+\end{itemize}
+
+These features are reserved for future use.
+
 \chapter{Device Types}\label{sec:Device Types}
 
 On top of the queues, config space and feature negotiation facilities
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 07/10] ccw: document ADMIN_VQ as reserved

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> Adding relevant registers needs more work and it's not
> clear what the use-case will be as currently only
> the PCI transport is supported. But let's keep the
> door open on this.
> We already say it's reserved in a central place, but it
> does not hurt to remind implementers to mask it.
> 
s/implementers/implementation

> Note: there are more features to add to this list.
> Will be done later with a patch on top.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index f7446bf..1213d48 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2978,6 +2978,18 @@ \subsubsection{Resetting Devices}\label{sec:Virtio Transport Options / Virtio ov
>   MAY also choose to verify reset completion by reading \field{device status} via
>   CCW_CMD_READ_STATUS and checking whether it is 0 afterwards.
>   
> +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio over channel I/O / Features reserved for future use}
> +
> +At this time, devices and drivers utilizing Virtio over channel I/O
> +do not support the following features:
> +\begin{itemize}
> +
> +\item VIRTIO_F_ADMIN_VQ
> +
> +\end{itemize}
> +
> +These features are reserved for future use.
> +
>   \chapter{Device Types}\label{sec:Device Types}
>   
>   On top of the queues, config space and feature negotiation facilities

Same nit as that of mmio.

Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 07/10] ccw: document ADMIN_VQ as reserved

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 11:55:29AM -0400, Parav Pandit wrote:
> 
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > Adding relevant registers needs more work and it's not
> > clear what the use-case will be as currently only
> > the PCI transport is supported. But let's keep the
> > door open on this.
> > We already say it's reserved in a central place, but it
> > does not hurt to remind implementers to mask it.
> > 
> s/implementers/implementation
> 
> > Note: there are more features to add to this list.
> > Will be done later with a patch on top.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >   content.tex | 12 ++++++++++++
> >   1 file changed, 12 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index f7446bf..1213d48 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -2978,6 +2978,18 @@ \subsubsection{Resetting Devices}\label{sec:Virtio Transport Options / Virtio ov
> >   MAY also choose to verify reset completion by reading \field{device status} via
> >   CCW_CMD_READ_STATUS and checking whether it is 0 afterwards.
> > +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio over channel I/O / Features reserved for future use}
> > +
> > +At this time, devices and drivers utilizing Virtio over channel I/O
> > +do not support the following features:
> > +\begin{itemize}
> > +
> > +\item VIRTIO_F_ADMIN_VQ
> > +
> > +\end{itemize}
> > +
> > +These features are reserved for future use.
> > +
> >   \chapter{Device Types}\label{sec:Device Types}
> >   On top of the queues, config space and feature negotiation facilities
> 
> Same nit as that of mmio.

Sorry I don't get what this refers to.

> Reviewed-by: Parav Pandit <parav@nvidia.com>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 07/10] ccw: document ADMIN_VQ as reserved

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> Adding relevant registers needs more work and it's not
> clear what the use-case will be as currently only
> the PCI transport is supported. But let's keep the
> door open on this.
> We already say it's reserved in a central place, but it
> does not hurt to remind implementers to mask it.
> 
> Note: there are more features to add to this list.
> Will be done later with a patch on top.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index f7446bf..1213d48 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2978,6 +2978,18 @@ \subsubsection{Resetting Devices}\label{sec:Virtio Transport Options / Virtio ov
>   MAY also choose to verify reset completion by reading \field{device status} via
>   CCW_CMD_READ_STATUS and checking whether it is 0 afterwards.
>   
> +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio over channel I/O / Features reserved for future use}
> +
> +At this time, devices and drivers utilizing Virtio over channel I/O
> +do not support the following features:
> +\begin{itemize}
> +
> +\item VIRTIO_F_ADMIN_VQ
> +
> +\end{itemize}
> +
> +These features are reserved for future use.
> +
>   \chapter{Device Types}\label{sec:Device Types}
>   
>   On top of the queues, config space and feature negotiation facilities

Reviewed-by: Parav Pandit <parav@nvidia.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v11 07/10] ccw: document ADMIN_VQ as reserved

[Zhu Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Adding relevant registers needs more work and it's not
> clear what the use-case will be as currently only
> the PCI transport is supported. But let's keep the
> door open on this.
> We already say it's reserved in a central place, but it
> does not hurt to remind implementers to mask it.
>
> Note: there are more features to add to this list.
> Will be done later with a patch on top.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index f7446bf..1213d48 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2978,6 +2978,18 @@ \subsubsection{Resetting Devices}\label{sec:Virtio Transport Options / Virtio ov
>   MAY also choose to verify reset completion by reading \field{device status} via
>   CCW_CMD_READ_STATUS and checking whether it is 0 afterwards.
>   
> +\subsection{Features reserved for future use}\label{sec:Virtio Transport Options / Virtio over channel I/O / Features reserved for future use}
> +
> +At this time, devices and drivers utilizing Virtio over channel I/O
> +do not support the following features:
> +\begin{itemize}
> +
> +\item VIRTIO_F_ADMIN_VQ
> +
> +\end{itemize}
> +
> +These features are reserved for future use.
> +
>   \chapter{Device Types}\label{sec:Device Types}
>   
>   On top of the queues, config space and feature negotiation facilities
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 08/10] admin: command list discovery

[Michael S. Tsirkin]

Add commands to find out which commands does each group support,
as well as enable their use by driver.
This will be especially useful once we have multiple group types.

An alternative is per-type VQs. This is possible but will
require more per-transport work. Discovery through the vq
helps keep things contained.

e.g. lack of support for some command can switch to a legacy mode

note that commands are expected to be avolved by adding new
fields to command specific data at the tail, so
we generally do not need feature bits for compatibility.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
dropped S.O.B by Max
explain in commit log how commands will evolve, comment by Stefan
replace will use with is capable of use, comment by Stefan
typo fix reported by David and Lingshan
rename cmds to cmd_opcodes suggested by Jiri
list group_type explicitly in command description suggested by Jiri
clarify how can device not support some commands
always say group administration commands, comment by Jiri
---
 admin.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 101 insertions(+), 1 deletion(-)

diff --git a/admin.tex b/admin.tex
index f201bcd..9552797 100644
--- a/admin.tex
+++ b/admin.tex
@@ -113,7 +113,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \hline
 opcode & Name & Command Description \\
 \hline \hline
-0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
+0x0000 & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands supported for this group type    \\
+0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used for this group type \\
+0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
 \hline
 0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
 \hline
@@ -183,6 +185,104 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 depends on these structures and is described separately or is
 implicit in the structure description.
 
+Before sending any group administration commands to the device, the driver
+needs to communicate to the device which commands it is going to
+use. Initially (after reset), only two commands are assumed to be used:
+VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
+
+Before sending any other commands for any member of a specific group to
+the device, the driver queries the supported commands via
+VIRTIO_ADMIN_CMD_LIST_QUERY and sends the commands it is
+capable of using via VIRTIO_ADMIN_CMD_LIST_USE.
+
+Commands VIRTIO_ADMIN_CMD_LIST_QUERY and
+VIRTIO_ADMIN_CMD_LIST_USE
+both use the following structure describing the
+command opcodes:
+
+\begin{lstlisting}
+struct virtio_admin_cmd_list {
+       /* Indicates which of the below fields were returned
+       le64 device_admin_cmd_opcodes[];
+};
+\end{lstlisting}
+
+This structure is an array of 64 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported. Thus, \field{device_admin_cmd_opcodes[0]} refers to the
+first 64-bit value in this array corresponding to opcodes 0 to
+63, \field{device_admin_cmd_opcodes[1]} is the second 64-bit value
+corresponding to opcodes 64 to 127, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmd_opcodes[0]}
+and 0x1 in \field{device_admin_cmd_opcodes[1]} indicates that only
+opcodes 0, 1 and 64 are supported.
+The length of the array depends on the supported opcodes - it is
+large enough to include bits set for all supported opcodes,
+that is the length can be calculated by starting with the largest
+supported opcode adding one, dividing by 64 and rounding up.
+In other words, for
+VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE the
+length of \field{command_specific_result} and
+\field{command_specific_data} respectively will be
+$DIV_ROUND_UP(max_cmd, 64) * 8$ where DIV_ROUND_UP is integer division
+with round up and \field{max_cmd} is the largest available command opcode.
+
+The array is also allowed to be larger and to additionally
+include an arbitrary number of all-zero entries.
+
+Accordingly, bits 0 and 1 corresponding to opcode 0
+(VIRTIO_ADMIN_CMD_LIST_QUERY) and 1
+(VIRTIO_ADMIN_CMD_LIST_USE) are
+always set in \field{device_admin_cmd_opcodes[0]} returned by VIRTIO_ADMIN_CMD_LIST_QUERY.
+
+For the command VIRTIO_ADMIN_CMD_LIST_QUERY, \field{opcode} is set to 0x0.
+The \field{group_member_id} is unused. It is set to zero by driver.
+This command has no command specific data.
+The device, upon success, returns a result in
+\field{command_specific_result} in the format
+\field{struct virtio_admin_cmd_list} describing the
+list of group administration commands supported for the group type
+specified by \field{group_type}.
+
+For the command VIRTIO_ADMIN_CMD_LIST_USE, \field{opcode}
+is set to 0x1.
+The \field{group_member_id} is unused. It is set to zero by driver.
+The \field{command_specific_data} is in the format
+\field{struct virtio_admin_cmd_list} describing the
+list of group administration commands used by the driver
+with the group type specified by \field{group_type}.
+
+This command has no command specific result.
+
+The driver issues the command VIRTIO_ADMIN_CMD_LIST_QUERY to
+query the list of commands valid for this group and before sending
+any commands for any member of a group.
+
+The driver then enables use of some of the opcodes by sending to
+the device the command VIRTIO_ADMIN_CMD_LIST_USE with a subset
+of the list returned by VIRTIO_ADMIN_CMD_LIST_QUERY that is
+both understood and used by the driver.
+
+If the device supports the command list used by the driver, the
+device completes the command with status VIRTIO_ADMIN_STATUS_OK.
+If the device does not support the command list
+(for example, if the driver is not capable to use
+some required commands), the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in
+device_admin_cmd_opcodes
+if they are not familiar with how the command opcode
+is used, since devices could have dependencies between
+command opcodes.
+
+It is assumed that all members in a group support and are used
+with the same list of commands. However, for owner devices
+supporting multiple group types, the list of supported commands
+might differ between different group types.
+
 \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 
 An administration virtqueue of an owner device is used to submit
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 08/10] admin: command list discovery

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]

On Mon, Apr 03, 2023 at 11:03:49AM -0400, Michael S. Tsirkin wrote:
> Add commands to find out which commands does each group support,
> as well as enable their use by driver.
> This will be especially useful once we have multiple group types.
> 
> An alternative is per-type VQs. This is possible but will
> require more per-transport work. Discovery through the vq
> helps keep things contained.
> 
> e.g. lack of support for some command can switch to a legacy mode
> 
> note that commands are expected to be avolved by adding new
> fields to command specific data at the tail, so
> we generally do not need feature bits for compatibility.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> dropped S.O.B by Max
> explain in commit log how commands will evolve, comment by Stefan
> replace will use with is capable of use, comment by Stefan
> typo fix reported by David and Lingshan
> rename cmds to cmd_opcodes suggested by Jiri
> list group_type explicitly in command description suggested by Jiri
> clarify how can device not support some commands
> always say group administration commands, comment by Jiri
> ---
>  admin.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [PATCH v11 08/10] admin: command list discovery

[Parav Pandit]

On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> Add commands to find out which commands does each group support,
> as well as enable their use by driver.
> This will be especially useful once we have multiple group types.
> 
> An alternative is per-type VQs. This is possible but will
> require more per-transport work. Discovery through the vq
> helps keep things contained.
> 
> e.g. lack of support for some command can switch to a legacy mode
>
Above description about legacy is not clear. what is legacy mode in 
context of admin vq?
> note that commands are expected to be avolved by adding new
s/to be avolved/to evolve/

> fields to command specific data at the tail, so
> we generally do not need feature bits for compatibility.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> dropped S.O.B by Max
> explain in commit log how commands will evolve, comment by Stefan
> replace will use with is capable of use, comment by Stefan
> typo fix reported by David and Lingshan
> rename cmds to cmd_opcodes suggested by Jiri
> list group_type explicitly in command description suggested by Jiri
> clarify how can device not support some commands
> always say group administration commands, comment by Jiri
> ---
>   admin.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 101 insertions(+), 1 deletion(-)
> 
> diff --git a/admin.tex b/admin.tex
> index f201bcd..9552797 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -113,7 +113,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   \hline
>   opcode & Name & Command Description \\
>   \hline \hline
> -0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> +0x0000 & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands supported for this group type    \\
> +0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used for this group type \\
> +0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
>   \hline
>   0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
>   \hline
> @@ -183,6 +185,104 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   depends on these structures and is described separately or is
>   implicit in the structure description.
>   
> +Before sending any group administration commands to the device, the driver
> +needs to communicate to the device which commands it is going to
> +use. Initially (after reset), only two commands are assumed to be used:
> +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
> +
> +Before sending any other commands for any member of a specific group to
> +the device, the driver queries the supported commands via
> +VIRTIO_ADMIN_CMD_LIST_QUERY and sends the commands it is
> +capable of using via VIRTIO_ADMIN_CMD_LIST_USE.
> +
> +Commands VIRTIO_ADMIN_CMD_LIST_QUERY and
> +VIRTIO_ADMIN_CMD_LIST_USE
> +both use the following structure describing the
> +command opcodes:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_list {
> +       /* Indicates which of the below fields were returned
> +       le64 device_admin_cmd_opcodes[];
> +};
> +\end{lstlisting}
> +
> +This structure is an array of 64 bit values in little-endian byte
> +order, in which a bit is set if the specific command opcode
> +is supported. Thus, \field{device_admin_cmd_opcodes[0]} refers to the
> +first 64-bit value in this array corresponding to opcodes 0 to
> +63, \field{device_admin_cmd_opcodes[1]} is the second 64-bit value
> +corresponding to opcodes 64 to 127, etc.
> +For example, the array of size 2 including
> +the values 0x3 in \field{device_admin_cmd_opcodes[0]}
> +and 0x1 in \field{device_admin_cmd_opcodes[1]} indicates that only
> +opcodes 0, 1 and 64 are supported.
> +The length of the array depends on the supported opcodes - it is
> +large enough to include bits set for all supported opcodes,
> +that is the length can be calculated by starting with the largest
> +supported opcode adding one, dividing by 64 and rounding up.
> +In other words, for
> +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE the
> +length of \field{command_specific_result} and
> +\field{command_specific_data} respectively will be
> +$DIV_ROUND_UP(max_cmd, 64) * 8$ where DIV_ROUND_UP is integer division
> +with round up and \field{max_cmd} is the largest available command opcode.
> +
> +The array is also allowed to be larger and to additionally
> +include an arbitrary number of all-zero entries.
> +
> +Accordingly, bits 0 and 1 corresponding to opcode 0
> +(VIRTIO_ADMIN_CMD_LIST_QUERY) and 1
> +(VIRTIO_ADMIN_CMD_LIST_USE) are
> +always set in \field{device_admin_cmd_opcodes[0]} returned by VIRTIO_ADMIN_CMD_LIST_QUERY.
> +
> +For the command VIRTIO_ADMIN_CMD_LIST_QUERY, \field{opcode} is set to 0x0.
> +The \field{group_member_id} is unused. It is set to zero by driver.
> +This command has no command specific data.
> +The device, upon success, returns a result in
> +\field{command_specific_result} in the format
> +\field{struct virtio_admin_cmd_list} describing the
> +list of group administration commands supported for the group type
> +specified by \field{group_type}.
> +
> +For the command VIRTIO_ADMIN_CMD_LIST_USE, \field{opcode}
> +is set to 0x1.
> +The \field{group_member_id} is unused. It is set to zero by driver.
> +The \field{command_specific_data} is in the format
> +\field{struct virtio_admin_cmd_list} describing the
> +list of group administration commands used by the driver
> +with the group type specified by \field{group_type}.
> +
> +This command has no command specific result.
> +
> +The driver issues the command VIRTIO_ADMIN_CMD_LIST_QUERY to
> +query the list of commands valid for this group and before sending
> +any commands for any member of a group.
> +
> +The driver then enables use of some of the opcodes by sending to
> +the device the command VIRTIO_ADMIN_CMD_LIST_USE with a subset
> +of the list returned by VIRTIO_ADMIN_CMD_LIST_QUERY that is
> +both understood and used by the driver.
> +
We need to clarify that if LIST_USE command can be called only one time 
or multiple times.

For example first time for limited functionality, it enabled 10 commands 
specific to SR-IOV.
Later when user wants to use SIOV, it enables additional SIOV specific 
command set.

> +If the device supports the command list used by the driver, the
> +device completes the command with status VIRTIO_ADMIN_STATUS_OK.
> +If the device does not support the command list
> +(for example, if the driver is not capable to use
> +some required commands), the device
> +completes the command with status
> +VIRTIO_ADMIN_STATUS_INVALID_FIELD.
> +
> +Note: drivers are assumed not to set bits in
> +device_admin_cmd_opcodes
> +if they are not familiar with how the command opcode
> +is used, since devices could have dependencies between
> +command opcodes.
> +
Can you please change to use the singular notion for device and driver 
in the note like rest of the above description?

> +It is assumed that all members in a group support and are used
> +with the same list of commands. However, for owner devices
> +supporting multiple group types, the list of supported commands
> +might differ between different group types.
> +
>   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
>   
>   An administration virtqueue of an owner device is used to submit

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Re: [PATCH v11 08/10] admin: command list discovery

[Michael S. Tsirkin]

On Thu, Apr 06, 2023 at 12:27:36AM -0400, Parav Pandit wrote:
> 
> On 4/3/2023 11:03 AM, Michael S. Tsirkin wrote:
> > Add commands to find out which commands does each group support,
> > as well as enable their use by driver.
> > This will be especially useful once we have multiple group types.
> > 
> > An alternative is per-type VQs. This is possible but will
> > require more per-transport work. Discovery through the vq
> > helps keep things contained.
> > 
> > e.g. lack of support for some command can switch to a legacy mode
> > 
> Above description about legacy is not clear. what is legacy mode in context
> of admin vq?
> > note that commands are expected to be avolved by adding new
> s/to be avolved/to evolve/
> 
> > fields to command specific data at the tail, so
> > we generally do not need feature bits for compatibility.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > dropped S.O.B by Max
> > explain in commit log how commands will evolve, comment by Stefan
> > replace will use with is capable of use, comment by Stefan
> > typo fix reported by David and Lingshan
> > rename cmds to cmd_opcodes suggested by Jiri
> > list group_type explicitly in command description suggested by Jiri
> > clarify how can device not support some commands
> > always say group administration commands, comment by Jiri
> > ---
> >   admin.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >   1 file changed, 101 insertions(+), 1 deletion(-)
> > 
> > diff --git a/admin.tex b/admin.tex
> > index f201bcd..9552797 100644
> > --- a/admin.tex
> > +++ b/admin.tex
> > @@ -113,7 +113,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> >   \hline
> >   opcode & Name & Command Description \\
> >   \hline \hline
> > -0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> > +0x0000 & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands supported for this group type    \\
> > +0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used for this group type \\
> > +0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> >   \hline
> >   0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
> >   \hline
> > @@ -183,6 +185,104 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> >   depends on these structures and is described separately or is
> >   implicit in the structure description.
> > +Before sending any group administration commands to the device, the driver
> > +needs to communicate to the device which commands it is going to
> > +use. Initially (after reset), only two commands are assumed to be used:
> > +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
> > +
> > +Before sending any other commands for any member of a specific group to
> > +the device, the driver queries the supported commands via
> > +VIRTIO_ADMIN_CMD_LIST_QUERY and sends the commands it is
> > +capable of using via VIRTIO_ADMIN_CMD_LIST_USE.
> > +
> > +Commands VIRTIO_ADMIN_CMD_LIST_QUERY and
> > +VIRTIO_ADMIN_CMD_LIST_USE
> > +both use the following structure describing the
> > +command opcodes:
> > +
> > +\begin{lstlisting}
> > +struct virtio_admin_cmd_list {
> > +       /* Indicates which of the below fields were returned
> > +       le64 device_admin_cmd_opcodes[];
> > +};
> > +\end{lstlisting}
> > +
> > +This structure is an array of 64 bit values in little-endian byte
> > +order, in which a bit is set if the specific command opcode
> > +is supported. Thus, \field{device_admin_cmd_opcodes[0]} refers to the
> > +first 64-bit value in this array corresponding to opcodes 0 to
> > +63, \field{device_admin_cmd_opcodes[1]} is the second 64-bit value
> > +corresponding to opcodes 64 to 127, etc.
> > +For example, the array of size 2 including
> > +the values 0x3 in \field{device_admin_cmd_opcodes[0]}
> > +and 0x1 in \field{device_admin_cmd_opcodes[1]} indicates that only
> > +opcodes 0, 1 and 64 are supported.
> > +The length of the array depends on the supported opcodes - it is
> > +large enough to include bits set for all supported opcodes,
> > +that is the length can be calculated by starting with the largest
> > +supported opcode adding one, dividing by 64 and rounding up.
> > +In other words, for
> > +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE the
> > +length of \field{command_specific_result} and
> > +\field{command_specific_data} respectively will be
> > +$DIV_ROUND_UP(max_cmd, 64) * 8$ where DIV_ROUND_UP is integer division
> > +with round up and \field{max_cmd} is the largest available command opcode.
> > +
> > +The array is also allowed to be larger and to additionally
> > +include an arbitrary number of all-zero entries.
> > +
> > +Accordingly, bits 0 and 1 corresponding to opcode 0
> > +(VIRTIO_ADMIN_CMD_LIST_QUERY) and 1
> > +(VIRTIO_ADMIN_CMD_LIST_USE) are
> > +always set in \field{device_admin_cmd_opcodes[0]} returned by VIRTIO_ADMIN_CMD_LIST_QUERY.
> > +
> > +For the command VIRTIO_ADMIN_CMD_LIST_QUERY, \field{opcode} is set to 0x0.
> > +The \field{group_member_id} is unused. It is set to zero by driver.
> > +This command has no command specific data.
> > +The device, upon success, returns a result in
> > +\field{command_specific_result} in the format
> > +\field{struct virtio_admin_cmd_list} describing the
> > +list of group administration commands supported for the group type
> > +specified by \field{group_type}.
> > +
> > +For the command VIRTIO_ADMIN_CMD_LIST_USE, \field{opcode}
> > +is set to 0x1.
> > +The \field{group_member_id} is unused. It is set to zero by driver.
> > +The \field{command_specific_data} is in the format
> > +\field{struct virtio_admin_cmd_list} describing the
> > +list of group administration commands used by the driver
> > +with the group type specified by \field{group_type}.
> > +
> > +This command has no command specific result.
> > +
> > +The driver issues the command VIRTIO_ADMIN_CMD_LIST_QUERY to
> > +query the list of commands valid for this group and before sending
> > +any commands for any member of a group.
> > +
> > +The driver then enables use of some of the opcodes by sending to
> > +the device the command VIRTIO_ADMIN_CMD_LIST_USE with a subset
> > +of the list returned by VIRTIO_ADMIN_CMD_LIST_QUERY that is
> > +both understood and used by the driver.
> > +
> We need to clarify that if LIST_USE command can be called only one time or
> multiple times.
> 
> For example first time for limited functionality, it enabled 10 commands
> specific to SR-IOV.
> Later when user wants to use SIOV, it enables additional SIOV specific
> command set.


clarified in a conformance statement


> > +If the device supports the command list used by the driver, the
> > +device completes the command with status VIRTIO_ADMIN_STATUS_OK.
> > +If the device does not support the command list
> > +(for example, if the driver is not capable to use
> > +some required commands), the device
> > +completes the command with status
> > +VIRTIO_ADMIN_STATUS_INVALID_FIELD.
> > +
> > +Note: drivers are assumed not to set bits in
> > +device_admin_cmd_opcodes
> > +if they are not familiar with how the command opcode
> > +is used, since devices could have dependencies between
> > +command opcodes.
> > +
> Can you please change to use the singular notion for device and driver in
> the note like rest of the above description?
> 
> > +It is assumed that all members in a group support and are used
> > +with the same list of commands. However, for owner devices
> > +supporting multiple group types, the list of supported commands
> > +might differ between different group types.
> > +
> >   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> >   An administration virtqueue of an owner device is used to submit
> 
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v11 08/10] admin: command list discovery

[Zhu Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Add commands to find out which commands does each group support,
> as well as enable their use by driver.
> This will be especially useful once we have multiple group types.
>
> An alternative is per-type VQs. This is possible but will
> require more per-transport work. Discovery through the vq
> helps keep things contained.
>
> e.g. lack of support for some command can switch to a legacy mode
>
> note that commands are expected to be avolved by adding new
> fields to command specific data at the tail, so
> we generally do not need feature bits for compatibility.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
> dropped S.O.B by Max
> explain in commit log how commands will evolve, comment by Stefan
> replace will use with is capable of use, comment by Stefan
> typo fix reported by David and Lingshan
> rename cmds to cmd_opcodes suggested by Jiri
> list group_type explicitly in command description suggested by Jiri
> clarify how can device not support some commands
> always say group administration commands, comment by Jiri
> ---
>   admin.tex | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/admin.tex b/admin.tex
> index f201bcd..9552797 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -113,7 +113,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   \hline
>   opcode & Name & Command Description \\
>   \hline \hline
> -0x0000 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
> +0x0000 & VIRTIO_ADMIN_CMD_LIST_QUERY & Provides to driver list of commands supported for this group type    \\
> +0x0001 & VIRTIO_ADMIN_CMD_LIST_USE & Provides to device list of commands used for this group type \\
> +0x0002 - 0x7FFF & - & Commands using \field{struct virtio_admin_cmd}    \\
>   \hline
>   0x8000 - 0xFFFF & - & Reserved for future commands (possibly using a different structure)    \\
>   \hline
> @@ -183,6 +185,104 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   depends on these structures and is described separately or is
>   implicit in the structure description.
>   
> +Before sending any group administration commands to the device, the driver
> +needs to communicate to the device which commands it is going to
> +use. Initially (after reset), only two commands are assumed to be used:
> +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
> +
> +Before sending any other commands for any member of a specific group to
> +the device, the driver queries the supported commands via
> +VIRTIO_ADMIN_CMD_LIST_QUERY and sends the commands it is
> +capable of using via VIRTIO_ADMIN_CMD_LIST_USE.
> +
> +Commands VIRTIO_ADMIN_CMD_LIST_QUERY and
> +VIRTIO_ADMIN_CMD_LIST_USE
> +both use the following structure describing the
> +command opcodes:
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_list {
> +       /* Indicates which of the below fields were returned
> +       le64 device_admin_cmd_opcodes[];
> +};
> +\end{lstlisting}
> +
> +This structure is an array of 64 bit values in little-endian byte
> +order, in which a bit is set if the specific command opcode
> +is supported. Thus, \field{device_admin_cmd_opcodes[0]} refers to the
> +first 64-bit value in this array corresponding to opcodes 0 to
> +63, \field{device_admin_cmd_opcodes[1]} is the second 64-bit value
> +corresponding to opcodes 64 to 127, etc.
> +For example, the array of size 2 including
> +the values 0x3 in \field{device_admin_cmd_opcodes[0]}
> +and 0x1 in \field{device_admin_cmd_opcodes[1]} indicates that only
> +opcodes 0, 1 and 64 are supported.
> +The length of the array depends on the supported opcodes - it is
> +large enough to include bits set for all supported opcodes,
> +that is the length can be calculated by starting with the largest
> +supported opcode adding one, dividing by 64 and rounding up.
> +In other words, for
> +VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE the
> +length of \field{command_specific_result} and
> +\field{command_specific_data} respectively will be
> +$DIV_ROUND_UP(max_cmd, 64) * 8$ where DIV_ROUND_UP is integer division
> +with round up and \field{max_cmd} is the largest available command opcode.
> +
> +The array is also allowed to be larger and to additionally
> +include an arbitrary number of all-zero entries.
> +
> +Accordingly, bits 0 and 1 corresponding to opcode 0
> +(VIRTIO_ADMIN_CMD_LIST_QUERY) and 1
> +(VIRTIO_ADMIN_CMD_LIST_USE) are
> +always set in \field{device_admin_cmd_opcodes[0]} returned by VIRTIO_ADMIN_CMD_LIST_QUERY.
> +
> +For the command VIRTIO_ADMIN_CMD_LIST_QUERY, \field{opcode} is set to 0x0.
> +The \field{group_member_id} is unused. It is set to zero by driver.
> +This command has no command specific data.
> +The device, upon success, returns a result in
> +\field{command_specific_result} in the format
> +\field{struct virtio_admin_cmd_list} describing the
> +list of group administration commands supported for the group type
> +specified by \field{group_type}.
> +
> +For the command VIRTIO_ADMIN_CMD_LIST_USE, \field{opcode}
> +is set to 0x1.
> +The \field{group_member_id} is unused. It is set to zero by driver.
> +The \field{command_specific_data} is in the format
> +\field{struct virtio_admin_cmd_list} describing the
> +list of group administration commands used by the driver
> +with the group type specified by \field{group_type}.
> +
> +This command has no command specific result.
> +
> +The driver issues the command VIRTIO_ADMIN_CMD_LIST_QUERY to
> +query the list of commands valid for this group and before sending
> +any commands for any member of a group.
> +
> +The driver then enables use of some of the opcodes by sending to
> +the device the command VIRTIO_ADMIN_CMD_LIST_USE with a subset
> +of the list returned by VIRTIO_ADMIN_CMD_LIST_QUERY that is
> +both understood and used by the driver.
> +
> +If the device supports the command list used by the driver, the
> +device completes the command with status VIRTIO_ADMIN_STATUS_OK.
> +If the device does not support the command list
> +(for example, if the driver is not capable to use
> +some required commands), the device
> +completes the command with status
> +VIRTIO_ADMIN_STATUS_INVALID_FIELD.
> +
> +Note: drivers are assumed not to set bits in
> +device_admin_cmd_opcodes
> +if they are not familiar with how the command opcode
> +is used, since devices could have dependencies between
> +command opcodes.
> +
> +It is assumed that all members in a group support and are used
> +with the same list of commands. However, for owner devices
> +supporting multiple group types, the list of supported commands
> +might differ between different group types.
> +
>   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
>   
>   An administration virtqueue of an owner device is used to submit
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

RE: [virtio-dev] [PATCH v11 08/10] admin: command list discovery

[Parav Pandit]

> From: Zhu Lingshan <lingshan.zhu@linux.intel.com>
> Sent: Thursday, April 6, 2023 3:20 AM

I have a dumb question.
Zhu responded to these patches.
Some patches have prefix appended as "virtio", some has "virtio-dev", and some have "virtio-comment".
Whichever tool is adding it prefix, why it cannot keep the simple consistency?

Do we need to ask OASIS about it?

Re: [virtio-dev] [PATCH v11 08/10] admin: command list discovery

[Zhu, Lingshan]

On 4/6/2023 10:05 PM, Parav Pandit wrote:
>
>> From: Zhu Lingshan <lingshan.zhu@linux.intel.com>
>> Sent: Thursday, April 6, 2023 3:20 AM
> I have a dumb question.
> Zhu responded to these patches.
> Some patches have prefix appended as "virtio", some has "virtio-dev", and some have "virtio-comment".
> Whichever tool is adding it prefix, why it cannot keep the simple consistency?
>
> Do we need to ask OASIS about it?
one of the reasons is that I subscribed virtio, virtio-comment and 
virtio-dev, and these threads list are all in the to or cc recipients.

I agree OASIS mailing server sometimes behave oddly, it is reliable most 
of the time, but I didn't receive any direct emails from you about
the virtio-net Sub-TC proposal, but I can receive emails from Stefan. 
That's odd, I should see all your emails via virtio@list.oasis-open.org.
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH v11 08/10] admin: command list discovery

[Michael S. Tsirkin]

On Thu, Apr 06, 2023 at 02:05:07PM +0000, Parav Pandit wrote:
> 
> 
> > From: Zhu Lingshan <lingshan.zhu@linux.intel.com>
> > Sent: Thursday, April 6, 2023 3:20 AM
> 
> I have a dumb question.
> Zhu responded to these patches.
> Some patches have prefix appended as "virtio", some has "virtio-dev", and some have "virtio-comment".
> Whichever tool is adding it prefix, why it cannot keep the simple consistency?
> 
> Do we need to ask OASIS about it?
> 

Guess: are you using gmail?  Each list adds its own prefix.  Which is
admittedly a bad idea but there you are, OASIS outsources this and can't
change it. Adding a footer is an even worse idea but they won't change
this either.

So if you are subscribed to multiple lists you are getting
multiple copies, which is handy if each list goes to its
own folder. However, gmail is unusual in that it deduplicates
email discarding all copies but one. There are work-arounds
with filters but the end result in any case will be that
you will get one of the copies, selected at random.
As they have different prefixes and footers added, there you go,
a mix of different prefixes in a single thread.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 09/10] admin: conformance clauses

[Michael S. Tsirkin]

Add conformance clauses for admin commands and admin virtqueues.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
typo and wording fixes reported by David
clarify cmd list use can be repeated but not in parallel with other
commands, and returning same value across uses - comment by Lingshan
add conformance clauses for new error codes
---
 admin.tex | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 227 insertions(+)

diff --git a/admin.tex b/admin.tex
index 9552797..a398c51 100644
--- a/admin.tex
+++ b/admin.tex
@@ -283,6 +283,150 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 supporting multiple group types, the list of supported commands
 might differ between different group types.
 
+\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
+
+The device MUST validate \field{opcode}, \field{group_type} and
+\field{group_member_id}, and if any of these has an invalid or
+unsupported value, set \field{status} to
+VIRTIO_ADMIN_STATUS_EINVAL and set \field{status_qualifier}
+accordingly:
+\begin{itemize}
+\item if \field{group_type} is invalid, \field{status_qualifier}
+	MUST be set to VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
+\item otherwise, if \field{opcode} is invalid,
+	\field{status_qualifier} MUST be set to
+	VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE;
+\item otherwise, if \field{group_member_id} is used by the
+	specific command and is invalid, \field{status_qualifier} MUST be
+	set to VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER.
+\end{itemize}
+
+If a command completes successfully, the device MUST set
+\field{status} to VIRTIO_ADMIN_STATUS_OK.
+
+If a command fails, the device MUST set
+\field{status} to a value different from VIRTIO_ADMIN_STATUS_OK.
+
+If \field{status} is set to VIRTIO_ADMIN_STATUS_EINVAL, the
+device state MUST NOT change, that is the command MUST NOT have
+any side effects on the device, in particular the device MUST NOT
+enter an error state as a result of this command.
+
+If a command fails, the device state generally SHOULD NOT change,
+as far as possible.
+
+The device MAY enforce additional restrictions and dependencies on
+opcodes used by the driver and MAY fail the command
+VIRTIO_ADMIN_CMD_LIST_USE with \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
+and \field{status_qualifier} set to VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD
+if the list of commands used violate internal device dependencies.
+
+If the device supports multiple group types, commands for each group
+type MUST operate independently of each other, in particular,
+the device MAY return different results for VIRTIO_ADMIN_CMD_LIST_QUERY
+for different group types.
+
+After reset, if the device supports a given group type
+and before receiving VIRTIO_ADMIN_CMD_LIST_USE for this group type
+the device MUST assume
+that the list of legal commands used by the driver consists of
+the two commands VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
+
+After completing VIRTIO_ADMIN_CMD_LIST_USE successfully,
+the device MUST set the list of legal commands used by the driver
+to the one supplied in \field{command_specific_data}.
+
+The device MUST validate commands against the list used by
+the driver and MUST fail any commands not in the list with
+\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
+and \field{status_qualifier} set to
+VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE.
+
+The list of supported commands reported by the device MUST NOT
+shrink (but MAY expand): after reporting a given command as
+supported through VIRTIO_ADMIN_CMD_LIST_QUERY the device MUST NOT
+later report it as unsupported.  Further, after a given set of
+commands has been used (via a successful
+VIRTIO_ADMIN_CMD_LIST_USE), then after a device or system reset
+the device SHOULD complete successfully any following calls to
+VIRTIO_ADMIN_CMD_LIST_USE with the same list of commands; if this
+command VIRTIO_ADMIN_CMD_LIST_USE fails after a device or system
+reset, the device MUST not fail it solely because of the command
+list used.  Failure to do so would interfere with resuming from
+suspend and error recovery. Exceptions MAY apply if the system
+configuration assures, in some way, that the driver does not
+cache the previous value of VIRTIO_ADMIN_CMD_LIST_USE,
+such as in the case of a firmware upgrade or downgrade.
+
+When processing a command with the SR-IOV group type,
+if the device does not have an SR-IOV Extended Capability or
+if \field{VF Enable} is clear
+then the device MUST fail all commands with
+\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
+\field{status_qualifier} set to
+VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
+otherwise, if \field{group_member_id} is not
+between $1$ and \field{NumVFs} inclusive,
+the device MUST fail all commands with
+\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
+\field{status_qualifier} set to
+VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER;
+\field{NumVFs}, \field{VF Migration Capable}  and
+\field{VF Enable} refer to registers within the SR-IOV Extended
+Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
+
+\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
+
+The driver MAY discover whether device supports a specific group type
+by issuing VIRTIO_ADMIN_CMD_LIST_QUERY with the matching
+\field{group_type}.
+
+The driver MUST issue VIRTIO_ADMIN_CMD_LIST_USE
+and wait for it to be completed with status
+VIRTIO_ADMIN_STATUS_OK before issuing any commands
+(except for the initial VIRTIO_ADMIN_CMD_LIST_QUERY
+and VIRTIO_ADMIN_CMD_LIST_USE).
+
+The driver MAY issue VIRTIO_ADMIN_CMD_LIST_USE any number
+of times but MUST NOT issue VIRTIO_ADMIN_CMD_LIST_USE commands
+if any other command is currently being processed by the device.
+
+The driver SHOULD NOT set bits in device_admin_cmd_opcodes
+if it is not familiar with how the command opcode
+is used, as dependencies between command opcodes might exist.
+
+The driver MUST NOT request (via VIRTIO_ADMIN_CMD_LIST_USE)
+the use of any commands not previously reported as
+supported for the same group type by VIRTIO_ADMIN_CMD_LIST_QUERY.
+
+The driver MUST NOT use any commands for a given group type
+before sending VIRTIO_ADMIN_CMD_LIST_USE with the correct
+list of command opcodes and group type.
+
+The driver MAY block use of VIRTIO_ADMIN_CMD_LIST_QUERY and
+VIRTIO_ADMIN_CMD_LIST_USE by issuing VIRTIO_ADMIN_CMD_LIST_USE
+with respective bits cleared in \field{command_specific_data}.
+
+The driver MUST handle a command error with a reserved \field{status}
+value in the same way as \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
+(except possibly for different error reporting/diagnostic messages).
+
+The driver MUST handle a command error with a reserved
+\field{status_qualifier} value in the same way as
+\field{status_qualifier} set to
+VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND (except possibly for
+different error reporting/diagnostic messages).
+
+When sending commands with the SR-IOV group type,
+the driver specify a value for \field{group_member_id}
+between $1$ and \field{NumVFs} inclusive,
+the driver MUST also make sure that as long as any such command
+is outstanding, \field{VF Migration Capable} is clear and
+\field{VF Enable} is set;
+\field{NumVFs}, \field{VF Migration Capable}  and
+\field{VF Enable} refer to registers within the SR-IOV Extended
+Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
+
 \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 
 An administration virtqueue of an owner device is used to submit
@@ -357,3 +501,86 @@ \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Devic
 of the specification are designed, new fields can be added to the
 tail of a structure, with the driver/device using the full
 structure without concern for versioning.
+
+\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
+
+The device MUST support device-readable and device-writeable buffers
+shorter than described in this specification, by
+\begin{enumerate}
+\item acting as if any data that would be read outside the
+device-readable buffers is set to zero, and
+\item discarding data that would be written outside the
+specified device-writeable buffers.
+\end{enumerate}
+
+The device MUST support device-readable and device-writeable buffers
+longer than described in this specification, by
+\begin{enumerate}
+\item ignoring any data in device-readable buffers outside
+the expected length, and
+\item only writing the expected structure to the device-writeable
+buffers, ignoring any extra buffers, and reporting the
+actual length of data written, in bytes,
+as buffer used length.
+\end{enumerate}
+
+The device SHOULD initialize the device-writeable buffer
+up to the length of the structure described by this specification or
+the length of the buffer supplied by the driver (even if the buffer is
+all set to zero), whichever is shorter.
+
+The device MUST NOT fail a command solely because the buffers
+provided are shorter or longer than described in this
+specification.
+
+The device MUST initialize the device-writeable part of
+\field{struct virtio_admin_cmd} that is a multiple of 64 bit in
+size.
+
+The device MUST initialize \field{status} and
+\field{status_qualifier} in \field{struct virtio_admin_cmd}.
+
+The device MUST process commands on a given administration virtqueue
+in the order in which they are queued.
+
+If multiple administration virtqueues have been configured,
+device MAY process commands on distinct virtqueues with
+no order constraints.
+
+If the device sets \field{status} to either VIRTIO_ADMIN_STATUS_EAGAIN
+or VIRTIO_ADMIN_STATUS_ENOMEM, then the command MUST NOT
+have any side effects, making it safe to retry.
+
+\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
+
+The driver MAY supply device-readable or device-writeable parts
+of \field{struct virtio_admin_cmd} that are longer than described in
+this specification.
+
+The driver SHOULD supply device-readable part of
+\field{struct virtio_admin_cmd} that is at least as
+large as the structure described by this specification
+(even if the structure is all set to zero).
+
+The driver MUST supply both device-readable or device-writeable parts
+of \field{struct virtio_admin_cmd} that are a multiple of 64 bit
+in length.
+
+The device MUST supply both device-readable or device-writeable parts
+of \field{struct virtio_admin_cmd} that are larger than zero in
+length. However, \field{command_specific_data} and
+\field{command_specific_result} MAY be zero in length, unless
+specified otherwise for the command.
+
+The driver MUST NOT assume that the device will initialize the whole
+device-writeable part of \field{struct virtio_admin_cmd} as described in the specification; instead,
+the driver MUST act as if the structure
+outside the part of the buffer used by the device
+is set to zero.
+
+If multiple administration virtqueues have been configured,
+the driver MUST ensure ordering for commands
+placed on different administration virtqueues.
+
+The driver SHOULD retry a command that completed with
+\field{status} VIRTIO_ADMIN_STATUS_EAGAIN.
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 09/10] admin: conformance clauses

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Mon, Apr 03, 2023 at 11:03:56AM -0400, Michael S. Tsirkin wrote:
> Add conformance clauses for admin commands and admin virtqueues.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> typo and wording fixes reported by David
> clarify cmd list use can be repeated but not in parallel with other
> commands, and returning same value across uses - comment by Lingshan
> add conformance clauses for new error codes
> ---
>  admin.tex | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 227 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[virtio-dev] Re: [virtio] [PATCH v11 09/10] admin: conformance clauses

[Zhu, Lingshan]

On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> Add conformance clauses for admin commands and admin virtqueues.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
> typo and wording fixes reported by David
> clarify cmd list use can be repeated but not in parallel with other
> commands, and returning same value across uses - comment by Lingshan
> add conformance clauses for new error codes
> ---
>   admin.tex | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 227 insertions(+)
>
> diff --git a/admin.tex b/admin.tex
> index 9552797..a398c51 100644
> --- a/admin.tex
> +++ b/admin.tex
> @@ -283,6 +283,150 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
>   supporting multiple group types, the list of supported commands
>   might differ between different group types.
>   
> +\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
> +
> +The device MUST validate \field{opcode}, \field{group_type} and
> +\field{group_member_id}, and if any of these has an invalid or
> +unsupported value, set \field{status} to
> +VIRTIO_ADMIN_STATUS_EINVAL and set \field{status_qualifier}
> +accordingly:
> +\begin{itemize}
> +\item if \field{group_type} is invalid, \field{status_qualifier}
> +	MUST be set to VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
> +\item otherwise, if \field{opcode} is invalid,
> +	\field{status_qualifier} MUST be set to
> +	VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE;
> +\item otherwise, if \field{group_member_id} is used by the
> +	specific command and is invalid, \field{status_qualifier} MUST be
> +	set to VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER.
> +\end{itemize}
> +
> +If a command completes successfully, the device MUST set
> +\field{status} to VIRTIO_ADMIN_STATUS_OK.
> +
> +If a command fails, the device MUST set
> +\field{status} to a value different from VIRTIO_ADMIN_STATUS_OK.
> +
> +If \field{status} is set to VIRTIO_ADMIN_STATUS_EINVAL, the
> +device state MUST NOT change, that is the command MUST NOT have
> +any side effects on the device, in particular the device MUST NOT
> +enter an error state as a result of this command.
> +
> +If a command fails, the device state generally SHOULD NOT change,
> +as far as possible.
> +
> +The device MAY enforce additional restrictions and dependencies on
> +opcodes used by the driver and MAY fail the command
> +VIRTIO_ADMIN_CMD_LIST_USE with \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> +and \field{status_qualifier} set to VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD
> +if the list of commands used violate internal device dependencies.
> +
> +If the device supports multiple group types, commands for each group
> +type MUST operate independently of each other, in particular,
> +the device MAY return different results for VIRTIO_ADMIN_CMD_LIST_QUERY
> +for different group types.
> +
> +After reset, if the device supports a given group type
> +and before receiving VIRTIO_ADMIN_CMD_LIST_USE for this group type
> +the device MUST assume
> +that the list of legal commands used by the driver consists of
> +the two commands VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
> +
> +After completing VIRTIO_ADMIN_CMD_LIST_USE successfully,
> +the device MUST set the list of legal commands used by the driver
> +to the one supplied in \field{command_specific_data}.
> +
> +The device MUST validate commands against the list used by
> +the driver and MUST fail any commands not in the list with
> +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> +and \field{status_qualifier} set to
> +VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE.
> +
> +The list of supported commands reported by the device MUST NOT
> +shrink (but MAY expand): after reporting a given command as
> +supported through VIRTIO_ADMIN_CMD_LIST_QUERY the device MUST NOT
> +later report it as unsupported.  Further, after a given set of
> +commands has been used (via a successful
> +VIRTIO_ADMIN_CMD_LIST_USE), then after a device or system reset
> +the device SHOULD complete successfully any following calls to
> +VIRTIO_ADMIN_CMD_LIST_USE with the same list of commands; if this
> +command VIRTIO_ADMIN_CMD_LIST_USE fails after a device or system
> +reset, the device MUST not fail it solely because of the command
> +list used.  Failure to do so would interfere with resuming from
> +suspend and error recovery. Exceptions MAY apply if the system
> +configuration assures, in some way, that the driver does not
> +cache the previous value of VIRTIO_ADMIN_CMD_LIST_USE,
> +such as in the case of a firmware upgrade or downgrade.
> +
> +When processing a command with the SR-IOV group type,
> +if the device does not have an SR-IOV Extended Capability or
> +if \field{VF Enable} is clear
> +then the device MUST fail all commands with
> +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
> +\field{status_qualifier} set to
> +VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
> +otherwise, if \field{group_member_id} is not
> +between $1$ and \field{NumVFs} inclusive,
> +the device MUST fail all commands with
> +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
> +\field{status_qualifier} set to
> +VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER;
> +\field{NumVFs}, \field{VF Migration Capable}  and
> +\field{VF Enable} refer to registers within the SR-IOV Extended
> +Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
> +
> +\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
> +
> +The driver MAY discover whether device supports a specific group type
> +by issuing VIRTIO_ADMIN_CMD_LIST_QUERY with the matching
> +\field{group_type}.
> +
> +The driver MUST issue VIRTIO_ADMIN_CMD_LIST_USE
> +and wait for it to be completed with status
> +VIRTIO_ADMIN_STATUS_OK before issuing any commands
> +(except for the initial VIRTIO_ADMIN_CMD_LIST_QUERY
> +and VIRTIO_ADMIN_CMD_LIST_USE).
> +
> +The driver MAY issue VIRTIO_ADMIN_CMD_LIST_USE any number
> +of times but MUST NOT issue VIRTIO_ADMIN_CMD_LIST_USE commands
> +if any other command is currently being processed by the device.
Please correct me if I misunderstood this,
IMHO when the driver re-negotiates the features by issuing _USE command,
the driver can choose to shrink the feature bits and there
may be some already queued descriptors require the previous negotiated
feature bits. So I think the driver must make sure the admin queues are
empty(like all descriptors are marked as used) before it re-negotiating the
features.

Others look good to me.
> +
> +The driver SHOULD NOT set bits in device_admin_cmd_opcodes
> +if it is not familiar with how the command opcode
> +is used, as dependencies between command opcodes might exist.
> +
> +The driver MUST NOT request (via VIRTIO_ADMIN_CMD_LIST_USE)
> +the use of any commands not previously reported as
> +supported for the same group type by VIRTIO_ADMIN_CMD_LIST_QUERY.
> +
> +The driver MUST NOT use any commands for a given group type
> +before sending VIRTIO_ADMIN_CMD_LIST_USE with the correct
> +list of command opcodes and group type.
> +
> +The driver MAY block use of VIRTIO_ADMIN_CMD_LIST_QUERY and
> +VIRTIO_ADMIN_CMD_LIST_USE by issuing VIRTIO_ADMIN_CMD_LIST_USE
> +with respective bits cleared in \field{command_specific_data}.
> +
> +The driver MUST handle a command error with a reserved \field{status}
> +value in the same way as \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> +(except possibly for different error reporting/diagnostic messages).
> +
> +The driver MUST handle a command error with a reserved
> +\field{status_qualifier} value in the same way as
> +\field{status_qualifier} set to
> +VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND (except possibly for
> +different error reporting/diagnostic messages).
> +
> +When sending commands with the SR-IOV group type,
> +the driver specify a value for \field{group_member_id}
> +between $1$ and \field{NumVFs} inclusive,
> +the driver MUST also make sure that as long as any such command
> +is outstanding, \field{VF Migration Capable} is clear and
> +\field{VF Enable} is set;
> +\field{NumVFs}, \field{VF Migration Capable}  and
> +\field{VF Enable} refer to registers within the SR-IOV Extended
> +Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
> +
>   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
>   
>   An administration virtqueue of an owner device is used to submit
> @@ -357,3 +501,86 @@ \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Devic
>   of the specification are designed, new fields can be added to the
>   tail of a structure, with the driver/device using the full
>   structure without concern for versioning.
> +
> +\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
> +
> +The device MUST support device-readable and device-writeable buffers
> +shorter than described in this specification, by
> +\begin{enumerate}
> +\item acting as if any data that would be read outside the
> +device-readable buffers is set to zero, and
> +\item discarding data that would be written outside the
> +specified device-writeable buffers.
> +\end{enumerate}
> +
> +The device MUST support device-readable and device-writeable buffers
> +longer than described in this specification, by
> +\begin{enumerate}
> +\item ignoring any data in device-readable buffers outside
> +the expected length, and
> +\item only writing the expected structure to the device-writeable
> +buffers, ignoring any extra buffers, and reporting the
> +actual length of data written, in bytes,
> +as buffer used length.
> +\end{enumerate}
> +
> +The device SHOULD initialize the device-writeable buffer
> +up to the length of the structure described by this specification or
> +the length of the buffer supplied by the driver (even if the buffer is
> +all set to zero), whichever is shorter.
> +
> +The device MUST NOT fail a command solely because the buffers
> +provided are shorter or longer than described in this
> +specification.
> +
> +The device MUST initialize the device-writeable part of
> +\field{struct virtio_admin_cmd} that is a multiple of 64 bit in
> +size.
> +
> +The device MUST initialize \field{status} and
> +\field{status_qualifier} in \field{struct virtio_admin_cmd}.
> +
> +The device MUST process commands on a given administration virtqueue
> +in the order in which they are queued.
> +
> +If multiple administration virtqueues have been configured,
> +device MAY process commands on distinct virtqueues with
> +no order constraints.
> +
> +If the device sets \field{status} to either VIRTIO_ADMIN_STATUS_EAGAIN
> +or VIRTIO_ADMIN_STATUS_ENOMEM, then the command MUST NOT
> +have any side effects, making it safe to retry.
> +
> +\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
> +
> +The driver MAY supply device-readable or device-writeable parts
> +of \field{struct virtio_admin_cmd} that are longer than described in
> +this specification.
> +
> +The driver SHOULD supply device-readable part of
> +\field{struct virtio_admin_cmd} that is at least as
> +large as the structure described by this specification
> +(even if the structure is all set to zero).
> +
> +The driver MUST supply both device-readable or device-writeable parts
> +of \field{struct virtio_admin_cmd} that are a multiple of 64 bit
> +in length.
> +
> +The device MUST supply both device-readable or device-writeable parts
> +of \field{struct virtio_admin_cmd} that are larger than zero in
> +length. However, \field{command_specific_data} and
> +\field{command_specific_result} MAY be zero in length, unless
> +specified otherwise for the command.
> +
> +The driver MUST NOT assume that the device will initialize the whole
> +device-writeable part of \field{struct virtio_admin_cmd} as described in the specification; instead,
> +the driver MUST act as if the structure
> +outside the part of the buffer used by the device
> +is set to zero.
> +
> +If multiple administration virtqueues have been configured,
> +the driver MUST ensure ordering for commands
> +placed on different administration virtqueues.
> +
> +The driver SHOULD retry a command that completed with
> +\field{status} VIRTIO_ADMIN_STATUS_EAGAIN.
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] Re: [virtio] [PATCH v11 09/10] admin: conformance clauses

[Michael S. Tsirkin]

On Thu, Apr 06, 2023 at 03:30:56PM +0800, Zhu, Lingshan wrote:
> 
> 
> On 4/3/2023 11:03 PM, Michael S. Tsirkin wrote:
> > Add conformance clauses for admin commands and admin virtqueues.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > typo and wording fixes reported by David
> > clarify cmd list use can be repeated but not in parallel with other
> > commands, and returning same value across uses - comment by Lingshan
> > add conformance clauses for new error codes
> > ---
> >   admin.tex | 227 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 227 insertions(+)
> > 
> > diff --git a/admin.tex b/admin.tex
> > index 9552797..a398c51 100644
> > --- a/admin.tex
> > +++ b/admin.tex
> > @@ -283,6 +283,150 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
> >   supporting multiple group types, the list of supported commands
> >   might differ between different group types.
> > +\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
> > +
> > +The device MUST validate \field{opcode}, \field{group_type} and
> > +\field{group_member_id}, and if any of these has an invalid or
> > +unsupported value, set \field{status} to
> > +VIRTIO_ADMIN_STATUS_EINVAL and set \field{status_qualifier}
> > +accordingly:
> > +\begin{itemize}
> > +\item if \field{group_type} is invalid, \field{status_qualifier}
> > +	MUST be set to VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
> > +\item otherwise, if \field{opcode} is invalid,
> > +	\field{status_qualifier} MUST be set to
> > +	VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE;
> > +\item otherwise, if \field{group_member_id} is used by the
> > +	specific command and is invalid, \field{status_qualifier} MUST be
> > +	set to VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER.
> > +\end{itemize}
> > +
> > +If a command completes successfully, the device MUST set
> > +\field{status} to VIRTIO_ADMIN_STATUS_OK.
> > +
> > +If a command fails, the device MUST set
> > +\field{status} to a value different from VIRTIO_ADMIN_STATUS_OK.
> > +
> > +If \field{status} is set to VIRTIO_ADMIN_STATUS_EINVAL, the
> > +device state MUST NOT change, that is the command MUST NOT have
> > +any side effects on the device, in particular the device MUST NOT
> > +enter an error state as a result of this command.
> > +
> > +If a command fails, the device state generally SHOULD NOT change,
> > +as far as possible.
> > +
> > +The device MAY enforce additional restrictions and dependencies on
> > +opcodes used by the driver and MAY fail the command
> > +VIRTIO_ADMIN_CMD_LIST_USE with \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> > +and \field{status_qualifier} set to VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD
> > +if the list of commands used violate internal device dependencies.
> > +
> > +If the device supports multiple group types, commands for each group
> > +type MUST operate independently of each other, in particular,
> > +the device MAY return different results for VIRTIO_ADMIN_CMD_LIST_QUERY
> > +for different group types.
> > +
> > +After reset, if the device supports a given group type
> > +and before receiving VIRTIO_ADMIN_CMD_LIST_USE for this group type
> > +the device MUST assume
> > +that the list of legal commands used by the driver consists of
> > +the two commands VIRTIO_ADMIN_CMD_LIST_QUERY and VIRTIO_ADMIN_CMD_LIST_USE.
> > +
> > +After completing VIRTIO_ADMIN_CMD_LIST_USE successfully,
> > +the device MUST set the list of legal commands used by the driver
> > +to the one supplied in \field{command_specific_data}.
> > +
> > +The device MUST validate commands against the list used by
> > +the driver and MUST fail any commands not in the list with
> > +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> > +and \field{status_qualifier} set to
> > +VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE.
> > +
> > +The list of supported commands reported by the device MUST NOT
> > +shrink (but MAY expand): after reporting a given command as
> > +supported through VIRTIO_ADMIN_CMD_LIST_QUERY the device MUST NOT
> > +later report it as unsupported.  Further, after a given set of
> > +commands has been used (via a successful
> > +VIRTIO_ADMIN_CMD_LIST_USE), then after a device or system reset
> > +the device SHOULD complete successfully any following calls to
> > +VIRTIO_ADMIN_CMD_LIST_USE with the same list of commands; if this
> > +command VIRTIO_ADMIN_CMD_LIST_USE fails after a device or system
> > +reset, the device MUST not fail it solely because of the command
> > +list used.  Failure to do so would interfere with resuming from
> > +suspend and error recovery. Exceptions MAY apply if the system
> > +configuration assures, in some way, that the driver does not
> > +cache the previous value of VIRTIO_ADMIN_CMD_LIST_USE,
> > +such as in the case of a firmware upgrade or downgrade.
> > +
> > +When processing a command with the SR-IOV group type,
> > +if the device does not have an SR-IOV Extended Capability or
> > +if \field{VF Enable} is clear
> > +then the device MUST fail all commands with
> > +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
> > +\field{status_qualifier} set to
> > +VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP;
> > +otherwise, if \field{group_member_id} is not
> > +between $1$ and \field{NumVFs} inclusive,
> > +the device MUST fail all commands with
> > +\field{status} set to VIRTIO_ADMIN_STATUS_EINVAL and
> > +\field{status_qualifier} set to
> > +VIRTIO_ADMIN_STATUS_Q_INVALID_MEMBER;
> > +\field{NumVFs}, \field{VF Migration Capable}  and
> > +\field{VF Enable} refer to registers within the SR-IOV Extended
> > +Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
> > +
> > +\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Device groups / Group administration commands}
> > +
> > +The driver MAY discover whether device supports a specific group type
> > +by issuing VIRTIO_ADMIN_CMD_LIST_QUERY with the matching
> > +\field{group_type}.
> > +
> > +The driver MUST issue VIRTIO_ADMIN_CMD_LIST_USE
> > +and wait for it to be completed with status
> > +VIRTIO_ADMIN_STATUS_OK before issuing any commands
> > +(except for the initial VIRTIO_ADMIN_CMD_LIST_QUERY
> > +and VIRTIO_ADMIN_CMD_LIST_USE).
> > +
> > +The driver MAY issue VIRTIO_ADMIN_CMD_LIST_USE any number
> > +of times but MUST NOT issue VIRTIO_ADMIN_CMD_LIST_USE commands
> > +if any other command is currently being processed by the device.
> Please correct me if I misunderstood this,
> IMHO when the driver re-negotiates the features by issuing _USE command,
> the driver can choose to shrink the feature bits and there
> may be some already queued descriptors require the previous negotiated
> feature bits. So I think the driver must make sure the admin queues are
> empty(like all descriptors are marked as used) before it re-negotiating the
> features.

oh so not just processed, queued also. good point.

> Others look good to me.
> > +
> > +The driver SHOULD NOT set bits in device_admin_cmd_opcodes
> > +if it is not familiar with how the command opcode
> > +is used, as dependencies between command opcodes might exist.
> > +
> > +The driver MUST NOT request (via VIRTIO_ADMIN_CMD_LIST_USE)
> > +the use of any commands not previously reported as
> > +supported for the same group type by VIRTIO_ADMIN_CMD_LIST_QUERY.
> > +
> > +The driver MUST NOT use any commands for a given group type
> > +before sending VIRTIO_ADMIN_CMD_LIST_USE with the correct
> > +list of command opcodes and group type.
> > +
> > +The driver MAY block use of VIRTIO_ADMIN_CMD_LIST_QUERY and
> > +VIRTIO_ADMIN_CMD_LIST_USE by issuing VIRTIO_ADMIN_CMD_LIST_USE
> > +with respective bits cleared in \field{command_specific_data}.
> > +
> > +The driver MUST handle a command error with a reserved \field{status}
> > +value in the same way as \field{status} set to VIRTIO_ADMIN_STATUS_EINVAL
> > +(except possibly for different error reporting/diagnostic messages).
> > +
> > +The driver MUST handle a command error with a reserved
> > +\field{status_qualifier} value in the same way as
> > +\field{status_qualifier} set to
> > +VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND (except possibly for
> > +different error reporting/diagnostic messages).
> > +
> > +When sending commands with the SR-IOV group type,
> > +the driver specify a value for \field{group_member_id}
> > +between $1$ and \field{NumVFs} inclusive,
> > +the driver MUST also make sure that as long as any such command
> > +is outstanding, \field{VF Migration Capable} is clear and
> > +\field{VF Enable} is set;
> > +\field{NumVFs}, \field{VF Migration Capable}  and
> > +\field{VF Enable} refer to registers within the SR-IOV Extended
> > +Capability as specified by \hyperref[intro:PCIe]{[PCIe]}.
> > +
> >   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
> >   An administration virtqueue of an owner device is used to submit
> > @@ -357,3 +501,86 @@ \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Devic
> >   of the specification are designed, new fields can be added to the
> >   tail of a structure, with the driver/device using the full
> >   structure without concern for versioning.
> > +
> > +\devicenormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
> > +
> > +The device MUST support device-readable and device-writeable buffers
> > +shorter than described in this specification, by
> > +\begin{enumerate}
> > +\item acting as if any data that would be read outside the
> > +device-readable buffers is set to zero, and
> > +\item discarding data that would be written outside the
> > +specified device-writeable buffers.
> > +\end{enumerate}
> > +
> > +The device MUST support device-readable and device-writeable buffers
> > +longer than described in this specification, by
> > +\begin{enumerate}
> > +\item ignoring any data in device-readable buffers outside
> > +the expected length, and
> > +\item only writing the expected structure to the device-writeable
> > +buffers, ignoring any extra buffers, and reporting the
> > +actual length of data written, in bytes,
> > +as buffer used length.
> > +\end{enumerate}
> > +
> > +The device SHOULD initialize the device-writeable buffer
> > +up to the length of the structure described by this specification or
> > +the length of the buffer supplied by the driver (even if the buffer is
> > +all set to zero), whichever is shorter.
> > +
> > +The device MUST NOT fail a command solely because the buffers
> > +provided are shorter or longer than described in this
> > +specification.
> > +
> > +The device MUST initialize the device-writeable part of
> > +\field{struct virtio_admin_cmd} that is a multiple of 64 bit in
> > +size.
> > +
> > +The device MUST initialize \field{status} and
> > +\field{status_qualifier} in \field{struct virtio_admin_cmd}.
> > +
> > +The device MUST process commands on a given administration virtqueue
> > +in the order in which they are queued.
> > +
> > +If multiple administration virtqueues have been configured,
> > +device MAY process commands on distinct virtqueues with
> > +no order constraints.
> > +
> > +If the device sets \field{status} to either VIRTIO_ADMIN_STATUS_EAGAIN
> > +or VIRTIO_ADMIN_STATUS_ENOMEM, then the command MUST NOT
> > +have any side effects, making it safe to retry.
> > +
> > +\drivernormative{\paragraph}{Group administration commands}{Basic Facilities of a Virtio Device / Administration virtqueues}
> > +
> > +The driver MAY supply device-readable or device-writeable parts
> > +of \field{struct virtio_admin_cmd} that are longer than described in
> > +this specification.
> > +
> > +The driver SHOULD supply device-readable part of
> > +\field{struct virtio_admin_cmd} that is at least as
> > +large as the structure described by this specification
> > +(even if the structure is all set to zero).
> > +
> > +The driver MUST supply both device-readable or device-writeable parts
> > +of \field{struct virtio_admin_cmd} that are a multiple of 64 bit
> > +in length.
> > +
> > +The device MUST supply both device-readable or device-writeable parts
> > +of \field{struct virtio_admin_cmd} that are larger than zero in
> > +length. However, \field{command_specific_data} and
> > +\field{command_specific_result} MAY be zero in length, unless
> > +specified otherwise for the command.
> > +
> > +The driver MUST NOT assume that the device will initialize the whole
> > +device-writeable part of \field{struct virtio_admin_cmd} as described in the specification; instead,
> > +the driver MUST act as if the structure
> > +outside the part of the buffer used by the device
> > +is set to zero.
> > +
> > +If multiple administration virtqueues have been configured,
> > +the driver MUST ensure ordering for commands
> > +placed on different administration virtqueues.
> > +
> > +The driver SHOULD retry a command that completed with
> > +\field{status} VIRTIO_ADMIN_STATUS_EAGAIN.
> Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>
> 
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
> 
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
> 
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] [PATCH v11 10/10] ccw: document more reserved features

[Michael S. Tsirkin]

vq reset and shared memory are unsupported, too.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Fixes: https://github.com/oasis-tcs/virtio-spec/issues/160
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 content.tex | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/content.tex b/content.tex
index 1213d48..fca3827 100644
--- a/content.tex
+++ b/content.tex
@@ -2985,6 +2985,8 @@ \subsection{Features reserved for future use}\label{sec:Virtio Transport Options
 \begin{itemize}
 
 \item VIRTIO_F_ADMIN_VQ
+\item VIRTIO_F_RING_RESET
+\item Shared memory regions including VIRTIO_PMEM_F_SHMEM_REGION
 
 \end{itemize}
 
-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [virtio-comment] [PATCH v11 10/10] ccw: document more reserved features

[Zhu Lingshan]

On 4/3/2023 11:04 PM, Michael S. Tsirkin wrote:
> vq reset and shared memory are unsupported, too.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/160
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   content.tex | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 1213d48..fca3827 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2985,6 +2985,8 @@ \subsection{Features reserved for future use}\label{sec:Virtio Transport Options
>   \begin{itemize}
>   
>   \item VIRTIO_F_ADMIN_VQ
> +\item VIRTIO_F_RING_RESET
> +\item Shared memory regions including VIRTIO_PMEM_F_SHMEM_REGION
>   
>   \end{itemize}
>   
Reviewed-by: Zhu Lingshan <lingshan.zhu@intel.com>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 00/10] Introduce device group and device management

[Parav Pandit]

On 4/3/2023 11:02 AM, Michael S. Tsirkin wrote:
> 
> Change log:
> 
> since v10:
> 	addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, Max
> 	Cornelia, do you want to pick up 10/10 separately?
>
structuring prep patches as 7/10, 10/10 as first patches followed by 
6/10 is more clear as it builds the framework first for existing items.

But not a must item for me.

I am ok with the current patch sequence as the change in 10/10 is really 
small and issue number is already listed in commit log of it.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 00/10] Introduce device group and device management

[Cornelia Huck]

On Mon, Apr 03 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> Change log:
>
> since v10:
> 	addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, Max
> 	Cornelia, do you want to pick up 10/10 separately?

I'm not sure that makes sense -- if we refactor to introduce a patch
that deals with all of the currently unimplemented features (and respin
this series with the patch that lists the admin vq as unsupported on
top) we could do that, but I don't think it's worth the effort.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 00/10] Introduce device group and device management

[Michael S. Tsirkin]

On Mon, Apr 03, 2023 at 06:39:12PM +0200, Cornelia Huck wrote:
> On Mon, Apr 03 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > Change log:
> >
> > since v10:
> > 	addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, Max
> > 	Cornelia, do you want to pick up 10/10 separately?
> 
> I'm not sure that makes sense -- if we refactor to introduce a patch
> that deals with all of the currently unimplemented features (and respin
> this series with the patch that lists the admin vq as unsupported on
> top) we could do that, but I don't think it's worth the effort.

My concern is that people are already kind of familiar with the
patchset, reshuffling for no end result will cause everyone to re-read.

-- 
MST


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[virtio-dev] Re: [PATCH v11 00/10] Introduce device group and device management

[Cornelia Huck]

On Mon, Apr 03 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Apr 03, 2023 at 06:39:12PM +0200, Cornelia Huck wrote:
>> On Mon, Apr 03 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > Change log:
>> >
>> > since v10:
>> > 	addressed lots of comments by Jiri, Stefan. Cornelia, Lngshan, Parav, Max
>> > 	Cornelia, do you want to pick up 10/10 separately?
>> 
>> I'm not sure that makes sense -- if we refactor to introduce a patch
>> that deals with all of the currently unimplemented features (and respin
>> this series with the patch that lists the admin vq as unsupported on
>> top) we could do that, but I don't think it's worth the effort.
>
> My concern is that people are already kind of familiar with the
> patchset, reshuffling for no end result will cause everyone to re-read.

Yeah, let's just have 10/10 walk along with the rest.


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org