Introduce device group and device management

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

Suggested-by Halil Pasic <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644

content.tex +9 -0

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

content.tex +8 -0

+
 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

--
MST

Quoted 19 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 126

Quoted 6 lines

Likewise, it is important that

content.tex +5 -0

+drivers \em{do not} accept feature bits they do not know how to handle
+(even though devices are not supposed to offer them in the first place,
+according to this version of the specification.) The preferred way for
+handling reserved and unexpected features is that the driver ignores
+them.

I'm not sure this is the best way or whether we've defined the "unexpected features" before. It might be better to say that to accept the features that the driver understands.

Quoted 14 lines

content.tex +9 -0

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

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 148

Quoted 8 lines

content.tex +8 -0

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

Do we need a similar part for the device?

Quoted 14 lines

content.tex +4 -0

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

Quoted 4 lines

content.tex +4 -0

+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

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 176

Quoted 10 lines

content.tex +7 -0

+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

This sentence is incomplete.

Thanks

Quoted 3 lines

--
MST

Quoted 20 lines · Jason Wang

Quoted 19 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 10 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

Quoted 12 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 126

Quoted 8 lines

Likewise, it is important that

content.tex +5 -0

+drivers \em{do not} accept feature bits they do not know how to handle
+(even though devices are not supposed to offer them in the first place,
+according to this version of the specification.) The preferred way for
+handling reserved and unexpected features is that the driver ignores
+them.

I'm not sure this is the best way or whether we've defined the "unexpected features" before. It might be better to say that to accept the features that the driver understands.

So specifically, if feature is documented as pci only and then down the road we enable it for ccw. Does driver "understand" it? Or not? It is clearly unexpected.
I am not too worried about using terms we didn't define this is not part of a conformance statement.
And people seem to get the meaning ;)

Quoted 33 lines · Michael S. Tsirkin

Quoted 20 lines · Jason Wang

Quoted 19 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 10 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

Quoted 3 lines

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

E.g:

For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle.

?

Quoted 13 lines

Quoted 12 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 126

Quoted 8 lines

Likewise, it is important that

content.tex +5 -0

+drivers \em{do not} accept feature bits they do not know how to handle
+(even though devices are not supposed to offer them in the first place,
+according to this version of the specification.) The preferred way for
+handling reserved and unexpected features is that the driver ignores
+them.

I'm not sure this is the best way or whether we've defined the "unexpected features" before. It might be better to say that to accept the features that the driver understands.

Quoted 6 lines

So specifically, if feature is documented as pci only and then down the road we enable it for ccw. Does driver "understand" it? Or not? It is clearly unexpected.
I am not too worried about using terms we didn't define this is not part of a conformance statement.
And people seem to get the meaning ;)

Ok.

Thanks

Quoted 38 lines · Jason Wang

Quoted 33 lines · Michael S. Tsirkin

Quoted 20 lines · Jason Wang

Quoted 19 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 10 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

Quoted 3 lines

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

Quoted 7 lines

E.g:

For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle.

?

Jason I think what you miss here is this part:

"even though drivers are not
supposed to accept them in the first place even if offered, according to this version of the specification"

does not apply to status and virtqueue size.

Let me clarify what all this means.
It seems safe for a device to offer a reserved feature bit since drivers are not supposed to accept it.
This text says device must not rely on this.

How would this apply to status or vq size? I don't see.

Quoted 47 lines · Michael S. Tsirkin

Quoted 33 lines · Jason Wang

Quoted 29 lines · Michael S. Tsirkin

Quoted 19 lines · Jason Wang

Quoted 18 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 9 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

Quoted 3 lines

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

Quoted 7 lines

E.g:

For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle.

?

Jason I think what you miss here is this part:

"even though drivers are not
supposed to accept them in the first place even if offered, according to this version of the specification"

Quoted 5 lines

does not apply to status and virtqueue size.

Let me clarify what all this means.
It seems safe for a device to offer a reserved feature bit

Quoted 1 line

since drivers are not supposed to accept it.

Quoted 3 lines

This text says device must not rely on this.

How would this apply to status or vq size? I don't see.

Me neither... for the status, it's about either the driver noting its progress, or the device indicating that a reset is needed. The only case where setting something requires kind of an ack is FEATURES_OK, and there we already spell out the conditions clearly.

For the queue size,
we specify that the device states what it can support, and that the driver may only reduce it, that seems clear enough to me.

Quoted 49 lines · Cornelia Huck

Quoted 47 lines · Michael S. Tsirkin

Quoted 33 lines · Jason Wang

Quoted 29 lines · Michael S. Tsirkin

Quoted 19 lines · Jason Wang

Quoted 18 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 9 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 121

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

Quoted 3 lines

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

Quoted 7 lines

E.g:

For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle.

?

Jason I think what you miss here is this part:

"even though drivers are not
supposed to accept them in the first place even if offered, according to this version of the specification"

Quoted 5 lines

Quoted 5 lines

does not apply to status and virtqueue size.

Let me clarify what all this means.
It seems safe for a device to offer a reserved feature bit

This depends really on the behaviour of the drivers.

Quoted 1 line

Quoted 1 line

since drivers are not supposed to accept it.

So this is the case of the ADMIN_VQ.

Quoted 7 lines

Quoted 3 lines

This text says device must not rely on this.

How would this apply to status or vq size? I don't see.

Me neither... for the status, it's about either the driver noting its progress, or the device indicating that a reset is needed. The only case where setting something requires kind of an ack is FEATURES_OK, and there we already spell out the conditions clearly.

I basically meant something like:

Assuming we have a feature like VIRTIO_RING_F_NEW and a new status bit was mapped to this feature, VIRTIO_CONFIG_S_NEW. And for some reason this feature is reserved for some transports. Should we mention device does not offer VIRTIO_CONFIG_S_NEW as well, or we assume it is implied that we don't offer VIRTIO_CONFIG_S_NEW in this case?

Quoted 5 lines

