[Qemu-devel] [virtio-dev][RFC 0/2] Signal Distribution Module virtio device specification

[Qemu-devel] [virtio-dev][RFC 0/2] Signal Distribution Module virtio device specification

[Christian Pinto]

Hi all,

This patch series proposes the specification of a new virtio device on which we
are working on, namely the Signal Distribution Module (SDM). 
The SDM routes inter-processor signals intra and inter QEMU
instances, using a user-defined communication channel. At the current state the 
SDM provides a local channel, for intra-QEMU signals, and a channel based on
sockets (UNIX or TCP) to exchange signals between processors in different
instances of QEMU. Each communication channel exports a common interface for 
the sake of ease of extension and integration of new channels.

In addition to the virtio version, a platform device version is available as
well to be used for cases where a processor is not running Linux but another
OS/firmware that does not support virtio.

This patch is related to :

[Qemu-devel][RFC v3 0/6] SDM Interface

where you can find the latest RFC patch set for the QEMU code of the virtio SDM 
device.

Kernel code is publicly accessible from:

https://git.virtualopensystems.com/dev/qemu-het-tools

branch sdm_test_virtio_mod_v2.


QEMU code is accessible from:

https://git.virtualopensystems.com/dev/qemu-het-tools

branch sdm-dev-v3

Thanks,

Christian

Christian Pinto (2):
  content: reserve virtio device ID
  virtio-sdm: new device specification

 content.tex    |   4 ++
 virtio-sdm.tex | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+)
 create mode 100644 virtio-sdm.tex

-- 
1.9.1

[Qemu-devel] [virtio-dev][RFC 1/2] content: reserve virtio device ID

[Christian Pinto]

Signed-off-by: Christian Pinto <c.pinto@virtualopensystems.com>
---
 content.tex | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/content.tex b/content.tex
index d989d98..0d98926 100644
--- a/content.tex
+++ b/content.tex
@@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device    \\
 \hline
 18         &   Input device \\
 \hline
+19         &   Signal Distribution Module \\
+\hline
 \end{tabular}
 
 Some of the devices above are unspecified by this document,
@@ -5641,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:
-- 
1.9.1

Re: [Qemu-devel] [virtio-dev][RFC 1/2] content: reserve virtio device ID

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 968 bytes --]

On Fri, Jun 17, 2016 at 06:03:13PM +0200, Christian Pinto wrote:
> Signed-off-by: Christian Pinto <c.pinto@virtualopensystems.com>
> ---
>  content.tex | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index d989d98..0d98926 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device    \\
>  \hline
>  18         &   Input device \\
>  \hline
> +19         &   Signal Distribution Module \\
> +\hline

Please use a higher number, 19 is already proposed for virtio-vsock:
https://lists.oasis-open.org/archives/virtio-dev/201603/msg00042.html

> @@ -5641,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}

Please move this to the patch that adds virtio-sdm.tex.  Each patch must
be self-contained and build successfully.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [virtio-dev][RFC 1/2] content: reserve virtio device ID

[Christian Pinto]

Hello Stefan,

On Thu, Jun 23, 2016 at 8:57 PM, Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> On Fri, Jun 17, 2016 at 06:03:13PM +0200, Christian Pinto wrote:
> > Signed-off-by: Christian Pinto <c.pinto@virtualopensystems.com>
> > ---
> >  content.tex | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/content.tex b/content.tex
> > index d989d98..0d98926 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device    \\
> >  \hline
> >  18         &   Input device \\
> >  \hline
> > +19         &   Signal Distribution Module \\
> > +\hline
>
> Please use a higher number, 19 is already proposed for virtio-vsock:
> https://lists.oasis-open.org/archives/virtio-dev/201603/msg00042.html


Thanks, I will use 20 then.


>
>
> > @@ -5641,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}
>
> Please move this to the patch that adds virtio-sdm.tex.  Each patch must
> be self-contained and build successfully.
>

Right, I will add this change to the patch adding the device specification
text.


Best,

Christian

Re: [Qemu-devel] [virtio-dev][RFC 1/2] content: reserve virtio device ID

[Cornelia Huck]

On Fri, 24 Jun 2016 14:45:29 +0200
Christian Pinto <c.pinto@virtualopensystems.com> wrote:

> On Thu, Jun 23, 2016 at 8:57 PM, Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> 
> > On Fri, Jun 17, 2016 at 06:03:13PM +0200, Christian Pinto wrote:

> > > @@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device    \\
> > >  \hline
> > >  18         &   Input device \\
> > >  \hline
> > > +19         &   Signal Distribution Module \\
> > > +\hline
> >
> > Please use a higher number, 19 is already proposed for virtio-vsock:
> > https://lists.oasis-open.org/archives/virtio-dev/201603/msg00042.html
> 
> 
> Thanks, I will use 20 then.

