[virtio-dev] Re: [PATCH 4/4] vhost-user: add vhost-user device type

[virtio-dev] Re: [PATCH 4/4] vhost-user: add vhost-user device type

[Michael S. Tsirkin]

On Wed, Mar 30, 2022 at 04:21:05PM +0100, Usama Arif wrote:
> The vhost-user device backend facilitates vhost-user device emulation
> through vhost-user protocol exchanges and access to shared memory.
> Software-defined networking, storage, and other I/O appliances can
> provide services through this device.
> 
> This device is based on Wei Wang's vhost-pci work.  The virtio
> vhost-user device differs from vhost-pci because it is a single virtio
> device type that exposes the vhost-user protocol instead of a family of
> new virtio device types, one for each vhost-user device type.
> 
> This device supports vhost-user backend and vhost-user frontend
> reconnection. It also contains a UUID so that vhost-user backend programs
> can identify a specific device among many without using bus addresses.
> 
> virtio-vhost-user makes use of additional resources introduced in earlier
> patches including device aux. notifications, driver aux. notifications,
> as well as shared memory.
> 
> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>

Sorry about the delay in review.




> ---
>  conformance.tex       |  27 ++++-
>  content.tex           |   3 +
>  introduction.tex      |   3 +
>  virtio-vhost-user.tex | 259 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 288 insertions(+), 4 deletions(-)
>  create mode 100644 virtio-vhost-user.tex
> 
> diff --git a/conformance.tex b/conformance.tex
> index cddaf75..fab49c3 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -32,8 +32,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
> +\ref{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}.
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -58,8 +59,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>  \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>  \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
> +\ref{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}.
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -324,6 +326,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Device Types / PMEM Device / Device Initialization}
>  \end{itemize}
>  
> +\conformance{\subsection}{Vhost-user Backend Driver Conformance}\label{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}
> +
> +A vhost-user backend driver MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device configuration layout}
> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device Initialization}
> +\end{itemize}
> +
>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>  
>  A device MUST conform to the following normative statements:
> @@ -595,6 +606,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue return}
>  \end{itemize}
>  
> +\conformance{\subsection}{Vhost-user Backend Device Conformance}\label{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}
> +
> +A Vhost-user backend device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
> +\end{itemize}
> +
>  \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>  A conformant implementation MUST be either transitional or
>  non-transitional, see \ref{intro:Legacy
> diff --git a/content.tex b/content.tex
> index 0fc50c4..8bf114d 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3122,6 +3122,8 @@ \chapter{Device Types}\label{sec:Device Types}
>  \hline
>  42         &   RDMA device \\
>  \hline
> +43         &   vhost-user device backend \ \\
> +\hline
>  \end{tabular}
>  
>  Some of the devices above are unspecified by this document,
> @@ -6878,6 +6880,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>  \input{virtio-scmi.tex}
>  \input{virtio-gpio.tex}
>  \input{virtio-pmem.tex}
> +\input{virtio-vhost-user.tex}
>  
>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  
> diff --git a/introduction.tex b/introduction.tex
> index 6d52717..5bd1b95 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -79,6 +79,9 @@ \section{Normative References}\label{sec:Normative References}
>  	\phantomsection\label{intro:SCMI}\textbf{[SCMI]} &
>  	Arm System Control and Management Interface, DEN0056,
>  	\newline\url{https://developer.arm.com/docs/den0056/c}, version C and any future revisions\\
> +	\phantomsection\label{intro:Vhost-user Protocol}\textbf{[Vhost-user Protocol]}
> +	& Vhost-user Protocol,
> +	\newline\url{https://qemu.readthedocs.io/en/latest/interop/vhost-user.html}\\
>  
>  \end{longtable}
>  
> diff --git a/virtio-vhost-user.tex b/virtio-vhost-user.tex
> new file mode 100644
> index 0000000..303054f
> --- /dev/null
> +++ b/virtio-vhost-user.tex
> @@ -0,0 +1,259 @@
> +\section{Vhost-user Device Backend}\label{sec:Device Types / Vhost-user Device
> +Backend}
> +
> +The vhost-user device backend facilitates vhost-user device emulation through
> +vhost-user protocol exchanges and access to shared memory. Software-defined
> +networking, storage, and other I/O appliances can provide services through this
> +device.
> +
> +This section relies on definitions from the \hyperref[intro:Vhost-user
> +Protocol]{Vhost-user Protocol}.  Knowledge of the vhost-user protocol is a
> +prerequisite for understanding this device.
> +
> +The \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol} was originally
> +designed for processes on a single system communicating over UNIX domain
> +sockets. The virtio vhost-user device backend allows the vhost-user backend to
> +communicate with the vhost-user frontend over the device instead of a UNIX domain
> +socket. This allows the backend and frontend to run on two separate systems such
> +as a virtual machine and a hypervisor.
> +
> +The vhost-user backend program exchanges vhost-user protocol messages with the
> +vhost-user frontend through this device. How the device implementation
> +communicates with the vhost-user frontend is beyond the scope of this
> +specification.  One possible device implementation uses a UNIX domain socket to
> +relay messages to a vhost-user frontend process running on the same host.
> +
> +Existing vhost-user backend programs that communicate over UNIX domain sockets
> +can support the virtio vhost-user device backend without invasive changes
> +because the pre-existing vhost-user wire protocol is used.
> +
> +\subsection{Device ID}\label{sec:Device Types / Vhost-user Device Backend / Device ID}
> +  43
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Vhost-user Device Backend / Virtqueues}
> +
> +\begin{description}
> +\item[0] rxq (device-to-driver vhost-user protocol messages)
> +\item[1] txq (driver-to-device vhost-user protocol messages)
> +\end{description}
> +
> +\subsection{Feature bits}\label{sec:Device Types / Vhost-user Device Backend / Feature bits}
> +
> +No feature bits are defined at this time.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Vhost-user Device Backend / Device configuration layout}
> +
> +  All fields of this configuration are always available.
> +
> +\begin{lstlisting}
> +struct virtio_vhost_user_config {
> +        le32 status;
> +#define VIRTIO_VHOST_USER_STATUS_BACKEND_UP (1 << 0)
> +#define VIRTIO_VHOST_USER_STATUS_FRONTEND_UP (1 << 1)
> +        le32 max_vhost_queues;
> +        u8 uuid[16];
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{status}] contains the vhost-user operational status.  The default
> +    value of this field is 0.
> +
> +    The driver sets VIRTIO_VHOST_USER_STATUS_BACKEND_UP to indicate readiness for
> +    the vhost-user frontend to connect.  The vhost-user frontend cannot connect
> +    unless the driver has set this bit first.
> +
> +    The device sets VIRTIO_VHOST_USER_STATUS_FRONTEND_UP to indicate that the
> +    vhost-user frontend is connected.
> +
> +    When the driver clears VIRTIO_VHOST_USER_STATUS_BACKEND_UP while the
> +    vhost-user frontend is connected, the vhost-user frontend is disconnected.
> +
> +    When the vhost-user frontend disconnects, both
> +    VIRTIO_VHOST_USER_STATUS_BACKEND_UP and VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
> +    are cleared by the device.  Communication can be restarted by the driver
> +    setting VIRTIO_VHOST_USER_STATUS_BACKEND_UP again.
> +
> +    A configuration change notification is sent when the device changes
> +    this field, unless a write to the field by the driver caused the change.


So backend might disconnect by the time we check it's UP again?
Seems racy, I suspect you need a handshake not unlike handshake.

> +
> +\item[\field{max_vhost_queues}] is the maximum number of vhost-user queues
> +    supported by this device.  This field is always greater than 0.
> +
> +\item[\field{uuid}] is the Universally Unique Identifier (UUID) for this
> +    device. If the device has no UUID then this field contains the nil
> +    UUID (all zeroes).  The UUID allows vhost-user backend programs to identify a
> +    specific vhost-user device backend among many without relying on bus
> +    addresses.

Why UUID specifically? Is there a need for them to be globally unique
and not just within a VM? These are annoying to generate and maintain
...




> +\end{description}

So feel this basically means that for frontend driver to attack to device
backend driver must be up and responding to messages.
I wonder whether we can make things a bit more robust
by having backend driver send at least basic device
config and feature bits to the vhost-user device
at boot.

Of course device can request these itself if it wants to -
so we do not need to change the spec for this, but maybe
add an implementation note explaining this idea.






> +
> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Vhost-user Device Backend / Device configuration layout}
> +
> +The driver MUST NOT write to device configuration fields other than \field{status}.
> +
> +The driver MUST NOT set undefined bits in the \field{status} configuration field.
> +
> +\subsection{Device Initialization}\label{sec:Device Types / Vhost-user Device Backend / Device Initialization}
> +
> +The driver initializes the rxq/txq virtqueues and then it sets
> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the \field{status} field of the device
> +configuration structure.
> +
> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Vhost-user Device Backend / Device Initialization}
> +
> +The driver SHOULD check the \field{max_vhost_queues} configuration field to
> +determine how many queues the vhost-user backend will be able to support.
> +
> +The driver SHOULD fetch the \field{uuid} configuration field to allow
> +vhost-user backend programs to identify a specific device among many.
> +
> +The driver SHOULD place at least one buffer in rxq before setting the
> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP bit in the \field{status} configuration field.
> +
> +The driver MUST handle rxq virtqueue notifications that occur before the
> +configuration change notification.  It is possible that a vhost-user protocol
> +message from the vhost-user frontend arrives before the driver has seen the
> +configuration change notification for the VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
> +\field{status} change.
> +
> +\subsection{Device Operation}\label{sec:Device Types / Vhost-user Device Backend / Device Operation}
> +
> +Device operation consists of operating request queues and response queues.
> +
> +\subsubsection{Device Operation: Request Queues}\label{sec:Device Types / Vhost-user Device Backend / Device Operation / Device Operation: RX/TX Queues}
> +
> +The driver receives vhost-user protocol messages from the vhost-user frontend on
> +rxq. The driver sends responses to the vhost-user frontend on txq.
> +
> +The driver sends backend-initiated requests on txq. The driver receives
> +responses from the vhost-user frontend on rxq.
> +
> +All virtqueues offer in-order guaranteed delivery semantics for vhost-user
> +protocol messages.
> +
> +Each buffer is a vhost-user protocol message as defined by the
> +\hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}.  In order to enable
> +cross-endian communication, all message fields are little-endian instead of the
> +native byte order normally used by the protocol.


I wonder how clear this is to the reader.

I feel maybe being more explicit is called for.
For example explain that "u64" should be interpreted as "le64".