For the queue size,
we specify that the device states what it can support, and that the driver may only reduce it, that seems clear enough to me.

Similar to the above, assuming a feature VIRTIO_R_F_MAXSIZE_XXX, and it is reserved. Should we mention that the new max virtqueue size should not be advertised or it is implied in the feature advertisement?

Quoted 74 lines · Jason Wang

Quoted 49 lines · Cornelia Huck

Quoted 47 lines · Michael S. Tsirkin

Quoted 33 lines · Jason Wang

Quoted 29 lines · Michael S. Tsirkin

Quoted 19 lines · Jason Wang

Quoted 18 lines · 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 <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
   content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++
   1 file changed, 42 insertions(+)

content.tex +0 -0

index 3051399..e3203be 100644
@@ -114,21 +114,63 @@ \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.
 

Quoted 9 lines

content.tex \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B · line 121

Quoted 5 lines

content.tex +5 -0

+To keep te 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 them in the first place even if offered, according to
+this version of the specification.)

It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).

Quoted 3 lines

I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.

Quoted 7 lines

E.g:

For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle.

?

Jason I think what you miss here is this part:

"even though drivers are not
supposed to accept them in the first place even if offered, according to this version of the specification"

Quoted 5 lines

Quoted 5 lines

does not apply to status and virtqueue size.

Let me clarify what all this means.
It seems safe for a device to offer a reserved feature bit

This depends really on the behaviour of the drivers.

Quoted 1 line

Quoted 1 line

since drivers are not supposed to accept it.

So this is the case of the ADMIN_VQ.

Quoted 7 lines

Quoted 3 lines

This text says device must not rely on this.

How would this apply to status or vq size? I don't see.

Me neither... for the status, it's about either the driver noting its progress, or the device indicating that a reset is needed. The only case where setting something requires kind of an ack is FEATURES_OK, and there we already spell out the conditions clearly.

I basically meant something like:

Quoted 5 lines

Assuming we have a feature like VIRTIO_RING_F_NEW and a new status bit was mapped to this feature, VIRTIO_CONFIG_S_NEW. And for some reason this feature is reserved for some transports. Should we mention device does not offer VIRTIO_CONFIG_S_NEW as well, or we assume it is implied that we don't offer VIRTIO_CONFIG_S_NEW in this case?

I'm not sure that adding a feature-specific status bit would make sense, given that the status bits either need to work before feature negotiation is complete, or are actually needed for feature negotiation.

Also, the status-bit space is way more limited than the feature-bit space. Therefore, I think we can safely ignore the status bits.

Quoted 1 line

Quoted 7 lines

Quoted 3 lines

For the queue size,
we specify that the device states what it can support, and that the driver may only reduce it, that seems clear enough to me.

Similar to the above, assuming a feature VIRTIO_R_F_MAXSIZE_XXX, and it is reserved. Should we mention that the new max virtqueue size should not be advertised or it is implied in the feature advertisement?

I'd say it's implied in the feature bit handling already.

Each device group has a type. For now, define one initial group:

SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given PCI SR-IOV physical function (PF). This group may contain one or more virtio devices.

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 content.tex |  2 ++
 2 files changed, 47 insertions(+)
 create mode 100644 admin.tex

admin.tex new file +21 -0

new file mode 100644
index 0000000..6ebdd05
@@ -0,0 +1,45 @@
+\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 is the member identifier is interpreted
+       and what kind of control the owner has.
+       A given owner can control a single group of a given type,

admin.tex new file +14 -0

+
+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 0x1 to 0xFFFF
+and equals the SR-IOV VF number of the member device (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}
+
+

content.tex +2 -1

index e3203be..3f3585d 100644
@@ -491,6 +491,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

Quoted 55 lines · Michael S. Tsirkin

Each device group has a type. For now, define one initial group:

SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given PCI SR-IOV physical function (PF). This group may contain one or more virtio devices.

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 content.tex |  2 ++
 2 files changed, 47 insertions(+)
 create mode 100644 admin.tex

admin.tex new file +21 -0

new file mode 100644
index 0000000..6ebdd05
@@ -0,0 +1,45 @@
+\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 is the member identifier is interpreted
+       and what kind of control the owner has.
+       A given owner can control a single group of a given type,

admin.tex line 31

Quoted 10 lines

admin.tex new file +10 -0

+       thus the type and the owner together identify the group.
+\end{description}
+
+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Thanks

Quoted 30 lines

admin.tex new file +14 -0

+
+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 0x1 to 0xFFFF
+and equals the SR-IOV VF number of the member device (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}
+
+

content.tex +2 -1

index e3203be..3f3585d 100644
@@ -491,6 +491,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

Quoted 72 lines · Jason Wang

Quoted 55 lines · Michael S. Tsirkin

Each device group has a type. For now, define one initial group:

SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given PCI SR-IOV physical function (PF). This group may contain one or more virtio devices.

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 content.tex |  2 ++
 2 files changed, 47 insertions(+)
 create mode 100644 admin.tex

admin.tex new file +21 -0

new file mode 100644
index 0000000..6ebdd05
@@ -0,0 +1,45 @@
+\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 is the member identifier is interpreted
+       and what kind of control the owner has.
+       A given owner can control a single group of a given type,

admin.tex line 31

Quoted 10 lines

admin.tex new file +10 -0

+       thus the type and the owner together identify the group.
+\end{description}
+
+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Quoted 25 lines

Quoted 25 lines

admin.tex new file +14 -0

+
+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 0x1 to 0xFFFF
+and equals the SR-IOV VF number of the member device (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}
+
+

content.tex +0 -0

index e3203be..3f3585d 100644
@@ -491,6 +491,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]}.
 

Quoted 8 lines

Quoted 8 lines

content.tex +2 -1

+\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: [email protected] For additional commands, e-mail: [email protected]

Quoted 74 lines · Michael S. Tsirkin

Quoted 72 lines · Jason Wang

Quoted 55 lines · Michael S. Tsirkin

Each device group has a type. For now, define one initial group:

SR-IOV type - PCI SR-IOV virtual functions (VFs) of a given PCI SR-IOV physical function (PF). This group may contain one or more virtio devices.

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 content.tex |  2 ++
 2 files changed, 47 insertions(+)
 create mode 100644 admin.tex

admin.tex new file +21 -0

new file mode 100644
index 0000000..6ebdd05
@@ -0,0 +1,45 @@
+\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 is the member identifier is interpreted
+       and what kind of control the owner has.
+       A given owner can control a single group of a given type,

admin.tex line 31

Quoted 10 lines

admin.tex new file +10 -0

+       thus the type and the owner together identify the group.
+\end{description}
+
+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 4 lines

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

Because it is transport specific. The basic facility should be transport independent.

Quoted 2 lines

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Probably, but can it be modeled exactly as what SR-IOV looks like? Or anyhow, it's not too late to define this for MMIO at that time. For example, we know MSI-X may work for MMIO but we define it only for PCI now.

Thanks

Quoted 39 lines

Quoted 25 lines

Quoted 25 lines

admin.tex new file +14 -0

+
+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 0x1 to 0xFFFF
+and equals the SR-IOV VF number of the member device (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}
+
+

content.tex +0 -0

index e3203be..3f3585d 100644
@@ -491,6 +491,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]}.
 

Quoted 8 lines

Quoted 8 lines

content.tex +2 -1

+\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: [email protected] For additional commands, e-mail: [email protected]

Quoted 17 lines · Jason Wang

Quoted 16 lines · Michael S. Tsirkin

Quoted 14 lines · Jason Wang

admin.tex line 31

Quoted 7 lines · Michael S. Tsirkin

admin.tex new file +7 -0

+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 7 lines

Quoted 4 lines

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

Because it is transport specific. The basic facility should be transport independent.

I tried this but the result is spread all over the spec and does not result in a readable cohesive whole.
So we give up on the transport independent purity a bit and it seems worth it.
Also explained this in the cover letter - have you seen that?

Quoted 8 lines

Quoted 2 lines

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Probably, but can it be modeled exactly as what SR-IOV looks like? Or anyhow, it's not too late to define this for MMIO at that time. For example, we know MSI-X may work for MMIO but we define it only for PCI now.

Quoted 1 line

Thanks

Right. So if we reserve the id for all transports then it will be easy to do.

Quoted 16 lines · Michael S. Tsirkin

Quoted 15 lines · Jason Wang

Quoted 14 lines · Michael S. Tsirkin

Quoted 13 lines · Jason Wang

admin.tex line 31

Quoted 7 lines · Michael S. Tsirkin

admin.tex new file +7 -0

+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

Because it is transport specific. The basic facility should be transport independent.

I tried this but the result is spread all over the spec and does not result in a readable cohesive whole.

Quoted 3 lines

So we give up on the transport independent purity a bit and it seems worth it.
Also explained this in the cover letter - have you seen that?

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Probably, but can it be modeled exactly as what SR-IOV looks like? Or anyhow, it's not too late to define this for MMIO at that time. For example, we know MSI-X may work for MMIO but we define it only for PCI now.

Quoted 3 lines

Quoted 1 line

Thanks

Right. So if we reserve the id for all transports then it will be easy to do.

Also, if we go with transport-specific ids, we might end up with different ids per transport when we add a future transport-independent feature.

The global id space really should be big enough to accommodate
even single-transport groups.

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

Quoted 18 lines · Cornelia Huck

Quoted 16 lines · Michael S. Tsirkin

Quoted 15 lines · Jason Wang

Quoted 14 lines · Michael S. Tsirkin

Quoted 13 lines · Jason Wang

Review

Quoted 7 lines · Michael S. Tsirkin

(quoted diff) +7 -0

+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

Because it is transport specific. The basic facility should be transport independent.

I tried this but the result is spread all over the spec and does not result in a readable cohesive whole.

I may miss something, but it looks to me it's just a subsection in the PCI transport to describe the SR-IOV group identifier.

Quoted 3 lines

Quoted 3 lines

So we give up on the transport independent purity a bit and it seems worth it.
Also explained this in the cover letter - have you seen that?

Sorry, I don't.

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Probably, but can it be modeled exactly as what SR-IOV looks like? Or anyhow, it's not too late to define this for MMIO at that time. For example, we know MSI-X may work for MMIO but we define it only for PCI now.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

Thanks

Right. So if we reserve the id for all transports then it will be easy to do.

Also, if we go with transport-specific ids, we might end up with different ids per transport when we add a future transport-independent feature.

I'm not sure this is a real problem:

1) all the basic facilities are now transport-independent but they are implemented via different transport specific registers/commands/offsets etc.

2) I'm not quite sure there would be a transport-independent group identifier other than the virtqueue transport, so I wonder if it makes sense to split the id space into global ones as well as the transport specific ones

Quoted 4 lines

The global id space really should be big enough to accommodate
even single-transport groups.

Yes, so it's not about the space but about whether or it it's worth/correct/expensive to describe a transport specific identifier in the basic facility part.

Quoted 21 lines · Cornelia Huck

Quoted 19 lines · Michael S. Tsirkin

Quoted 18 lines · Jason Wang

Quoted 17 lines · Michael S. Tsirkin

Quoted 13 lines · Jason Wang

Review

Quoted 7 lines · Michael S. Tsirkin

(quoted diff) +7 -0

+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]}).

So I wonder what's the advantage of using a global identifier over the transport specific one. There's almost no way for CCW/MMIO to use SR-IOV. Limiting it to PCI seems much easier and avoids layer violation.

Quoted 1 line

Thanks

So we burn up an identifier, ccw and mmio won't be able to use it. Big deal? Why?

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

Because it is transport specific. The basic facility should be transport independent.

I tried this but the result is spread all over the spec and does not result in a readable cohesive whole.

I may miss something, but it looks to me it's just a subsection in the PCI transport to describe the SR-IOV group identifier.

Quoted 3 lines

Quoted 3 lines