I think virtio-crypto already wants to use 20. We sadly have a bit of a
backlog...

Re: [Qemu-devel] [virtio-dev][RFC 1/2] content: reserve virtio device ID

[Christian Pinto]

On Fri, Jun 24, 2016 at 3:04 PM, Cornelia Huck <cornelia.huck@de.ibm.com>
wrote:

> On Fri, 24 Jun 2016 14:45:29 +0200
> Christian Pinto <c.pinto@virtualopensystems.com> wrote:
>
> > On Thu, Jun 23, 2016 at 8:57 PM, Stefan Hajnoczi <stefanha@redhat.com>
> > wrote:
> >
> > > On Fri, Jun 17, 2016 at 06:03:13PM +0200, Christian Pinto wrote:
>
> > > > @@ -2990,6 +2990,8 @@ Device ID  &  Virtio Device    \\
> > > >  \hline
> > > >  18         &   Input device \\
> > > >  \hline
> > > > +19         &   Signal Distribution Module \\
> > > > +\hline
> > >
> > > Please use a higher number, 19 is already proposed for virtio-vsock:
> > > https://lists.oasis-open.org/archives/virtio-dev/201603/msg00042.html
> >
> >
> > Thanks, I will use 20 then.
>
> I think virtio-crypto already wants to use 20. We sadly have a bit of a
> backlog...
>

Thanks Cornelia,

I'll use another number.

BTW, Is there a way to know which one is the first free device ID? I was
not following virtio-dev before, so not aware of all the pending device
specs.

Thanks,

Christian

[Qemu-devel] [virtio-dev][RFC 2/2] virtio-sdm: new device specification

[Christian Pinto]

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>
---
 virtio-sdm.tex | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 virtio-sdm.tex

diff --git a/virtio-sdm.tex b/virtio-sdm.tex
new file mode 100644
index 0000000..abbdb80
--- /dev/null
+++ b/virtio-sdm.tex
@@ -0,0 +1,126 @@
+\section{Signal Distribution Module}\label{sec:Device Types / SDM Device}
+
+The virtio Signal Distribution Module is meant to enable Inter Processor
+interrupts in QEMU/KVM environment. The SDM enables different processors in the
+same or different QEMU instances to send mutual signals. 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.
+
+\subsection{Device ID}\label{sec:Device Types / SDM Device / Device ID}
+
+19
+
+\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 host-guest communication (i.e., notification of a signal),
+while queue 1 for guest-host communication.
+
+\subsection{Feature bits}\label{sec:Device Types / SDM Device / Feature bits}
+
+None.
+
+\subsection{Device configuration layout}\label{sec:Device Types / SDM Device / 
+Device configuration layout}
+
+The device configuration is composed by three fields: \texttt{master},
+\texttt{max_slaves} and \texttt{current_slaves}.
+
+\begin{lstlisting}
+struct virtio_sdm_config {
+	u8    master;
+	u16   max_slaves;
+	u16   current_slaves;
+};
+\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
+Linux 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. 
+The value of \texttt{max_slaves} is initially set from the command line of QEMU,
+but can be changed during operations through the configuration space.
+Finally the \texttt{current_slaves} field contains the actual number of slaves
+registered to the master SDM. This field is a read only 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 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 that is specificied as a parameter when configuring the device. At the
+current state there are two types of communication channels: local and
+socket(TCP or UNIX).
+The local channel is meant for SDM devices attached to the same QEMU instance,
+while the network channel makes it possible to exchange signals between
+SDMs in different instances of QEMU. 
+The type of communication channel is chosen at QEMU boot time and cannot be
+changed during operations. It has to be noted that the behavior of the device is
+independent from the communication channel used.
+
+\subsection{Device Operation}\label{sec:Device Types / SDM Device / Device
+peration}
+
+The SDM device  handles signal coming from the two following sources:
+
+\begin{enumerate}
+\item The local processor to which the devics 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 SDM device instance (being it the same QEMU instance or a
+separate one).
+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 themsleves 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,
+};
+
+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 two 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{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.
+The \texttt{payload} is used to pass extra accompanying information with the
+signal. The use case for the payload field is a hardware mailbox, where data is
+pushed into the mailbox and an interrupt is triggered towards all the devices
+registered with the mailbox.
+
+The \texttt{SDMSignalData} structure is first filled by the source SDM kernel
+virtio driver and sent over the gh_vq.
-- 
1.9.1

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 7714 bytes --]