> +
> +The appropriate size of rxq buffers is at least as large as the largest message
> +defined by the \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}
> +standard version that the driver supports.  If the vhost-user frontend sends a
> +message that is too large for an rxq buffer, then DEVICE_NEEDS_RESET is set and
> +the driver must reset the device.
> +
> +File descriptor passing is handled differently by the vhost-user device
> +backend. When a frontend-initiated message is received that carries one or more file
> +descriptors according to the vhost-user protocol, additional device resources
> +become available to the driver.
> +
> +\subsection{Additional Device Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources}
> +
> +The vhost-user device backend uses the following facilities from virtio device
> +\ref{sec:Basic Facilities of a Virtio Device} for the vhost-user frontend and
> +backend to exchange notifications and data through the device:
> +
> +\begin{description}
> +  \item[Device auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
> +The driver signals the vhost-user frontend through device auxiliary notifications. The signal does not
> +carry any data, it is purely an event.
> +  \item[Driver auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
> +The vhost-user frontend signals the driver for events besides virtqueue activity
> +and configuration changes by sending driver auxiliary notification.
> +  \item[Shared memory] \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
> +The vhost-user frontend gives access to memory that can be mapped by the driver.
> +\end{description}
> +
> +\subsubsection{Device auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Device auxiliary notifications}
> +
> +The vhost-user device backend provides all (or part) of the following device auxiliary notifications:
> +
> +\begin{description}
> +\item[0] Vring call for vhost-user queue 0
> +\item[\ldots]
> +\item[N-1] Vring call for vhost-user queue N-1
> +\item[N] Vring err for vhost-user queue 0
> +\item[\ldots]
> +\item[2N-1] Vring err for vhost-user queue N-1
> +\item[2N] Log
> +\end{description}
> +
> +where N is the number of the vhost-user virtqueues.
> +
> +\subsubsection{Driver auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Driver auxiliary notifications}
> +
> +The vhost-user device backend provides all (or part) of the following driver auxiliary notifications:
> +
> +\begin{description}
> +\item[0] Vring kick for vhost-user queue 0
> +\item[\ldots]
> +\item[N-1] Vring kick for vhost-user queue N-1
> +\end{description}
> +
> +where N is the number of the vhost-user virtqueues.
> +
> +\subsubsection{Shared Memory}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory}
> +
> +The vhost-user device backend provides all (or part) of the following shared memory regions:
> +
> +\begin{description}
> +\item[0] Vhost-user memory region 0
> +\item[1] Vhost-user memory region 1
> +\item[\ldots]
> +\item[M-1] Vhost-user memory region M-1
> +\item[M] Log memory region
> +\end{description}
> +
> +where M is the total number of memory regions shared.
> +
> +\devicenormative{\paragraph}{Shared Memory layout}{Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
> +
> +The device exports all memory regions reported by the vhost-user frontend as a
> +single shared memory region \ref{sec:Basic Facilities of a Virtio Device /
> +Shared Memory Regions}.
> +
> +The size of this shared memory region exported by the device MUST be at least
> +as much as the sum of the sizes of all the memory regions reported by the
> +vhost-user frontend.
> +
> +The memory regions exported by the device MUST be laid out in the same order
> +in which they are reported by the frontend with vhost-user messages.

This is vague. The driver is the front end here, isn't it?
Are you talking about the "Memory regions description" section
in the vhost-user spec?


> +
> +The offsets in which the memory regions are mapped inside the shared memory
> +region MUST be the following:
> +
> +\begin{description}
> +\item[0] Offset for vhost-user memory region 0
> +\item[SIZE0] Offset for vhost-user memory region 1
> +\item[\ldots]
> +\item[SIZE0 + SIZE1 + \ldots] Offset for vhost-user memory region M
> +\end{description}
> +    
> +where SIZEi is the size of the vhost-user memory region i.
> +
> +\subsubsection{Availability of Additional Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Availability of Additional Resources}
> +
> +The following vhost-user protocol messages convey access to additional device
> +resources:
> +
> +\begin{description}
> +\item[VHOST_USER_SET_MEM_TABLE] Contents of vhost-user memory regions are
> +available to the driver as device memory. Region contents are laid out in the
> +same order as the vhost-user memory region list.
> +\item[VHOST_USER_SET_LOG_BASE] Contents of the log memory region are available
> +to the driver as device memory.
> +\item[VHOST_USER_SET_LOG_FD] The log device auxiliary notification is available to the driver.
> +Writes to the log device auxiliary notification before this message is received produce no effect.
> +\item[VHOST_USER_SET_VRING_KICK] The vring kick notification for this queue is
> +available to the driver. The first notification may occur before the driver has
> +processed this message.
> +\item[VHOST_USER_SET_VRING_CALL] The vring call device auxiliary notification for this queue is
> +available to the driver. Writes to the vring call device auxiliary notification before this message
> +is received produce no effect.
> +\item[VHOST_USER_SET_VRING_ERR] The vring err device auxiliary notification for this queue is
> +available to the driver. Writes to the vring err device auxiliary notification before this message
> +is received produce no effect.
> +\item[VHOST_USER_SET_SLAVE_REQ_FD] The driver may send vhost-user protocol
> +backend messages on txq. Backend-initiated messages put onto txq before this
> +message is received are discarded by the device.

What about VHOST_USER_GET_INFLIGHT_FD?

Generally I worry that new versions of the protocol
add new features without regard for this reuse.

Do we want to refer to a specific version of the spec maybe?



> +\end{description}
> -- 
> 2.25.1


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

Re: [PATCH 4/4] vhost-user: add vhost-user device type

[Usama Arif]

On 09/08/2022 21:06, Michael S. Tsirkin wrote:
> On Wed, Mar 30, 2022 at 04:21:05PM +0100, Usama Arif wrote:
>> The vhost-user device backend facilitates vhost-user device emulation
>> through vhost-user protocol exchanges and access to shared memory.
>> Software-defined networking, storage, and other I/O appliances can
>> provide services through this device.
>>
>> This device is based on Wei Wang's vhost-pci work.  The virtio
>> vhost-user device differs from vhost-pci because it is a single virtio
>> device type that exposes the vhost-user protocol instead of a family of
>> new virtio device types, one for each vhost-user device type.
>>
>> This device supports vhost-user backend and vhost-user frontend
>> reconnection. It also contains a UUID so that vhost-user backend programs
>> can identify a specific device among many without using bus addresses.
>>
>> virtio-vhost-user makes use of additional resources introduced in earlier
>> patches including device aux. notifications, driver aux. notifications,
>> as well as shared memory.
>>
>> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
> 
> Sorry about the delay in review.
> 
> 

Hi,

No worries, Thanks for the review!

I had sent a v2 
(https://lists.oasis-open.org/archives/virtio-dev/202204/msg00022.html) 
of the patch in April to address Stefans' comments, so it might be good 
to review that. I have replied inline below to the comments.

> 
> 
>> ---
>>   conformance.tex       |  27 ++++-
>>   content.tex           |   3 +
>>   introduction.tex      |   3 +
>>   virtio-vhost-user.tex | 259 ++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 288 insertions(+), 4 deletions(-)
>>   create mode 100644 virtio-vhost-user.tex
>>
>> diff --git a/conformance.tex b/conformance.tex
>> index cddaf75..fab49c3 100644
>> --- a/conformance.tex
>> +++ b/conformance.tex
>> @@ -32,8 +32,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>   \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>>   \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>>   \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
>> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
>> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
>> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
>> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
>> +\ref{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}.
>>   
>>       \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>     \end{itemize}
>> @@ -58,8 +59,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>   \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>>   \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>>   \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
>> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
>> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
>> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
>> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
>> +\ref{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}.
>>   
>>       \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>     \end{itemize}
>> @@ -324,6 +326,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>   \item \ref{drivernormative:Device Types / PMEM Device / Device Initialization}
>>   \end{itemize}
>>   
>> +\conformance{\subsection}{Vhost-user Backend Driver Conformance}\label{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}
>> +
>> +A vhost-user backend driver MUST conform to the following normative statements:
>> +
>> +\begin{itemize}
>> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device configuration layout}
>> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device Initialization}
>> +\end{itemize}
>> +
>>   \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>>   
>>   A device MUST conform to the following normative statements:
>> @@ -595,6 +606,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>   \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue return}
>>   \end{itemize}
>>   
>> +\conformance{\subsection}{Vhost-user Backend Device Conformance}\label{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}
>> +
>> +A Vhost-user backend device MUST conform to the following normative statements:
>> +
>> +\begin{itemize}
>> +\item \ref{devicenormative:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
>> +\end{itemize}
>> +
>>   \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>>   A conformant implementation MUST be either transitional or
>>   non-transitional, see \ref{intro:Legacy
>> diff --git a/content.tex b/content.tex
>> index 0fc50c4..8bf114d 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -3122,6 +3122,8 @@ \chapter{Device Types}\label{sec:Device Types}
>>   \hline
>>   42         &   RDMA device \\
>>   \hline
>> +43         &   vhost-user device backend \ \\
>> +\hline
>>   \end{tabular}
>>   
>>   Some of the devices above are unspecified by this document,
>> @@ -6878,6 +6880,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>>   \input{virtio-scmi.tex}
>>   \input{virtio-gpio.tex}
>>   \input{virtio-pmem.tex}
>> +\input{virtio-vhost-user.tex}
>>   
>>   \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>>   
>> diff --git a/introduction.tex b/introduction.tex
>> index 6d52717..5bd1b95 100644
>> --- a/introduction.tex
>> +++ b/introduction.tex
>> @@ -79,6 +79,9 @@ \section{Normative References}\label{sec:Normative References}
>>   	\phantomsection\label{intro:SCMI}\textbf{[SCMI]} &
>>   	Arm System Control and Management Interface, DEN0056,
>>   	\newline\url{https://developer.arm.com/docs/den0056/c}, version C and any future revisions\\
>> +	\phantomsection\label{intro:Vhost-user Protocol}\textbf{[Vhost-user Protocol]}
>> +	& Vhost-user Protocol,
>> +	\newline\url{https://qemu.readthedocs.io/en/latest/interop/vhost-user.html}\\
>>   
>>   \end{longtable}
>>   
>> diff --git a/virtio-vhost-user.tex b/virtio-vhost-user.tex
>> new file mode 100644
>> index 0000000..303054f
>> --- /dev/null
>> +++ b/virtio-vhost-user.tex
>> @@ -0,0 +1,259 @@
>> +\section{Vhost-user Device Backend}\label{sec:Device Types / Vhost-user Device
>> +Backend}
>> +
>> +The vhost-user device backend facilitates vhost-user device emulation through
>> +vhost-user protocol exchanges and access to shared memory. Software-defined
>> +networking, storage, and other I/O appliances can provide services through this
>> +device.
>> +
>> +This section relies on definitions from the \hyperref[intro:Vhost-user
>> +Protocol]{Vhost-user Protocol}.  Knowledge of the vhost-user protocol is a
>> +prerequisite for understanding this device.
>> +
>> +The \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol} was originally
>> +designed for processes on a single system communicating over UNIX domain
>> +sockets. The virtio vhost-user device backend allows the vhost-user backend to
>> +communicate with the vhost-user frontend over the device instead of a UNIX domain
>> +socket. This allows the backend and frontend to run on two separate systems such
>> +as a virtual machine and a hypervisor.
>> +
>> +The vhost-user backend program exchanges vhost-user protocol messages with the
>> +vhost-user frontend through this device. How the device implementation
>> +communicates with the vhost-user frontend is beyond the scope of this
>> +specification.  One possible device implementation uses a UNIX domain socket to
>> +relay messages to a vhost-user frontend process running on the same host.
>> +
>> +Existing vhost-user backend programs that communicate over UNIX domain sockets
>> +can support the virtio vhost-user device backend without invasive changes
>> +because the pre-existing vhost-user wire protocol is used.
>> +
>> +\subsection{Device ID}\label{sec:Device Types / Vhost-user Device Backend / Device ID}
>> +  43
>> +
>> +\subsection{Virtqueues}\label{sec:Device Types / Vhost-user Device Backend / Virtqueues}
>> +
>> +\begin{description}
>> +\item[0] rxq (device-to-driver vhost-user protocol messages)
>> +\item[1] txq (driver-to-device vhost-user protocol messages)
>> +\end{description}
>> +
>> +\subsection{Feature bits}\label{sec:Device Types / Vhost-user Device Backend / Feature bits}
>> +
>> +No feature bits are defined at this time.
>> +
>> +\subsection{Device configuration layout}\label{sec:Device Types / Vhost-user Device Backend / Device configuration layout}
>> +
>> +  All fields of this configuration are always available.
>> +
>> +\begin{lstlisting}
>> +struct virtio_vhost_user_config {
>> +        le32 status;
>> +#define VIRTIO_VHOST_USER_STATUS_BACKEND_UP (1 << 0)
>> +#define VIRTIO_VHOST_USER_STATUS_FRONTEND_UP (1 << 1)
>> +        le32 max_vhost_queues;
>> +        u8 uuid[16];
>> +};
>> +\end{lstlisting}
>> +
>> +\begin{description}
>> +\item[\field{status}] contains the vhost-user operational status.  The default
>> +    value of this field is 0.
>> +
>> +    The driver sets VIRTIO_VHOST_USER_STATUS_BACKEND_UP to indicate readiness for
>> +    the vhost-user frontend to connect.  The vhost-user frontend cannot connect
>> +    unless the driver has set this bit first.
>> +
>> +    The device sets VIRTIO_VHOST_USER_STATUS_FRONTEND_UP to indicate that the
>> +    vhost-user frontend is connected.
>> +
>> +    When the driver clears VIRTIO_VHOST_USER_STATUS_BACKEND_UP while the
>> +    vhost-user frontend is connected, the vhost-user frontend is disconnected.
>> +
>> +    When the vhost-user frontend disconnects, both
>> +    VIRTIO_VHOST_USER_STATUS_BACKEND_UP and VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
>> +    are cleared by the device.  Communication can be restarted by the driver
>> +    setting VIRTIO_VHOST_USER_STATUS_BACKEND_UP again.
>> +
>> +    A configuration change notification is sent when the device changes
>> +    this field, unless a write to the field by the driver caused the change.
> 
> 
> So backend might disconnect by the time we check it's UP again?
> Seems racy, I suspect you need a handshake not unlike handshake.
> 

I am assuming over here you mean when vhost-user frontend disconnects.

I realized that there is no need for VIRTIO_VHOST_USER_STATUS_BACKEND_UP 
to be cleared when the frontend disconnects.
The backend driver can just stay connected to the backend device when 
the frontend disconnect. In the next revision, I can change the above 
paragraph to:

"When the vhost-user frontend disconnects, 
VIRTIO_VHOST_USER_STATUS_FRONTEND_UP is cleared by the device." (Nothing 
is done with VIRTIO_VHOST_USER_STATUS_BACKEND_UP)

I might have misunderstood, but I don't see a race condition or the 
advantage of a handshake connection when changed to above.
Do you mean handshake connection between the backend driver and backend 
device, or between the frontend and the backend device? Will there still 
be a race condition if we dont disconnect the backend driver when the 
frontend disconnects?


>> +
>> +\item[\field{max_vhost_queues}] is the maximum number of vhost-user queues
>> +    supported by this device.  This field is always greater than 0.
>> +
>> +\item[\field{uuid}] is the Universally Unique Identifier (UUID) for this
>> +    device. If the device has no UUID then this field contains the nil
>> +    UUID (all zeroes).  The UUID allows vhost-user backend programs to identify a
>> +    specific vhost-user device backend among many without relying on bus
>> +    addresses.
> 
> Why UUID specifically? Is there a need for them to be globally unique
> and not just within a VM? These are annoying to generate and maintain
> ...
> 
> 

That makes sense, no need for UUID, just need an ID unique within a VM. 
I will change it to just ID in next version.

> 
> 
>> +\end{description}
> 
> So feel this basically means that for frontend driver to attack to device
> backend driver must be up and responding to messages.
> I wonder whether we can make things a bit more robust
> by having backend driver send at least basic device
> config and feature bits to the vhost-user device
> at boot.
> 
> Of course device can request these itself if it wants to -
> so we do not need to change the spec for this, but maybe
> add an implementation note explaining this idea.
> 
> 

Yes, for the frontend to connect to backend device, the backend driver 
must be up.


In the prototype as well, the backend driver in DPDK does check if the 
virtio features are OK during init.

I will change the Device Initialization section below to:

The driver checks if the virtio features supported by it are provided by 
the device, initializes the rxq/txq virtqueues and then sets 
VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the status field of the device 
configuration structure.

if that makes things better?

> 
> 
> 
> 
>> +
>> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Vhost-user Device Backend / Device configuration layout}
>> +
>> +The driver MUST NOT write to device configuration fields other than \field{status}.
>> +
>> +The driver MUST NOT set undefined bits in the \field{status} configuration field.
>> +
>> +\subsection{Device Initialization}\label{sec:Device Types / Vhost-user Device Backend / Device Initialization}
>> +
>> +The driver initializes the rxq/txq virtqueues and then it sets
>> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the \field{status} field of the device
>> +configuration structure.
>> +
>> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Vhost-user Device Backend / Device Initialization}
>> +
>> +The driver SHOULD check the \field{max_vhost_queues} configuration field to
>> +determine how many queues the vhost-user backend will be able to support.
>> +
>> +The driver SHOULD fetch the \field{uuid} configuration field to allow
>> +vhost-user backend programs to identify a specific device among many.
>> +
>> +The driver SHOULD place at least one buffer in rxq before setting the
>> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP bit in the \field{status} configuration field.
>> +
>> +The driver MUST handle rxq virtqueue notifications that occur before the
>> +configuration change notification.  It is possible that a vhost-user protocol
>> +message from the vhost-user frontend arrives before the driver has seen the
>> +configuration change notification for the VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
>> +\field{status} change.
>> +
>> +\subsection{Device Operation}\label{sec:Device Types / Vhost-user Device Backend / Device Operation}
>> +
>> +Device operation consists of operating request queues and response queues.
>> +
>> +\subsubsection{Device Operation: Request Queues}\label{sec:Device Types / Vhost-user Device Backend / Device Operation / Device Operation: RX/TX Queues}
>> +
>> +The driver receives vhost-user protocol messages from the vhost-user frontend on
>> +rxq. The driver sends responses to the vhost-user frontend on txq.
>> +
>> +The driver sends backend-initiated requests on txq. The driver receives
>> +responses from the vhost-user frontend on rxq.
>> +
>> +All virtqueues offer in-order guaranteed delivery semantics for vhost-user
>> +protocol messages.
>> +
>> +Each buffer is a vhost-user protocol message as defined by the
>> +\hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}.  In order to enable
>> +cross-endian communication, all message fields are little-endian instead of the
>> +native byte order normally used by the protocol.
> 
> 
> I wonder how clear this is to the reader.
> 
> I feel maybe being more explicit is called for.
> For example explain that "u64" should be interpreted as "le64".
> 
> 

Will change to:
"all message fields are little-endian instead of the native byte order 
normally used by the protocol, i.e. u64 message fields are interpreted 
as le64"

if thats better?

