Re: [virtio] [PATCH v10 03/10] admin: introduce group administration commands

[Jiri Pirko <jiri@nvidia.com>]

Thu, Mar 02, 2023 at 02:05:02PM CET, mst@redhat.com wrote:
>This introduces a general structure for group administration commands,
>used to control device groups through their owner.
>
>Following patches will introduce specific commands and an interface for
>submitting these commands to the owner.
>
>Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>---
> admin.tex        | 108 +++++++++++++++++++++++++++++++++++++++++++++++
> introduction.tex |   3 ++
> 2 files changed, 111 insertions(+)
>
>diff --git a/admin.tex b/admin.tex
>index 3dc47be..7e28b77 100644
>--- a/admin.tex
>+++ b/admin.tex
>@@ -46,4 +46,112 @@ \section{Device groups}\label{sec:Basic Facilities of a Virtio Device / Device g
> PCI transport (see \ref{sec:Virtio Transport Options / Virtio Over PCI Bus}).
> \end{description}
> 
>+\subsection{Group administration commands}\label{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands}
> 
>+The driver sends group administration commands to the owner device of
>+a group to control member devices of the group.
>+This mechanism can
>+be used, for example, to configure a member device before it is
>+initialized by its driver.
>+\footnote{The term "administration" is intended to be interpreted
>+widely to include any kind of control. See specific commands
>+for detail.}
>+
>+All the group administration commands are of the following form:
>+
>+\begin{lstlisting}
>+struct virtio_admin_cmd {
>+        /* Device-readable part */
>+        le16 opcode;
>+        /*
>+         * 1 - SR-IOV
>+         * 2 - 65535 reserved
>+         */
>+        le16 group_type;
>+        /* unused, reserved for future extensions */
>+        u8 reserved1[12];
>+        le64 group_member_id;

As I wrote in the reply to patch 4, I think this struct should be
generic and things like this (group_*) should be carried in
command_specific_data[];


>+        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}.
>+
>+Generally, any unused device-readable fields are set to zero by the driver
>+and ignored by the device.  Any unused device-writeable fields are set to zero
>+by the device and ignored by the driver.
>+
>+\field{opcode} specifies the command. The valid
>+values for \field{opcode} can be found in the following table:
>+
>+\begin{tabular}{|l|l|}
>+\hline
>+opcode & Name & Command Description \\
>+\hline \hline
>+0x0000 - 0x7FFF & - &  Group administration commands    \\

Isn't this inconsistent with opcode comment which describes values
in "struct virtio_admin_cmd"?


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

[...]