So we give up on the transport independent purity a bit and it seems worth it.
Also explained this in the cover letter - have you seen that?

Sorry, I don't.

Quoted 10 lines

Quoted 10 lines

Quoted 10 lines

Quoted 5 lines

And I think we might find a use for this with MMIO down the road with some kind of passthrough. Who knows.

Probably, but can it be modeled exactly as what SR-IOV looks like? Or anyhow, it's not too late to define this for MMIO at that time. For example, we know MSI-X may work for MMIO but we define it only for PCI now.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

Thanks

Right. So if we reserve the id for all transports then it will be easy to do.

Also, if we go with transport-specific ids, we might end up with different ids per transport when we add a future transport-independent feature.

I'm not sure this is a real problem:

1) all the basic facilities are now transport-independent but they are implemented via different transport specific registers/commands/offsets etc.
2) I'm not quite sure there would be a transport-independent group identifier other than the virtqueue transport, so I wonder if it makes sense to split the id space into global ones as well as the transport specific ones

Quoted 2 lines

The global id space really should be big enough to accommodate
even single-transport groups.

Yes, so it's not about the space but about whether or it it's worth/correct/expensive to describe a transport specific identifier in the basic facility part.

Thanks

Quoted 6 lines

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644

admin.tex +40 -0

+
+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;
+        u8 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}.
+
+As a rule, 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|}

admin.tex +35 -1

+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline
+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEM    & unsupported or invalid \field{group_member_id} \\
+\hline
+0x05-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
--

MST

Quoted 21 lines · 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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644
@@ -42,4 +42,102 @@ \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}
 

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 50

Quoted 6 lines

admin.tex +5 -0

+\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
 
+Group administration commands can be issued through an owner
+device to control member devices of a group.  This mechanism can
+be used, for example, to configure a member device before it is
+initialized by its driver.

This excludes any possible runtime configuration, is this intended?

Quoted 41 lines

admin.tex +40 -0

+
+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;
+        u8 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}.
+
+As a rule, 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|}

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 100

Quoted 10 lines

admin.tex +10 -0

+\hline
+opcode & Name & Command Description \\
+\hline \hline
+0x0000 - 0x7FFF & - &  Group administration commands    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved    \\
+\hline
+\end{tabular}
+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 108

Quoted 8 lines

admin.tex +8 -0

+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 following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common Linux names and error numbers:

I may miss something but I don't see how it matches Linux error numbers.

Thanks

Quoted 38 lines

admin.tex +35 -1

+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline
+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEM    & unsupported or invalid \field{group_member_id} \\
+\hline
+0x05-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
--

MST

Quoted 22 lines · Jason Wang

Quoted 21 lines · 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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644
@@ -42,4 +42,102 @@ \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}
 

Quoted 8 lines

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 50

Quoted 6 lines

admin.tex +5 -0

+\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
 
+Group administration commands can be issued through an owner
+device to control member devices of a group.  This mechanism can
+be used, for example, to configure a member device before it is
+initialized by its driver.

This excludes any possible runtime configuration, is this intended?

No idea - what kind of runtime configuration are you talking about? This explicitly says "to configure a member device" and that's certainly at runtime.

Quoted 57 lines

Quoted 43 lines

admin.tex +40 -0

+
+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;
+        u8 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}.
+
+As a rule, 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|}

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 100

Quoted 10 lines

admin.tex +10 -0

+\hline
+opcode & Name & Command Description \\
+\hline \hline
+0x0000 - 0x7FFF & - &  Group administration commands    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved    \\
+\hline
+\end{tabular}
+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

Quoted 10 lines

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 108

Quoted 8 lines

admin.tex +8 -0

+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 following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common Linux names and error numbers:

I may miss something but I don't see how it matches Linux error numbers.

include/uapi/asm-generic/errno-base.h:#define EINVAL 22 /* Invalid argument */

Quoted 42 lines

Thanks

Quoted 38 lines

admin.tex +35 -1

+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline
+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEM    & unsupported or invalid \field{group_member_id} \\
+\hline
+0x05-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
--

MST

Quoted 33 lines · Michael S. Tsirkin

Quoted 30 lines · Jason Wang

Quoted 18 lines · 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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 50

Quoted 9 lines

admin.tex +5 -0

@@ -42,4 +42,102 @@ \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}
 
+Group administration commands can be issued through an owner
+device to control member devices of a group.  This mechanism can
+be used, for example, to configure a member device before it is
+initialized by its driver.

This excludes any possible runtime configuration, is this intended?

Quoted 3 lines

No idea - what kind of runtime configuration are you talking about? This explicitly says "to configure a member device" and that's certainly at runtime.

Not sure, but if we say something like we, it means we need a group of commands to support runtime stuff in the future?

Quoted 69 lines

Quoted 67 lines

Quoted 43 lines

admin.tex +40 -0

+
+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;
+        u8 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}.
+
+As a rule, 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|}

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 100

Quoted 10 lines

admin.tex +10 -0

+\hline
+opcode & Name & Command Description \\
+\hline \hline
+0x0000 - 0x7FFF & - &  Group administration commands    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved    \\
+\hline
+\end{tabular}
+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 108

Quoted 8 lines

admin.tex +8 -0

+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 following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common Linux names and error numbers:

I may miss something but I don't see how it matches Linux error numbers.

Quoted 1 line

include/uapi/asm-generic/errno-base.h:#define EINVAL 22 /* Invalid argument */

Ok, only this one was matched but any advantages of menting the matching? I believe not all the error numbers are meaningful for a device interface (e.g EINTR).

Thanks

Quoted 43 lines

Quoted 42 lines

Thanks

Quoted 38 lines

admin.tex +35 -1

+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline
+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEM    & unsupported or invalid \field{group_member_id} \\
+\hline
+0x05-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
--

MST

Quoted 30 lines · Jason Wang

Quoted 29 lines · Michael S. Tsirkin

Quoted 28 lines · Jason Wang

Quoted 17 lines · 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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 50

Quoted 9 lines