> 
> 
>> +
>> +The appropriate size of rxq buffers is at least as large as the largest message
>> +defined by the \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}
>> +standard version that the driver supports.  If the vhost-user frontend sends a
>> +message that is too large for an rxq buffer, then DEVICE_NEEDS_RESET is set and
>> +the driver must reset the device.
>> +
>> +File descriptor passing is handled differently by the vhost-user device
>> +backend. When a frontend-initiated message is received that carries one or more file
>> +descriptors according to the vhost-user protocol, additional device resources
>> +become available to the driver.
>> +
>> +\subsection{Additional Device Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources}
>> +
>> +The vhost-user device backend uses the following facilities from virtio device
>> +\ref{sec:Basic Facilities of a Virtio Device} for the vhost-user frontend and
>> +backend to exchange notifications and data through the device:
>> +
>> +\begin{description}
>> +  \item[Device auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
>> +The driver signals the vhost-user frontend through device auxiliary notifications. The signal does not
>> +carry any data, it is purely an event.
>> +  \item[Driver auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
>> +The vhost-user frontend signals the driver for events besides virtqueue activity
>> +and configuration changes by sending driver auxiliary notification.
>> +  \item[Shared memory] \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
>> +The vhost-user frontend gives access to memory that can be mapped by the driver.
>> +\end{description}
>> +
>> +\subsubsection{Device auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Device auxiliary notifications}
>> +
>> +The vhost-user device backend provides all (or part) of the following device auxiliary notifications:
>> +
>> +\begin{description}
>> +\item[0] Vring call for vhost-user queue 0
>> +\item[\ldots]
>> +\item[N-1] Vring call for vhost-user queue N-1
>> +\item[N] Vring err for vhost-user queue 0
>> +\item[\ldots]
>> +\item[2N-1] Vring err for vhost-user queue N-1
>> +\item[2N] Log
>> +\end{description}
>> +
>> +where N is the number of the vhost-user virtqueues.
>> +
>> +\subsubsection{Driver auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Driver auxiliary notifications}
>> +
>> +The vhost-user device backend provides all (or part) of the following driver auxiliary notifications:
>> +
>> +\begin{description}
>> +\item[0] Vring kick for vhost-user queue 0
>> +\item[\ldots]
>> +\item[N-1] Vring kick for vhost-user queue N-1
>> +\end{description}
>> +
>> +where N is the number of the vhost-user virtqueues.
>> +
>> +\subsubsection{Shared Memory}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory}
>> +
>> +The vhost-user device backend provides all (or part) of the following shared memory regions:
>> +
>> +\begin{description}
>> +\item[0] Vhost-user memory region 0
>> +\item[1] Vhost-user memory region 1
>> +\item[\ldots]
>> +\item[M-1] Vhost-user memory region M-1
>> +\item[M] Log memory region
>> +\end{description}
>> +
>> +where M is the total number of memory regions shared.
>> +
>> +\devicenormative{\paragraph}{Shared Memory layout}{Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
>> +
>> +The device exports all memory regions reported by the vhost-user frontend as a
>> +single shared memory region \ref{sec:Basic Facilities of a Virtio Device /
>> +Shared Memory Regions}.
>> +
>> +The size of this shared memory region exported by the device MUST be at least
>> +as much as the sum of the sizes of all the memory regions reported by the
>> +vhost-user frontend.
>> +
>> +The memory regions exported by the device MUST be laid out in the same order
>> +in which they are reported by the frontend with vhost-user messages.
> 
> This is vague. The driver is the front end here, isn't it?
> Are you talking about the "Memory regions description" section
> in the vhost-user spec?
> 
> 

This section changed in v2, when Stefan suggested that this doesn't take 
into account VHOST_USER_ADD_MEM_REG/VHOST_USER_REM_MEM_REG. So might be 
better to re-review in v2.

In the document there are references to 3 things, the frontend (which is 
the same as the the frontend in vhost-user spec), the backend driver and 
the backend device. I see that I have just written driver/device in many 
places in this doc without prefixing with backend which might make 
things vague? I will prefix all driver/device instances in the doc with 
backend if it makes it better.

>> +
>> +The offsets in which the memory regions are mapped inside the shared memory
>> +region MUST be the following:
>> +
>> +\begin{description}
>> +\item[0] Offset for vhost-user memory region 0
>> +\item[SIZE0] Offset for vhost-user memory region 1
>> +\item[\ldots]
>> +\item[SIZE0 + SIZE1 + \ldots] Offset for vhost-user memory region M
>> +\end{description}
>> +
>> +where SIZEi is the size of the vhost-user memory region i.
>> +
>> +\subsubsection{Availability of Additional Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Availability of Additional Resources}
>> +
>> +The following vhost-user protocol messages convey access to additional device
>> +resources:
>> +
>> +\begin{description}
>> +\item[VHOST_USER_SET_MEM_TABLE] Contents of vhost-user memory regions are
>> +available to the driver as device memory. Region contents are laid out in the
>> +same order as the vhost-user memory region list.
>> +\item[VHOST_USER_SET_LOG_BASE] Contents of the log memory region are available
>> +to the driver as device memory.
>> +\item[VHOST_USER_SET_LOG_FD] The log device auxiliary notification is available to the driver.
>> +Writes to the log device auxiliary notification before this message is received produce no effect.
>> +\item[VHOST_USER_SET_VRING_KICK] The vring kick notification for this queue is
>> +available to the driver. The first notification may occur before the driver has
>> +processed this message.
>> +\item[VHOST_USER_SET_VRING_CALL] The vring call device auxiliary notification for this queue is
>> +available to the driver. Writes to the vring call device auxiliary notification before this message
>> +is received produce no effect.
>> +\item[VHOST_USER_SET_VRING_ERR] The vring err device auxiliary notification for this queue is
>> +available to the driver. Writes to the vring err device auxiliary notification before this message
>> +is received produce no effect.
>> +\item[VHOST_USER_SET_SLAVE_REQ_FD] The driver may send vhost-user protocol
>> +backend messages on txq. Backend-initiated messages put onto txq before this
>> +message is received are discarded by the device.
> 
> What about VHOST_USER_GET_INFLIGHT_FD?
> 
> Generally I worry that new versions of the protocol
> add new features without regard for this reuse.
> 
> Do we want to refer to a specific version of the spec maybe?
> 
> 

I think it would be a good idea to link to a specific version of 
vhost-user spec.

I didn't take into account VHOST_USER_GET_INFLIGHT_FD. From reading 
about it on the vhost-user spec now, it looks like another region is 
needed in shared memory layout for this.

The access to additional device resources will look the same for both 
get/set inflight_fd.

\item[VHOST_USER_GET_INFLIGHT_FD] Contents of the inflight memory region 
are available to the backend driver as backend device memory.
\item[VHOST_USER_SET_INFLIGHT_FD] Contents of the inflight memory region 
are available to the backend driver as backend device memory.

and we will need to add another region shared memory when referencing 
the v2 patch. i.e.

\begin{description}
\item[0] Vhost-user memory region 0
\item[1] Vhost-user memory region 1
\item[\ldots]
\item[M-2] Vhost-user memory region M-1
\item[M-1] Log memory region
\item[M] Inflight memory region
\end{description}


I will send these changes in v3 of the patch if this and the above 
comments look good.

Thanks!

> 
>> +\end{description}
>> -- 
>> 2.25.1
> 

Re: [PATCH 4/4] vhost-user: add vhost-user device type

[Michael S. Tsirkin]

On Thu, Aug 11, 2022 at 11:05:16AM +0100, Usama Arif wrote:
> 
> 
> On 09/08/2022 21:06, Michael S. Tsirkin wrote:
> > On Wed, Mar 30, 2022 at 04:21:05PM +0100, Usama Arif wrote:
> > > The vhost-user device backend facilitates vhost-user device emulation
> > > through vhost-user protocol exchanges and access to shared memory.
> > > Software-defined networking, storage, and other I/O appliances can
> > > provide services through this device.
> > > 
> > > This device is based on Wei Wang's vhost-pci work.  The virtio
> > > vhost-user device differs from vhost-pci because it is a single virtio
> > > device type that exposes the vhost-user protocol instead of a family of
> > > new virtio device types, one for each vhost-user device type.
> > > 
> > > This device supports vhost-user backend and vhost-user frontend
> > > reconnection. It also contains a UUID so that vhost-user backend programs
> > > can identify a specific device among many without using bus addresses.
> > > 
> > > virtio-vhost-user makes use of additional resources introduced in earlier
> > > patches including device aux. notifications, driver aux. notifications,
> > > as well as shared memory.
> > > 
> > > Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
> > 
> > Sorry about the delay in review.
> > 
> > 
> 
> Hi,
> 
> No worries, Thanks for the review!
> 
> I had sent a v2
> (https://lists.oasis-open.org/archives/virtio-dev/202204/msg00022.html) of
> the patch in April to address Stefans' comments, so it might be good to
> review that. I have replied inline below to the comments.
> 
> > 
> > 
> > > ---
> > >   conformance.tex       |  27 ++++-
> > >   content.tex           |   3 +
> > >   introduction.tex      |   3 +
> > >   virtio-vhost-user.tex | 259 ++++++++++++++++++++++++++++++++++++++++++
> > >   4 files changed, 288 insertions(+), 4 deletions(-)
> > >   create mode 100644 virtio-vhost-user.tex
> > > 
> > > diff --git a/conformance.tex b/conformance.tex
> > > index cddaf75..fab49c3 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -32,8 +32,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
> > >   \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
> > >   \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
> > > -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
> > > -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
> > > +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
> > > +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
> > > +\ref{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}.
> > >       \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> > >     \end{itemize}
> > > @@ -58,8 +59,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
> > >   \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
> > >   \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
> > > -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
> > > -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
> > > +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
> > > +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
> > > +\ref{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}.
> > >       \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
> > >     \end{itemize}
> > > @@ -324,6 +326,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \item \ref{drivernormative:Device Types / PMEM Device / Device Initialization}
> > >   \end{itemize}
> > > +\conformance{\subsection}{Vhost-user Backend Driver Conformance}\label{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}
> > > +
> > > +A vhost-user backend driver MUST conform to the following normative statements:
> > > +
> > > +\begin{itemize}
> > > +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device configuration layout}
> > > +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device Initialization}
> > > +\end{itemize}
> > > +
> > >   \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
> > >   A device MUST conform to the following normative statements:
> > > @@ -595,6 +606,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue return}
> > >   \end{itemize}
> > > +\conformance{\subsection}{Vhost-user Backend Device Conformance}\label{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}
> > > +
> > > +A Vhost-user backend device MUST conform to the following normative statements:
> > > +
> > > +\begin{itemize}
> > > +\item \ref{devicenormative:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
> > > +\end{itemize}
> > > +
> > >   \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
> > >   A conformant implementation MUST be either transitional or
> > >   non-transitional, see \ref{intro:Legacy
> > > diff --git a/content.tex b/content.tex
> > > index 0fc50c4..8bf114d 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3122,6 +3122,8 @@ \chapter{Device Types}\label{sec:Device Types}
> > >   \hline
> > >   42         &   RDMA device \\
> > >   \hline
> > > +43         &   vhost-user device backend \ \\
> > > +\hline
> > >   \end{tabular}
> > >   Some of the devices above are unspecified by this document,
> > > @@ -6878,6 +6880,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >   \input{virtio-scmi.tex}
> > >   \input{virtio-gpio.tex}
> > >   \input{virtio-pmem.tex}
> > > +\input{virtio-vhost-user.tex}
> > >   \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > diff --git a/introduction.tex b/introduction.tex
> > > index 6d52717..5bd1b95 100644
> > > --- a/introduction.tex
> > > +++ b/introduction.tex
> > > @@ -79,6 +79,9 @@ \section{Normative References}\label{sec:Normative References}
> > >   	\phantomsection\label{intro:SCMI}\textbf{[SCMI]} &
> > >   	Arm System Control and Management Interface, DEN0056,
> > >   	\newline\url{https://developer.arm.com/docs/den0056/c}, version C and any future revisions\\
> > > +	\phantomsection\label{intro:Vhost-user Protocol}\textbf{[Vhost-user Protocol]}
> > > +	& Vhost-user Protocol,
> > > +	\newline\url{https://qemu.readthedocs.io/en/latest/interop/vhost-user.html}\\
> > >   \end{longtable}
> > > diff --git a/virtio-vhost-user.tex b/virtio-vhost-user.tex
> > > new file mode 100644
> > > index 0000000..303054f
> > > --- /dev/null
> > > +++ b/virtio-vhost-user.tex
> > > @@ -0,0 +1,259 @@
> > > +\section{Vhost-user Device Backend}\label{sec:Device Types / Vhost-user Device
> > > +Backend}
> > > +
> > > +The vhost-user device backend facilitates vhost-user device emulation through
> > > +vhost-user protocol exchanges and access to shared memory. Software-defined
> > > +networking, storage, and other I/O appliances can provide services through this
> > > +device.
> > > +
> > > +This section relies on definitions from the \hyperref[intro:Vhost-user
> > > +Protocol]{Vhost-user Protocol}.  Knowledge of the vhost-user protocol is a
> > > +prerequisite for understanding this device.
> > > +
> > > +The \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol} was originally
> > > +designed for processes on a single system communicating over UNIX domain
> > > +sockets. The virtio vhost-user device backend allows the vhost-user backend to
> > > +communicate with the vhost-user frontend over the device instead of a UNIX domain
> > > +socket. This allows the backend and frontend to run on two separate systems such
> > > +as a virtual machine and a hypervisor.
> > > +
> > > +The vhost-user backend program exchanges vhost-user protocol messages with the
> > > +vhost-user frontend through this device. How the device implementation
> > > +communicates with the vhost-user frontend is beyond the scope of this
> > > +specification.  One possible device implementation uses a UNIX domain socket to
> > > +relay messages to a vhost-user frontend process running on the same host.
> > > +
> > > +Existing vhost-user backend programs that communicate over UNIX domain sockets
> > > +can support the virtio vhost-user device backend without invasive changes
> > > +because the pre-existing vhost-user wire protocol is used.
> > > +
> > > +\subsection{Device ID}\label{sec:Device Types / Vhost-user Device Backend / Device ID}
> > > +  43
> > > +
> > > +\subsection{Virtqueues}\label{sec:Device Types / Vhost-user Device Backend / Virtqueues}
> > > +
> > > +\begin{description}
> > > +\item[0] rxq (device-to-driver vhost-user protocol messages)
> > > +\item[1] txq (driver-to-device vhost-user protocol messages)
> > > +\end{description}
> > > +
> > > +\subsection{Feature bits}\label{sec:Device Types / Vhost-user Device Backend / Feature bits}
> > > +
> > > +No feature bits are defined at this time.
> > > +
> > > +\subsection{Device configuration layout}\label{sec:Device Types / Vhost-user Device Backend / Device configuration layout}
> > > +
> > > +  All fields of this configuration are always available.
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_vhost_user_config {
> > > +        le32 status;
> > > +#define VIRTIO_VHOST_USER_STATUS_BACKEND_UP (1 << 0)
> > > +#define VIRTIO_VHOST_USER_STATUS_FRONTEND_UP (1 << 1)
> > > +        le32 max_vhost_queues;
> > > +        u8 uuid[16];
> > > +};
> > > +\end{lstlisting}
> > > +
> > > +\begin{description}
> > > +\item[\field{status}] contains the vhost-user operational status.  The default
> > > +    value of this field is 0.
> > > +
> > > +    The driver sets VIRTIO_VHOST_USER_STATUS_BACKEND_UP to indicate readiness for
> > > +    the vhost-user frontend to connect.  The vhost-user frontend cannot connect
> > > +    unless the driver has set this bit first.
> > > +
> > > +    The device sets VIRTIO_VHOST_USER_STATUS_FRONTEND_UP to indicate that the
> > > +    vhost-user frontend is connected.
> > > +
> > > +    When the driver clears VIRTIO_VHOST_USER_STATUS_BACKEND_UP while the
> > > +    vhost-user frontend is connected, the vhost-user frontend is disconnected.
> > > +
> > > +    When the vhost-user frontend disconnects, both
> > > +    VIRTIO_VHOST_USER_STATUS_BACKEND_UP and VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
> > > +    are cleared by the device.  Communication can be restarted by the driver
> > > +    setting VIRTIO_VHOST_USER_STATUS_BACKEND_UP again.
> > > +
> > > +    A configuration change notification is sent when the device changes
> > > +    this field, unless a write to the field by the driver caused the change.
> > 
> > 
> > So backend might disconnect by the time we check it's UP again?
> > Seems racy, I suspect you need a handshake not unlike handshake.
> > 
> 
> I am assuming over here you mean when vhost-user frontend disconnects.
> 
> I realized that there is no need for VIRTIO_VHOST_USER_STATUS_BACKEND_UP to
> be cleared when the frontend disconnects.
> The backend driver can just stay connected to the backend device when the
> frontend disconnect. In the next revision, I can change the above paragraph
> to:
> 
> "When the vhost-user frontend disconnects,
> VIRTIO_VHOST_USER_STATUS_FRONTEND_UP is cleared by the device." (Nothing is
> done with VIRTIO_VHOST_USER_STATUS_BACKEND_UP)
> 
> I might have misunderstood, but I don't see a race condition or the
> advantage of a handshake connection when changed to above.
> Do you mean handshake connection between the backend driver and backend
> device, or between the frontend and the backend device? Will there still be
> a race condition if we dont disconnect the backend driver when the frontend
> disconnects?
> 

Oh I was confused between front end and back end. Sorry.
Actually this spec patch only describes backend device and driver.
There is front end in the picture but it is not defined by this spec.


So my question is this. Let's say frontend crashes and is restarted.
I think we want backend to notice and react right?
However, at the moment VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
will just go down then up by the time driver goes to look at
the status it might be up again.



> > > +
> > > +\item[\field{max_vhost_queues}] is the maximum number of vhost-user queues
> > > +    supported by this device.  This field is always greater than 0.
> > > +
> > > +\item[\field{uuid}] is the Universally Unique Identifier (UUID) for this
> > > +    device. If the device has no UUID then this field contains the nil
> > > +    UUID (all zeroes).  The UUID allows vhost-user backend programs to identify a
> > > +    specific vhost-user device backend among many without relying on bus
> > > +    addresses.
> > 
> > Why UUID specifically? Is there a need for them to be globally unique
> > and not just within a VM? These are annoying to generate and maintain
> > ...
> > 
> > 
> 
> That makes sense, no need for UUID, just need an ID unique within a VM. I
> will change it to just ID in next version.
> 
> > 
> > 
> > > +\end{description}
> > 
> > So feel this basically means that for frontend driver to attack to device
> > backend driver must be up and responding to messages.
> > I wonder whether we can make things a bit more robust
> > by having backend driver send at least basic device
> > config and feature bits to the vhost-user device
> > at boot.
> > 
> > Of course device can request these itself if it wants to -
> > so we do not need to change the spec for this, but maybe
> > add an implementation note explaining this idea.
> > 
> > 
> 
> Yes, for the frontend to connect to backend device, the backend driver must
> be up.
> 
> 
> In the prototype as well, the backend driver in DPDK does check if the
> virtio features are OK during init.
> 
> I will change the Device Initialization section below to:
> 
> The driver checks if the virtio features supported by it are provided by the
> device, initializes the rxq/txq virtqueues and then sets
> VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the status field of the device
> configuration structure.
> 
> if that makes things better?

So I was confused with frontend/backend. Is backend ever down then
while driver is attached? Maybe just DRIVER_OK is enough - no?


> > 
> > 
> > 
> > 
> > > +
> > > +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Vhost-user Device Backend / Device configuration layout}
> > > +
> > > +The driver MUST NOT write to device configuration fields other than \field{status}.
> > > +
> > > +The driver MUST NOT set undefined bits in the \field{status} configuration field.
> > > +
> > > +\subsection{Device Initialization}\label{sec:Device Types / Vhost-user Device Backend / Device Initialization}
> > > +
> > > +The driver initializes the rxq/txq virtqueues and then it sets
> > > +VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the \field{status} field of the device
> > > +configuration structure.
> > > +
> > > +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Vhost-user Device Backend / Device Initialization}
> > > +
> > > +The driver SHOULD check the \field{max_vhost_queues} configuration field to
> > > +determine how many queues the vhost-user backend will be able to support.
> > > +
> > > +The driver SHOULD fetch the \field{uuid} configuration field to allow
> > > +vhost-user backend programs to identify a specific device among many.
> > > +
> > > +The driver SHOULD place at least one buffer in rxq before setting the
> > > +VIRTIO_VHOST_USER_STATUS_BACKEND_UP bit in the \field{status} configuration field.
> > > +
> > > +The driver MUST handle rxq virtqueue notifications that occur before the
> > > +configuration change notification.  It is possible that a vhost-user protocol
> > > +message from the vhost-user frontend arrives before the driver has seen the
> > > +configuration change notification for the VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
> > > +\field{status} change.
> > > +
> > > +\subsection{Device Operation}\label{sec:Device Types / Vhost-user Device Backend / Device Operation}
> > > +
> > > +Device operation consists of operating request queues and response queues.
> > > +
> > > +\subsubsection{Device Operation: Request Queues}\label{sec:Device Types / Vhost-user Device Backend / Device Operation / Device Operation: RX/TX Queues}
> > > +
> > > +The driver receives vhost-user protocol messages from the vhost-user frontend on
> > > +rxq. The driver sends responses to the vhost-user frontend on txq.
> > > +
> > > +The driver sends backend-initiated requests on txq. The driver receives
> > > +responses from the vhost-user frontend on rxq.
> > > +
> > > +All virtqueues offer in-order guaranteed delivery semantics for vhost-user
> > > +protocol messages.
> > > +
> > > +Each buffer is a vhost-user protocol message as defined by the
> > > +\hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}.  In order to enable
> > > +cross-endian communication, all message fields are little-endian instead of the
> > > +native byte order normally used by the protocol.
> > 
> > 
> > I wonder how clear this is to the reader.
> > 
> > I feel maybe being more explicit is called for.
> > For example explain that "u64" should be interpreted as "le64".
> > 
> > 
> 
> Will change to:
> "all message fields are little-endian instead of the native byte order
> normally used by the protocol, i.e. u64 message fields are interpreted as
> le64"
> 
> if thats better?

