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

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Wed, Sep 07, 2016 at 09:24:39AM +0200, Christian Pinto wrote:
> Hello Edgar,
> 
> thanks for your comments.
>

Thanks for the clarification, I have a few follow-up questions/comments.
 
> 
> On 06/09/2016 23:43, Edgar E. Iglesias wrote:
> >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.
> 
> Usually in AMP-like systems CPUs signal each other with IPIs by exploiting
> a dedicated hardware peripheral, like a hardware mailbox. So I see the
> concept of the SDM (master, slave and communication channels) as the
> hardware device to actually implement and deliver the IPIs.
> In this case the SDM is designed in a way to be able to deliver multiple
> signals (e.g., BOOT, RESET) and not just a single interrupt.
> 
> >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?
> 
> In the SDM multiple slaves can connect to the same SDM channel (e.g.,
> socket)
> and the master uses the channel to deliver the signal (e.g., IRQ) to the
> destination slave device. The other way around is also possible, slaves
> signaling
> the master.
> I guess this is the multiplexing features you were talking about.

Right, but for this to be more useful, I think you should pass on some
payload with the IRQ signal. That will allow this channel to be used
to enable notification mechanisms for many other channels over a single
HW irq (e.g the payload would be like the irq vector or irq line nr).

Another question:
For the reverese path, I don't see how it will be reasonably implemented
in the remote cpus various sw stacks sharing a single virtio queue.
It would seem like you would need a reverese channel per remoteproc
to avoid locking (very messy in AMP systems) to safely update
the reverese gh_vq queue?

Or how do you see multiple AMP systems safely updating a single virtio
queue in parallel?

Best regards,
Edgar


> 
> However it is worth to notice that one SDM channel is not dedicated to one
> specific signal, but rather it is used to deliver all the signals
> implemented
> by the device.
> 
> >
> >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?
> 
> The purpose of the BOOT signal is also to pass the remote CPU its startup
> address. So the idea is to use the payload of the signal to send the slave
> processor
> its startup address over the SDM channel and SDM slave device. Our
> implementation
> for QEMU uses the BOOT signal for this purpose.
> 
> This part of the semantic of the BOOT signal is not described in the
> documentation
> since I believe it is a choice of the actual implementation of the SDM
> whether to
> pass the startup address or not (i.e. there might be a platform where the
> boot
> address of slave processors is somehow hard-coded).
> I can add the explanation to the device specification to make it more clear.
> 
> 
> Thanks,
> 
> Christian
> 
> >
> >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 <c.pinto@virtualopensystems.com>
> >>>Signed-off-by: Baptiste Reynal <b.reynal@virtualopensystems.com>
> >>>
> >>>---
> >>>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.
> >>
>