admin.tex +5 -0

@@ -42,4 +42,102 @@ \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}
 
+Group administration commands can be issued through an owner
+device to control member devices of a group.  This mechanism can
+be used, for example, to configure a member device before it is
+initialized by its driver.

This excludes any possible runtime configuration, is this intended?

Quoted 5 lines

Quoted 3 lines

No idea - what kind of runtime configuration are you talking about? This explicitly says "to configure a member device" and that's certainly at runtime.

Not sure, but if we say something like we, it means we need a group of commands to support runtime stuff in the future?

we'll need commands to provision members - is this what you mean? for sure.

Quoted 66 lines

Quoted 66 lines

Quoted 55 lines

Quoted 42 lines

admin.tex +40 -0

+
+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;
+        u8 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}.
+
+As a rule, 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|}

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 100

Quoted 10 lines

admin.tex +10 -0

+\hline
+opcode & Name & Command Description \\
+\hline \hline
+0x0000 - 0x7FFF & - &  Group administration commands    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved    \\
+\hline
+\end{tabular}
+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

Quoted 9 lines

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 108

Quoted 8 lines

admin.tex +8 -0

+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 following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common Linux names and error numbers:

I may miss something but I don't see how it matches Linux error numbers.

include/uapi/asm-generic/errno-base.h:#define EINVAL 22 /* Invalid argument */

Quoted 5 lines

Ok, only this one was matched but any advantages of menting the matching? I believe not all the error numbers are meaningful for a device interface (e.g EINTR).

Thanks

Makes it easier for driver to decide on an error to report to userspace. If we don't use all error codes that's fine.

Quoted 37 lines

Quoted 37 lines

Quoted 37 lines

Thanks

Quoted 30 lines

admin.tex +30 -0

+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\
+\hline
+0x02   & VIRTIO_ADMIN_STATUS_Q_INVALID_FIELD    & unsupported or invalid field within \field{command_specific_data}  \\
+\hline
+0x03   & VIRTIO_ADMIN_STATUS_Q_INVALID_GROUP    & unsupported or invalid \field{group_type} \\
+\hline

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

admin.tex +5 -1

+0x04   & VIRTIO_ADMIN_STATUS_Q_INVALID_MEM    & unsupported or invalid \field{group_member_id} \\
+\hline
+0x05-0xFFFF   & -    & reserved for future use \\
+\hline
+\end{tabular}
--

MST

Quoted 44 lines · Michael S. Tsirkin

admin.tex +44 -0

+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;
+        u8 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}.
+
+As a rule, 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 & - &  Group administration commands    \\
+\hline

Review

Quoted 5 lines

+0x8000 - 0xFFFF & - & Reserved \\
+\hline
+\end{tabular}

(quoted diff) +2 -0

+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

admin.tex \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g · line 104

Quoted 5 lines

admin.tex +4 -0

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

Quoted 44 lines · Jason Wang

Quoted 43 lines · Michael S. Tsirkin

admin.tex +43 -0

+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;
+        u8 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}.
+
+As a rule, 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 & - &  Group administration commands    \\

Quoted 8 lines

Review

Quoted 6 lines

+\hline
+0x8000 - 0xFFFF & - & Reserved \\
+\hline
+\end{tabular}

(quoted diff) +2 -0

+
+The \field{group_type} specifies the group type identifier.

So this means there's no way for having a group specific adminq? E.g one queue for SR-IOV and another for SIOV.

I don't see anything that prevents the device from exposing two admin queues... do you mean that we should provide a mechanism for the device to mandate that only commands for a certain group id can go on a certain admin queue?

Quoted 5 lines

Quoted 5 lines

admin.tex +4 -0

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

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  admin.tex | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 98 insertions(+)

admin.tex +0 -0

index 6ebdd05..1dca136 100644

admin.tex +83 -0

+
+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;
+        u8 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}.
+
+As a rule, 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 & - &  Group administration commands    \\
+\hline
+0x8000 - 0xFFFF & - & Reserved    \\
+\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 following table describes possible \field{status} values;
+to simplify common implementations, they are intentionally
+matching common Linux names and error numbers:
+
+\begin{tabular}{|l|l|l|}
+\hline
+Status (decimal) & Name & Description \\
+\hline \hline
+00   & VIRTIO_ADMIN_STATUS_OK    & successful completion  \\
+\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_qialifier}
+is reserved and set to zero by the device.
+
+When \field{status} is VIRTIO_ADMIN_STATUS_EINVAL,
+the following table describes possible \field{status_qialifier} values:
+\begin{tabular}{|l|l|l|}
+\hline
+Status & Name & Description \\
+\hline \hline
+0x00   & VIRTIO_ADMIN_STATUS_Q_INVALID_COMMAND   & command error: no additional information  \\
+\hline
+0x01   & VIRTIO_ADMIN_STATUS_Q_INVALID_OPCODE    & unsupported or invalid \field{opcode}  \\

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

Currently virtio specification defines control virtqueue to manipulate features and configuration of the device it operates on. However, control virtqueue commands are device type specific, which makes it very difficult to extend for device agnostic commands.

To support this requirement in a more generic way, this patch introduces a new admin virtqueue interface.

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

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  admin.tex   | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
  content.tex |  7 +++++--
  2 files changed, 59 insertions(+), 2 deletions(-)

admin.tex +25 -0

index 1dca136..eeccd2e 100644
@@ -141,3 +141,57 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
  0x05-0xFFFF   & -    & reserved for future use \\
  \hline
  \end{tabular}
+
+\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 submits commands by queueing them to an arbitrary
+administration virtqueue, and they are processed by the
+device in the order in which they are queued. It is the
+responsibility of the driver to ensure ordering for commands
+placed on different administration virtqueues, because they will
+be executed with no order constraints.
+
+Administration virtqueues are used as follows:
+\begin{itemize}
+\item The driver submits the command using the \field{struct virtio_admin_cmd}
+structure using two buffers: a device-readable one followed by a
+device-writable one.
+\item the device-readable buffer includes fields from \field{opcode}