I suggest listing all types in the vhost-user spec and how they
should be interpreted, not by example.

> > 
> > 
> > > +
> > > +The appropriate size of rxq buffers is at least as large as the largest message
> > > +defined by the \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}
> > > +standard version that the driver supports.  If the vhost-user frontend sends a
> > > +message that is too large for an rxq buffer, then DEVICE_NEEDS_RESET is set and
> > > +the driver must reset the device.
> > > +
> > > +File descriptor passing is handled differently by the vhost-user device
> > > +backend. When a frontend-initiated message is received that carries one or more file
> > > +descriptors according to the vhost-user protocol, additional device resources
> > > +become available to the driver.
> > > +
> > > +\subsection{Additional Device Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources}
> > > +
> > > +The vhost-user device backend uses the following facilities from virtio device
> > > +\ref{sec:Basic Facilities of a Virtio Device} for the vhost-user frontend and
> > > +backend to exchange notifications and data through the device:
> > > +
> > > +\begin{description}
> > > +  \item[Device auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
> > > +The driver signals the vhost-user frontend through device auxiliary notifications. The signal does not
> > > +carry any data, it is purely an event.
> > > +  \item[Driver auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
> > > +The vhost-user frontend signals the driver for events besides virtqueue activity
> > > +and configuration changes by sending driver auxiliary notification.
> > > +  \item[Shared memory] \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
> > > +The vhost-user frontend gives access to memory that can be mapped by the driver.
> > > +\end{description}
> > > +
> > > +\subsubsection{Device auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Device auxiliary notifications}
> > > +
> > > +The vhost-user device backend provides all (or part) of the following device auxiliary notifications:
> > > +
> > > +\begin{description}
> > > +\item[0] Vring call for vhost-user queue 0
> > > +\item[\ldots]
> > > +\item[N-1] Vring call for vhost-user queue N-1
> > > +\item[N] Vring err for vhost-user queue 0
> > > +\item[\ldots]
> > > +\item[2N-1] Vring err for vhost-user queue N-1
> > > +\item[2N] Log
> > > +\end{description}
> > > +
> > > +where N is the number of the vhost-user virtqueues.
> > > +
> > > +\subsubsection{Driver auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Driver auxiliary notifications}
> > > +
> > > +The vhost-user device backend provides all (or part) of the following driver auxiliary notifications:
> > > +
> > > +\begin{description}
> > > +\item[0] Vring kick for vhost-user queue 0
> > > +\item[\ldots]
> > > +\item[N-1] Vring kick for vhost-user queue N-1
> > > +\end{description}
> > > +
> > > +where N is the number of the vhost-user virtqueues.
> > > +
> > > +\subsubsection{Shared Memory}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory}
> > > +
> > > +The vhost-user device backend provides all (or part) of the following shared memory regions:
> > > +
> > > +\begin{description}
> > > +\item[0] Vhost-user memory region 0
> > > +\item[1] Vhost-user memory region 1
> > > +\item[\ldots]
> > > +\item[M-1] Vhost-user memory region M-1
> > > +\item[M] Log memory region
> > > +\end{description}
> > > +
> > > +where M is the total number of memory regions shared.
> > > +
> > > +\devicenormative{\paragraph}{Shared Memory layout}{Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
> > > +
> > > +The device exports all memory regions reported by the vhost-user frontend as a
> > > +single shared memory region \ref{sec:Basic Facilities of a Virtio Device /
> > > +Shared Memory Regions}.
> > > +
> > > +The size of this shared memory region exported by the device MUST be at least
> > > +as much as the sum of the sizes of all the memory regions reported by the
> > > +vhost-user frontend.
> > > +
> > > +The memory regions exported by the device MUST be laid out in the same order
> > > +in which they are reported by the frontend with vhost-user messages.
> > 
> > This is vague. The driver is the front end here, isn't it?
> > Are you talking about the "Memory regions description" section
> > in the vhost-user spec?
> > 
> > 
> 
> This section changed in v2, when Stefan suggested that this doesn't take
> into account VHOST_USER_ADD_MEM_REG/VHOST_USER_REM_MEM_REG. So might be
> better to re-review in v2.
> 
> In the document there are references to 3 things, the frontend (which is the
> same as the the frontend in vhost-user spec), the backend driver and the
> backend device. I see that I have just written driver/device in many places
> in this doc without prefixing with backend which might make things vague? I
> will prefix all driver/device instances in the doc with backend if it makes
> it better.

Maybe the device should be named vhost user backend device.
And add a paragraph repeating what you just wrote here:
device and driver together serve to implement the vhost-user backend.

You don't have to prefix it all, just once in a while is enough.


> > > +
> > > +The offsets in which the memory regions are mapped inside the shared memory
> > > +region MUST be the following:
> > > +
> > > +\begin{description}
> > > +\item[0] Offset for vhost-user memory region 0
> > > +\item[SIZE0] Offset for vhost-user memory region 1
> > > +\item[\ldots]
> > > +\item[SIZE0 + SIZE1 + \ldots] Offset for vhost-user memory region M
> > > +\end{description}
> > > +
> > > +where SIZEi is the size of the vhost-user memory region i.
> > > +
> > > +\subsubsection{Availability of Additional Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Availability of Additional Resources}
> > > +
> > > +The following vhost-user protocol messages convey access to additional device
> > > +resources:
> > > +
> > > +\begin{description}
> > > +\item[VHOST_USER_SET_MEM_TABLE] Contents of vhost-user memory regions are
> > > +available to the driver as device memory. Region contents are laid out in the
> > > +same order as the vhost-user memory region list.
> > > +\item[VHOST_USER_SET_LOG_BASE] Contents of the log memory region are available
> > > +to the driver as device memory.
> > > +\item[VHOST_USER_SET_LOG_FD] The log device auxiliary notification is available to the driver.
> > > +Writes to the log device auxiliary notification before this message is received produce no effect.
> > > +\item[VHOST_USER_SET_VRING_KICK] The vring kick notification for this queue is
> > > +available to the driver. The first notification may occur before the driver has
> > > +processed this message.
> > > +\item[VHOST_USER_SET_VRING_CALL] The vring call device auxiliary notification for this queue is
> > > +available to the driver. Writes to the vring call device auxiliary notification before this message
> > > +is received produce no effect.
> > > +\item[VHOST_USER_SET_VRING_ERR] The vring err device auxiliary notification for this queue is
> > > +available to the driver. Writes to the vring err device auxiliary notification before this message
> > > +is received produce no effect.
> > > +\item[VHOST_USER_SET_SLAVE_REQ_FD] The driver may send vhost-user protocol
> > > +backend messages on txq. Backend-initiated messages put onto txq before this
> > > +message is received are discarded by the device.
> > 
> > What about VHOST_USER_GET_INFLIGHT_FD?
> > 
> > Generally I worry that new versions of the protocol
> > add new features without regard for this reuse.
> > 
> > Do we want to refer to a specific version of the spec maybe?
> > 
> > 
> 
> I think it would be a good idea to link to a specific version of vhost-user
> spec.
> 
> I didn't take into account VHOST_USER_GET_INFLIGHT_FD. From reading about it
> on the vhost-user spec now, it looks like another region is needed in shared
> memory layout for this.
> 
> The access to additional device resources will look the same for both
> get/set inflight_fd.
> 
> \item[VHOST_USER_GET_INFLIGHT_FD] Contents of the inflight memory region are
> available to the backend driver as backend device memory.
> \item[VHOST_USER_SET_INFLIGHT_FD] Contents of the inflight memory region are
> available to the backend driver as backend device memory.
> 
> and we will need to add another region shared memory when referencing the v2
> patch. i.e.
> 
> \begin{description}
> \item[0] Vhost-user memory region 0
> \item[1] Vhost-user memory region 1
> \item[\ldots]
> \item[M-2] Vhost-user memory region M-1
> \item[M-1] Log memory region
> \item[M] Inflight memory region
> \end{description}

Do we then assume that driver sets the inflight bits?

Should we allow both to do it?

> 
> I will send these changes in v3 of the patch if this and the above comments
> look good.
> 
> Thanks!

Note these are all gated by protocol features.

I wonder whether protocol features should be up to driver or
up to device. What do you think?


> > 
> > > +\end{description}
> > > -- 
> > > 2.25.1
> > 

Re: [External] Re: [PATCH 4/4] vhost-user: add vhost-user device type

[Usama Arif]

On 11/08/2022 12:20, Michael S. Tsirkin wrote:
> On Thu, Aug 11, 2022 at 11:05:16AM +0100, Usama Arif wrote:
>>
>>
>> On 09/08/2022 21:06, Michael S. Tsirkin wrote:
>>> On Wed, Mar 30, 2022 at 04:21:05PM +0100, Usama Arif wrote:
>>>> The vhost-user device backend facilitates vhost-user device emulation
>>>> through vhost-user protocol exchanges and access to shared memory.
>>>> Software-defined networking, storage, and other I/O appliances can
>>>> provide services through this device.
>>>>
>>>> This device is based on Wei Wang's vhost-pci work.  The virtio
>>>> vhost-user device differs from vhost-pci because it is a single virtio
>>>> device type that exposes the vhost-user protocol instead of a family of
>>>> new virtio device types, one for each vhost-user device type.
>>>>
>>>> This device supports vhost-user backend and vhost-user frontend
>>>> reconnection. It also contains a UUID so that vhost-user backend programs
>>>> can identify a specific device among many without using bus addresses.
>>>>
>>>> virtio-vhost-user makes use of additional resources introduced in earlier
>>>> patches including device aux. notifications, driver aux. notifications,
>>>> as well as shared memory.
>>>>
>>>> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
>>>
>>> Sorry about the delay in review.
>>>
>>>
>>
>> Hi,
>>
>> No worries, Thanks for the review!
>>
>> I had sent a v2
>> (https://lists.oasis-open.org/archives/virtio-dev/202204/msg00022.html) of
>> the patch in April to address Stefans' comments, so it might be good to
>> review that. I have replied inline below to the comments.
>>
>>>
>>>
>>>> ---
>>>>    conformance.tex       |  27 ++++-
>>>>    content.tex           |   3 +
>>>>    introduction.tex      |   3 +
>>>>    virtio-vhost-user.tex | 259 ++++++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 288 insertions(+), 4 deletions(-)
>>>>    create mode 100644 virtio-vhost-user.tex
>>>>
>>>> diff --git a/conformance.tex b/conformance.tex
>>>> index cddaf75..fab49c3 100644
>>>> --- a/conformance.tex
>>>> +++ b/conformance.tex
>>>> @@ -32,8 +32,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>    \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
>>>>    \ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
>>>>    \ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance},
>>>> -\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance} or
>>>> -\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance}.
>>>> +\ref{sec:Conformance / Driver Conformance / GPIO Driver Conformance},
>>>> +\ref{sec:Conformance / Driver Conformance / PMEM Driver Conformance} or
>>>> +\ref{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}.
>>>>        \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>>      \end{itemize}
>>>> @@ -58,8 +59,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>    \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
>>>>    \ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
>>>>    \ref{sec:Conformance / Device Conformance / SCMI Device Conformance},
>>>> -\ref{sec:Conformance / Device Conformance / GPIO Device Conformance} or
>>>> -\ref{sec:Conformance / Device Conformance / PMEM Device Conformance}.
>>>> +\ref{sec:Conformance / Device Conformance / GPIO Device Conformance},
>>>> +\ref{sec:Conformance / Device Conformance / PMEM Device Conformance} or
>>>> +\ref{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}.
>>>>        \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>>>>      \end{itemize}
>>>> @@ -324,6 +326,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>    \item \ref{drivernormative:Device Types / PMEM Device / Device Initialization}
>>>>    \end{itemize}
>>>> +\conformance{\subsection}{Vhost-user Backend Driver Conformance}\label{sec:Conformance / Driver Conformance / Vhost-user Backend Driver Conformance}
>>>> +
>>>> +A vhost-user backend driver MUST conform to the following normative statements:
>>>> +
>>>> +\begin{itemize}
>>>> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device configuration layout}
>>>> +\item \ref{drivernormative:Device Types / Vhost-user Device Backend / Device Initialization}
>>>> +\end{itemize}
>>>> +
>>>>    \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>>>>    A device MUST conform to the following normative statements:
>>>> @@ -595,6 +606,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>>>>    \item \ref{devicenormative:Device Types / PMEM Device / Device Operation / Virtqueue return}
>>>>    \end{itemize}
>>>> +\conformance{\subsection}{Vhost-user Backend Device Conformance}\label{sec:Conformance / Device Conformance / Vhost-user Backend Device Conformance}
>>>> +
>>>> +A Vhost-user backend device MUST conform to the following normative statements:
>>>> +
>>>> +\begin{itemize}
>>>> +\item \ref{devicenormative:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
>>>> +\end{itemize}
>>>> +
>>>>    \conformance{\section}{Legacy Interface: Transitional Device and Transitional Driver Conformance}\label{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}
>>>>    A conformant implementation MUST be either transitional or
>>>>    non-transitional, see \ref{intro:Legacy
>>>> diff --git a/content.tex b/content.tex
>>>> index 0fc50c4..8bf114d 100644
>>>> --- a/content.tex
>>>> +++ b/content.tex
>>>> @@ -3122,6 +3122,8 @@ \chapter{Device Types}\label{sec:Device Types}
>>>>    \hline
>>>>    42         &   RDMA device \\
>>>>    \hline
>>>> +43         &   vhost-user device backend \ \\
>>>> +\hline
>>>>    \end{tabular}
>>>>    Some of the devices above are unspecified by this document,
>>>> @@ -6878,6 +6880,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>>>>    \input{virtio-scmi.tex}
>>>>    \input{virtio-gpio.tex}
>>>>    \input{virtio-pmem.tex}
>>>> +\input{virtio-vhost-user.tex}
>>>>    \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>>>> diff --git a/introduction.tex b/introduction.tex
>>>> index 6d52717..5bd1b95 100644
>>>> --- a/introduction.tex
>>>> +++ b/introduction.tex
>>>> @@ -79,6 +79,9 @@ \section{Normative References}\label{sec:Normative References}
>>>>    	\phantomsection\label{intro:SCMI}\textbf{[SCMI]} &
>>>>    	Arm System Control and Management Interface, DEN0056,
>>>>    	\newline\url{https://developer.arm.com/docs/den0056/c}, version C and any future revisions\\
>>>> +	\phantomsection\label{intro:Vhost-user Protocol}\textbf{[Vhost-user Protocol]}
>>>> +	& Vhost-user Protocol,
>>>> +	\newline\url{https://qemu.readthedocs.io/en/latest/interop/vhost-user.html}\\
>>>>    \end{longtable}
>>>> diff --git a/virtio-vhost-user.tex b/virtio-vhost-user.tex
>>>> new file mode 100644
>>>> index 0000000..303054f
>>>> --- /dev/null
>>>> +++ b/virtio-vhost-user.tex
>>>> @@ -0,0 +1,259 @@
>>>> +\section{Vhost-user Device Backend}\label{sec:Device Types / Vhost-user Device
>>>> +Backend}
>>>> +
>>>> +The vhost-user device backend facilitates vhost-user device emulation through
>>>> +vhost-user protocol exchanges and access to shared memory. Software-defined
>>>> +networking, storage, and other I/O appliances can provide services through this
>>>> +device.
>>>> +
>>>> +This section relies on definitions from the \hyperref[intro:Vhost-user
>>>> +Protocol]{Vhost-user Protocol}.  Knowledge of the vhost-user protocol is a
>>>> +prerequisite for understanding this device.
>>>> +
>>>> +The \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol} was originally
>>>> +designed for processes on a single system communicating over UNIX domain
>>>> +sockets. The virtio vhost-user device backend allows the vhost-user backend to
>>>> +communicate with the vhost-user frontend over the device instead of a UNIX domain
>>>> +socket. This allows the backend and frontend to run on two separate systems such
>>>> +as a virtual machine and a hypervisor.
>>>> +
>>>> +The vhost-user backend program exchanges vhost-user protocol messages with the
>>>> +vhost-user frontend through this device. How the device implementation
>>>> +communicates with the vhost-user frontend is beyond the scope of this
>>>> +specification.  One possible device implementation uses a UNIX domain socket to
>>>> +relay messages to a vhost-user frontend process running on the same host.
>>>> +
>>>> +Existing vhost-user backend programs that communicate over UNIX domain sockets
>>>> +can support the virtio vhost-user device backend without invasive changes
>>>> +because the pre-existing vhost-user wire protocol is used.
>>>> +
>>>> +\subsection{Device ID}\label{sec:Device Types / Vhost-user Device Backend / Device ID}
>>>> +  43
>>>> +
>>>> +\subsection{Virtqueues}\label{sec:Device Types / Vhost-user Device Backend / Virtqueues}
>>>> +
>>>> +\begin{description}
>>>> +\item[0] rxq (device-to-driver vhost-user protocol messages)
>>>> +\item[1] txq (driver-to-device vhost-user protocol messages)
>>>> +\end{description}
>>>> +
>>>> +\subsection{Feature bits}\label{sec:Device Types / Vhost-user Device Backend / Feature bits}
>>>> +
>>>> +No feature bits are defined at this time.
>>>> +
>>>> +\subsection{Device configuration layout}\label{sec:Device Types / Vhost-user Device Backend / Device configuration layout}
>>>> +
>>>> +  All fields of this configuration are always available.
>>>> +
>>>> +\begin{lstlisting}
>>>> +struct virtio_vhost_user_config {
>>>> +        le32 status;
>>>> +#define VIRTIO_VHOST_USER_STATUS_BACKEND_UP (1 << 0)
>>>> +#define VIRTIO_VHOST_USER_STATUS_FRONTEND_UP (1 << 1)
>>>> +        le32 max_vhost_queues;
>>>> +        u8 uuid[16];
>>>> +};
>>>> +\end{lstlisting}
>>>> +
>>>> +\begin{description}
>>>> +\item[\field{status}] contains the vhost-user operational status.  The default
>>>> +    value of this field is 0.
>>>> +
>>>> +    The driver sets VIRTIO_VHOST_USER_STATUS_BACKEND_UP to indicate readiness for
>>>> +    the vhost-user frontend to connect.  The vhost-user frontend cannot connect
>>>> +    unless the driver has set this bit first.
>>>> +
>>>> +    The device sets VIRTIO_VHOST_USER_STATUS_FRONTEND_UP to indicate that the
>>>> +    vhost-user frontend is connected.
>>>> +
>>>> +    When the driver clears VIRTIO_VHOST_USER_STATUS_BACKEND_UP while the
>>>> +    vhost-user frontend is connected, the vhost-user frontend is disconnected.
>>>> +
>>>> +    When the vhost-user frontend disconnects, both
>>>> +    VIRTIO_VHOST_USER_STATUS_BACKEND_UP and VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
>>>> +    are cleared by the device.  Communication can be restarted by the driver
>>>> +    setting VIRTIO_VHOST_USER_STATUS_BACKEND_UP again.
>>>> +
>>>> +    A configuration change notification is sent when the device changes
>>>> +    this field, unless a write to the field by the driver caused the change.
>>>
>>>
>>> So backend might disconnect by the time we check it's UP again?
>>> Seems racy, I suspect you need a handshake not unlike handshake.
>>>
>>
>> I am assuming over here you mean when vhost-user frontend disconnects.
>>
>> I realized that there is no need for VIRTIO_VHOST_USER_STATUS_BACKEND_UP to
>> be cleared when the frontend disconnects.
>> The backend driver can just stay connected to the backend device when the
>> frontend disconnect. In the next revision, I can change the above paragraph
>> to:
>>
>> "When the vhost-user frontend disconnects,
>> VIRTIO_VHOST_USER_STATUS_FRONTEND_UP is cleared by the device." (Nothing is
>> done with VIRTIO_VHOST_USER_STATUS_BACKEND_UP)
>>
>> I might have misunderstood, but I don't see a race condition or the
>> advantage of a handshake connection when changed to above.
>> Do you mean handshake connection between the backend driver and backend
>> device, or between the frontend and the backend device? Will there still be
>> a race condition if we dont disconnect the backend driver when the frontend
>> disconnects?
>>
> 
> Oh I was confused between front end and back end. Sorry.
> Actually this spec patch only describes backend device and driver.
> There is front end in the picture but it is not defined by this spec.
> 
> 
> So my question is this. Let's say frontend crashes and is restarted.
> I think we want backend to notice and react right?
> However, at the moment VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
> will just go down then up by the time driver goes to look at
> the status it might be up again.
> 
> 

When the frontend crashes, the backend device must clean up. For e.g. 
close the character device that was connected to socket to frontend and 
cleanup the kickfds and callfds. This is implementation specifc, so I 
guess we don't add this information to the virtio spec.

After that there are 2 possibilites, I tried both clearing/not clearing 
VIRTIO_VHOST_USER_STATUS_BACKEND_UP bit in the DPDK prototype and they 
both work:

- the backend device clears VIRTIO_VHOST_USER_STATUS_BACKEND_UP and 
notifies the backend driver via virtio config change. The driver 
receives the interrupt, does any required clean up and sets 
VIRTIO_VHOST_USER_STATUS_BACKEND_UP showing the willingness to connect 
again. If the frontend connects while the driver is still processing the 
first interrupt, and the backend device sets 
VIRTIO_VHOST_USER_STATUS_FRONTEND_UP it will
just be another virtio config change notifying the driver which 
processes a new connection. I think both interrupts will be processed 
one after the other and the
2nd interrupt wont just be ignored if the first interrupt of the 
disconnect is still being processed.

- the device doesnt clear VIRTIO_VHOST_USER_STATUS_BACKEND_UP and the 
driver isnt notified, there wont be any clean up from driver side.
When the frontend connects again, the device sets 
VIRTIO_VHOST_USER_STATUS_FRONTEND_UP, which is shown as a virtio config 
change notifying the driver as
an interrupt which processes a new connection overwriting the previous 
one. I tried this now and it worked, but I think there might be an issue 
if there is some complicated cleanup later required when frontend 
disconnects.

Any configuration change by device is sent as an interrupt to driver, 
and I guess if another change happens while the driver is processing an 
interrupt, the driver will just process them sequentially and it 
shouldnt really cause a race condition?

I think it is better to use the first approach and keep the line which 
was originally there, i.e.

"When the vhost-user frontend disconnects, both
VIRTIO_VHOST_USER_STATUS_BACKEND_UP and VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
are cleared by the device. Communication can be restarted by the driver
setting VIRTIO_VHOST_USER_STATUS_BACKEND_UP again.

A configuration change notification is sent when the device changes
this field, unless a write to the field by the driver caused the change."


> 
>>>> +
>>>> +\item[\field{max_vhost_queues}] is the maximum number of vhost-user queues
>>>> +    supported by this device.  This field is always greater than 0.
>>>> +
>>>> +\item[\field{uuid}] is the Universally Unique Identifier (UUID) for this
>>>> +    device. If the device has no UUID then this field contains the nil
>>>> +    UUID (all zeroes).  The UUID allows vhost-user backend programs to identify a
>>>> +    specific vhost-user device backend among many without relying on bus
>>>> +    addresses.
>>>
>>> Why UUID specifically? Is there a need for them to be globally unique
>>> and not just within a VM? These are annoying to generate and maintain
>>> ...
>>>
>>>
>>
>> That makes sense, no need for UUID, just need an ID unique within a VM. I
>> will change it to just ID in next version.
>>
>>>
>>>
>>>> +\end{description}
>>>
>>> So feel this basically means that for frontend driver to attack to device
>>> backend driver must be up and responding to messages.
>>> I wonder whether we can make things a bit more robust
>>> by having backend driver send at least basic device
>>> config and feature bits to the vhost-user device
>>> at boot.
>>>
>>> Of course device can request these itself if it wants to -
>>> so we do not need to change the spec for this, but maybe
>>> add an implementation note explaining this idea.
>>>
>>>
>>
>> Yes, for the frontend to connect to backend device, the backend driver must
>> be up.
>>
>>
>> In the prototype as well, the backend driver in DPDK does check if the
>> virtio features are OK during init.
>>
>> I will change the Device Initialization section below to:
>>
>> The driver checks if the virtio features supported by it are provided by the
>> device, initializes the rxq/txq virtqueues and then sets
>> VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the status field of the device
>> configuration structure.
>>
>> if that makes things better?
> 
> So I was confused with frontend/backend. Is backend ever down then
> while driver is attached? Maybe just DRIVER_OK is enough - no?
> 

I am not sure by "driver is attached" if you meant frontend driver or 
backend driver :), will reply for both:

If the backend goes down while the frontend is attached, the frontend 
networking/storage will also stop working.
Similar to what happens when simple vhost-user backend running in server 
mode goes down.

The backend device is up from the start e.g. from VM start and should 
never go down as long as the VM is working.
I think if the backend device goes down, probably something went wrong 
in the VM and backend driver will stop working as well.

> 
>>>
>>>
>>>
>>>
>>>> +
>>>> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Vhost-user Device Backend / Device configuration layout}
>>>> +
>>>> +The driver MUST NOT write to device configuration fields other than \field{status}.
>>>> +
>>>> +The driver MUST NOT set undefined bits in the \field{status} configuration field.
>>>> +
>>>> +\subsection{Device Initialization}\label{sec:Device Types / Vhost-user Device Backend / Device Initialization}
>>>> +
>>>> +The driver initializes the rxq/txq virtqueues and then it sets
>>>> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP to the \field{status} field of the device
>>>> +configuration structure.
>>>> +
>>>> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Vhost-user Device Backend / Device Initialization}
>>>> +
>>>> +The driver SHOULD check the \field{max_vhost_queues} configuration field to
>>>> +determine how many queues the vhost-user backend will be able to support.
>>>> +
>>>> +The driver SHOULD fetch the \field{uuid} configuration field to allow
>>>> +vhost-user backend programs to identify a specific device among many.
>>>> +
>>>> +The driver SHOULD place at least one buffer in rxq before setting the
>>>> +VIRTIO_VHOST_USER_STATUS_BACKEND_UP bit in the \field{status} configuration field.
>>>> +
>>>> +The driver MUST handle rxq virtqueue notifications that occur before the
>>>> +configuration change notification.  It is possible that a vhost-user protocol
>>>> +message from the vhost-user frontend arrives before the driver has seen the
>>>> +configuration change notification for the VIRTIO_VHOST_USER_STATUS_FRONTEND_UP
>>>> +\field{status} change.
>>>> +
>>>> +\subsection{Device Operation}\label{sec:Device Types / Vhost-user Device Backend / Device Operation}
>>>> +
>>>> +Device operation consists of operating request queues and response queues.
>>>> +
>>>> +\subsubsection{Device Operation: Request Queues}\label{sec:Device Types / Vhost-user Device Backend / Device Operation / Device Operation: RX/TX Queues}
>>>> +
>>>> +The driver receives vhost-user protocol messages from the vhost-user frontend on
>>>> +rxq. The driver sends responses to the vhost-user frontend on txq.
>>>> +
>>>> +The driver sends backend-initiated requests on txq. The driver receives
>>>> +responses from the vhost-user frontend on rxq.
>>>> +
>>>> +All virtqueues offer in-order guaranteed delivery semantics for vhost-user
>>>> +protocol messages.
>>>> +
>>>> +Each buffer is a vhost-user protocol message as defined by the
>>>> +\hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}.  In order to enable
>>>> +cross-endian communication, all message fields are little-endian instead of the
>>>> +native byte order normally used by the protocol.
>>>
>>>
>>> I wonder how clear this is to the reader.
>>>
>>> I feel maybe being more explicit is called for.
>>> For example explain that "u64" should be interpreted as "le64".
>>>
>>>
>>
>> Will change to:
>> "all message fields are little-endian instead of the native byte order
>> normally used by the protocol, i.e. u64 message fields are interpreted as
>> le64"
>>
>> if thats better?
> 
> I suggest listing all types in the vhost-user spec and how they
> should be interpreted, not by example.
> 

