From: Max Gurtovoy <mgurtovoy@nvidia.com>
To: Jason Wang <jasowang@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
Cc: virtio-comment@lists.oasis-open.org, cohuck@redhat.com,
stefanha@redhat.com, oren@nvidia.com, parav@nvidia.com,
shahafs@nvidia.com, eperezma@redhat.com, aadam@redhat.com,
bodong@nvidia.com, amikheev@nvidia.com
Subject: Re: [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification
Date: Mon, 2 Aug 2021 12:51:30 +0300 [thread overview]
Message-ID: <29acb062-e541-3359-cbdb-b777249c222c@nvidia.com> (raw)
In-Reply-To: <6de5eb17-1d0d-45d5-b1fa-e21989341b4f@redhat.com>
On 8/2/2021 5:25 AM, Jason Wang wrote:
>
> 在 2021/7/28 下午8:48, Michael S. Tsirkin 写道:
>> On Mon, Jul 26, 2021 at 07:52:53PM +0300, Max Gurtovoy wrote:
>>> Admin virtqueues will be used to send administrative commands to
>>> manipulate various features of the device which would not easily map
>>> into the configuration space.
>>>
>>> The same Admin command format will be used for all virtio devices. The
>>> Admin command set will include 4 types of command classes:
>>> 1. The generic common class
>>> 2. The transport specific class
>>> 3. The device specific class
>>> 4. The vendor specific class
>>>
>>> The above mechanism will enable adding various features to the virtio
>>> specification, e.g.:
>>> 1. Format virtio-blk devices in various configurations (512B block
>>> size,
>>> 512B + 8B T10-DIF, 4K block size, 4k + 8B T10-DIF, etc..).
>>> 2. Live migration management.
>>> 3. Encrypt/Decrypt descriptors.
>>> 4. Virtualization management.
>>> 5. Get device error logs.
>>> 6. Implement advanced vendor/device/transport specific features.
>>> 7. Run device health test.
>>> 8. More.
>>>
>>> As virtio evolves beyond the para-virt/sw-emulated world, it's
>>> mandatory
>>> for the specification to become flexible and allow a wider feature set.
>>> The corrent ctrl virtq that is defined for some of the virtio
>>> devices is
>>> device specific and wasn't designed to be a generic virtq for
>>> admininistration.
>>>
>>> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> Lots of things on this list seem to make sense when
>> done from PF and affecting a VF.
>> I think from this POV the generic structure should include
>> a way to address one device from another.
>>
>>
>>> ---
>>> admin-virtq.tex | 241
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> content.tex | 4 +
>>> 2 files changed, 245 insertions(+)
>>> create mode 100644 admin-virtq.tex
>>>
>>> diff --git a/admin-virtq.tex b/admin-virtq.tex
>>> new file mode 100644
>>> index 0000000..ccec2ca
>>> --- /dev/null
>>> +++ b/admin-virtq.tex
>>> @@ -0,0 +1,241 @@
>>> +\section{Admin Virtqueues}\label{sec:Basic Facilities of a Virtio
>>> Device / Admin Virtqueues}
>>> +
>>> +Admin virtqueues are used to send administrative commands to
>>> manipulate
>>> +various features of the device which would not easily map into the
>>> +configuration space.
>>> +
>>> +Use of Admin virtqueues is negotiated by the VIRTIO_F_ADMIN_VQ
>>> +feature bit.
>>> +
>>> +Admin virtqueue index may vary among different device types.
>>> +
>>> +All commands are of the following form:
>>> +
>>> +\begin{lstlisting}
>>> +struct virtio_admin_cmd {
>>> + /* Device-readable part */
>>> + u8 class;
>>> + u8 command;
>>> + u8 command-specific-data[];
>>> +
>>> + /* Device-writable part */
>>> + u8 command-specific-result[];
>>> + u8 status_type : 4;
>>> + u8 reserved : 4;
>>> + u8 status;
>>> +};
>>> +
>>> +/* Status type values */
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_GENERIC 0
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_CLASS_SPECIFIC 1
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_COMMAND_SPECIFIC 2
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_TRANSPORT_SPECIFIC 3
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_DEVICE_SPECIFIC 4
>>> +#define VIRTIO_ADMIN_STATUS_TYPE_VENDOR_SPECIFIC 5
>>> +
>>> +/* Generic status values */
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_OK 0
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_ERR 1
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_INVALID_CLASS 2
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_INVALID_COMMAND 3
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_DATA_TRANSFER_ERR 4
>>> +#define VIRTIO_ADMIN_STATUS_GENERIC_DEVICE_INTERNAL_ERR 5
>>> +\end{lstlisting}
>>> +
>>> +The \field{class}, \field{command} and
>>> \field{command-specific-data} are
>>> +set by the driver, and the device sets the \field{status_type}, the
>>> +\field{status} and the \field{command-specific-result}, if needed.
>>> +
>>> +The virtio Admin command class codes are divided in the following
>>> form:
>>> +
>>> +\begin{lstlisting}
>>> +/* class values that are transport, device and vendor independent */
>>> +#define VIRTIO_ADMIN_COMMON_CLASS_START 0
>>> +#define VIRTIO_ADMIN_COMMON_CLASS_END 63
>>> +
>>> +/* class values that are transport specific */
>>> +#define VIRTIO_ADMIN_TRANSPORT_CLASS_START 64
>>> +#define VIRTIO_ADMIN_TRANSPORT_CLASS_END 127
>>> +
>>> +/* class values that are device specific */
>>> +#define VIRTIO_ADMIN_DEVICE_CLASS_START 128
>>> +#define VIRTIO_ADMIN_DEVICE_CLASS_END 191
>>> +
>>> +/* class values that are vendor specific */
>>> +#define VIRTIO_ADMIN_VENDOR_CLASS_START 192
>>> +#define VIRTIO_ADMIN_VENDOR_CLASS_END 255
>>> +\end{lstlisting}
>>> +
>>> +\subsection{Admin command set}\label{sec:Basic Facilities of a
>>> Virtio Device / Admin Virtqueues / Admin command set}
>>> +
>>> +Each virtio device that advertise VIRTIO_F_ADMIN_VQ feature, MUST
>>> +support all the mandatory admin commands. A device MAY support also
>>> +one or more optional admin commands.
>>> +
>>> +\subsubsection{Common command set}\label{sec:Basic Facilities of a
>>> Virtio Device / Admin Virtqueues / Admin command set / Common
>>> command set}
>>> +
>>> +The Common command set is a group of classes and commands within each
>>> +of these classes which are transport, device and vendor independent.
>>> +A mandatory class is a class that has at least one mandatory command.
>>> +The Common command set is summarized in following table:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Class & Description & M/O \\
>>> +\hline \hline
>>> +0 & VIRTIO_ADMIN_DISCOVER_DEVICE & M \\
>>> +\hline
>>> +1 & VIRTIO_ADMIN_DISCOVER_DEVICE_CLASS_COMMANDS & M \\
>>> +\hline
>>> +2-63 & reserved & - \\
>>> +\hline
>>> +\end{tabular}
>>> +
>>> +\paragraph{Discover device class}\label{sec:Basic Facilities of a
>>> Virtio Device / Admin Virtqueues / Admin command set / Common
>>> command set / Discover device class}
>>> +
>>> +This class (opcode: 0) of commands is used to query generic device
>>> +information. The following table describes the commands supported for
>>> +this class:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Command & Description & M/O \\
>>> +\hline \hline
>>> +0 & VIRTIO_ADMIN_DISCOVER_DEVICE_IDENTITY & M \\
>>> +\hline
>>> +1 & VIRTIO_ADMIN_DISCOVER_DEVICE_SUPPORTED_CLASSES & M \\
>>> +\hline
>>> +2-255 & reserved & - \\
>>> +\hline
>>> +\end{tabular}
>> So there are several problems with this approach.
>> One is that there is no two way negotiation.
>> No way for device to know what will driver use in the end.
>> This breaks things like e.g. accelerating some configurations
>> but not others.
>>
>> Another is that everything is going through the admin vq.
>> Hard for hypervisor to intervene and change just some aspects
>> of the device.
>>
>> This is also a problem for validation, tricky
>> to find out during probe what does device support and whether
>> driver can work with it.
>
>
> Yes, so I think it makes sense to make the admin virtqueue as a
> transport and then build other stuffs on top.
what transport exactly ?
it should be a virtq for devices to open.
>
>
>>
>>
>> Can't we map this stuff into memory instead?
>> Maybe have an admin config structure
>> that is separate from regular config?
>> Only issue we can't then make it too big.
>> But so far I don't see a lot of info used, most is reserved.
>
>
> It should work (especially for PCI).
you're trying to invent some mechanism that is not flexible and non trivial.
You have already a virtq - so lets use it.
>
> Thanks
>
>
>>
>>
>>
>>> +
>>> +\subparagraph{Device identity command}\label{sec:Basic Facilities
>>> of a Virtio Device / Admin Virtqueues / Admin command set / Common
>>> command set / Discover device class / Device identity command}
>>> +
>>> +This mandatory command should return device identity in the following
>>> +structure:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Bytes & Description & M/O \\
>>> +\hline \hline
>>> +03:00 & VIRTIO DEVICE ID & M \\
>>> +\hline
>>> +05:04 & VIRTIO TRANSPORT ID & M \\
>>> +\hline
>>> +4095:06 & reserved & - \\
>>> + \hline
>>> +\end{tabular}
>>> +
>>> +\subparagraph{Device supported classes command}\label{sec:Basic
>>> Facilities of a Virtio Device / Admin Virtqueues / Admin command set
>>> / Common command set / Discover device class / Device supported
>>> classes command}
>>> +
>>> +This mandatory command should return device identity in the following
>>> +structure:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Bytes & Description & M/O \\
>>> +\hline \hline
>>> +03:00 & VIRTIO_ADMIN_CLASS_0_PROPERTIES & M \\
>>> +\hline
>>> +07:04 & VIRTIO_ADMIN_CLASS_1_PROPERTIES & M \\
>>> + \hline
>>> +11:08 & VIRTIO_ADMIN_CLASS_2_PROPERTIES & M \\
>>> +\hline
>>> +... & ... & M \\
>>> +\hline
>>> +1023:1020 & VIRTIO_ADMIN_CLASS_255_PROPERTIES & M \\
>>> +\hline
>>> +4095:1024 & reserved & - \\
>>> +\hline
>>> +\end{tabular}
>>> +
>>> +The above structure is divided to 256 sections of 4B each, that will
>>> +describe whether a device supports a certain class code. The
>>> +following 3072B are reserved. each of the non-reserved 4B sections
>>> will
>>> +follow the following data structure:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Bits & Description & M/O \\
>>> +\hline \hline
>>> +0 & SUPPORTED (1: yes, 0:no) & M \\
>>> +\hline
>>> +31:01 & reserved & - \\
>>> + \hline
>>> +\end{tabular}
>>> +
>>> +\paragraph{Discover device class commands class}\label{sec:Basic
>>> Facilities of a Virtio Device / Admin Virtqueues / Admin command set
>>> / Common command set / Discover device class commands class}
>>> +
>>> +This class (opcode: 1) of commands is used to query supported commands
>>> +for each supported device class.
>>> The following table describes the commands
>>> +supported for this class:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Command & Description & M/O \\
>>> +\hline \hline
>>> +0 & VIRTIO_ADMIN_CLASS_0_COMMANDS_IDENTITY & M \\
>>> +\hline
>>> +1 & VIRTIO_ADMIN_CLASS_1_COMMANDS_IDENTITY & M \\
>>> +\hline
>>> +2 & VIRTIO_ADMIN_CLASS_2_COMMANDS_IDENTITY & M \\
>>> +\hline
>>> +... & ... & M \\
>>> +\hline
>>> +255 & VIRTIO_ADMIN_CLASS_255_COMMANDS_IDENTITY & M \\
>>> +\hline
>>> +\end{tabular}
>>> +
>>> +Each command in this class, will return class identity in the
>>> following structure:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Bytes & Description & M/O \\
>>> +\hline \hline
>>> +03:00 & VIRTIO_ADMIN_COMMAND_0_PROPERTIES & M \\
>>> +\hline
>>> +07:04 & VIRTIO_ADMIN_COMMAND_1_PROPERTIES & M \\
>>> + \hline
>>> +11:08 & VIRTIO_ADMIN_COMMAND_2_PROPERTIES & M \\
>>> +\hline
>>> +... & ... & M \\
>>> +\hline
>>> +1023:1020 & VIRTIO_ADMIN_COMMAND_255_PROPERTIES & M \\
>>> +\hline
>>> +4095:1024 & reserved & - \\
>>> +\hline
>>> +\end{tabular}
>>> +
>>> +The above structure is divided to 256 sections of 4B each, that will
>>> +describe whether a class supports a certain command code. The
>>> +following 3072B are reserved. each of the non-reserved 4B sections
>>> will
>>> +follow the following data structure:
>>> +
>>> +\begin{tabular}{|l|l|l|}
>>> +\hline
>>> +Bits & Description & M/O \\
>>> +\hline \hline
>>> +0 & SUPPORTED (1: yes, 0:no) & M \\
>>> +\hline
>>> +31:01 & reserved & - \\
>>> + \hline
>>> +\end{tabular}
>>> +
>>> +\subsubsection{Transport command set}\label{sec:Basic Facilities of
>>> a Virtio Device / Admin Virtqueues / Admin command set / Transport
>>> command set}
>>> +
>>> +The Transport command set is a group of classes and commands within
>>> +each of these classes which are transport specific. Refer to each
>>> +transport section for more information on the supported commands.
>>> +
>>> +\subsubsection{Device command set}\label{sec:Basic Facilities of a
>>> Virtio Device / Admin Virtqueues / Admin command set / Device
>>> command set}
>>> +
>>> +The Device command set is a group of classes and commands within
>>> +each of these classes which are device specific. Refer to each
>>> +device section for more information on the supported commands.
>>> +
>>> +\subsubsection{Vendor specific command set}\label{sec:Basic
>>> Facilities of a Virtio Device / Admin Virtqueues / Admin command set
>>> / Vendor specific command set}
>>> +
>>> +The Vendor specific command set is a group of classes and commands
>>> +within each of these classes which are vendor specific. Refer to
>>> +each vendor specification for more information on the supported
>>> +commands.
>> Here's another example.
>> It's important that there is a way to make a device completely
>> generic without vendor specific expensions.
>> Would be hard to do here.
>>
>> That's a generic comment.
>>
>> but specifically I am very reluctant to add vendor specific stuff like
>> this directly in the spec. We already have VIRTIO_PCI_CAP_VENDOR_CFG
>> and if that is not sufficient I would like to know why
>> before we add more vendor specific stuff.
>>
>>
>>> diff --git a/content.tex b/content.tex
>>> index c266fd5..d350175 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -407,6 +407,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-virtq.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
>>> @@ -6671,6 +6673,8 @@ \chapter{Reserved Feature
>>> Bits}\label{sec:Reserved Feature Bits}
>>> data to be provided in driver notification and the delivery
>>> method is
>>> transport specific.
>>> For more details about driver notifications over PCI see
>>> \ref{sec:Virtio Transport Options / Virtio Over PCI Bus /
>>> PCI-specific Initialization And Device Operation / Available Buffer
>>> Notifications}.
>>> + \item[VIRTIO_F_ADMIN_VQ(40)] This feature indicates that
>>> + the device supports administration virtqueue negotiation.
>>> \end{description}
>>> --
>>> 2.21.0
>
next prev parent reply other threads:[~2021-08-02 9:51 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-26 16:52 [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification Max Gurtovoy
2021-07-26 16:52 ` [RFC PATCH v2 2/2] virtio-blk: add support for VIRTIO_F_ADMIN_VQ Max Gurtovoy
2021-07-27 12:24 ` Stefan Hajnoczi
2021-07-27 16:08 ` [virtio-comment] " Max Gurtovoy
2021-07-28 8:25 ` Stefan Hajnoczi
2021-07-27 10:27 ` [RFC PATCH v2 1/2] Add virtio Admin Virtqueue specification Stefan Hajnoczi
2021-07-27 14:28 ` [virtio-comment] " Cornelia Huck
2021-07-27 15:29 ` Max Gurtovoy
2021-07-28 8:52 ` Stefan Hajnoczi
2021-07-28 10:59 ` Max Gurtovoy
2021-07-28 13:42 ` Stefan Hajnoczi
2021-07-28 14:20 ` Max Gurtovoy
2021-07-29 8:48 ` Stefan Hajnoczi
2021-08-01 10:46 ` [virtio-comment] " Max Gurtovoy
2021-08-02 12:58 ` Stefan Hajnoczi
2021-07-28 12:53 ` Michael S. Tsirkin
2021-07-30 6:45 ` [virtio-comment] " Cornelia Huck
2021-07-28 12:48 ` Michael S. Tsirkin
2021-07-29 14:51 ` Max Gurtovoy
2021-07-30 7:05 ` [virtio-comment] " Cornelia Huck
2021-07-31 11:34 ` Max Gurtovoy
2021-07-31 22:26 ` Michael S. Tsirkin
2021-07-31 22:53 ` Max Gurtovoy
2021-08-01 8:16 ` Michael S. Tsirkin
2021-08-01 8:38 ` Max Gurtovoy
2021-08-02 2:17 ` Jason Wang
2021-08-02 2:19 ` Jason Wang
2021-08-02 9:54 ` Max Gurtovoy
2021-08-02 14:51 ` [virtio-comment] " Cornelia Huck
2021-08-02 15:27 ` Max Gurtovoy
2021-08-02 17:28 ` Michael S. Tsirkin
2021-08-03 3:39 ` Jason Wang
2021-08-03 8:32 ` Max Gurtovoy
2021-08-03 9:01 ` Jason Wang
2021-08-03 9:21 ` Max Gurtovoy
2021-08-03 10:04 ` [virtio-comment] " Jason Wang
2021-07-30 7:36 ` Michael S. Tsirkin
2021-07-31 11:53 ` Max Gurtovoy
2021-07-31 22:17 ` Michael S. Tsirkin
2021-07-31 23:46 ` Max Gurtovoy
2021-08-02 13:22 ` Stefan Hajnoczi
2021-08-02 14:34 ` [virtio-comment] " Cornelia Huck
2021-08-02 14:58 ` Max Gurtovoy
2021-08-02 16:39 ` Stefan Hajnoczi
2021-08-02 15:21 ` [virtio-comment] " Cornelia Huck
2021-08-02 16:03 ` Max Gurtovoy
2021-08-02 17:05 ` Michael S. Tsirkin
2021-08-03 6:28 ` [virtio-comment] " Cornelia Huck
2021-08-03 6:41 ` Jason Wang
2021-08-03 6:51 ` [virtio-comment] " Cornelia Huck
2021-08-03 7:55 ` Max Gurtovoy
2021-08-03 8:55 ` Cornelia Huck
2021-08-03 9:04 ` Max Gurtovoy
2021-08-02 2:25 ` Jason Wang
2021-08-02 9:51 ` Max Gurtovoy [this message]
2021-08-02 17:07 ` Michael S. Tsirkin
2021-08-03 3:22 ` Jason Wang
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=29acb062-e541-3359-cbdb-b777249c222c@nvidia.com \
--to=mgurtovoy@nvidia.com \
--cc=aadam@redhat.com \
--cc=amikheev@nvidia.com \
--cc=bodong@nvidia.com \
--cc=cohuck@redhat.com \
--cc=eperezma@redhat.com \
--cc=jasowang@redhat.com \
--cc=mst@redhat.com \
--cc=oren@nvidia.com \
--cc=parav@nvidia.com \
--cc=shahafs@nvidia.com \
--cc=stefanha@redhat.com \
--cc=virtio-comment@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