admin.tex +19 -0

+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 assumed to be
+set to zero.
+
+Similarly, it is legal for the device to use, for a specific
+command, a shorter part of the \field{command_specific_result}
+buffer than the length of the structure described in this
+specification. The driver silently ignores any data falling outside
+the used buffer length reported by the device.  Any missing
+fields are assumed to be set to zero.
+
+This simplifies driver implementations since the driver can
+simply maintain a single large 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 using the full structure without concern
+for versioning.

content.tex +2 -2

index 3f3585d..04235da 100644
@@ -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}

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 Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

content.tex +0 -0

index 04235da..aa68926 100644

content.tex +30 -1

 };
 \end{lstlisting}
 

@@ -1031,6 +1035,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}) inclusive serve as administration
+       virtqueues.
+       Thus the number of administration virtqueues equals
+       (\field{admin_queue_num} + 1).
+       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}

@@ -1117,6 +1134,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} 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

@@ -6992,6 +7017,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

Quoted 19 lines · 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 Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

content.tex +0 -0

index 04235da..aa68926 100644

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 952

Quoted 7 lines

content.tex +4 -0

@@ -946,6 +946,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 */

If we are adding more queue related functionality, it would be placed under the above two fields which seems not elegant. I wonder if a new structure is needed here.

Thanks

Quoted 57 lines

content.tex +30 -1

 };
 \end{lstlisting}
 

@@ -1031,6 +1035,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}) inclusive serve as administration
+       virtqueues.
+       Thus the number of administration virtqueues equals
+       (\field{admin_queue_num} + 1).
+       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}

@@ -1117,6 +1134,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} 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

@@ -6992,6 +7017,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

Quoted 32 lines · Jason Wang

Quoted 19 lines · 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 Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

content.tex +0 -0

index 04235da..aa68926 100644

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 952

Quoted 7 lines

content.tex +4 -0

@@ -946,6 +946,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 */

If we are adding more queue related functionality, it would be placed under the above two fields which seems not elegant. I wonder if a new structure is needed here.

Quoted 1 line

Thanks

I don't expect more, if that happens we can always refactor it.

Generally this is not C, no one compiles this directly. So we do not need to add structures unless they are referred to in spec text.

Quoted 57 lines

Quoted 57 lines

content.tex +30 -1

 };
 \end{lstlisting}
 

@@ -1031,6 +1035,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}) inclusive serve as administration
+       virtqueues.
+       Thus the number of administration virtqueues equals
+       (\field{admin_queue_num} + 1).
+       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}

@@ -1117,6 +1134,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} 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

@@ -6992,6 +7017,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

Quoted 34 lines · Michael S. Tsirkin

Quoted 32 lines · Jason Wang

Quoted 19 lines · 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 Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 content.tex | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

content.tex +0 -0

index 04235da..aa68926 100644

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 952

Quoted 7 lines

content.tex +4 -0

@@ -946,6 +946,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 */

If we are adding more queue related functionality, it would be placed under the above two fields which seems not elegant. I wonder if a new structure is needed here.

Quoted 3 lines

Quoted 1 line

Thanks

I don't expect more, if that happens we can always refactor it.

I may miss something but it should be a part of ABI. How could we refactor it without breaking ABI?

Thanks

Quoted 62 lines

Generally this is not C, no one compiles this directly. So we do not need to add structures unless they are referred to in spec text.

Quoted 57 lines

Quoted 57 lines

content.tex +30 -1

 };
 \end{lstlisting}
 

@@ -1031,6 +1035,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}) inclusive serve as administration
+       virtqueues.
+       Thus the number of administration virtqueues equals
+       (\field{admin_queue_num} + 1).
+       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}

@@ -1117,6 +1134,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} 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

@@ -6992,6 +7017,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

Quoted 32 lines · Jason Wang

Quoted 31 lines · Michael S. Tsirkin

Quoted 30 lines · Jason Wang

Quoted 18 lines · 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 Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  content.tex | 34 ++++++++++++++++++++++++++++++++++
  1 file changed, 34 insertions(+)

content.tex +0 -0

index 04235da..aa68926 100644

content.tex \subsubsection{Common configuration structure layout}\label{sec:Virtio Transport · line 952

Quoted 7 lines

content.tex +4 -0

@@ -946,6 +946,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 */

If we are adding more queue related functionality, it would be placed under the above two fields which seems not elegant. I wonder if a new structure is needed here.

Quoted 6 lines

Quoted 2 lines

Quoted 1 line

Thanks

I don't expect more, if that happens we can always refactor it.

I may miss something but it should be a part of ABI. How could we refactor it without breaking ABI?

Thanks

          /* About the administration virtqueue. */

(quoted diff) +4 -2

-        le16 admin_queue_index;         /* read-only for driver */
-        le16 admin_queue_num;         /* read-only for driver */
+        struct { /* read-only for driver */
+        	le16 index;
+        	le16 num;
+	 } admin_queue;

does not affect the ABI. But the order of things makes it less readable.

We would also have to change references to admin_queue_index with admin_queue.index and explain the "." notation in introduction.

All doable but this patchset is large enough as it is.

Quoted 64 lines

Quoted 64 lines

Generally this is not C, no one compiles this directly. So we do not need to add structures unless they are referred to in spec text.

Quoted 57 lines

Quoted 57 lines

content.tex +30 -1

  };
  \end{lstlisting}
 

@@ -1031,6 +1035,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}) inclusive serve as administration
+       virtqueues.
+       Thus the number of administration virtqueues equals
+       (\field{admin_queue_num} + 1).
+       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}

@@ -1117,6 +1134,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} 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

@@ -6992,6 +7017,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

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 <[email protected]>

---
 content.tex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

content.tex +0 -0

index aa68926..023f9b9 100644

content.tex +5 -1

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

Quoted 18 lines · 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 <[email protected]>

---
 content.tex | 12 ++++++++++++
 1 file changed, 12 insertions(+)