I was thinking more about this, and supporting cross-endian is probably 
not the right approach?

The backend doesn't really have a way to know if the messages from 
front-end are of a different endianness. So it wont really know if it 
needs to swap the bytes or not.

I think its better to restrict the backend VM to have the same 
endianness as the host, so the VhostUserMsg it receives from the 
frontend will be the same endian as the backend, if that makes sense?

>>>
>>>
>>>> +
>>>> +The appropriate size of rxq buffers is at least as large as the largest message
>>>> +defined by the \hyperref[intro:Vhost-user Protocol]{Vhost-user Protocol}
>>>> +standard version that the driver supports.  If the vhost-user frontend sends a
>>>> +message that is too large for an rxq buffer, then DEVICE_NEEDS_RESET is set and
>>>> +the driver must reset the device.
>>>> +
>>>> +File descriptor passing is handled differently by the vhost-user device
>>>> +backend. When a frontend-initiated message is received that carries one or more file
>>>> +descriptors according to the vhost-user protocol, additional device resources
>>>> +become available to the driver.
>>>> +
>>>> +\subsection{Additional Device Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources}
>>>> +
>>>> +The vhost-user device backend uses the following facilities from virtio device
>>>> +\ref{sec:Basic Facilities of a Virtio Device} for the vhost-user frontend and
>>>> +backend to exchange notifications and data through the device:
>>>> +
>>>> +\begin{description}
>>>> +  \item[Device auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
>>>> +The driver signals the vhost-user frontend through device auxiliary notifications. The signal does not
>>>> +carry any data, it is purely an event.
>>>> +  \item[Driver auxiliary notification] \ref{sec:Basic Facilities of a Virtio Device / Notifications}
>>>> +The vhost-user frontend signals the driver for events besides virtqueue activity
>>>> +and configuration changes by sending driver auxiliary notification.
>>>> +  \item[Shared memory] \ref{sec:Basic Facilities of a Virtio Device / Shared Memory Regions}
>>>> +The vhost-user frontend gives access to memory that can be mapped by the driver.
>>>> +\end{description}
>>>> +
>>>> +\subsubsection{Device auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Device auxiliary notifications}
>>>> +
>>>> +The vhost-user device backend provides all (or part) of the following device auxiliary notifications:
>>>> +
>>>> +\begin{description}
>>>> +\item[0] Vring call for vhost-user queue 0
>>>> +\item[\ldots]
>>>> +\item[N-1] Vring call for vhost-user queue N-1
>>>> +\item[N] Vring err for vhost-user queue 0
>>>> +\item[\ldots]
>>>> +\item[2N-1] Vring err for vhost-user queue N-1
>>>> +\item[2N] Log
>>>> +\end{description}
>>>> +
>>>> +where N is the number of the vhost-user virtqueues.
>>>> +
>>>> +\subsubsection{Driver auxiliary notifications}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Driver auxiliary notifications}
>>>> +
>>>> +The vhost-user device backend provides all (or part) of the following driver auxiliary notifications:
>>>> +
>>>> +\begin{description}
>>>> +\item[0] Vring kick for vhost-user queue 0
>>>> +\item[\ldots]
>>>> +\item[N-1] Vring kick for vhost-user queue N-1
>>>> +\end{description}
>>>> +
>>>> +where N is the number of the vhost-user virtqueues.
>>>> +
>>>> +\subsubsection{Shared Memory}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory}
>>>> +
>>>> +The vhost-user device backend provides all (or part) of the following shared memory regions:
>>>> +
>>>> +\begin{description}
>>>> +\item[0] Vhost-user memory region 0
>>>> +\item[1] Vhost-user memory region 1
>>>> +\item[\ldots]
>>>> +\item[M-1] Vhost-user memory region M-1
>>>> +\item[M] Log memory region
>>>> +\end{description}
>>>> +
>>>> +where M is the total number of memory regions shared.
>>>> +
>>>> +\devicenormative{\paragraph}{Shared Memory layout}{Device Types / Vhost-user Device Backend / Additional Device Resources / Shared Memory layout}
>>>> +
>>>> +The device exports all memory regions reported by the vhost-user frontend as a
>>>> +single shared memory region \ref{sec:Basic Facilities of a Virtio Device /
>>>> +Shared Memory Regions}.
>>>> +
>>>> +The size of this shared memory region exported by the device MUST be at least
>>>> +as much as the sum of the sizes of all the memory regions reported by the
>>>> +vhost-user frontend.
>>>> +
>>>> +The memory regions exported by the device MUST be laid out in the same order
>>>> +in which they are reported by the frontend with vhost-user messages.
>>>
>>> This is vague. The driver is the front end here, isn't it?
>>> Are you talking about the "Memory regions description" section
>>> in the vhost-user spec?
>>>
>>>
>>
>> This section changed in v2, when Stefan suggested that this doesn't take
>> into account VHOST_USER_ADD_MEM_REG/VHOST_USER_REM_MEM_REG. So might be
>> better to re-review in v2.
>>
>> In the document there are references to 3 things, the frontend (which is the
>> same as the the frontend in vhost-user spec), the backend driver and the
>> backend device. I see that I have just written driver/device in many places
>> in this doc without prefixing with backend which might make things vague? I
>> will prefix all driver/device instances in the doc with backend if it makes
>> it better.
> 
> Maybe the device should be named vhost user backend device.
> And add a paragraph repeating what you just wrote here:
> device and driver together serve to implement the vhost-user backend.
> 
> You don't have to prefix it all, just once in a while is enough.
> 

