Re: [Qemu-devel] [virtio-dev][RFC v3] virtio-sdm: new device specification

[Edgar E. Iglesias]

Hi,

Sorry for the delay. I have a few questions.

I don't fully understand the purpose of this. Could you elaborate a little on 
that?
You need a real IPI signal to drive this I guess, so it's a little bit of a
chicken and egg problem.

A useful feature I can see is multiplexing a single underlying IPI (driving
an sdm channel) into multiple "virtual" IPI signals. Is that the main
use-case?

Regarding the boot and reset. Typically there's a reset signal you release and
the remote CPU starts running. You can ofcourse reset the CPU and restart
at anytime. Not sure if you need an additional BOOT virtual signal.

There may also be additional mechanisms to control the startup address of the 
remote CPU.
Any thoughts on that?

Best regards,
Edgar


On Tue, Aug 30, 2016 at 01:22:26PM +0200, Christian Pinto wrote:
> Hello,
> 
> are there any comments?
> 
> 
> Christian
> 
> 
> On 09/08/2016 09:37, Christian Pinto wrote:
> >This patch adds the specification of the Signal Dristribution Module virtio
> >device to the current virtio specification document.
> >
> >Signed-off-by: Christian Pinto <address@hidden>
> >Signed-off-by: Baptiste Reynal <address@hidden>
> >
> >---
> >v2 -> v3:
> >- Removed master field from configuration and replaced with device_id
> >- Added new RESET signal
> >- Added signals definition into device specs
> >- Added feature bits associated to signals
> >
> >v1 -> v2:
> >- Fixed some typos
> >- Removed dependencies from QEMU
> >- Added explanation on how SDM can be used in AMP systems
> >- Explained semantics of payload field in SDMSignalData struct
> >---
> >  content.tex    |   2 +
> >  virtio-sdm.tex | 166 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 168 insertions(+)
> >  create mode 100644 virtio-sdm.tex
> >
> >diff --git a/content.tex b/content.tex
> >index 3317916..7fcf779 100644
> >--- a/content.tex
> >+++ b/content.tex
> >@@ -5643,6 +5643,8 @@ descriptor for the \field{sense_len}, \field{residual},
> >  \field{status_qualifier}, \field{status}, \field{response} and
> >  \field{sense} fields.
> >+\input{virtio-sdm.tex}
> >+
> >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >  Currently there are three device-independent feature bits defined:
> >diff --git a/virtio-sdm.tex b/virtio-sdm.tex
> >new file mode 100644
> >index 0000000..ee43e23
> >--- /dev/null
> >+++ b/virtio-sdm.tex
> >@@ -0,0 +1,166 @@
> >+\section{Signal Distribution Module}\label{sec:Device Types / SDM Device}
> >+
> >+The virtio Signal Distribution Module is meant to enable Inter Processor 
> >signal
> >+exchange.
> >+An example are Inter Processor Interrupts used in AMP systems, for the
> >+processors involved to notify the presence of new data in the communication
> >+queues.
> >+In AMP systems signals are used for various purposes, an example are 
> >remoteproc
> >+or RPMSG. In the former signals are used by a master processor to trigger
> >+the boot of a slave processor. While the latter, RPMSG, uses virtio queues 
> >as a
> >+message exchange medium between processors. In this case the SDM can be 
> >used to
> >+generate the interrupt associated to the kick of a virtio queue.
> >+
> >+There are three signal types supported by the device, namely the
> >+\textit{IRQ} signal, \textit{BOOT} signal and \textit{RESET} signal. Such 
> >set of
> >+signals covers most of the needs of an AMP system, where a master processor 
> >can
> >+trigger the boot or reset of slave processors, and processors send IRQs 
> >between
> >+each other.
> >+
> >+\subsection{Device ID}\label{sec:Device Types / SDM Device / Device ID}
> >+
> >+21
> >+
> >+\subsection{Virtqueues}\label{sec:Device Types / SDM Device / Virtqueues}
> >+
> >+\begin{description}
> >+\item[0] hg_vq
> >+\item[1] gh_vq
> >+\end{description}
> >+
> >+Queue 0 is used for device-to-driver communication (i.e., notification of a
> >+signal), while queue 1 for driver-to-device communication.
> >+
> >+\subsection{Feature bits}\label{sec:Device Types / SDM Device / Feature 
> >bits}
> >+
> >+\begin{description}
> >+\item[VIRTIO_SDM_F_IRQ_SIG (0)] Device supports the IRQ signal.
> >+
> >+\item[VIRTIO_SDM_F_BOOT_SIG (1)] Device supports the BOOT signal.
> >+
> >+\item[VIRTIO_SDM_F_RESET_SIG (2)] Device supports the RESET signal.
> >+\end{description}
> >+
> >+This set of bits is used by each virtio SDM device to declare which of the
> >+signals it supports. By specification each device can support all or a 
> >subset of
> >+the defined signals.
> >+
> >+\subsection{Device configuration layout}\label{sec:Device Types / SDM 
> >Device /
> >+Device configuration layout}
> >+
> >+The device configuration is composed by three fields:
> >+\texttt{max_slaves}, \texttt{current_slaves} and the \texttt{device_id}.
> >+
> >+\begin{lstlisting}
> >+struct virtio_sdm_config {
> >+    u16   max_slaves;
> >+    u16   current_slaves;
> >+    u32   device_id;
> >+};
> >+\end{lstlisting}
> >+
> >+The SDM device can be instantiated either as a master or as a slave. The 
> >master
> >+slave behavior is meant on purpose to reflect the AMP like type of 
> >communication
> >+where a master processor controls one or more slave processors.
> >+A master SDM instance can send a signal to any of the slaves instances,
> >+while slaves can only signal the master.
> >+
> >+The \texttt{master} field of the configuration space is meant to be read by 
> >the
> >+driver to figure out whether a specific instance is a master or a slave.
> >+The \texttt{max_slaves} field contains the maximum number of slaves 
> >supported by
> >+the SDM device. A configuration change notification is sent to the driver 
> >each
> >+time the value of \texttt{max_slaves} is changed from the device side.
> >+Finally, the \texttt{current_slaves} field contains the actual number of 
> >slaves
> >+registered to the master SDM. This field is a read only field. Finally the
> >+\texttt{device_id} field is used by each driver to know the ID of the 
> >device it
> >+is attached to, the field contains 0 in case of a master instance. A driver
> >+figures out whether it is attached to a master or a slave instance thanks 
> >to this
> >+field.
> >+
> >+\subsection{Device Initialization}\label{sec:Device Types / SDM Device /
> >+evice Initialization}
> >+
> >+During initialization the \texttt{hg_vq} and \texttt{gh_vq} are identified 
> >and
> >+the device is immediately operational. A master driver instance can access 
> >the
> >+number of slaves registered at any time by reading the configuration space 
> >of
> >+the device.
> >+
> >+During the initialization phase the device connects also to the 
> >communication
> >+channel. It has to be noted that the behavior of the device is
> >+independent from the communication channel used, that is a detail of each
> >+specific implementation of the SDM device.
> >+
> >+The last phase of the initialization is the negotiation of the feature bits.
> >+Each device implementation declares the signals supported by offering all 
> >or a
> >+subset of the three feature bits (VIRTIO_SDM_F_IRQ_SIG, 
> >VIRTIO_SDM_F_BOOT_SIG,
> >+VIRTIO_SDM_F_RESET_SIG). The SDM driver will be aware of the set of signals 
> >to
> >+handle thanks to this negotiation phase.
> >+
> >+\subsection{Device Operation}\label{sec:Device Types / SDM Device / Device
> >+peration}
> >+
> >+The SDM device handles signals coming from the two following sources:
> >+
> >+\begin{enumerate}
> >+\item The local processor to which the device is attached to.
> >+\item The communication channel connecting to other slaves/master.
> >+\end{enumerate}
> >+
> >+The first case is verified when the processor attached to the SDM device 
> >wants
> >+to send a signal to a second SDM device instance.
> >+The second case is instead when an SDM device instance receives a signal 
> >from
> >+another SDM device, to be forwarded to the local processor.
> >+It is important to note that due to the master/slave behavior, slaves cannot
> >+signal among themselves but only with the master SDM instance.
> >+
> >+In both cases, before sending over the communication channel, the signal is
> >+packed in the \texttt{SDMSignalData} data structure.
> >+
> >+\begin{lstlisting}
> >+enum sdm_signal_type {
> >+    SDM_IRQ,
> >+    SDM_BOOT,
> >+    SDM_RESET,
> >+};
> >+
> >+struct SDMSignalData {
> >+    uint32_t type;
> >+    uint32_t slave;
> >+    uint32_t payload[2];
> >+};
> >+\end{lstlisting}
> >+
> >+The \texttt{type} field indicates the type of signal to be sent to the
> >+destination SDM. The current implementation supports three signal types.
> >+The \texttt{SDM_IRQ} signal is used to send an inter processor interrupt, 
> >while
> >+the \texttt{SDM_BOOT} signal is sent to trigger the boot of the destination
> >+processor. The boot signal is meant to be used in an AMP like scenario 
> >where a
> >+master processor boots one or more slave processors (e.g., via remoteproc).
> >+The \texttt{SDM_RESET} is also meant to be used in AMP like scenarios, to
> >+trigger the reset of the target slave processor. As an assumption a driver
> >+cannot trigger the reset of the device it is attached to, so each driver
> >+implementation should ignore reset signals where the source slave 
> >corresponds to
> >+the device ID the driver is attached to.
> >+This is done by comparing, when a message is recevied, the value of
> >+the \texttt{slave} field of the \texttt{SDMSignalData} data structure with 
> >the
> >+\texttt{device_id} field of the configuration space.
> >+The \texttt{slave} field contains the id of the SDM instance destination of 
> >the
> >+signal. Id 0 is reserved for the master, from 1 onwards for the slaves.
> >+This means that the \texttt{slave} field will always contain 0 when the 
> >source
> >+of the signal is a slave SDM instance, while the actual id of the slave in 
> >case
> >+of a master. When instead a message is recevied, this field contains the ID 
> >of
> >+the slave source of the signal.
> >+The \texttt{payload} is used to pass extra accompanying information with the
> >+signal.
> >+The semantics of the payload field depends on the signal itself. In case of
> >+\texttt{SDM_IRQ} signal, the payload field is ignored since interrupts do 
> >not
> >+need any extra information to be handled. In the case of \texttt{SDM_BOOT}
> >+signal the payload contains the boot address of the slave processor, to be 
> >used
> >+at the destination to initialize the program counter register before the 
> >actual
> >+boot process is started. Finally the payload field is ignored also in case 
> >of
> >+\texttt{SDM_RESET} signal, since no extra information is needed to trigger 
> >the
> >+reset of the target processor.
> >+
> >+
> >+The \texttt{SDMSignalData} structure is first filled by the source SDM 
> >kernel
> >+virtio driver and sent over the gh_vq.
> 
>