content.tex +0 -0

index aa68926..023f9b9 100644

content.tex \subsection{Legacy interface}\label{sec:Virtio Transport Options / Virtio Over M · line 2371

Quoted 10 lines

content.tex +7 -0

@@ -2362,6 +2362,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

Do we need to add VIRTIO_F_SR_IOV here? And probably SR-IOV group type?

Thanks

Quoted 11 lines

content.tex +5 -1

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

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.

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +18 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \hline
 opcode & Name & Command Description \\
 \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
 \hline
 0x8000 - 0xFFFF & - & Reserved    \\
 \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \hline
 \end{tabular}
 
+Before sending any 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 will use 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:
+

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex +15 -1

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
 \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

Quoted 5 lines · 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.

Quoted 3 lines

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

This seems to complicate the migration (compatibility) of the adminq itself. We need check both features and command list.

More below.

Quoted 47 lines

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +24 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \hline
 opcode & Name & Command Description \\
 \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
 \hline
 0x8000 - 0xFFFF & - & Reserved    \\
 \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
 \hline
 \end{tabular}
 
+Before sending any 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 will use 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
+       le32 device_admin_cmds[];
+};
+\end{lstlisting}

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti

Quoted 4 lines

admin.tex +4 -0

+
+This structure is an array of 32 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported.

Is this simpler if we map group type instead of a single opcode? E.g virtqueue transport will introduce 10+ commands, using a bit for a specific opcode will complicate things.

Quoted 29 lines

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti · line 209

Quoted 10 lines

admin.tex +10 -0

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

Any reason we need to invent a negotiation process here? (This seems only useful when some commands/opcodes are mutually exclusive)

Thanks

Quoted 21 lines

admin.tex +15 -1

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
 \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

Quoted 10 lines · Jason Wang

Quoted 7 lines · 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.

This seems to complicate the migration (compatibility) of the adminq itself. We need check both features and command list.

Yes. There's no way around this though - commands need to be per group and a pci device can host at least both SIOV and SRIOV children.

Quoted 47 lines

More below.

Quoted 45 lines

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +23 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
  \hline
  opcode & Name & Command Description \\
  \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
  \hline
  0x8000 - 0xFFFF & - & Reserved    \\
  \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
  \hline
  \end{tabular}
 
+Before sending any 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 will use 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
+       le32 device_admin_cmds[];
+};

Quoted 8 lines

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti

Quoted 5 lines

admin.tex +5 -0

+\end{lstlisting}
+
+This structure is an array of 32 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported.

Is this simpler if we map group type instead of a single opcode? E.g virtqueue transport will introduce 10+ commands, using a bit for a specific opcode will complicate things.

I think more flexibility is better, and bits are cheap.

Quoted 44 lines

Quoted 31 lines

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti · line 209

Quoted 10 lines

admin.tex +10 -0

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

Any reason we need to invent a negotiation process here? (This seems only useful when some commands/opcodes are mutually exclusive)

Quoted 1 line

Thanks

yes, features don't help because they are per owner. this is per group.

imagine vq transport. SET_FEATURES commands sets the features for the child. we need something to negotiate with owner but still within the vq.

Quoted 19 lines

Quoted 19 lines

admin.tex +15 -0

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
  \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 

Quoted 4 lines

Quoted 4 lines

An administration virtqueue of an owner device is used to submit
--
MST

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: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
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/

Quoted 71 lines · Michael S. Tsirkin

Quoted 10 lines · Jason Wang

Quoted 7 lines · 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.

This seems to complicate the migration (compatibility) of the adminq itself. We need check both features and command list.

Yes. There's no way around this though - commands need to be per group and a pci device can host at least both SIOV and SRIOV children.

Quoted 47 lines

More below.

Quoted 45 lines

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
  admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +23 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
  \hline
  opcode & Name & Command Description \\
  \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
  \hline
  0x8000 - 0xFFFF & - & Reserved    \\
  \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
  \hline
  \end{tabular}
 
+Before sending any 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 will use 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
+       le32 device_admin_cmds[];
+};

Quoted 8 lines

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti

Quoted 5 lines

admin.tex +5 -0

+\end{lstlisting}
+
+This structure is an array of 32 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported.

Is this simpler if we map group type instead of a single opcode? E.g virtqueue transport will introduce 10+ commands, using a bit for a specific opcode will complicate things.

Quoted 1 line

I think more flexibility is better, and bits are cheap.

So we may end up with something like:

If VIRTIO_F_TRANSPORT_VQ is supported, device MUST return the following opcodes via VIRTIO_ADMIN_CMD_LIST_QUERY

[opX, opY, ....]

Probably fine.

Quoted 45 lines

Quoted 45 lines

Quoted 32 lines

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti · line 209

Quoted 10 lines

admin.tex +10 -0

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

Any reason we need to invent a negotiation process here? (This seems only useful when some commands/opcodes are mutually exclusive)

Quoted 7 lines

Quoted 1 line

Thanks

yes, features don't help because they are per owner. this is per group.

imagine vq transport. SET_FEATURES commands sets the features for the child. we need something to negotiate with owner but still within the vq.

Should we document what happens if driver use a opcode that it doesn't negotiate? (It looks to me we don't document this for at leave virtio-net control virtqueue).

Thanks

Quoted 40 lines

Quoted 19 lines

Quoted 19 lines

admin.tex +15 -0

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
  \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 

Quoted 4 lines

Quoted 4 lines

An administration virtqueue of an owner device is used to submit
--
MST

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: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
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/

Quoted 76 lines · Jason Wang

Quoted 71 lines · Michael S. Tsirkin

Quoted 10 lines · Jason Wang

Quoted 7 lines · 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.

This seems to complicate the migration (compatibility) of the adminq itself. We need check both features and command list.

Yes. There's no way around this though - commands need to be per group and a pci device can host at least both SIOV and SRIOV children.

Quoted 47 lines

More below.