I will make sure to add to add a few lines at the start of the doc in 
the next version.


> 
>>>> +
>>>> +The offsets in which the memory regions are mapped inside the shared memory
>>>> +region MUST be the following:
>>>> +
>>>> +\begin{description}
>>>> +\item[0] Offset for vhost-user memory region 0
>>>> +\item[SIZE0] Offset for vhost-user memory region 1
>>>> +\item[\ldots]
>>>> +\item[SIZE0 + SIZE1 + \ldots] Offset for vhost-user memory region M
>>>> +\end{description}
>>>> +
>>>> +where SIZEi is the size of the vhost-user memory region i.
>>>> +
>>>> +\subsubsection{Availability of Additional Resources}\label{sec:Device Types / Vhost-user Device Backend / Additional Device Resources / Availability of Additional Resources}
>>>> +
>>>> +The following vhost-user protocol messages convey access to additional device
>>>> +resources:
>>>> +
>>>> +\begin{description}
>>>> +\item[VHOST_USER_SET_MEM_TABLE] Contents of vhost-user memory regions are
>>>> +available to the driver as device memory. Region contents are laid out in the
>>>> +same order as the vhost-user memory region list.
>>>> +\item[VHOST_USER_SET_LOG_BASE] Contents of the log memory region are available
>>>> +to the driver as device memory.
>>>> +\item[VHOST_USER_SET_LOG_FD] The log device auxiliary notification is available to the driver.
>>>> +Writes to the log device auxiliary notification before this message is received produce no effect.
>>>> +\item[VHOST_USER_SET_VRING_KICK] The vring kick notification for this queue is
>>>> +available to the driver. The first notification may occur before the driver has
>>>> +processed this message.
>>>> +\item[VHOST_USER_SET_VRING_CALL] The vring call device auxiliary notification for this queue is
>>>> +available to the driver. Writes to the vring call device auxiliary notification before this message
>>>> +is received produce no effect.
>>>> +\item[VHOST_USER_SET_VRING_ERR] The vring err device auxiliary notification for this queue is
>>>> +available to the driver. Writes to the vring err device auxiliary notification before this message
>>>> +is received produce no effect.
>>>> +\item[VHOST_USER_SET_SLAVE_REQ_FD] The driver may send vhost-user protocol
>>>> +backend messages on txq. Backend-initiated messages put onto txq before this
>>>> +message is received are discarded by the device.
>>>
>>> What about VHOST_USER_GET_INFLIGHT_FD?
>>>
>>> Generally I worry that new versions of the protocol
>>> add new features without regard for this reuse.
>>>
>>> Do we want to refer to a specific version of the spec maybe?
>>>
>>>
>>
>> I think it would be a good idea to link to a specific version of vhost-user
>> spec.
>>
>> I didn't take into account VHOST_USER_GET_INFLIGHT_FD. From reading about it
>> on the vhost-user spec now, it looks like another region is needed in shared
>> memory layout for this.
>>
>> The access to additional device resources will look the same for both
>> get/set inflight_fd.
>>
>> \item[VHOST_USER_GET_INFLIGHT_FD] Contents of the inflight memory region are
>> available to the backend driver as backend device memory.
>> \item[VHOST_USER_SET_INFLIGHT_FD] Contents of the inflight memory region are
>> available to the backend driver as backend device memory.
>>
>> and we will need to add another region shared memory when referencing the v2
>> patch. i.e.
>>
>> \begin{description}
>> \item[0] Vhost-user memory region 0
>> \item[1] Vhost-user memory region 1
>> \item[\ldots]
>> \item[M-2] Vhost-user memory region M-1
>> \item[M-1] Log memory region
>> \item[M] Inflight memory region
>> \end{description}
> 
> Do we then assume that driver sets the inflight bits?
> 
> Should we allow both to do it?
> 

I believe the information about the vhost_virtqueues will only be setup 
in the driver and not the device, So i guess the driver would handle the 
inflight bits.

>>
>> I will send these changes in v3 of the patch if this and the above comments
>> look good.
>>
>> Thanks!
> 
> Note these are all gated by protocol features.
> 
> I wonder whether protocol features should be up to driver or
> up to device. What do you think?
> 
> 

Even if the device is just forwarding the messages to driver, it still 
would need the logic to forward it.
Reading from the vhost-user spec, I believe only the driver will be able 
to do the actual logging of used ring writes and fill the buffer with 
inflight descriptors.
So I believe both backend device and backend driver will need to support 
the specific protocol features.


>>>
>>>> +\end{description}
>>>> -- 
>>>> 2.25.1
>>>
> 

Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Michael S. Tsirkin]

On Wed, Mar 30, 2022 at 04:21:02PM +0100, Usama Arif wrote:
> Driver auxiliary notifications allow the device to send notifications
> other than configuration changes and used buffer notifications to the
> driver, these are optional and their meaning is device-specific.
> 
> Device auxiliary notifcations allow the driver to send notifcations
> other than available buffer notifications to the device for example
> through a device register, these are optional and their meaning is
> device-specific.
> 
> These device-specific notifications are needed later when adding support
> for virtio-vhost-user device.
> 
> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>

I see ccw is missing. Cornelia, any suggestions?