On Fri, Jun 17, 2016 at 06:03:14PM +0200, 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>
> ---
>  virtio-sdm.tex | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
>  create mode 100644 virtio-sdm.tex
> 
> diff --git a/virtio-sdm.tex b/virtio-sdm.tex
> new file mode 100644
> index 0000000..abbdb80
> --- /dev/null
> +++ b/virtio-sdm.tex
> @@ -0,0 +1,126 @@
> +\section{Signal Distribution Module}\label{sec:Device Types / SDM Device}
> +
> +The virtio Signal Distribution Module is meant to enable Inter Processor
> +interrupts in QEMU/KVM environment. The SDM enables different processors in the
> +same or different QEMU instances to send mutual signals. 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.

Please rephrase without mentioning QEMU/KVM.  Focus on the device
functionality and purpose.  VIRTIO's scope is more general than
QEMU/KVM.

I'm not familiar with AMP systems.  Can you explain how SDM relates to
remoteproc and rpmsg?  There seems to be overlap because remoteproc can
also boot a remote processor.

> +
> +\subsection{Device ID}\label{sec:Device Types / SDM Device / Device ID}
> +
> +19
> +
> +\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 host-guest communication (i.e., notification of a signal),
> +while queue 1 for guest-host communication.

I think the VIRTIO terminology is "device" (host) and "driver" (guest).
So it would be:

Queue 0 is used for device-to-driver communication (i.e. notification of
a signal), while queue 1 is for driver-to-device communication.

> +
> +\subsection{Feature bits}\label{sec:Device Types / SDM Device / Feature bits}
> +
> +None.
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / SDM Device / 
> +Device configuration layout}
> +
> +The device configuration is composed by three fields: \texttt{master},
> +\texttt{max_slaves} and \texttt{current_slaves}.
> +
> +\begin{lstlisting}
> +struct virtio_sdm_config {
> +	u8    master;
> +	u16   max_slaves;
> +	u16   current_slaves;
> +};
> +\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
> +Linux driver to figure out whether a specific instance is a master or a slave.

The scope of VIRTIO is wider than Linux, just saying "the driver" is
preferred.

> +The \texttt{max_slaves} field contains the maximum number of slaves supported by
> +the SDM device. 
> +The value of \texttt{max_slaves} is initially set from the command line of QEMU,

The QEMU command-line is out of scope for the VIRTIO specification.
Please drop this part of the sentence.  I suggest saying that a
configuration change notification is sent when the value of max_slaves
changes.

> +but can be changed during operations through the configuration space.
> +Finally the \texttt{current_slaves} field contains the actual number of slaves
> +registered to the master SDM. This field is a read only 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 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 that is specificied as a parameter when configuring the device. At the
> +current state there are two types of communication channels: local and
> +socket(TCP or UNIX).
> +The local channel is meant for SDM devices attached to the same QEMU instance,
> +while the network channel makes it possible to exchange signals between
> +SDMs in different instances of QEMU. 
> +The type of communication channel is chosen at QEMU boot time and cannot be
> +changed during operations. It has to be noted that the behavior of the device is
> +independent from the communication channel used.

This paragraph is specific to QEMU and not directly relevant to the
device specification.  It can be dropped (feel free to extract any
pieces of information in there that you consider relevant to the device
spec).

> +
> +\subsection{Device Operation}\label{sec:Device Types / SDM Device / Device
> +peration}
> +
> +The SDM device  handles signal coming from the two following sources:

s/  / /
s/signal/signals/

> +
> +\begin{enumerate}
> +\item The local processor to which the devics is attached to.

s/devics is attached to/device is attached/

> +\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 SDM device instance (being it the same QEMU instance or a
> +separate one).
> +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 themsleves 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,
> +};
> +
> +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 two 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{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.
> +The \texttt{payload} is used to pass extra accompanying information with the
> +signal. The use case for the payload field is a hardware mailbox, where data is
> +pushed into the mailbox and an interrupt is triggered towards all the devices
> +registered with the mailbox.

This spec cannot be implemented because it does not explain the
semantics of the payload.  Please provide those details.  Even if it
doesn't make it into the final spec it will help reviewers understand
how SDM works.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Christian Pinto]

Hello Stefan,

On Thu, Jun 23, 2016 at 9:39 PM, Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> On Fri, Jun 17, 2016 at 06:03:14PM +0200, 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>
> > ---
> >  virtio-sdm.tex | 126
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 126 insertions(+)
> >  create mode 100644 virtio-sdm.tex
> >
> > diff --git a/virtio-sdm.tex b/virtio-sdm.tex
> > new file mode 100644
> > index 0000000..abbdb80
> > --- /dev/null
> > +++ b/virtio-sdm.tex
> > @@ -0,0 +1,126 @@
> > +\section{Signal Distribution Module}\label{sec:Device Types / SDM
> Device}
> > +
> > +The virtio Signal Distribution Module is meant to enable Inter Processor
> > +interrupts in QEMU/KVM environment. The SDM enables different
> processors in the
> > +same or different QEMU instances to send mutual signals. 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.
>
> Please rephrase without mentioning QEMU/KVM.  Focus on the device
> functionality and purpose.  VIRTIO's scope is more general than
> QEMU/KVM.
>