Quoted 45 lines

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
   admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
   1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +23 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
   \hline
   opcode & Name & Command Description \\
   \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
   \hline
   0x8000 - 0xFFFF & - & Reserved    \\
   \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
   \hline
   \end{tabular}
 
+Before sending any 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 will use 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
+       le32 device_admin_cmds[];
+};

Quoted 8 lines

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti

Quoted 5 lines

admin.tex +5 -0

+\end{lstlisting}
+
+This structure is an array of 32 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported.

Is this simpler if we map group type instead of a single opcode? E.g virtqueue transport will introduce 10+ commands, using a bit for a specific opcode will complicate things.

Quoted 1 line

I think more flexibility is better, and bits are cheap.

So we may end up with something like:

Quoted 7 lines

If VIRTIO_F_TRANSPORT_VQ is supported, device MUST return the following opcodes via VIRTIO_ADMIN_CMD_LIST_QUERY

[opX, opY, ....]

Probably fine.

I think I would tie the commands to a group type.

Quoted 52 lines

Quoted 44 lines

Quoted 44 lines

Quoted 31 lines

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti · line 209

Quoted 10 lines

admin.tex +10 -0

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

Any reason we need to invent a negotiation process here? (This seems only useful when some commands/opcodes are mutually exclusive)

Quoted 7 lines

Quoted 1 line

Thanks

yes, features don't help because they are per owner. this is per group.

imagine vq transport. SET_FEATURES commands sets the features for the child. we need something to negotiate with owner but still within the vq.

Quoted 6 lines

Should we document what happens if driver use a opcode that it doesn't negotiate? (It looks to me we don't document this for at leave virtio-net control virtqueue).

Thanks

documented in the conformance section.

Quoted 38 lines

Quoted 38 lines

Quoted 18 lines

Quoted 18 lines

admin.tex +15 -0

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 

Quoted 4 lines

Quoted 4 lines

An administration virtqueue of an owner device is used to submit
--
MST

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: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
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/

Quoted 84 lines · Michael S. Tsirkin

Quoted 83 lines · Jason Wang

Quoted 71 lines · Michael S. Tsirkin

Quoted 10 lines · Jason Wang

Quoted 7 lines · 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.

This seems to complicate the migration (compatibility) of the adminq itself. We need check both features and command list.

Yes. There's no way around this though - commands need to be per group and a pci device can host at least both SIOV and SRIOV children.

Quoted 47 lines

More below.

Quoted 45 lines

Signed-off-by Max Gurtovoy <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
   admin.tex | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
   1 file changed, 81 insertions(+), 1 deletion(-)

admin.tex +23 -1

index eeccd2e..eec12a9 100644
@@ -91,7 +91,9 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
   \hline
   opcode & Name & Command Description \\
   \hline \hline
-0x0000 - 0x7FFF & - &  Group administration commands    \\
+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 & - &  Group administration commands    \\
   \hline
   0x8000 - 0xFFFF & - & Reserved    \\
   \hline

@@ -142,6 +144,84 @@ \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti
   \hline
   \end{tabular}
 
+Before sending any 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 will use 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
+       le32 device_admin_cmds[];
+};

Quoted 8 lines

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti

Quoted 5 lines

admin.tex +5 -0

+\end{lstlisting}
+
+This structure is an array of 32 bit values in little-endian byte
+order, in which a bit is set if the specific command opcode
+is supported.

Is this simpler if we map group type instead of a single opcode? E.g virtqueue transport will introduce 10+ commands, using a bit for a specific opcode will complicate things.

Quoted 1 line

I think more flexibility is better, and bits are cheap.

So we may end up with something like:

If VIRTIO_F_TRANSPORT_VQ is supported, device MUST return the following opcodes via VIRTIO_ADMIN_CMD_LIST_QUERY

[opX, opY, ....]

Probably fine.

Quoted 1 line

I think I would tie the commands to a group type.

Right, that's what I understood, but it looks to me it can't be achieved via LIST_QUERY now (which is based on opcodes).

Thanks

Quoted 99 lines

Quoted 52 lines

Quoted 44 lines

Quoted 44 lines

Quoted 31 lines

admin.tex +29 -0

+Thus, \field{device_admin_cmds[0]} refers to the first 32-bit value
+in this array corresponding to opcodes 0 to 31,
+\field{device_admin_cmds[1]} is the second 32-bit value
+corresponding to opcodes 32 to 63, etc.
+For example, the array of size 2 including
+the values 0x3 in \field{device_admin_cmds[0]}
+and 0x1 in \field{device_admin_cmds[1]} indicates that only opcodes 0, 1 and 32
+are supported.
+
+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_cmds[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 administration commands supported for the given group.
+
+
+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 administration commands used by the driver.

admin.tex \subsection{Group administration commands}\label{sec:Basic Facilities of a Virti · line 209

Quoted 10 lines

admin.tex +10 -0

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

Any reason we need to invent a negotiation process here? (This seems only useful when some commands/opcodes are mutually exclusive)

Quoted 7 lines

Quoted 1 line

Thanks

yes, features don't help because they are per owner. this is per group.

imagine vq transport. SET_FEATURES commands sets the features for the child. we need something to negotiate with owner but still within the vq.

Quoted 6 lines

Should we document what happens if driver use a opcode that it doesn't negotiate? (It looks to me we don't document this for at leave virtio-net control virtqueue).

Thanks

documented in the conformance section.

Quoted 38 lines

Quoted 38 lines

Quoted 18 lines

Quoted 18 lines

admin.tex +15 -0

+
+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, the device
+completes the command with status
+VIRTIO_ADMIN_STATUS_INVALID_FIELD.
+
+Note: drivers are assumed not to set bits in device_admin_cmds
+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.
+
   \section{Administration Virtqueues}\label{sec:Basic Facilities of a Virtio Device / Administration Virtqueues}
 

Quoted 4 lines

Quoted 4 lines

An administration virtqueue of an owner device is used to submit
--
MST

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: [email protected]
Unsubscribe: [email protected]
List help: [email protected]
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/