> ---
>  content.tex | 35 ++++++++++++++++++++++-------------
>  1 file changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index c6f116c..85980ac 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -160,29 +160,38 @@ \subsection{Legacy Interface: A Note on Feature
>  Specification text within these sections generally does not apply
>  to non-transitional devices.
>  
> -\section{Notifications}\label{sec:Basic Facilities of a Virtio Device
> -/ Notifications}
> +\section{Notifications}\label{sec:Basic Facilities of a Virtio Device / Notifications}
>  
>  The notion of sending a notification (driver to device or device
>  to driver) plays an important role in this specification. The
>  modus operandi of the notifications is transport specific.
>  
> -There are three types of notifications: 
> +There are five types of notifications:
>  \begin{itemize}
>  \item configuration change notification
>  \item available buffer notification
> -\item used buffer notification. 
> +\item used buffer notification
> +\item driver auxiliary notification
> +\item device auxiliary notification
>  \end{itemize}
>  
> -Configuration change notifications and used buffer notifications are sent
> -by the device, the recipient is the driver. A configuration change
> -notification indicates that the device configuration space has changed; a
> -used buffer notification indicates that a buffer may have been made used
> -on the virtqueue designated by the notification.
> -
> -Available buffer notifications are sent by the driver, the recipient is
> -the device. This type of notification indicates that a buffer may have
> -been made available on the virtqueue designated by the notification.
> +Configuration change notifications, used buffer notifications and
> +driver auxiliary notifications are sent by the device,
> +the recipient is the driver. A configuration change notification indicates
> +that the device configuration space has changed; a used buffer notification
> +indicates that a buffer may have been made used on the virtqueue designated
> +by the notification; driver auxiliary notifications allow the
> +device to send notifications other than configuration changes and used
> +buffer notifications to the driver, these are optional and their meaning
> +is device-specific.
> +
> +Available buffer notifications and device auxiliary notifications
> +are sent by the driver, the recipient is the device. Available buffer
> +notifications indicate that a buffer may have been made available on the
> +virtqueue designated by the notification; device auxiliary
> +notifcations allow the driver to send notifcations other than available
> +buffer notifications to the device for example through a device register, these
> +are optional and their meaning is device-specific.
>  
>  The semantics, the transport-specific implementations, and other
>  important aspects of the different notifications are specified in detail
> -- 
> 2.25.1

[virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Cornelia Huck]

On Tue, Aug 09 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Mar 30, 2022 at 04:21:02PM +0100, Usama Arif wrote:
>> Driver auxiliary notifications allow the device to send notifications
>> other than configuration changes and used buffer notifications to the
>> driver, these are optional and their meaning is device-specific.
>> 
>> Device auxiliary notifcations allow the driver to send notifcations
>> other than available buffer notifications to the device for example
>> through a device register, these are optional and their meaning is
>> device-specific.
>> 
>> These device-specific notifications are needed later when adding support
>> for virtio-vhost-user device.
>> 
>> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
>
> I see ccw is missing. Cornelia, any suggestions?

Hmm... I seem to be really behind on ccw things :(

We can probably use the following:

- for device->driver notification, use the next bit in the secondary
  indicators (bit 0 is used for config change notification)
- for driver->device notification, maybe use a new subcode for diagnose
  0x500 (4 is probably the next free one?)

I have not looked at the requirements deeply, though.

This highlights another problem, however: When we introduce new features
that require a transport-specific implementation, we often end up with a
PCI implementation, but sometimes MMIO and more often ccw are left
behind -- which is understandable, as PCI is what most people use, and
ccw is something only a very few people are familiar with. This sadly
means that we have a backlog of features supported in PCI, but not in
ccw... requiring implementations for ccw would put an undue burden on
contributors, as most of them are unlikely to write anything for a
mainframe, ever. On the flip side, I do not have enough bandwith to deal
with all of this.

Halil, any thoughts (on any of the above)?

>
>> ---
>>  content.tex | 35 ++++++++++++++++++++++-------------
>>  1 file changed, 22 insertions(+), 13 deletions(-)
>> 
>> diff --git a/content.tex b/content.tex
>> index c6f116c..85980ac 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -160,29 +160,38 @@ \subsection{Legacy Interface: A Note on Feature
>>  Specification text within these sections generally does not apply
>>  to non-transitional devices.
>>  
>> -\section{Notifications}\label{sec:Basic Facilities of a Virtio Device
>> -/ Notifications}
>> +\section{Notifications}\label{sec:Basic Facilities of a Virtio Device / Notifications}
>>  
>>  The notion of sending a notification (driver to device or device
>>  to driver) plays an important role in this specification. The
>>  modus operandi of the notifications is transport specific.
>>  
>> -There are three types of notifications: 
>> +There are five types of notifications:
>>  \begin{itemize}
>>  \item configuration change notification
>>  \item available buffer notification
>> -\item used buffer notification. 
>> +\item used buffer notification
>> +\item driver auxiliary notification
>> +\item device auxiliary notification
>>  \end{itemize}
>>  
>> -Configuration change notifications and used buffer notifications are sent
>> -by the device, the recipient is the driver. A configuration change
>> -notification indicates that the device configuration space has changed; a
>> -used buffer notification indicates that a buffer may have been made used
>> -on the virtqueue designated by the notification.
>> -
>> -Available buffer notifications are sent by the driver, the recipient is
>> -the device. This type of notification indicates that a buffer may have
>> -been made available on the virtqueue designated by the notification.
>> +Configuration change notifications, used buffer notifications and
>> +driver auxiliary notifications are sent by the device,
>> +the recipient is the driver. A configuration change notification indicates
>> +that the device configuration space has changed; a used buffer notification
>> +indicates that a buffer may have been made used on the virtqueue designated
>> +by the notification; driver auxiliary notifications allow the
>> +device to send notifications other than configuration changes and used
>> +buffer notifications to the driver, these are optional and their meaning
>> +is device-specific.
>> +
>> +Available buffer notifications and device auxiliary notifications
>> +are sent by the driver, the recipient is the device. Available buffer
>> +notifications indicate that a buffer may have been made available on the
>> +virtqueue designated by the notification; device auxiliary
>> +notifcations allow the driver to send notifcations other than available
>> +buffer notifications to the device for example through a device register, these
>> +are optional and their meaning is device-specific.
>>  
>>  The semantics, the transport-specific implementations, and other
>>  important aspects of the different notifications are specified in detail
>> -- 
>> 2.25.1


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

Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Michael S. Tsirkin]

On Wed, Aug 10, 2022 at 11:54:35AM +0200, Cornelia Huck wrote:
> On Tue, Aug 09 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Mar 30, 2022 at 04:21:02PM +0100, Usama Arif wrote:
> >> Driver auxiliary notifications allow the device to send notifications
> >> other than configuration changes and used buffer notifications to the
> >> driver, these are optional and their meaning is device-specific.
> >> 
> >> Device auxiliary notifcations allow the driver to send notifcations
> >> other than available buffer notifications to the device for example
> >> through a device register, these are optional and their meaning is
> >> device-specific.
> >> 
> >> These device-specific notifications are needed later when adding support
> >> for virtio-vhost-user device.
> >> 
> >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
> >
> > I see ccw is missing. Cornelia, any suggestions?
> 
> Hmm... I seem to be really behind on ccw things :(
> 
> We can probably use the following:
> 
> - for device->driver notification, use the next bit in the secondary
>   indicators (bit 0 is used for config change notification)
> - for driver->device notification, maybe use a new subcode for diagnose
>   0x500 (4 is probably the next free one?)
> 
> I have not looked at the requirements deeply, though.
> 
> This highlights another problem, however: When we introduce new features
> that require a transport-specific implementation, we often end up with a
> PCI implementation, but sometimes MMIO and more often ccw are left
> behind -- which is understandable, as PCI is what most people use, and
> ccw is something only a very few people are familiar with. This sadly
> means that we have a backlog of features supported in PCI, but not in
> ccw... requiring implementations for ccw would put an undue burden on
> contributors, as most of them are unlikely to write anything for a
> mainframe, ever. On the flip side, I do not have enough bandwith to deal
> with all of this.
> 
> Halil, any thoughts (on any of the above)?

Kind of depends. We Do we want to add a "universal config"
structure shared between transports?
Will help with some use-cases though not this one.


> >
> >> ---
> >>  content.tex | 35 ++++++++++++++++++++++-------------
> >>  1 file changed, 22 insertions(+), 13 deletions(-)
> >> 
> >> diff --git a/content.tex b/content.tex
> >> index c6f116c..85980ac 100644
> >> --- a/content.tex
> >> +++ b/content.tex
> >> @@ -160,29 +160,38 @@ \subsection{Legacy Interface: A Note on Feature
> >>  Specification text within these sections generally does not apply
> >>  to non-transitional devices.
> >>  
> >> -\section{Notifications}\label{sec:Basic Facilities of a Virtio Device
> >> -/ Notifications}
> >> +\section{Notifications}\label{sec:Basic Facilities of a Virtio Device / Notifications}
> >>  
> >>  The notion of sending a notification (driver to device or device
> >>  to driver) plays an important role in this specification. The
> >>  modus operandi of the notifications is transport specific.
> >>  
> >> -There are three types of notifications: 
> >> +There are five types of notifications:
> >>  \begin{itemize}
> >>  \item configuration change notification
> >>  \item available buffer notification
> >> -\item used buffer notification. 
> >> +\item used buffer notification
> >> +\item driver auxiliary notification
> >> +\item device auxiliary notification
> >>  \end{itemize}
> >>  
> >> -Configuration change notifications and used buffer notifications are sent
> >> -by the device, the recipient is the driver. A configuration change
> >> -notification indicates that the device configuration space has changed; a
> >> -used buffer notification indicates that a buffer may have been made used
> >> -on the virtqueue designated by the notification.
> >> -
> >> -Available buffer notifications are sent by the driver, the recipient is
> >> -the device. This type of notification indicates that a buffer may have
> >> -been made available on the virtqueue designated by the notification.
> >> +Configuration change notifications, used buffer notifications and
> >> +driver auxiliary notifications are sent by the device,
> >> +the recipient is the driver. A configuration change notification indicates
> >> +that the device configuration space has changed; a used buffer notification
> >> +indicates that a buffer may have been made used on the virtqueue designated
> >> +by the notification; driver auxiliary notifications allow the
> >> +device to send notifications other than configuration changes and used
> >> +buffer notifications to the driver, these are optional and their meaning
> >> +is device-specific.
> >> +
> >> +Available buffer notifications and device auxiliary notifications
> >> +are sent by the driver, the recipient is the device. Available buffer
> >> +notifications indicate that a buffer may have been made available on the
> >> +virtqueue designated by the notification; device auxiliary
> >> +notifcations allow the driver to send notifcations other than available
> >> +buffer notifications to the device for example through a device register, these
> >> +are optional and their meaning is device-specific.
> >>  
> >>  The semantics, the transport-specific implementations, and other
> >>  important aspects of the different notifications are specified in detail
> >> -- 
> >> 2.25.1

[virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Cornelia Huck]

On Wed, Aug 10 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Aug 10, 2022 at 11:54:35AM +0200, Cornelia Huck wrote:
>> On Tue, Aug 09 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Wed, Mar 30, 2022 at 04:21:02PM +0100, Usama Arif wrote:
>> >> Driver auxiliary notifications allow the device to send notifications
>> >> other than configuration changes and used buffer notifications to the
>> >> driver, these are optional and their meaning is device-specific.
>> >> 
>> >> Device auxiliary notifcations allow the driver to send notifcations
>> >> other than available buffer notifications to the device for example
>> >> through a device register, these are optional and their meaning is
>> >> device-specific.
>> >> 
>> >> These device-specific notifications are needed later when adding support
>> >> for virtio-vhost-user device.
>> >> 
>> >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>
>> >
>> > I see ccw is missing. Cornelia, any suggestions?
>> 
>> Hmm... I seem to be really behind on ccw things :(
>> 
>> We can probably use the following:
>> 
>> - for device->driver notification, use the next bit in the secondary
>>   indicators (bit 0 is used for config change notification)
>> - for driver->device notification, maybe use a new subcode for diagnose
>>   0x500 (4 is probably the next free one?)
>> 
>> I have not looked at the requirements deeply, though.
>> 
>> This highlights another problem, however: When we introduce new features
>> that require a transport-specific implementation, we often end up with a
>> PCI implementation, but sometimes MMIO and more often ccw are left
>> behind -- which is understandable, as PCI is what most people use, and
>> ccw is something only a very few people are familiar with. This sadly
>> means that we have a backlog of features supported in PCI, but not in
>> ccw... requiring implementations for ccw would put an undue burden on
>> contributors, as most of them are unlikely to write anything for a
>> mainframe, ever. On the flip side, I do not have enough bandwith to deal
>> with all of this.
>> 
>> Halil, any thoughts (on any of the above)?
>
> Kind of depends. We Do we want to add a "universal config"
> structure shared between transports?
> Will help with some use-cases though not this one.

A "universal config" structure seems like a good idea; but, as you said,
that will not help in all cases.

>
>
>> >
>> >> ---
>> >>  content.tex | 35 ++++++++++++++++++++++-------------
>> >>  1 file changed, 22 insertions(+), 13 deletions(-)
>> >> 
>> >> diff --git a/content.tex b/content.tex
>> >> index c6f116c..85980ac 100644
>> >> --- a/content.tex
>> >> +++ b/content.tex
>> >> @@ -160,29 +160,38 @@ \subsection{Legacy Interface: A Note on Feature
>> >>  Specification text within these sections generally does not apply
>> >>  to non-transitional devices.
>> >>  
>> >> -\section{Notifications}\label{sec:Basic Facilities of a Virtio Device
>> >> -/ Notifications}
>> >> +\section{Notifications}\label{sec:Basic Facilities of a Virtio Device / Notifications}
>> >>  
>> >>  The notion of sending a notification (driver to device or device
>> >>  to driver) plays an important role in this specification. The
>> >>  modus operandi of the notifications is transport specific.
>> >>  
>> >> -There are three types of notifications: 
>> >> +There are five types of notifications:
>> >>  \begin{itemize}
>> >>  \item configuration change notification
>> >>  \item available buffer notification
>> >> -\item used buffer notification. 
>> >> +\item used buffer notification
>> >> +\item driver auxiliary notification
>> >> +\item device auxiliary notification
>> >>  \end{itemize}
>> >>  
>> >> -Configuration change notifications and used buffer notifications are sent
>> >> -by the device, the recipient is the driver. A configuration change
>> >> -notification indicates that the device configuration space has changed; a
>> >> -used buffer notification indicates that a buffer may have been made used
>> >> -on the virtqueue designated by the notification.
>> >> -
>> >> -Available buffer notifications are sent by the driver, the recipient is
>> >> -the device. This type of notification indicates that a buffer may have
>> >> -been made available on the virtqueue designated by the notification.
>> >> +Configuration change notifications, used buffer notifications and
>> >> +driver auxiliary notifications are sent by the device,
>> >> +the recipient is the driver. A configuration change notification indicates
>> >> +that the device configuration space has changed; a used buffer notification
>> >> +indicates that a buffer may have been made used on the virtqueue designated
>> >> +by the notification; driver auxiliary notifications allow the
>> >> +device to send notifications other than configuration changes and used
>> >> +buffer notifications to the driver, these are optional and their meaning
>> >> +is device-specific.
>> >> +
>> >> +Available buffer notifications and device auxiliary notifications
>> >> +are sent by the driver, the recipient is the device. Available buffer
>> >> +notifications indicate that a buffer may have been made available on the
>> >> +virtqueue designated by the notification; device auxiliary
>> >> +notifcations allow the driver to send notifcations other than available
>> >> +buffer notifications to the device for example through a device register, these
>> >> +are optional and their meaning is device-specific.
>> >>  
>> >>  The semantics, the transport-specific implementations, and other
>> >>  important aspects of the different notifications are specified in detail
>> >> -- 
>> >> 2.25.1


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

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Halil Pasic]

On Wed, 10 Aug 2022 08:45:25 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Aug 10, 2022 at 11:54:35AM +0200, Cornelia Huck wrote:
> > On Tue, Aug 09 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Wed, Mar 30, 2022 at 04:21:02PM +0100, Usama Arif wrote:  
> > >> Driver auxiliary notifications allow the device to send notifications
> > >> other than configuration changes and used buffer notifications to the
> > >> driver, these are optional and their meaning is device-specific.
> > >> 
> > >> Device auxiliary notifcations allow the driver to send notifcations
> > >> other than available buffer notifications to the device for example
> > >> through a device register, these are optional and their meaning is
> > >> device-specific.
> > >> 
> > >> These device-specific notifications are needed later when adding support
> > >> for virtio-vhost-user device.
> > >> 
> > >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> > >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>  
> > >
> > > I see ccw is missing. Cornelia, any suggestions?  
> > 
> > Hmm... I seem to be really behind on ccw things :(
> > 
> > We can probably use the following:
> > 
> > - for device->driver notification, use the next bit in the secondary
> >   indicators (bit 0 is used for config change notification)
> > - for driver->device notification, maybe use a new subcode for diagnose
> >   0x500 (4 is probably the next free one?)
> > 
> > I have not looked at the requirements deeply, though.
> > 
> > This highlights another problem, however: When we introduce new features
> > that require a transport-specific implementation, we often end up with a
> > PCI implementation, but sometimes MMIO and more often ccw are left
> > behind -- which is understandable, as PCI is what most people use, and
> > ccw is something only a very few people are familiar with. This sadly
> > means that we have a backlog of features supported in PCI, but not in
> > ccw... requiring implementations for ccw would put an undue burden on
> > contributors, as most of them are unlikely to write anything for a
> > mainframe, ever. On the flip side, I do not have enough bandwith to deal
> > with all of this.
> > 
> > Halil, any thoughts (on any of the above)?  
> 
> Kind of depends. We Do we want to add a "universal config"
> structure shared between transports?
> Will help with some use-cases though not this one.

I'm for "unversal config"! Regarding this use case I have to dig a
little deeper to really understand!

Regards,
Halil 

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Halil Pasic]

On Wed, 10 Aug 2022 11:54:35 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> >> These device-specific notifications are needed later when adding support
> >> for virtio-vhost-user device.
> >> 
> >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>  
> >
> > I see ccw is missing. Cornelia, any suggestions?  
> 
> Hmm... I seem to be really behind on ccw things :(
> 
> We can probably use the following:
> 
> - for device->driver notification, use the next bit in the secondary
>   indicators (bit 0 is used for config change notification)
> - for driver->device notification, maybe use a new subcode for diagnose
>   0x500 (4 is probably the next free one?)
> 

Sounds reasonable! I will have to double check the DIAG stuff though. I'm
not sure where what needs to be reserved and documented. 

> I have not looked at the requirements deeply, though.
> 
> This highlights another problem, however: When we introduce new features
> that require a transport-specific implementation, we often end up with a
> PCI implementation, but sometimes MMIO and more often ccw are left
> behind -- which is understandable, as PCI is what most people use, and
> ccw is something only a very few people are familiar with. This sadly
> means that we have a backlog of features supported in PCI, but not in
> ccw... requiring implementations for ccw would put an undue burden on
> contributors, as most of them are unlikely to write anything for a
> mainframe, ever. On the flip side, I do not have enough bandwith to deal
> with all of this.

I'm completely with you in a sense that I see the same problem. I think
we have to get these resolved on a case by case basis. In my opinion at
least in theory it would make a big difference, whether the new feature
obligatory or not. But since VIRTIO is big on compatibility, and also
cares about the initial investment required, in practice, I think, we
are mostly good with the transports delivering features on their own
schedule. What I mean here is: it is kind of difficult to make a new
facility (like shm, or aux notifications) mandatory, because stuff
that conform to a previous incarnation of the spec would become
non-conform.

And the people who care about the particular transport, and the users
of the transport (indirectly also platforms) should make up their own
mind with regards to whether and when to invest into the new facilities
and the new tech and opportunities associated with those.

OTOH when reading the spec, it my strike one as strange, that for example
CCW does not mention aux notifications at all. One idea: maybe we could
add a note, or a subsection, or something, which states the list of
general optional virtio facilities or features not supported by the given
transport on the spec level for a given incarnation of the spec.

I think making the people not motivated to do the design and write the
spec for all the platforms add to that list is a reasonable middle
ground. It would also make the differences very clear, and the same goes
for the intention (i.e. not omitted by mistake).

> 
> Halil, any thoughts (on any of the above)?

Yes, definitely! See above. :) Thanks for drawing my attention to this.
I'm very interested in your opinion.

Regards,
Halil

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Michael S. Tsirkin]

On Wed, Aug 10, 2022 at 07:41:08PM +0200, Halil Pasic wrote:
> On Wed, 10 Aug 2022 11:54:35 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > >> These device-specific notifications are needed later when adding support
> > >> for virtio-vhost-user device.
> > >> 
> > >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> > >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>  
> > >
> > > I see ccw is missing. Cornelia, any suggestions?  
> > 
> > Hmm... I seem to be really behind on ccw things :(
> > 
> > We can probably use the following:
> > 
> > - for device->driver notification, use the next bit in the secondary
> >   indicators (bit 0 is used for config change notification)
> > - for driver->device notification, maybe use a new subcode for diagnose
> >   0x500 (4 is probably the next free one?)
> > 
> 
> Sounds reasonable! I will have to double check the DIAG stuff though. I'm
> not sure where what needs to be reserved and documented. 
> 
> > I have not looked at the requirements deeply, though.
> > 
> > This highlights another problem, however: When we introduce new features
> > that require a transport-specific implementation, we often end up with a
> > PCI implementation, but sometimes MMIO and more often ccw are left
> > behind -- which is understandable, as PCI is what most people use, and
> > ccw is something only a very few people are familiar with. This sadly
> > means that we have a backlog of features supported in PCI, but not in
> > ccw... requiring implementations for ccw would put an undue burden on
> > contributors, as most of them are unlikely to write anything for a
> > mainframe, ever. On the flip side, I do not have enough bandwith to deal
> > with all of this.
> 
> I'm completely with you in a sense that I see the same problem. I think
> we have to get these resolved on a case by case basis. In my opinion at
> least in theory it would make a big difference, whether the new feature
> obligatory or not. But since VIRTIO is big on compatibility, and also
> cares about the initial investment required, in practice, I think, we
> are mostly good with the transports delivering features on their own
> schedule. What I mean here is: it is kind of difficult to make a new
> facility (like shm, or aux notifications) mandatory, because stuff
> that conform to a previous incarnation of the spec would become
> non-conform.
> 
> And the people who care about the particular transport, and the users
> of the transport (indirectly also platforms) should make up their own
> mind with regards to whether and when to invest into the new facilities
> and the new tech and opportunities associated with those.
> 
> OTOH when reading the spec, it my strike one as strange, that for example
> CCW does not mention aux notifications at all. One idea: maybe we could
> add a note, or a subsection, or something, which states the list of
> general optional virtio facilities or features not supported by the given
> transport on the spec level for a given incarnation of the spec.

Yes I think each transport should list features it does not
support, and a feature specific to some transports must
also require that other transports disable it and
that drivers do not ack it.
Otherwise it's too easy for devices to offer the feature bit
by mistake.

> I think making the people not motivated to do the design and write the
> spec for all the platforms add to that list is a reasonable middle
> ground. It would also make the differences very clear, and the same goes
> for the intention (i.e. not omitted by mistake).
> 
> > 
> > Halil, any thoughts (on any of the above)?
> 
> Yes, definitely! See above. :) Thanks for drawing my attention to this.
> I'm very interested in your opinion.
> 
> Regards,
> Halil

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Cornelia Huck]

On Wed, Aug 10 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Aug 10, 2022 at 07:41:08PM +0200, Halil Pasic wrote:
>> On Wed, 10 Aug 2022 11:54:35 +0200
>> Cornelia Huck <cohuck@redhat.com> wrote:
>> > This highlights another problem, however: When we introduce new features
>> > that require a transport-specific implementation, we often end up with a
>> > PCI implementation, but sometimes MMIO and more often ccw are left
>> > behind -- which is understandable, as PCI is what most people use, and
>> > ccw is something only a very few people are familiar with. This sadly
>> > means that we have a backlog of features supported in PCI, but not in
>> > ccw... requiring implementations for ccw would put an undue burden on
>> > contributors, as most of them are unlikely to write anything for a
>> > mainframe, ever. On the flip side, I do not have enough bandwith to deal
>> > with all of this.
>> 
>> I'm completely with you in a sense that I see the same problem. I think
>> we have to get these resolved on a case by case basis. In my opinion at
>> least in theory it would make a big difference, whether the new feature
>> obligatory or not. But since VIRTIO is big on compatibility, and also
>> cares about the initial investment required, in practice, I think, we
>> are mostly good with the transports delivering features on their own
>> schedule. What I mean here is: it is kind of difficult to make a new
>> facility (like shm, or aux notifications) mandatory, because stuff
>> that conform to a previous incarnation of the spec would become
>> non-conform.

I don't think there's a big case for making new things mandatory;
everything should be guarded by a feature bit or similar.

>> 
>> And the people who care about the particular transport, and the users
>> of the transport (indirectly also platforms) should make up their own
>> mind with regards to whether and when to invest into the new facilities
>> and the new tech and opportunities associated with those.

PCI will probably satisfy the needs of the vast majority of users, and
MMIO is not too alien to just change at the same time. ccw is the big
problem. Is IBM still spending resources on virtio-ccw? [My own
involvement with s390x has dwindled a lot, so it would be great to see
some of it picked up by others. Certainly not trying to pin everything
on Halil, though.]

>> 
>> OTOH when reading the spec, it my strike one as strange, that for example
>> CCW does not mention aux notifications at all. One idea: maybe we could
>> add a note, or a subsection, or something, which states the list of
>> general optional virtio facilities or features not supported by the given
>> transport on the spec level for a given incarnation of the spec.
>
> Yes I think each transport should list features it does not
> support, and a feature specific to some transports must
> also require that other transports disable it and
> that drivers do not ack it.
> Otherwise it's too easy for devices to offer the feature bit
> by mistake.

Do we need to add a requirement for every transport-specific feature, or
would it be sufficient to add a statement like "if a feature requires a
transport-specific implementation, a device using that transport MUST
NOT offer that feature"?

>
>> I think making the people not motivated to do the design and write the
>> spec for all the platforms add to that list is a reasonable middle
>> ground. It would also make the differences very clear, and the same goes
>> for the intention (i.e. not omitted by mistake).

Hm. On the one hand, I like that it would add a laundry list for
transports regarding features that might be implemented. On the other
hand, I think the main problem is not enough people with enough
understanding and bandwidth to add new features everywhere... but I
suppose that needs to be fixed in a different place anyway.


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

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Michael S. Tsirkin]

On Thu, Aug 11, 2022 at 11:12:06AM +0200, Cornelia Huck wrote:
> >> OTOH when reading the spec, it my strike one as strange, that for example
> >> CCW does not mention aux notifications at all. One idea: maybe we could
> >> add a note, or a subsection, or something, which states the list of
> >> general optional virtio facilities or features not supported by the given
> >> transport on the spec level for a given incarnation of the spec.
> >
> > Yes I think each transport should list features it does not
> > support, and a feature specific to some transports must
> > also require that other transports disable it and
> > that drivers do not ack it.
> > Otherwise it's too easy for devices to offer the feature bit
> > by mistake.
> 
> Do we need to add a requirement for every transport-specific feature, or
> would it be sufficient to add a statement like "if a feature requires a
> transport-specific implementation, a device using that transport MUST
> NOT offer that feature"?


I'd prefer to list the features explicitly.

In particular is kind of forces whoever is proposing
the feature into at least be aware that other transports
exist and that the proposal is incomplete.


> >
> >> I think making the people not motivated to do the design and write the
> >> spec for all the platforms add to that list is a reasonable middle
> >> ground. It would also make the differences very clear, and the same goes
> >> for the intention (i.e. not omitted by mistake).
> 
> Hm. On the one hand, I like that it would add a laundry list for
> transports regarding features that might be implemented. On the other
> hand, I think the main problem is not enough people with enough
> understanding and bandwidth to add new features everywhere... but I
> suppose that needs to be fixed in a different place anyway.

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Halil Pasic]

On Thu, 11 Aug 2022 11:12:06 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> >  
> >> I think making the people not motivated to do the design and write the
> >> spec for all the platforms add to that list is a reasonable middle
> >> ground. It would also make the differences very clear, and the same goes
> >> for the intention (i.e. not omitted by mistake).  
> 
> Hm. On the one hand, I like that it would add a laundry list for
> transports regarding features that might be implemented. 

I believe, all three of us are in agreement on this one. I could hack up
a spec-patch for ccw at some point in time which rectifies things for
the features that are already in the spec.

> In the other
> hand, I think the main problem is not enough people with enough
> understanding and bandwidth to add new features everywhere... but I
> suppose that needs to be fixed in a different place anyway.

I kind of agree, especially with the part that this is not the right
place to fix that problem, if it is a problem. Actually I see this
rather as a strength of virtio: a large amount of freedom in deciding
how much and where to invest. I would rather not specify stuff without
having some sort of a reference implementation, so the ability to
delay specifying how some optional features are to work with virtio-ccw
is very welcome form my perspective.

Regards,
Halil

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Halil Pasic]