Right, thanks for pointing this out. I will remove any dependence from
QEMU/KVM from the device specs.


>
> I'm not familiar with AMP systems.  Can you explain how SDM relates to
> remoteproc and rpmsg?  There seems to be overlap because remoteproc can
> also boot a remote processor.
>

I will clarify this aspect in the cover letter of the next release.

In any case, to give you some preliminary information, the SDM is a device
to send generic signals among processors. Such signals can be customized
according to their purpose, as an example a payload can be added to a
signal in order to model a mailbox-like behavior.
 As an example it can be used by the remoteproc driver to send the BOOT
signal to the processor that needs to be booted. Similarly it is used in
rpmsg to send a signal to the destination every time a new message is
available. RPMSG uses virtio queues to exchange messages, and the SDM is
used when kicking a virtqueue to raise the associated interrupt in the
destination processor.



> > +
> > +\subsection{Device ID}\label{sec:Device Types / SDM Device / Device ID}
> > +
> > +19
> > +
> > +\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 host-guest communication (i.e., notification of a
> signal),
> > +while queue 1 for guest-host communication.
>
> I think the VIRTIO terminology is "device" (host) and "driver" (guest).
> So it would be:
>
> Queue 0 is used for device-to-driver communication (i.e. notification of
> a signal), while queue 1 is for driver-to-device communication.
>

Thanks, I will update the text according to this comment.


>
> > +
> > +\subsection{Feature bits}\label{sec:Device Types / SDM Device / Feature
> bits}
> > +
> > +None.
> > +
> > +\subsection{Device configuration layout}\label{sec:Device Types / SDM
> Device /
> > +Device configuration layout}
> > +
> > +The device configuration is composed by three fields: \texttt{master},
> > +\texttt{max_slaves} and \texttt{current_slaves}.
> > +
> > +\begin{lstlisting}
> > +struct virtio_sdm_config {
> > +     u8    master;
> > +     u16   max_slaves;
> > +     u16   current_slaves;
> > +};
> > +\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
> > +Linux driver to figure out whether a specific instance is a master or a
> slave.
>
> The scope of VIRTIO is wider than Linux, just saying "the driver" is
> preferred.
>

ACK


>
> > +The \texttt{max_slaves} field contains the maximum number of slaves
> supported by
> > +the SDM device.
> > +The value of \texttt{max_slaves} is initially set from the command line
> of QEMU,
>
> The QEMU command-line is out of scope for the VIRTIO specification.
> Please drop this part of the sentence.  I suggest saying that a
> configuration change notification is sent when the value of max_slaves
> changes.
>

I agree, this will make the description agnostic w.r.t. QEMU.


>
> > +but can be changed during operations through the configuration space.
> > +Finally the \texttt{current_slaves} field contains the actual number of
> slaves
> > +registered to the master SDM. This field is a read only 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 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 that is specificied as a parameter when configuring the device.
> At the
> > +current state there are two types of communication channels: local and
> > +socket(TCP or UNIX).
> > +The local channel is meant for SDM devices attached to the same QEMU
> instance,
> > +while the network channel makes it possible to exchange signals between
> > +SDMs in different instances of QEMU.
> > +The type of communication channel is chosen at QEMU boot time and
> cannot be
> > +changed during operations. It has to be noted that the behavior of the
> device is
> > +independent from the communication channel used.
>
> This paragraph is specific to QEMU and not directly relevant to the
> device specification.  It can be dropped (feel free to extract any
> pieces of information in there that you consider relevant to the device
> spec).
>
>
As already commented, I will remove all dependencies from QEMU.


> > +
> > +\subsection{Device Operation}\label{sec:Device Types / SDM Device /
> Device
> > +peration}
> > +
> > +The SDM device  handles signal coming from the two following sources:
>
> s/  / /
> s/signal/signals/
>

ACK


>
> > +
> > +\begin{enumerate}
> > +\item The local processor to which the devics is attached to.
>
> s/devics is attached to/device is attached/
>
>
ACK


> > +\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 SDM device instance (being it the same QEMU
> instance or a
> > +separate one).
> > +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 themsleves 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,
> > +};
> > +
> > +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 two 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{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.
> > +The \texttt{payload} is used to pass extra accompanying information
> with the
> > +signal. The use case for the payload field is a hardware mailbox, where
> data is
> > +pushed into the mailbox and an interrupt is triggered towards all the
> devices
> > +registered with the mailbox.
>
> This spec cannot be implemented because it does not explain the
> semantics of the payload.  Please provide those details.  Even if it
> doesn't make it into the final spec it will help reviewers understand
> how SDM works.
>

Thanks again for your comments, I will improve the description in  the next
release.


Best,

Christian