virtio-dev.lists.oasis-open.org archive mirror
 help / color / mirror / Atom feed
From: Parav Pandit <parav@nvidia.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	virtio-comment@lists.oasis-open.org,
	virtio-dev@lists.oasis-open.org, jasowang@redhat.com,
	cohuck@redhat.com, sgarzare@redhat.com, stefanha@redhat.com,
	nrupal.jani@intel.com, Piotr.Uminski@intel.com,
	hang.yuan@intel.com
Cc: virtio@lists.oasis-open.org, Jiri Pirko <jiri@nvidia.com>,
	Zhu Lingshan <lingshan.zhu@intel.com>,
	pasic@linux.ibm.com, Shahaf Shuler <shahafs@nvidia.com>,
	Max Gurtovoy <mgurtovoy@nvidia.com>
Subject: [virtio-dev] Re: [PATCH v12 03/10] admin: introduce group administration commands
Date: Mon, 24 Apr 2023 18:07:12 -0400	[thread overview]
Message-ID: <e9256f17-f9ab-de6d-0377-d9fc210a7cc0@nvidia.com> (raw)
In-Reply-To: <79c27bca6f72e4ea5d1d0e3a6b1a54369b03fb30.1682354275.git.mst@redhat.com>



On 4/24/2023 12:44 PM, Michael S. Tsirkin wrote:
> This introduces a general structure for group administration commands,
> used to control device groups through their owner.
> 
> Following patches will introduce specific commands and an interface for
> submitting these commands to the owner.
> 
> Note that the commands are focused on controlling device groups:
> this is why group related fields are in the generic part of
> the structure.

Below paragraph is no longer applies as we already discussed more use 
cases of the AQ such as accessing the PCI VF's legacy config space 
registers. Please drop below paragraph.

> Without this the admin vq would become a "whatever" vq which does not do
> anything specific at all, just a general transport like thing.
> I feel going this way opens the design space to the point where
> we no longer know what belongs in e.g. config space
> what in the control q and what in the admin q.
> As it is, whatever deals with groups is in the admin q; other
> things not in the admin q.
> 

A PF can use same or different AQ with a new struct 
virtio_legacy_register_access with a different opcode range.

AQ doesn't replace the control vq, so that part description is fine.

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

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

Other than commit log change,
Reviewed-by: Parav Pandit <parav@nvidia.com>

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


  parent reply	other threads:[~2023-04-24 22:07 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-24 16:44 [virtio-dev] [PATCH v12 00/10] Introduce device group and device management Michael S. Tsirkin
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 01/10] virtio: document forward compatibility guarantees Michael S. Tsirkin
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 02/10] admin: introduce device group and related concepts Michael S. Tsirkin
2023-04-24 21:47   ` [virtio-dev] " Parav Pandit
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 03/10] admin: introduce group administration commands Michael S. Tsirkin
     [not found]   ` <d9b0454c-747a-4ac9-b00d-005e066fa3e4@nvidia.com>
2023-04-24 21:33     ` [virtio-dev] " Michael S. Tsirkin
2023-04-24 22:07   ` Parav Pandit [this message]
2023-04-25  6:20     ` Michael S. Tsirkin
2023-04-25 13:25       ` [virtio-dev] " Parav Pandit
2023-04-25 13:31         ` [virtio-dev] " Michael S. Tsirkin
2023-04-25 13:38           ` [virtio-dev] " Parav Pandit
2023-04-25 20:04             ` [virtio-dev] " Michael S. Tsirkin
2023-04-25 20:18               ` [virtio-dev] " Parav Pandit
2023-04-25 20:55                 ` [virtio-dev] " Michael S. Tsirkin
2023-04-26 18:18                   ` [virtio-dev] RE: [virtio-comment] " Parav Pandit
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 04/10] admin: introduce virtio admin virtqueues Michael S. Tsirkin
2023-04-24 22:32   ` [virtio-dev] " Parav Pandit
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 05/10] pci: add admin vq registers to virtio over pci Michael S. Tsirkin
     [not found]   ` <5858e2e6-0b50-c155-85e9-eea6dfb533e1@nvidia.com>
2023-04-24 22:14     ` [virtio-dev] " Parav Pandit
     [not found]       ` <5cb79b37-a066-283c-eee7-afd26146d988@nvidia.com>
2023-04-26 22:11         ` [virtio-dev] " Parav Pandit
     [not found]           ` <b7addd39-d913-700f-ac67-1412825d580b@nvidia.com>
2023-04-27  0:11             ` Parav Pandit
2023-05-05 15:12               ` [virtio-dev] " Michael S. Tsirkin
2023-05-05 15:14                 ` [virtio-dev] " Parav Pandit
2023-04-27 17:57             ` [virtio-dev] " Michael S. Tsirkin
2023-05-05 15:22           ` Michael S. Tsirkin
2023-05-05 15:25             ` [virtio-dev] " Parav Pandit
2023-04-24 22:29   ` [virtio-dev] " Parav Pandit
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 06/10] mmio: document ADMIN_VQ as reserved Michael S. Tsirkin
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 07/10] ccw: " Michael S. Tsirkin
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 08/10] admin: command list discovery Michael S. Tsirkin
2023-04-24 22:27   ` [virtio-dev] " Parav Pandit
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 09/10] admin: conformance clauses Michael S. Tsirkin
2023-04-24 16:44 ` [virtio-dev] [PATCH v12 10/10] ccw: document more reserved features Michael S. Tsirkin
2023-04-24 22:17   ` [virtio-dev] " Parav Pandit
2023-04-24 21:34 ` [virtio-dev] Re: [PATCH v12 00/10] Introduce device group and device management Parav Pandit
2023-05-02  7:51 ` [virtio-dev] Re: [virtio] " David Edmondson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e9256f17-f9ab-de6d-0377-d9fc210a7cc0@nvidia.com \
    --to=parav@nvidia.com \
    --cc=Piotr.Uminski@intel.com \
    --cc=cohuck@redhat.com \
    --cc=hang.yuan@intel.com \
    --cc=jasowang@redhat.com \
    --cc=jiri@nvidia.com \
    --cc=lingshan.zhu@intel.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=mst@redhat.com \
    --cc=nrupal.jani@intel.com \
    --cc=pasic@linux.ibm.com \
    --cc=sgarzare@redhat.com \
    --cc=shahafs@nvidia.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=virtio@lists.oasis-open.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).