On Thu, 11 Aug 2022 11:12:06 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, Aug 10 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Aug 10, 2022 at 07:41:08PM +0200, Halil Pasic wrote:  
> >> On Wed, 10 Aug 2022 11:54:35 +0200
> >> Cornelia Huck <cohuck@redhat.com> wrote:  
> >> > This highlights another problem, however: When we introduce new features
> >> > that require a transport-specific implementation, we often end up with a
> >> > PCI implementation, but sometimes MMIO and more often ccw are left
> >> > behind -- which is understandable, as PCI is what most people use, and
> >> > ccw is something only a very few people are familiar with. This sadly
> >> > means that we have a backlog of features supported in PCI, but not in
> >> > ccw... requiring implementations for ccw would put an undue burden on
> >> > contributors, as most of them are unlikely to write anything for a
> >> > mainframe, ever. On the flip side, I do not have enough bandwith to deal
> >> > with all of this.  
> >> 
> >> I'm completely with you in a sense that I see the same problem. I think
> >> we have to get these resolved on a case by case basis. In my opinion at
> >> least in theory it would make a big difference, whether the new feature
> >> obligatory or not. But since VIRTIO is big on compatibility, and also
> >> cares about the initial investment required, in practice, I think, we
> >> are mostly good with the transports delivering features on their own
> >> schedule. What I mean here is: it is kind of difficult to make a new
> >> facility (like shm, or aux notifications) mandatory, because stuff
> >> that conform to a previous incarnation of the spec would become
> >> non-conform.  
> 
> I don't think there's a big case for making new things mandatory;
> everything should be guarded by a feature bit or similar.

Yes! I tried to say the same, just differently :)

> 
> >> 
> >> And the people who care about the particular transport, and the users
> >> of the transport (indirectly also platforms) should make up their own
> >> mind with regards to whether and when to invest into the new facilities
> >> and the new tech and opportunities associated with those.  
> 
> PCI will probably satisfy the needs of the vast majority of users, and
> MMIO is not too alien to just change at the same time. ccw is the big
> problem. Is IBM still spending resources on virtio-ccw? [My own
> involvement with s390x has dwindled a lot, so it would be great to see
> some of it picked up by others. Certainly not trying to pin everything
> on Halil, though.]

Yes! IBM is definitely (still) invested in virtio-ccw! I don't work on
Virtio on s390x full time any more, but I'm totally committed to
fulfilling my duties as a TC member and as the virtio-ccw
sub-maintainer. IBM is looking for a solution to at least replace what
the Virtio community has lost with me gaining new responsibilities not
closely related to Virtio.


BTW, should I not show up in time on some discussion, mentioning my
name in the body of the email should help. I auto-tag my emails,
and I have a separate tag for that. When I'm completely under water
I check for that tag to avoid not showing up when my name is called
:D

Thanks for calling my name occasionally!

Regards,
Halil

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Cornelia Huck]

On Wed, Aug 10 2022, Halil Pasic <pasic@linux.ibm.com> wrote:

> On Wed, 10 Aug 2022 11:54:35 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
>
>> >> These device-specific notifications are needed later when adding support
>> >> for virtio-vhost-user device.
>> >> 
>> >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>  
>> >
>> > I see ccw is missing. Cornelia, any suggestions?  
>> 
>> Hmm... I seem to be really behind on ccw things :(
>> 
>> We can probably use the following:
>> 
>> - for device->driver notification, use the next bit in the secondary
>>   indicators (bit 0 is used for config change notification)
>> - for driver->device notification, maybe use a new subcode for diagnose
>>   0x500 (4 is probably the next free one?)
>> 
>
> Sounds reasonable! I will have to double check the DIAG stuff though. I'm
> not sure where what needs to be reserved and documented. 

There's
https://gitlab.com/davidhildenbrand/s390x-os-virt-spec/-/merge_requests/1,
but nothing much has happened there recently. David?


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

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[Halil Pasic]

On Thu, 11 Aug 2022 10:53:35 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, Aug 10 2022, Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > On Wed, 10 Aug 2022 11:54:35 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >  
> >> >> These device-specific notifications are needed later when adding support
> >> >> for virtio-vhost-user device.
> >> >> 
> >> >> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
> >> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >> >> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>    
> >> >
> >> > I see ccw is missing. Cornelia, any suggestions?    
> >> 
> >> Hmm... I seem to be really behind on ccw things :(
> >> 
> >> We can probably use the following:
> >> 
> >> - for device->driver notification, use the next bit in the secondary
> >>   indicators (bit 0 is used for config change notification)
> >> - for driver->device notification, maybe use a new subcode for diagnose
> >>   0x500 (4 is probably the next free one?)
> >>   
> >
> > Sounds reasonable! I will have to double check the DIAG stuff though. I'm
> > not sure where what needs to be reserved and documented.   
> 
> There's
> https://gitlab.com/davidhildenbrand/s390x-os-virt-spec/-/merge_requests/1,
> but nothing much has happened there recently. David?

Hm, that seems to be private. I land on a sign-in page.

Regards,
Halil

Re: [virtio-dev] Re: [PATCH 1/4] content: Introduce driver/device auxiliary notifications

[David Hildenbrand]

On 11.08.22 16:09, Halil Pasic wrote:
> On Thu, 11 Aug 2022 10:53:35 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
>> On Wed, Aug 10 2022, Halil Pasic <pasic@linux.ibm.com> wrote:
>>
>>> On Wed, 10 Aug 2022 11:54:35 +0200
>>> Cornelia Huck <cohuck@redhat.com> wrote:
>>>  
>>>>>> These device-specific notifications are needed later when adding support
>>>>>> for virtio-vhost-user device.
>>>>>>
>>>>>> Signed-off-by: Usama Arif <usama.arif@bytedance.com>
>>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>>> Signed-off-by: Nikos Dragazis <ndragazis@arrikto.com>    
>>>>>
>>>>> I see ccw is missing. Cornelia, any suggestions?    
>>>>
>>>> Hmm... I seem to be really behind on ccw things :(
>>>>
>>>> We can probably use the following:
>>>>
>>>> - for device->driver notification, use the next bit in the secondary
>>>>   indicators (bit 0 is used for config change notification)
>>>> - for driver->device notification, maybe use a new subcode for diagnose
>>>>   0x500 (4 is probably the next free one?)
>>>>   
>>>
>>> Sounds reasonable! I will have to double check the DIAG stuff though. I'm
>>> not sure where what needs to be reserved and documented.   
>>
>> There's
>> https://gitlab.com/davidhildenbrand/s390x-os-virt-spec/-/merge_requests/1,
>> but nothing much has happened there recently. David?

Heh, I'd like to say it has high priority on my todo list ... but there
is a lot of stuff with even higher priority. I'll definetly come back to
this soonish.

> 
> Hm, that seems to be private. I land on a sign-in page.

Oh, it was private. It's public now.


-- 
Thanks,

David / dhildenb