[virtio-comment] Repost due mailist failure: virtio-gpio: add formal specification

[virtio-comment] Repost due mailist failure: virtio-gpio: add formal specification

[Enrico Weigelt, metux IT consult]

Hello folks,


this is repost of the patch to formally introduce specification of the already
implemented and announced virtio-gpio from late 2020. Multiple attempts of
posting this patch over the last week didn't go through the list server due to
(yet not fully understood) technical problems. Tried resubscribing to both lists,
if you receive that mail, the problems is probably solved (at least for me :))

This patch introduces a now tex'ified version of the spec already submitted
in late 2020. Back then the official specification process was blocked by yet
missing formal / editorial steps, namely ID allocation and tex'ifying the
ascii text into the official specification document. Unfortunately, I had to put
it aside for a while due to more pressing tasks (keeping public infrastructure
up and running while in the middle of lockdown).

The current specification is semantically equal (fully compatible) to the ASCII
text version from late 2020, it only received terminology changes (e.g. replacing
"host" and "guest" by "driver" and "device") some clarifications.

In the mean time, the existing implementations for both device and driver side
are running in the field, in embedded and industrial applications, indicating that
general semantics should be fine. Maybe just some minor ambiguities in the spec
text yet to be ironed out.

Note:

Maybe the protocol might look a bit strange for someone who's just looking from the
VM perspective. The design decisions also have simplicistic hardware implementations
in mind, e.g. reuse message buffers for rx and tx (no need for actual memory management),
pretty direct wireup between IO drivers and buffer latches and hardwired decoding
(no actual sram required), and most importantly: more sophisticated features (e.g.
features often implemented in pinmuxes, like stength control, trigger thresholds,
inverters, debounce filters, etc) have intentionally left out at this point and will
be added as purely optional features in the future. The idea behind that decision is
to allow very minimal implementations of the gpio itself, and leave enough time for
careful consideration where those extra units actually be placed (e.g. shall virtio-gpio
have pinmux features at all ? several pro's and con's have to be weighted yet).

I've received positive feedback HW/VHDL engieers, stating that first hw implementation
was pretty straigthforward - for ASICS they're planning more optimizations in order
to reduce the number of gates by a magnitude. (Since I'm not an HW/VHDL expert, so I
can't comment much on this)

Finally I'm feeling very confident that virtio-gpio might show good case for using
virtio even in lo-profile / lo-power hardware and good change of replacing proprietary
interchip/intrachip protocols.


best regards,
--mtx


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] [PATCH] virtio-gpio: add formal specification

[Enrico Weigelt, metux IT consult]

This patch adds specification for attaching general purpose IO (gpio) devices
via virtio. The protocol is specifically designed to be easily implemented in
software (e.g. hypervisors) as well as low end hardware (eg. silicon, FPGA,
tiny MCUs) and allows future extensions while retaining full backwards
compatibility.

Implementations for driver (Linux kernel) and device (Qemu) are publically
available and field tested since late 2020. Hardware implementations also
exist (but proprietary, cannot be published yet).

Device type ID 41 has been allocated by TC vote #3632.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 conformance.tex |  28 +++++-
 content.tex     |   3 +
 virtio-gpio.tex | 259 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 286 insertions(+), 4 deletions(-)
 create mode 100644 virtio-gpio.tex

diff --git a/conformance.tex b/conformance.tex
index a164cbb..e5956f4 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -29,8 +29,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Driver Conformance / IOMMU Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Sound Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
-\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance} or
-\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance} or
+\ref{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -52,8 +53,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Device Conformance / IOMMU Device Conformance},
 \ref{sec:Conformance / Device Conformance / Sound Device Conformance},
 \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
-\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance} or
-\ref{sec:Conformance / Device Conformance / SCMI Device Conformance}.
+\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
+\ref{sec:Conformance / Device Conformance / SCMI Device Conformance} or
+\ref{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -288,6 +290,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / SCMI Device / Device Operation / Setting Up eventq Buffers}
 \end{itemize}
 
+\conformance{\subsection}{General Purpose IO Driver Conformance}\label{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}
+
+An General Purpose IO driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
+\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
+\end{itemize}
+
 \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -527,6 +538,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / SCMI Device / Device Operation / Shared Memory Operation}
 \end{itemize}
 
+\conformance{\subsection}{General Purpose IO Device Conformance}\label{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}
+
+An General Purpose IO device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
+\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
+\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 90685fc..3e9d1a1 100644
--- a/content.tex
+++ b/content.tex
@@ -2876,6 +2876,8 @@ \chapter{Device Types}\label{sec:Device Types}
 \hline
 40         &   Bluetooth device \\
 \hline
+41         &   General Purpose IO device \\
+\hline
 \end{tabular}
 
 Some of the devices above are unspecified by this document,
@@ -6581,6 +6583,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
 \input{virtio-mem.tex}
 \input{virtio-i2c.tex}
 \input{virtio-scmi.tex}
+\input{virtio-gpio.tex}
 
 \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
diff --git a/virtio-gpio.tex b/virtio-gpio.tex
new file mode 100644
index 0000000..6ab0307
--- /dev/null
+++ b/virtio-gpio.tex
@@ -0,0 +1,259 @@
+\section{General Purpose IO Device}\label{sec:Device Types / General Purpose IO}
+
+The virtio gpio device is a general purpose IO device that supports a variable
+number of named IO lines that may be switched either as input or output and
+in logical level 0 or 1.
+
+\subsection{Device ID}\label{sec:Device Types / General Purpose IO / Device ID}
+  41
+
+\subsection{Version}\label{sec:Device Types / General Purpose IO / Version}
+  1
+
+\subsection{Device configuration layout}\label{sec:Device Types / General Purpose IO / Device configuration layout}
+
+General purpose IO configuration uses the following layout structure:
+
+\begin{tabular}{|l|l|l|}
+    \hline
+    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
+    \hline
+    0x00   & u8           & version \\
+    0x02   & u16 LE       & number of GPIO lines \\
+    0x04   & u32 LE       & size of gpio name block \\
+    0x20   & char$[$32$]$ & device name (0-terminated) \\
+    0x40   & char$[$$]$   & line names block \\
+    \hline
+\end{tabular}
+
+\begin{itemize}
+    \item for \field{version} field currently only value 1 supported.
+    \item the \field{line names block} holds a stream of zero-terminated strings,
+        containing the individual line names in ASCII. line names must unique.
+    \item unspecified fields are reserved for future use and should be zero.
+    \item future versions may extend this configuration space by additional fields.
+\end{itemize}
+
+\subsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues}
+\begin{description}
+\item[0] rx (device to CPU)
+\item[1] tx (CPU to device)
+\end{description}
+
+The virtqueues transport messages of the type struct virtio_gpio_msg from device to CPU or CPU to device.
+
+\subsubsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues / Message format}
+
+The queues transport messages of the struct virtio_gpio_msg:
+
+\begin{tabular}{|l|l|l|}
+    \hline
+    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
+    \hline
+    0x00   & uint16 LE & message type  \\
+    0x02   & uint16 LE & line id       \\
+    0x04   & uint32 LE & value         \\
+    \hline
+\end{tabular}
+
+\subsubsection{Message types}\label{sec:Device Types / General Purpose IO / Virtqueues / Message types}
+
+\begin{tabular}{|l|ll|}
+    \hline
+    \textbf{Code} & \textbf{Symbol} & \\
+    \hline
+    0x0001        & VIRTIO_GPIO_MSG_CPU_REQUEST            & request gpio line           \\
+    0x0002        & VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT    & set direction to input      \\
+    0x0003        & VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT   & set direction to output     \\
+    0x0004        & VIRTIO_GPIO_MSG_CPU_GET_DIRECTION      & read current direction      \\
+    0x0005        & VIRTIO_GPIO_MSG_CPU_GET_LEVEL          & read current level          \\
+    0x0006        & VIRTIO_GPIO_MSG_CPU_SET_LEVEL          & set current (out) level     \\
+    0x0011        & VIRTIO_GPIO_MSG_DEVICE_LEVEL           & level changed (device->CPU) \\
+    \hline
+    0x8000        & VIRTIO_GPIO_MSG_REPLY                  & device reply mask           \\
+    \hline
+\end{tabular}
+
+\devicenormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
+
+The device MUST read from the tx queue and write to rx queue.
+
+The device MUST NOT write to the tx queue.
+
+\drivernormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
+
+The device MUST read from the rx queue and write to tx queue.
+
+The device MUST NOT write to the rx queue.
+
+\subsection{Data flow}\label{sec:Device Types / General Purpose IO / Data flow}
+
+\begin{itemize}
+    \item all operations, except \field{VIRTIO_GPIO_MSG_DEVICE_LEVEL}, are initiated by CPU (tx queue)
+    \item device replies with the orinal \field{type} value OR'ed with \field{VIRTIO_GPIO_MSG_REPLY} (rx queue)
+    \item requests are processed and replied in the they had been sent
+    \item async notifications by the device may be interleaved with request responses
+    \item VIRTIO_GPIO_MSG_DEVICE_LEVEL is only sent asynchronously from device to CPU
+    \item in replies, a negative \field{value} field denotes an Unix-style / POSIX errno code
+    \item the actual error values \textit{(despite being negative or not)} is only of informational
+          nature -- a device or vm host \textit{may} report more detailed error cause but is not required to.
+    \item valid direction values are: 0 = output, 1 = input
+    \item valid line level values are: 0 = inactive, 1 = active
+    \item CPU should not send messages with unspecified \field{type} value
+    \item CPU should ignore ignore messages with unspecified \field{type} value
+\end{itemize}
+
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_REQUEST}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_REQUEST}
+
+Notify the device that given line number is going to be used.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT}
+
+Set line line direction to input.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT}
+
+Set line direction to output and given line level.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & output level (0=inactive, 1=active) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_DIRECTION}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_GET_DIRECTION}
+
+Retrieve line direction.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & direction (0=output, 1=input) or POSIX errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_GET_LEVEL}
+
+Retrieve line level.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & line level (0=inactive, 1=active) or errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_SET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_SET_LEVEL}
+
+Set line level (output only)
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & line level (0=inactive, 1=active) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & new line level or (negative) POSIX errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_DEVICE_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_DEVICE_LEVEL}
+
+Async notification from device to CPU: line level changed
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \field{value} field: & line level (0=inactive, 1=active) \\
+    \hline
+\end{tabular}
+
+\devicenormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
+
+The device MUST reply to all driver requests in they had been sent.
+
+The device MUST copy the \field{line} field from the request to its reply.
+
+Except for async notification, the device MUST reply the orignal message type, but with the highest bit set
+(or'ed with VIRTIO_GPIO_MSG_REPLY)
+
+In case of error the device MUST fill an negative value into the \field{value} field, it SHOULD use
+an fitting POSIX / Unix errno value when applicable.
+
+On switching to output, the device SHOULD set internal output level before switching the line to output.
+
+The device SHOULD send VIRTIO_GPIO_MSG_DEVICE_LEVEL message for a particular line when it is in input
+direction and line level changes.
+
+\drivernormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
+
+The driver MUST NOT send VIRTIO_GPIO_MSG_DEVICE_LEVEL and MUST NOT set the highest bit in the message type.
+
+The driver MUST NOT send messages with types not defined in this specification.
+
+\subsection{Future versions}\label{sec:Device Types / General Purpose IO / Future versions}
+
+\begin{itemize}
+    \item future versions must increment the ``version`` value
+    \item the basic data structures (config space, message format) should remain
+          backwards compatible, but may increased in size or use reserved fields
+    \item device needs to support commands in older versions
+    \item CPU should not send commands of newer versions that the device doesn't support
+\end{itemize}
+
+\subsection{Feature bits}\label{sec:Device Types / General Purpose IO / Feature bits}
+
+There are currently no feature bits defined for this device, but may be added in future versions.
-- 
2.20.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] Re: [virtio-dev] [PATCH] virtio-gpio: add formal specification

[Viresh Kumar]

+ couple of folks who have shown interest in this stuff.

On 17-06-21, 11:41, Enrico Weigelt, metux IT consult wrote:
> This patch adds specification for attaching general purpose IO (gpio) devices
> via virtio. The protocol is specifically designed to be easily implemented in
> software (e.g. hypervisors) as well as low end hardware (eg. silicon, FPGA,
> tiny MCUs) and allows future extensions while retaining full backwards
> compatibility.
> 
> Implementations for driver (Linux kernel) and device (Qemu) are publically
> available and field tested since late 2020. Hardware implementations also
> exist (but proprietary, cannot be published yet).
> 
> Device type ID 41 has been allocated by TC vote #3632.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  conformance.tex |  28 +++++-
>  content.tex     |   3 +
>  virtio-gpio.tex | 259 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 286 insertions(+), 4 deletions(-)
>  create mode 100644 virtio-gpio.tex

FWIW ./makehtml.sh reported a few missing references I believe, it
wasn't a clean build.

> diff --git a/conformance.tex b/conformance.tex
> index a164cbb..e5956f4 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -29,8 +29,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Driver Conformance / IOMMU Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Sound Driver Conformance},
>  \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
> -\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance} or
> -\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance}.
> +\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
> +\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance} or
> +\ref{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}.
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -52,8 +53,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \ref{sec:Conformance / Device Conformance / IOMMU Device Conformance},
>  \ref{sec:Conformance / Device Conformance / Sound Device Conformance},
>  \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
> -\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance} or
> -\ref{sec:Conformance / Device Conformance / SCMI Device Conformance}.
> +\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
> +\ref{sec:Conformance / Device Conformance / SCMI Device Conformance} or
> +\ref{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}.
>  
>      \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
>    \end{itemize}
> @@ -288,6 +290,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Device Types / SCMI Device / Device Operation / Setting Up eventq Buffers}
>  \end{itemize}
>  
> +\conformance{\subsection}{General Purpose IO Driver Conformance}\label{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}
> +
> +An General Purpose IO driver MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
> +\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
> +\end{itemize}
> +
>  \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
>  
>  A device MUST conform to the following normative statements:
> @@ -527,6 +538,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Device Types / SCMI Device / Device Operation / Shared Memory Operation}
>  \end{itemize}
>  
> +\conformance{\subsection}{General Purpose IO Device Conformance}\label{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}
> +
> +An General Purpose IO device MUST conform to the following normative statements:
> +
> +\begin{itemize}
> +\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
> +\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
> +\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 90685fc..3e9d1a1 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2876,6 +2876,8 @@ \chapter{Device Types}\label{sec:Device Types}
>  \hline
>  40         &   Bluetooth device \\
>  \hline
> +41         &   General Purpose IO device \\
> +\hline
>  \end{tabular}
>  
>  Some of the devices above are unspecified by this document,
> @@ -6581,6 +6583,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>  \input{virtio-mem.tex}
>  \input{virtio-i2c.tex}
>  \input{virtio-scmi.tex}
> +\input{virtio-gpio.tex}
>  
>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
>  
> diff --git a/virtio-gpio.tex b/virtio-gpio.tex
> new file mode 100644
> index 0000000..6ab0307
> --- /dev/null
> +++ b/virtio-gpio.tex
> @@ -0,0 +1,259 @@
> +\section{General Purpose IO Device}\label{sec:Device Types / General Purpose IO}
> +
> +The virtio gpio device is a general purpose IO device that supports a variable
> +number of named IO lines that may be switched either as input or output and
> +in logical level 0 or 1.
> +
> +\subsection{Device ID}\label{sec:Device Types / General Purpose IO / Device ID}
> +  41
> +
> +\subsection{Version}\label{sec:Device Types / General Purpose IO / Version}
> +  1
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / General Purpose IO / Device configuration layout}
> +
> +General purpose IO configuration uses the following layout structure:
> +
> +\begin{tabular}{|l|l|l|}
> +    \hline
> +    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
> +    \hline
> +    0x00   & u8           & version \\

As previously mentioned by Jason as well, most of the virtio protocol
specifications don't talk about versions but rather Features. The
features are normally more readable than saying version is 1.0 or 2.0.

This is a standard used by Virtio specification so we should reuse
that instead of creating protocol specific versions.

> +    0x02   & u16 LE       & number of GPIO lines \\
> +    0x04   & u32 LE       & size of gpio name block \\
> +    0x20   & char$[$32$]$ & device name (0-terminated) \\
> +    0x40   & char$[$$]$   & line names block \\

I understand from our previous discussions that you wanted to make it
possible for this structure to be extended in the future and so kept
the extra padding of 24 bytes. That is a right thing to do, as we may
need to add more fields here.

Having said that, the only thing which limits this structure to be
extended in future is the "line names block", simply because we don't
know the size of it in advance and we need to keep it at the end for
the same reason.

A better approach for that, as Jean Philippe suggested earlier, is to
rather add a new field, "gpio names offset", which will convey the
offset of the names block from the beginning of the structure. In your
case you can keep it to 0x40, while others can avoid the rather
unnecessary padding.

You may think today that 24 bytes of padding or reserved fields would
be sufficient for ever, but that may not be true. The requirements
change all the time and we shouldn't limit the possibility of
extension here, as you have earlier said.


> +    \hline
> +\end{tabular}
> +
> +\begin{itemize}
> +    \item for \field{version} field currently only value 1 supported.
> +    \item the \field{line names block} holds a stream of zero-terminated strings,
> +        containing the individual line names in ASCII. line names must unique.
> +    \item unspecified fields are reserved for future use and should be zero.
> +    \item future versions may extend this configuration space by additional fields.
> +\end{itemize}
> +
> +\subsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues}
> +\begin{description}
> +\item[0] rx (device to CPU)

What is CPU here? Driver/Guest ? I wonder if it would be better if we
talk in terms of Virtio spec standards here, which are "device" and
"driver".

> +\item[1] tx (CPU to device)
> +\end{description}
> +
> +The virtqueues transport messages of the type struct virtio_gpio_msg from device to CPU or CPU to device.
> +
> +\subsubsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues / Message format}
> +
> +The queues transport messages of the struct virtio_gpio_msg:
> +
> +\begin{tabular}{|l|l|l|}
> +    \hline
> +    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
> +    \hline
> +    0x00   & uint16 LE & message type  \\
> +    0x02   & uint16 LE & line id       \\
> +    0x04   & uint32 LE & value         \\
> +    \hline
> +\end{tabular}

It would be fine to have same format for request and response in our
use case.

> +
> +\subsubsection{Message types}\label{sec:Device Types / General Purpose IO / Virtqueues / Message types}
> +
> +\begin{tabular}{|l|ll|}
> +    \hline
> +    \textbf{Code} & \textbf{Symbol} & \\
> +    \hline
> +    0x0001        & VIRTIO_GPIO_MSG_CPU_REQUEST            & request gpio line           \\

This requires a corresponding "FREE" Msg type as well. This is already
well implemented and accepted in Linux kernel as well.

> +    0x0002        & VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT    & set direction to input      \\
> +    0x0003        & VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT   & set direction to output     \\
> +    0x0004        & VIRTIO_GPIO_MSG_CPU_GET_DIRECTION      & read current direction      \\
> +    0x0005        & VIRTIO_GPIO_MSG_CPU_GET_LEVEL          & read current level          \\
> +    0x0006        & VIRTIO_GPIO_MSG_CPU_SET_LEVEL          & set current (out) level     \\

These look good.

> +    0x0011        & VIRTIO_GPIO_MSG_DEVICE_LEVEL           & level changed (device->CPU) \\

I believe this is the interrupt from Device to Driver ? I believe it
would be more readable if it can be called as an interrupt.

I do not see a way for masking/unmasking interrupts right now, we need
them right now. I am fine with adding other request types, like
debounce, etc, later on, but interrupt support is bare minimum for us.
These are the new request types we would need for that, unless there
are more:

IRQ_TYPE	IRQ_MASK	IRQ_UNMASK

And these are the interrupt types we need to support, for now. Again
they are quite standard if you look at Linux's GPIO support.

IRQ_TYPE_NONE	
IRQ_TYPE_EDGE_RISING
IRQ_TYPE_EDGE_FALLING	
IRQ_TYPE_EDGE_BOTH	
IRQ_TYPE_LEVEL_HIGH	
IRQ_TYPE_LEVEL_LOW	

> +    \hline
> +    0x8000        & VIRTIO_GPIO_MSG_REPLY                  & device reply mask           \\
> +    \hline
> +\end{tabular}
> +
> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
> +
> +The device MUST read from the tx queue and write to rx queue.
> +
> +The device MUST NOT write to the tx queue.
> +
> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
> +
> +The device MUST read from the rx queue and write to tx queue.

s/device/driver/ ?

> +
> +The device MUST NOT write to the rx queue.

Same.

> +
> +\subsection{Data flow}\label{sec:Device Types / General Purpose IO / Data flow}
> +
> +\begin{itemize}
> +    \item all operations, except \field{VIRTIO_GPIO_MSG_DEVICE_LEVEL}, are initiated by CPU (tx queue)
> +    \item device replies with the orinal \field{type} value OR'ed with \field{VIRTIO_GPIO_MSG_REPLY} (rx queue)

original*

> +    \item requests are processed and replied in the they had been sent

in the *order* they...

Why is this important really ? If there are constraints in the control
flow (i.e. operation 1 must happen before operation 2), then the
request initiator needs to make sure of that. Otherwise, it doesn't
matter which one finishes first. The response message will have the
GPIO number in it and so the initiator will know whose response it is.

While on that, we must put a limit on number of messages started by
one side for the same GPIO line. i.e. Driver can not start another
operation for the same GPIO line, before receiving response for the
previous request for that GPIO line. Same goes for interrupts from
device to driver/CPU.

> +    \item async notifications by the device may be interleaved with request responses
> +    \item VIRTIO_GPIO_MSG_DEVICE_LEVEL is only sent asynchronously from device to CPU
> +    \item in replies, a negative \field{value} field denotes an Unix-style / POSIX errno code
> +    \item the actual error values \textit{(despite being negative or not)} is only of informational
> +          nature -- a device or vm host \textit{may} report more detailed error cause but is not required to.
> +    \item valid direction values are: 0 = output, 1 = input
> +    \item valid line level values are: 0 = inactive, 1 = active
> +    \item CPU should not send messages with unspecified \field{type} value
> +    \item CPU should ignore ignore messages with unspecified \field{type} value

double "ignore".

> +\end{itemize}
> +
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_REQUEST}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_REQUEST}
> +
> +Notify the device that given line number is going to be used.
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} & \\
> +    \hline
> +    \field{line}  field: & logical line number \\

s/logical/GPIO/ ?

> +    \field{value} field: & unused (should be zero) \\

Missing \hline here.

> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT}
> +
> +Set line line direction to input.
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & unused (should be zero) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT}
> +
> +Set line direction to output and given line level.
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & output level (0=inactive, 1=active) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & POSIX errno code (0 = success, non-zero = error) \\
> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_DIRECTION}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_GET_DIRECTION}
> +
> +Retrieve line direction.
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} & \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & unused (should be zero) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & direction (0=output, 1=input) or POSIX errno code \\
> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_GET_LEVEL}
> +
> +Retrieve line level.
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} & \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & unused (should be zero) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & line level (0=inactive, 1=active) or errno code \\
> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_CPU_SET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_SET_LEVEL}
> +
> +Set line level (output only)
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} & \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & line level (0=inactive, 1=active) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \hline
> +    \field{value} field: & new line level or (negative) POSIX errno code \\

Why not 0 for success? How will the driver use the returned value here ?

> +    \hline
> +\end{tabular}
> +
> +\subsubsection{VIRTIO_GPIO_MSG_DEVICE_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_DEVICE_LEVEL}
> +
> +Async notification from device to CPU: line level changed
> +
> +\begin{tabular}{ll}
> +    \hline
> +    \textbf{request:} & \\
> +    \hline
> +    \field{line}  field: & logical line number \\
> +    \field{value} field: & unused (should be zero) \\
> +    \hline
> +    \textbf{reply:} & \\
> +    \field{value} field: & line level (0=inactive, 1=active) \\
> +    \hline
> +\end{tabular}
> +
> +\devicenormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
> +
> +The device MUST reply to all driver requests in they had been sent.
> +
> +The device MUST copy the \field{line} field from the request to its reply.
> +
> +Except for async notification, the device MUST reply the orignal message type, but with the highest bit set
> +(or'ed with VIRTIO_GPIO_MSG_REPLY)
> +
> +In case of error the device MUST fill an negative value into the \field{value} field, it SHOULD use
> +an fitting POSIX / Unix errno value when applicable.
> +
> +On switching to output, the device SHOULD set internal output level before switching the line to output.
> +
> +The device SHOULD send VIRTIO_GPIO_MSG_DEVICE_LEVEL message for a particular line when it is in input
> +direction and line level changes.
> +
> +\drivernormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
> +
> +The driver MUST NOT send VIRTIO_GPIO_MSG_DEVICE_LEVEL and MUST NOT set the highest bit in the message type.

The device would need some kind of acknowledgement (as Linus also
asked earlier) for the interrupt from the driver, before which it is
eligible to send another interrupt for the same line.

> +
> +The driver MUST NOT send messages with types not defined in this specification.
> +
> +\subsection{Future versions}\label{sec:Device Types / General Purpose IO / Future versions}
> +
> +\begin{itemize}
> +    \item future versions must increment the ``version`` value
> +    \item the basic data structures (config space, message format) should remain
> +          backwards compatible, but may increased in size or use reserved fields
> +    \item device needs to support commands in older versions
> +    \item CPU should not send commands of newer versions that the device doesn't support
> +\end{itemize}
> +
> +\subsection{Feature bits}\label{sec:Device Types / General Purpose IO / Feature bits}
> +
> +There are currently no feature bits defined for this device, but may be added in future versions.

And yes please use a feature for adding IRQ support, as you previously
said some of the implementations may not define it.

Thanks.

-- 
viresh

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] Re: [virtio-dev] [PATCH] virtio-gpio: add formal specification

[Enrico Weigelt, metux IT consult]

On 17.06.21 13:33, Viresh Kumar wrote:

> FWIW ./makehtml.sh reported a few missing references I believe, it
> wasn't a clean build.

I'm seeing those (also w/ makepdf.sh) even in master. Since they only
appear pretty early, while the script does several xetex runs, I'd
suspect that's the usual tex behaviour:

When using xrefs, you have to do multiple runs, since it's working
linear and doesn't know the targets in the first runs. But it writes
the targets it encounters in a temporary file which is loaded in
subsequent runs - then the xrefs can be resolved and the warning goes
away. Same when using toc / index / bibliography.

>> +\begin{tabular}{|l|l|l|}
>> +    \hline
>> +    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
>> +    \hline
>> +    0x00   & u8           & version \\
> 
> As previously mentioned by Jason as well, most of the virtio protocol
> specifications don't talk about versions but rather Features. The
> features are normally more readable than saying version is 1.0 or 2.0.

Right, but these are separate things. I believe that most future
extensions can be done by feature bits. But it seems good to have
something in reserve in case that isn't enough some day - otherwise
we'd have to allocate a new ID for that.

Finally it's just one byte in a ROM :p

> I understand from our previous discussions that you wanted to make it
> possible for this structure to be extended in the future and so kept
> the extra padding of 24 bytes. That is a right thing to do, as we may
> need to add more fields here.

I see two options:
a) add some reserve space (but with unpredictable readout data)
b) increase the size in future versions

I'll have to check back w/ my HW folks whether an reserved but
unassigned address space adds a notable amount of gates. There might
be some special handling necessary, since we don't wanna end up in
faults when a fully valid driver reads out more rom space than the
device actually has.

> Having said that, the only thing which limits this structure to be
> extended in future is the "line names block", simply because we don't
> know the size of it in advance and we need to keep it at the end for
> the same reason.

We have it's size, so the driver can calculate it (the likehood that
driver is done by some - even little - cpu is much higher than on the
device side). So, the next block would start right after that.

> A better approach for that, as Jean Philippe suggested earlier, is to
> rather add a new field, "gpio names offset", which will convey the
> offset of the names block from the beginning of the structure. In your
> case you can keep it to 0x40, while others can avoid the rather
> unnecessary padding.

Okay, but we'd still need the size field.

> What is CPU here? Driver/Guest ? I wonder if it would be better if we
> talk in terms of Virtio spec standards here, which are "device" and
> "driver".

Right, should be called "driver". Seems some of the terminology things
slipped through.

>> +\subsubsection{Message types}\label{sec:Device Types / General Purpose IO / Virtqueues / Message types}
>> +
>> +\begin{tabular}{|l|ll|}
>> +    \hline
>> +    \textbf{Code} & \textbf{Symbol} & \\
>> +    \hline
>> +    0x0001        & VIRTIO_GPIO_MSG_CPU_REQUEST            & request gpio line           \\
> 
> This requires a corresponding "FREE" Msg type as well. This is already
> well implemented and accepted in Linux kernel as well.

Okay, lets add it.

>> +    0x0002        & VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT    & set direction to input      \\
>> +    0x0003        & VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT   & set direction to output     \\
>> +    0x0004        & VIRTIO_GPIO_MSG_CPU_GET_DIRECTION      & read current direction      \\
>> +    0x0005        & VIRTIO_GPIO_MSG_CPU_GET_LEVEL          & read current level          \\
>> +    0x0006        & VIRTIO_GPIO_MSG_CPU_SET_LEVEL          & set current (out) level     \\
> 
> These look good.
> 
>> +    0x0011        & VIRTIO_GPIO_MSG_DEVICE_LEVEL           & level changed (device->CPU) \\
> 
> I believe this is the interrupt from Device to Driver ? I believe it
> would be more readable if it can be called as an interrupt.

Well, you could call it interrupt, sort of. Actually it's a level state
change notification - pure IRQ wouldn't necessarily carry the extra
data. For an HW engineer, an IRQ usually would be essentially one edge,
and one has to look very closely to find out what some particular edge
is actually meaning. IRQs usually just tell that *something* happened,
but you'll need extra steps (e.g. read out some registers) to find out
what actually did happen.

In this case, the DEVICE_LEVEL message is on a much higher level than a
plain IRQ, it also carries the information what exactly happened.

I'd say let the TC folks decide on the appropriate terminology.

> I do not see a way for masking/unmasking interrupts right now, we need
> them right now. 

That's correct and intended. The problem is this needs extra logic on
the device for an interrupt controller, so I would hate to make it
mandatory for all. Not all gpio-controllers have such logic. On many
SoCs the gpio-controller has only *one* global IRQ line routed to some
IRQ controller somewhere else in the SOC. It depends on the actual setup
how we learn what actually happens. On Linux, when an gpio-consumer gets
a notification on some particular pin, a lot of (highly hw specific)
things already happened (eg. reading out several IRQ controllers to
follow the IRQ chain and finally reading the gpio line states from the
gpio-controller, possibly comparing w/ the last known state in order
to detect the edge direction, etc). Oh, and it even gets more complex:
some IRQ controllers automatically mask when IRQ is fired and need
explicit ack, some don't ... phuh.

Not trivial at all to find a good common ground for all scenarios as
well as minimal hardware requirements. That really needs some much
deeper thoughts to get it really right. Needs more discussions with
HW engineers.

You're certainly right that, at some point, we should have that. But it
really should be optional, so that actual silicon isn't required to have
its own IRQ controller.

> I am fine with adding other request types, like
> debounce, etc, later on, but interrupt support is bare minimum for us.

Do you have such a high load or extreme low-power requirements that you
cannot live without irq masking, until we have it as an optional feature
extension ? What HW are you using for the driver side ?

> These are the new request types we would need for that, unless there
> are more:
> 
> IRQ_TYPE	IRQ_MASK	IRQ_UNMASK

With the mask it's getting interesting. Note we're (theoretically)
supporting 2^16 lines (real implementations probably stay much below
that), so we'd need enough room for that.

Maybe we should do it individually per line. This causes more traffic
at setup, but also has the positive side effect that driver can easily
mask/unmask invidual lines without remembering the mask.

> And these are the interrupt types we need to support, for now. Again
> they are quite standard if you look at Linux's GPIO support.
> 
> IRQ_TYPE_NONE	
> IRQ_TYPE_EDGE_RISING
> IRQ_TYPE_EDGE_FALLING	
> IRQ_TYPE_EDGE_BOTH	
> IRQ_TYPE_LEVEL_HIGH	
> IRQ_TYPE_LEVEL_LOW

Well, "quite standard" is only when looking at what Linux gpio-subsys
supports. That doesn't mean that actual HW supports it directly. When
we make all these variants mandatory, we force a huge increase of the
hardware size (number of gates, chip size, power consumption).

We should be very careful in our decisions.

Note: I'm not doing this just for some host to vm signaling, but also
for actual hardware - embedded hardware. My goal also is laying out the
basis so this could become a standard interface even on-chip gpio-
controllers.

>> +    \hline
>> +    0x8000        & VIRTIO_GPIO_MSG_REPLY                  & device reply mask           \\
>> +    \hline
>> +\end{tabular}
>> +
>> +\devicenormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
>> +
>> +The device MUST read from the tx queue and write to rx queue.
>> +
>> +The device MUST NOT write to the tx queue.
>> +
>> +\drivernormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
>> +
>> +The device MUST read from the rx queue and write to tx queue.
> 
> s/device/driver/ ?

You're right. Fixed that.

>> +
>> +\subsection{Data flow}\label{sec:Device Types / General Purpose IO / Data flow}
>> +
>> +\begin{itemize}
>> +    \item all operations, except \field{VIRTIO_GPIO_MSG_DEVICE_LEVEL}, are initiated by CPU (tx queue)
>> +    \item device replies with the orinal \field{type} value OR'ed with \field{VIRTIO_GPIO_MSG_REPLY} (rx queue)
> 
> original*

fixed.

>> +    \item requests are processed and replied in the they had been sent
> 
> in the *order* they...

fixed.

> Why is this important really ? If there are constraints in the control
> flow (i.e. operation 1 must happen before operation 2), then the
> request initiator needs to make sure of that. 

The ordering is for associating request + reply. Otherwise we'd need
to add some kind of request identification (eq. seq numbers). I don't
see any good reason why some HW shouldn't keep the order inherently,
but it's probably better to leave no room for diferent interpretations
here.

> Otherwise, it doesn't
> matter which one finishes first. The response message will have the
> GPIO number in it and so the initiator will know whose response it is.

That's correct. At least for the currently defined operations, which
always have pin number. Who knows what the future will bring, so IMHO
it's better to care about that (even just hypothetical) issue now,
instead of running into trouble some time later.

Do you have any reason for NOT having this constraint.

> While on that, we must put a limit on number of messages started by
> one side for the same GPIO line. i.e. Driver can not start another
> operation for the same GPIO line, before receiving response for the
> previous request for that GPIO line. Same goes for interrupts from
> device to driver/CPU.

Queue size isn't enough for you ?

Or do you want an explicit per-line limit ? Why do you feel its needed ?
Is there anything that can go wrong with that ? Do you know any HW gpio
controller that has such a limit ?

Certainly, it might not be practically useful to fire too many changes
in a very short time, but could it cause any actual problem ?

>> +\end{itemize}
>> +
>> +
>> +\subsubsection{VIRTIO_GPIO_MSG_CPU_REQUEST}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO_GPIO_MSG_CPU_REQUEST}
>> +
>> +Notify the device that given line number is going to be used.
>> +
>> +\begin{tabular}{ll}
>> +    \hline
>> +    \textbf{request:} & \\
>> +    \hline
>> +    \field{line}  field: & logical line number \\
> 
> s/logical/GPIO/ ?

if you insist, i'll change that ;-)

By the way: we've got a lot of repeated text here that calls for putting
it into macros. Since it's a big document - do we have any constraints
on the naming of such local macros ? I'm reluctant to doing dirty tricks
with undefining commands :o

>> +\begin{tabular}{ll}
>> +    \hline
>> +    \textbf{request:} & \\
>> +    \hline
>> +    \field{line}  field: & logical line number \\
>> +    \field{value} field: & line level (0=inactive, 1=active) \\
>> +    \hline
>> +    \textbf{reply:} & \\
>> +    \hline
>> +    \field{value} field: & new line level or (negative) POSIX errno code \\
> 
> Why not 0 for success? How will the driver use the returned value here ?

The idea was device doesn't need to touch this field if everything's
fine.

>> +\drivernormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
>> +
>> +The driver MUST NOT send VIRTIO_GPIO_MSG_DEVICE_LEVEL and MUST NOT set the highest bit in the message type.
> 
> The device would need some kind of acknowledgement (as Linus also
> asked earlier) for the interrupt from the driver, before which it is
> eligible to send another interrupt for the same line.

No, this version does not have any kind of irq controller functionality.
It just sends this message unconditionally.

As already mentioned above, masking shall be an optional extra feature.

I'd suggest splitting the discussion into the basic functionality (that
we already have now) and additional features like IRQ controller etc.




--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] Re: [virtio-dev] [PATCH] virtio-gpio: add formal specification

[Viresh Kumar]

On 18-06-21, 11:19, Enrico Weigelt, metux IT consult wrote:
> On 17.06.21 13:33, Viresh Kumar wrote:
> 
> > FWIW ./makehtml.sh reported a few missing references I believe, it
> > wasn't a clean build.
> 
> I'm seeing those (also w/ makepdf.sh) even in master.

I don't see errors in master (build doesn't stop), but with your patch. With
makehtml build stops in between and I have to hit enter to let it continue. And
there are missing references from GPIO itself I think.

I haven't tried makepdf though, it looks different in that sense to me.

> Since they only
> appear pretty early, while the script does several xetex runs, I'd
> suspect that's the usual tex behaviour:
> 
> When using xrefs, you have to do multiple runs, since it's working
> linear and doesn't know the targets in the first runs. But it writes
> the targets it encounters in a temporary file which is loaded in
> subsequent runs - then the xrefs can be resolved and the warning goes
> away. Same when using toc / index / bibliography.

Try makehtml, you will see the GPIO ones.

> >> +\begin{tabular}{|l|l|l|}
> >> +    \hline
> >> +    \textbf{Offset} & \textbf{Type} & \textbf{Description} \\
> >> +    \hline
> >> +    0x00   & u8           & version \\
> > 
> > As previously mentioned by Jason as well, most of the virtio protocol
> > specifications don't talk about versions but rather Features. The
> > features are normally more readable than saying version is 1.0 or 2.0.
> 
> Right, but these are separate things. I believe that most future
> extensions can be done by feature bits. But it seems good to have
> something in reserve in case that isn't enough some day - otherwise
> we'd have to allocate a new ID for that.

If that day occurs, we will reserve a feature bit for version and a field in
config structure for the same. There is no need of it right now and it shouldn't
be there.

> Finally it's just one byte in a ROM :p

It doesn't really matter, this is not required. Virtio takes care of such stuff
with features and so should we.

> > I understand from our previous discussions that you wanted to make it
> > possible for this structure to be extended in the future and so kept
> > the extra padding of 24 bytes. That is a right thing to do, as we may
> > need to add more fields here.
> 
> I see two options:
> a) add some reserve space (but with unpredictable readout data)
> b) increase the size in future versions
> 
> I'll have to check back w/ my HW folks whether an reserved but
> unassigned address space adds a notable amount of gates. There might
> be some special handling necessary, since we don't wanna end up in
> faults when a fully valid driver reads out more rom space than the
> device actually has.
> 
> > Having said that, the only thing which limits this structure to be
> > extended in future is the "line names block", simply because we don't
> > know the size of it in advance and we need to keep it at the end for
> > the same reason.
> 
> We have it's size, so the driver can calculate it (the likehood that
> driver is done by some - even little - cpu is much higher than on the
> device side). So, the next block would start right after that.
> 
> > A better approach for that, as Jean Philippe suggested earlier, is to
> > rather add a new field, "gpio names offset", which will convey the
> > offset of the names block from the beginning of the structure. In your
> > case you can keep it to 0x40, while others can avoid the rather
> > unnecessary padding.
> 
> Okay, but we'd still need the size field.

Oh yes, you need size as well as offset. With these two, we can actually add
another such block after names block, in future if the need arises.

> > What is CPU here? Driver/Guest ? I wonder if it would be better if we
> > talk in terms of Virtio spec standards here, which are "device" and
> > "driver".
> 
> Right, should be called "driver". Seems some of the terminology things
> slipped through.
>
> >> +\subsubsection{Message types}\label{sec:Device Types / General Purpose IO / Virtqueues / Message types}
> >> +
> >> +\begin{tabular}{|l|ll|}
> >> +    \hline
> >> +    \textbf{Code} & \textbf{Symbol} & \\
> >> +    \hline
> >> +    0x0001        & VIRTIO_GPIO_MSG_CPU_REQUEST            & request gpio line           \\
> > 
> > This requires a corresponding "FREE" Msg type as well. This is already
> > well implemented and accepted in Linux kernel as well.
> 
> Okay, lets add it.
> 
> >> +    0x0002        & VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT    & set direction to input      \\
> >> +    0x0003        & VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT   & set direction to output     \\
> >> +    0x0004        & VIRTIO_GPIO_MSG_CPU_GET_DIRECTION      & read current direction      \\
> >> +    0x0005        & VIRTIO_GPIO_MSG_CPU_GET_LEVEL          & read current level          \\
> >> +    0x0006        & VIRTIO_GPIO_MSG_CPU_SET_LEVEL          & set current (out) level     \\
> > 
> > These look good.
> > 
> >> +    0x0011        & VIRTIO_GPIO_MSG_DEVICE_LEVEL           & level changed (device->CPU) \\
> > 
> > I believe this is the interrupt from Device to Driver ? I believe it
> > would be more readable if it can be called as an interrupt.
> 
> Well, you could call it interrupt, sort of. Actually it's a level state
> change notification - pure IRQ wouldn't necessarily carry the extra
> data. For an HW engineer, an IRQ usually would be essentially one edge,
> and one has to look very closely to find out what some particular edge
> is actually meaning. IRQs usually just tell that *something* happened,
> but you'll need extra steps (e.g. read out some registers) to find out
> what actually did happen.

Right, that is why the driver first needs to configure a particular GPIO line
for a particular IRQ type, like edge (falling/rising) or level, so it doesn't
need to get additional information. The irq came exactly for how it is
programmed to come.

> In this case, the DEVICE_LEVEL message is on a much higher level than a
> plain IRQ, it also carries the information what exactly happened.
> 
> I'd say let the TC folks decide on the appropriate terminology.
> 
> > I do not see a way for masking/unmasking interrupts right now, we need
> > them right now. 
> 
> That's correct and intended. The problem is this needs extra logic on
> the device for an interrupt controller, so I would hate to make it
> mandatory for all. Not all gpio-controllers have such logic. On many
> SoCs the gpio-controller has only *one* global IRQ line routed to some
> IRQ controller somewhere else in the SOC. It depends on the actual setup
> how we learn what actually happens. On Linux, when an gpio-consumer gets
> a notification on some particular pin, a lot of (highly hw specific)
> things already happened (eg. reading out several IRQ controllers to
> follow the IRQ chain and finally reading the gpio line states from the
> gpio-controller, possibly comparing w/ the last known state in order
> to detect the edge direction, etc). Oh, and it even gets more complex:
> some IRQ controllers automatically mask when IRQ is fired and need
> explicit ack, some don't ... phuh.
> 
> Not trivial at all to find a good common ground for all scenarios as
> well as minimal hardware requirements. That really needs some much
> deeper thoughts to get it really right. Needs more discussions with
> HW engineers.
> 
> You're certainly right that, at some point, we should have that. But it
> really should be optional, so that actual silicon isn't required to have
> its own IRQ controller.

I think we have already found a way for making it optional, using feature bit
i.e.. What's new now ?

> > I am fine with adding other request types, like
> > debounce, etc, later on, but interrupt support is bare minimum for us.
> 
> Do you have such a high load or extreme low-power requirements that you
> cannot live without irq masking, until we have it as an optional feature
> extension ? What HW are you using for the driver side ?

As I said earlier, I am looking for a generic interface for GPIO protocol and
irq support is bare minimum. I am not asking to force it to everyone, but make
it optional and keep it for the people who want it.

> > These are the new request types we would need for that, unless there
> > are more:
> > 
> > IRQ_TYPE	IRQ_MASK	IRQ_UNMASK
> 
> With the mask it's getting interesting. Note we're (theoretically)
> supporting 2^16 lines (real implementations probably stay much below
> that), so we'd need enough room for that.

Yeah, so the hardware (FPGA I think which you are working on) shouldn't need to
implement this. They will just not provide the associated feature bit.

> Maybe we should do it individually per line. This causes more traffic
> at setup, but also has the positive side effect that driver can easily
> mask/unmask invidual lines without remembering the mask.

Yeah, this is very much expected out of any basic GPIO controller or a spec for
the same.

> > And these are the interrupt types we need to support, for now. Again
> > they are quite standard if you look at Linux's GPIO support.
> > 
> > IRQ_TYPE_NONE	
> > IRQ_TYPE_EDGE_RISING
> > IRQ_TYPE_EDGE_FALLING	
> > IRQ_TYPE_EDGE_BOTH	
> > IRQ_TYPE_LEVEL_HIGH	
> > IRQ_TYPE_LEVEL_LOW
> 
> Well, "quite standard" is only when looking at what Linux gpio-subsys
> supports. That doesn't mean that actual HW supports it directly. When
> we make all these variants mandatory, we force a huge increase of the
> hardware size (number of gates, chip size, power consumption).
> 
> We should be very careful in our decisions.
> 
> Note: I'm not doing this just for some host to vm signaling, but also
> for actual hardware - embedded hardware. My goal also is laying out the
> basis so this could become a standard interface even on-chip gpio-
> controllers.

In such scenarios, the hardware doesn't need to implement these. You can just
fail such requests then or best don't even add the feature for your hardware.

Note that the same will happen for the requests Linus talked about during the
previous reviews, the list is present in
include/linux/pinctrl/pinconf-generic.h.

> >> +    \item requests are processed and replied in the they had been sent
> > 
> > in the *order* they...
> 
> fixed.
> 
> > Why is this important really ? If there are constraints in the control
> > flow (i.e. operation 1 must happen before operation 2), then the
> > request initiator needs to make sure of that. 
> 
> The ordering is for associating request + reply. Otherwise we'd need
> to add some kind of request identification (eq. seq numbers).

Yeah I thought about that earlier, but with the same layout for request/response
messages that you have chosen, it makes our life easy. The gpio pin number in
the request/response makes it work for us. We can use the gpio number as the
sequence number here.

> I don't
> see any good reason why some HW shouldn't keep the order inherently,
> but it's probably better to leave no room for diferent interpretations
> here.

This is a limitation, which isn't really required. It only adds restrictions,
which not everyone wants.

In your case you can just send the responses in FCFS form, while others can do
it the way they want, handle responses to come asynchronously.

> > Otherwise, it doesn't
> > matter which one finishes first. The response message will have the
> > GPIO number in it and so the initiator will know whose response it is.
> 
> That's correct. At least for the currently defined operations, which
> always have pin number. Who knows what the future will bring, so IMHO
> it's better to care about that (even just hypothetical) issue now,
> instead of running into trouble some time later.
> 
> Do you have any reason for NOT having this constraint.

Firstly, you need a _strong_ reason for adding a constraint and not vice versa,
we just can't add them hypothetically.

Secondly, It makes it less efficient.

Lets say device got Request A followed by Request B, but it was able to process
Request B faster than request A (which happens all the time with Linux because
of process scheduling and what not), then the device doesn't need to wait for a
response to request A before sending one for request B. There is no point in
that. Moreover it will increase the waiting time of another process at guest
(for request B), which may not anything to do with process attached with Request
A.

> > While on that, we must put a limit on number of messages started by
> > one side for the same GPIO line. i.e. Driver can not start another
> > operation for the same GPIO line, before receiving response for the
> > previous request for that GPIO line. Same goes for interrupts from
> > device to driver/CPU.
> 
> Queue size isn't enough for you ?

The queue is shared by all the GPIOs, lets say 256 of them. So I am not sure how
queue size works with it.

> Or do you want an explicit per-line limit ? Why do you feel its needed ?
> Is there anything that can go wrong with that ? Do you know any HW gpio
> controller that has such a limit ?
> 
> Certainly, it might not be practically useful to fire too many changes
> in a very short time,

Yes, so we don't normally start multiple requests for the same line. Any sane
code will always wait for the first operation to finish before the new one.

Like do a gpio-request, followed by get direction, and then set-direction or
something else. All of these are performed in sequence and the initiator will
always wait for the first request to finish before going for the second one.

Why would we ever want to send multiple instructions to the same line ?

> but could it cause any actual problem ?

I am not sure, because the software will always wait for the response to the
previous request for the same line. And we shall never have a case where two
requests for the same line come at the same time. Is there any such scenario
that I am missing ?

Moreover with interrupts, things become tricky. The device shall wait for the
first interrupt for a particular line to be finished, before sending another
one, else it can surely cause problems. Moreover, the kernel (on the guest side)
will try to mask the interrupt after receiving one, before sending the response
to the interrupt. So this kind of sequencing is actually better to have to avoid
deadlocks which may occur otherwise.

Linus thoughts ?

> >> +\begin{tabular}{ll}
> >> +    \hline
> >> +    \textbf{request:} & \\
> >> +    \hline
> >> +    \field{line}  field: & logical line number \\
> > 
> > s/logical/GPIO/ ?
> 
> if you insist, i'll change that ;-)

I don't insist for this TBH, but I wasn't sure if logical have a special meaning
here at all. We don't have virtual numbers for GPIOs and so logical ones also
don't make much sense.

> By the way: we've got a lot of repeated text here that calls for putting
> it into macros. Since it's a big document - do we have any constraints
> on the naming of such local macros ? I'm reluctant to doing dirty tricks
> with undefining commands :o

I am not sure, I would try to look at other documents inside virtio repository.

> >> +\begin{tabular}{ll}

On the same lines, your current document looks different than other protocols in
the spec right now. The spec maintainers may object to that and it may be better
to keep similar formats for such stuff. Like other protocols don't use tables
much but different section types for code like stuff.

Anyway, not my domain :)

> >> +    \hline
> >> +    \textbf{request:} & \\
> >> +    \hline
> >> +    \field{line}  field: & logical line number \\
> >> +    \field{value} field: & line level (0=inactive, 1=active) \\
> >> +    \hline
> >> +    \textbf{reply:} & \\
> >> +    \hline
> >> +    \field{value} field: & new line level or (negative) POSIX errno code \\
> > 
> > Why not 0 for success? How will the driver use the returned value here ?
> 
> The idea was device doesn't need to touch this field if everything's
> fine.

I think it is better for clarity to have response 0 for all success messages,
otherwise this will rather add special handling in code. I can see on the guest
Linux driver, a single function can take care of setting this field for all type
of response messages, but with current spec, we will require different handling
for 1 request.

> >> +\drivernormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
> >> +
> >> +The driver MUST NOT send VIRTIO_GPIO_MSG_DEVICE_LEVEL and MUST NOT set the highest bit in the message type.
> > 
> > The device would need some kind of acknowledgement (as Linus also
> > asked earlier) for the interrupt from the driver, before which it is
> > eligible to send another interrupt for the same line.
> 
> No, this version does not have any kind of irq controller functionality.
> It just sends this message unconditionally.

It is interrupt, even if you don't want to call it one :)

> As already mentioned above, masking shall be an optional extra feature.
> 
> I'd suggest splitting the discussion into the basic functionality (that
> we already have now) and additional features like IRQ controller etc.

You can always skip implementing this for your hardware, but since we are
defining the spec now, we should implement all basic functionality (according to
different people). For you irq isn't basic, for me it is.

So lets make it optional but implement it right away :)

Thanks.

-- 
viresh

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] Re: [virtio-dev] [PATCH] virtio-gpio: add formal specification

[Viresh Kumar]

On 17-06-21, 17:03, Viresh Kumar wrote:
> + couple of folks who have shown interest in this stuff.
> 
> On 17-06-21, 11:41, Enrico Weigelt, metux IT consult wrote:
> > This patch adds specification for attaching general purpose IO (gpio) devices
> > via virtio. The protocol is specifically designed to be easily implemented in
> > software (e.g. hypervisors) as well as low end hardware (eg. silicon, FPGA,
> > tiny MCUs) and allows future extensions while retaining full backwards
> > compatibility.

Hi Enrico,

It has been two weeks since the previous post, since no one else
replied yet, I think it is good time now for the next version to be
posted. :)

-- 
viresh

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

Re: [virtio-comment] Re: [virtio-dev] [PATCH] virtio-gpio: add formal specification

[Enrico Weigelt, metux IT consult]

On 30.06.21 05:58, Viresh Kumar wrote:
> Hi Enrico,
> 
> It has been two weeks since the previous post, since no one else
> replied yet, I think it is good time now for the next version to be
> posted. :)

yeah, I'll do that in a few minutes .. had been busy w/ other stuff :o

And I've still been struggling with the build error you've reported.
It only happens when on html build (which I hadn't tested yet).
Seems that htlatex has problems with underscores in labels. Fixed that
now by using - instead of _ in the label.

I'm now doing some more typographic fixes before sending v2.


--mtx

-- 
---
Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
GPG/PGP-Schlüssel zu.
---
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/

[virtio-comment] [PATCH] virtio-gpio: add formal specification

[Enrico Weigelt, metux IT consult]

This patch adds specification for attaching general purpose IO (gpio) devices
via virtio. The protocol is specifically designed to be easily implemented in
software (e.g. hypervisors) as well as low end hardware (eg. silicon, FPGA,
tiny MCUs) both both device and driver end, and allows future extensions while
retaining full backwards compatibility.

Implementations for driver (Linux kernel) and device (Qemu) are publically
available and field tested since late 2020. Hardware implementations also
exist (but proprietary, cannot be published yet).

Device type ID 41 has been allocated by TC vote #3632.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

---

changes v3: * moved arbitration into an optional feature
            * clarification on feature bits vs versions
            * clarification on max concurrent requests (queue size)
            * explicit definition of error codes
            * optional IRQ masking
            * moved spec text into subdir 'devices'
            * added explaination of important design considerations

changes v2: * fixed htlatex build error with underscores in labels
            * using code listings for structs and define's
            * fixed mixed-up device/driver wording
---
 conformance.tex         |  28 ++-
 content.tex             |   3 +-
 devices/virtio-gpio.tex | 381 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 407 insertions(+), 5 deletions(-)
 create mode 100644 devices/virtio-gpio.tex

diff --git a/conformance.tex b/conformance.tex
index 94d7a06..4bdaac4 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -30,8 +30,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Driver Conformance / IOMMU Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Sound Driver Conformance},
 \ref{sec:Conformance / Driver Conformance / Memory Driver Conformance},
-\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance} or
-\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance}.
+\ref{sec:Conformance / Driver Conformance / I2C Adapter Driver Conformance},
+\ref{sec:Conformance / Driver Conformance / SCMI Driver Conformance} or
+\ref{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -54,8 +55,9 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \ref{sec:Conformance / Device Conformance / IOMMU Device Conformance},
 \ref{sec:Conformance / Device Conformance / Sound Device Conformance},
 \ref{sec:Conformance / Device Conformance / Memory Device Conformance},
-\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance} or
-\ref{sec:Conformance / Device Conformance / SCMI Device Conformance}.
+\ref{sec:Conformance / Device Conformance / I2C Adapter Device Conformance},
+\ref{sec:Conformance / Device Conformance / SCMI Device Conformance} or
+\ref{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}.
 
     \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}.
   \end{itemize}
@@ -301,6 +303,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / SCMI Device / Device Operation / Setting Up eventq Buffers}
 \end{itemize}
 
+\conformance{\subsection}{General Purpose IO Driver Conformance}\label{sec:Conformance / Driver Conformance / General Purpose IO Driver Conformance}
+
+An General Purpose IO driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
+\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
+\end{itemize}
+
 \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -550,6 +561,15 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / SCMI Device / Device Operation / Shared Memory Operation}
 \end{itemize}
 
+\conformance{\subsection}{General Purpose IO Device Conformance}\label{sec:Conformance / Device Conformance / General Purpose IO Device Conformance}
+
+An General Purpose IO device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{devicenormative:Device Types / General Purpose IO / Virtqueues}
+\item \ref{devicenormative:Device Types / General Purpose IO / Data flow}
+\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 ceb2562..a5e1ec4 100644
--- a/content.tex
+++ b/content.tex
@@ -2876,7 +2876,7 @@ \chapter{Device Types}\label{sec:Device Types}
 \hline
 40         &   Bluetooth device \\
 \hline
-41         &   GPIO device \\
+41         &   General Purpose IO device \\
 \hline
 \end{tabular}
 
@@ -6583,6 +6583,7 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
 \input{virtio-mem.tex}
 \input{virtio-i2c.tex}
 \input{virtio-scmi.tex}
+\input{devices/virtio-gpio.tex}
 
 \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
diff --git a/devices/virtio-gpio.tex b/devices/virtio-gpio.tex
new file mode 100644
index 0000000..8417acb
--- /dev/null
+++ b/devices/virtio-gpio.tex
@@ -0,0 +1,381 @@
+\section{General Purpose IO Device}\label{sec:Device Types / General Purpose IO}
+
+The virtio gpio device is a general purpose IO device that supports a variable
+number of named IO lines that may be switched either as input or output and
+in logical level 0 or 1.
+
+\subsection{Device ID}\label{sec:Device Types / General Purpose IO / Device ID}
+  41
+
+\subsection{Version}\label{sec:Device Types / General Purpose IO / Version}
+  1
+
+\subsection{Device configuration layout}\label{sec:Device Types / General Purpose IO / Device configuration layout}
+
+General purpose IO configuration uses the following layout structure:
+
+\begin{lstlisting}
+struct virtio_gpio_config {
+    __u8    version;
+    __u8    reserved0;
+    __u16   num_gpios;
+    __u32   names_size;
+    __u8    reserved1[24];
+    __u8    name[32];
+    __u8    line_names[];
+};
+\end{lstlisting}
+
+\begin{itemize}
+    \item for \field{version} field currently only value 1 supported.
+    \item the \field{line names block} holds a stream of zero-terminated strings,
+        containing the individual line names in ASCII. line names must unique.
+    \item the \field{name} field may contain a zero terminated device name
+          or serial number in ASCII.
+    \item unspecified fields are reserved for future use and should be zero.
+    \item future versions may extend this configuration space by additional fields.
+\end{itemize}
+
+\subsection{Feature bits}\label{sec:Device Types / General Purpose IO / Feature bits}
+
+\begin{description}
+\item[VIRTIO_GPIO_F_ARBITRATION (0)] line arbitration -- REQUEST and RELEASE operations.
+\item[VIRTIO_GPIO_F_IRQ (1)] IRQ control
+\end{description}
+
+\subsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues}
+\begin{description}
+\item[0] rx (device to CPU)
+\item[1] tx (CPU to device)
+\end{description}
+
+The virtqueues transport messages of the type struct virtio_gpio_msg from device to CPU or CPU to device.
+
+\subsubsection{Virtqueues}\label{sec:Device Types / General Purpose IO / Virtqueues / Message format}
+
+The queues transport messages of the struct virtio_gpio_msg:
+
+\begin{lstlisting}
+struct virtio_gpio_msg {
+    __le16  type;
+    __le16  pin;
+    __le32  value;
+};
+\end{lstlisting}
+
+\subsubsection{Message types}\label{sec:Device Types / General Purpose IO / Virtqueues / Message types}
+
+\begin{lstlisting}
+/* messages types: driver -> device */
+#define VIRTIO_GPIO_MSG_CPU_REQUEST             0x0001
+#define VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT     0x0002
+#define VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT    0x0003
+#define VIRTIO_GPIO_MSG_CPU_GET_DIRECTION       0x0004
+#define VIRTIO_GPIO_MSG_CPU_GET_LEVEL           0x0005
+#define VIRTIO_GPIO_MSG_CPU_SET_LEVEL           0x0006
+#define VIRTIO_GPIO_MSG_CPU_RELEASE             0x0007
+#define VIRTIO_GPIO_MSG_CPU_SET_IRQ             0x0008
+
+/* message types: device -> driver */
+#define VIRTIO_GPIO_MSG_DEVICE_LEVEL            0x0011
+
+/* reply mask: device sets this bit on replies (along with request's message type)
+#define VIRTIO_GPIO_MSG_REPLY                   0x8000
+\end{lstlisting}
+
+\subsubsection{Error codes}\label{sec:Device Types / General Purpose IO / Virtqueues / Error codes}
+
+If requests result in an error, the device shall respond with an appropriate
+error code, which is encoded as \textbf{negative} value in the reply message's
+\textit{value} field.
+
+\begin{lstlisting}
+/* success, no error */
+#define VIRTIO_GPIO_ERR_OK      0x0000
+/* internal fault */
+#define VIRTIO_GPIO_ERR_FAULT   0x0001
+/* requested pin does not exist */
+#define VIRTIO_GPIO_ERR_NO_PIN  0x0002
+/* communications error */
+#define VIRTIO_GPIO_ERR_COMM    0x0003
+/* configuration not supported */
+#define VIRTIO_GPIO_ERR_INVAL   0x0004
+\end{lstlisting}
+
+\devicenormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
+
+The device MUST read from the tx queue and write to rx queue.
+
+The device MUST NOT write to the tx queue.
+
+\drivernormative{\subsubsection}{Virtqueues}{Device Types / General Purpose IO / Virtqueues}
+
+The device MUST read from the rx queue and write to tx queue.
+
+The device MUST NOT write to the rx queue.
+
+\subsection{Data flow}\label{sec:Device Types / General Purpose IO / Data flow}
+
+\begin{itemize}
+    \item all operations, except \field{VIRTIO_GPIO_MSG_DEVICE_LEVEL}, are initiated by CPU (tx queue)
+    \item device replies with the orinal \field{type} value OR'ed with \field{VIRTIO_GPIO_MSG_REPLY} (rx queue)
+    \item requests are processed and replied in the they had been sent
+    \item the number of concurrent requests is limited by the device's queue size
+    \item async notifications by the device may be interleaved with request responses
+    \item VIRTIO_GPIO_MSG_DEVICE_LEVEL is only sent asynchronously from device to CPU
+    \item in replies, a negative \field{value} field denotes an error code
+    \item in level or direction values, only bit 0 shall be used
+    \item valid direction values are: bit 0: 0 = output, 1 = input
+    \item valid line level values are: bit 0: 0 = inactive, 1 = active
+    \item CPU should not send messages with unspecified \field{type} value
+    \item CPU should ignore ignore messages with unspecified \field{type} value
+\end{itemize}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_REQUEST}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-REQUEST}
+
+Notify the device that given line number is going to be used.
+
+Only available if feature bit VIRTIO_GPIO_F_ARBITRATION is set.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & error code (0 = success) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_RELEASE}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-RELEASE}
+
+Notify the device that given line number is not used anymore.
+
+Only available if feature bit VIRTIO_GPIO_F_ARBITRATION is set.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & error code (0 = success) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-DIRECTION-INPUT}
+
+Set line line direction to input.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & error code (0 = success) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-DIRECTION-OUTPUT}
+
+Set line direction to output and given line level.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & bit 0: output level (0=inactive, 1=active) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & error code (0 = success) \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_DIRECTION}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-GET-DIRECTION}
+
+Retrieve line direction.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & bit 0: direction (0=output, 1=input) or errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_GET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-GET-LEVEL}
+
+Retrieve line level.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & bit 0: line level (0=inactive, 1=active) or errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_SET_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-SET-LEVEL}
+
+Set line level (output only)
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & bit 0: line level (0=inactive, 1=active) \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & bit 0: new line level or (negative) errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_CPU_SET_IRQ}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-CPU-SET-IRQ}
+
+Set the IRQ mask. See \ref{sec:Device Types / General Purpose IO / IRQ handling}.
+
+Only available if feature bit VIRTIO_GPIO_F_IRQ has been negotiated.
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & IRQ mask \\
+    \hline
+    \textbf{reply:} & \\
+    \hline
+    \field{value} field: & (negative) errno code \\
+    \hline
+\end{tabular}
+
+\subsubsection{VIRTIO_GPIO_MSG_DEVICE_LEVEL}\label{sec:Device Types / General Purpose IO / Data flow / VIRTIO-GPIO-MSG-DEVICE-LEVEL}
+
+Async notification from device to CPU: line level changed
+
+\begin{tabular}{ll}
+    \hline
+    \textbf{request:} & \\
+    \hline
+    \field{line}  field: & logical line number \\
+    \field{value} field: & unused (should be zero) \\
+    \hline
+    \textbf{reply:} & \\
+    \field{value} field: & bit 0: line level (0=inactive, 1=active) \\
+    \hline
+\end{tabular}
+
+\devicenormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
+
+The device MUST reply to all driver requests in they had been sent.
+
+The device MUST copy the \field{line} field from the request to its reply.
+
+Except for async notification, the device MUST reply the orignal message type, but with the highest bit set
+(or'ed with VIRTIO_GPIO_MSG_REPLY)
+
+In case of error the device MUST fill an \textbf{negative} value into the \field{value} field.
+
+On switching to output, the device SHOULD set internal output level before switching the line to output.
+
+The device SHOULD send VIRTIO_GPIO_MSG_DEVICE_LEVEL message for a particular line when it is in input
+direction and line level changes.
+
+\drivernormative{\subsubsection}{Data flow}{Device Types / General Purpose IO / Data flow}
+
+The driver MUST NOT send VIRTIO_GPIO_MSG_DEVICE_LEVEL and MUST NOT set the highest bit in the message type.
+
+The driver MUST NOT send messages with types not defined in this specification.
+
+The driver MUST treat all negative values of the \field{value} field as errors, regardless whether
+the actual codes are explicitly defined in this specification.
+
+\subsection{IRQ handling}\label{sec:Device Types / General Purpose IO / IRQ handling}
+
+IRQ handling is only enabled if feature VIRTIO_GPIO_F_IRQ had been negotiated -- in
+this case only explicitly configured events are sent via VIRTIO_GPIO_MSG_DEVICE_LEVEL
+message. Otherwise level state changes are sent unconditionally.
+
+IRQ masking is configured via VIRTIO_GPIO_MSG_CPU_IRQ message, using the following
+(or'ed) mask bits:
+
+\begin{lstlisting}
+/* fire on levels */
+#define VIRTIO_GPIO_IRQ_HI      0x0001
+#define VIRTIO_GPIO_IRQ_LOW     0x0002
+
+/* fire on edge */
+#define VIRTIO_GPIO_IRQ_RAISING 0x0004
+#define VIRTIO_GPIO_IRQ_FALLING 0x0008
+
+/* enabled */
+#define VIRTIO_GPIO_IRQ_ARMED   0x0016
+#define VIRTIO_GPIO_IRQ_ALWAYS  0x0032
+\end{lstlisting}
+
+The actual kind of IRQ fired is determined on whether / how the line state changed
+from the previous message.
+
+IRQs are fired only, if either \textbf{ARMED} or \textbf{ALWAYS} are set. The
+\textbf{ARMED} flag is automatically cleared on IRQ signal, thus needs explicit
+rearming / unmasking.
+
+\devicenormative{\subsubsection}{IRQ handling}{sec:Device Types / General Purpose IO / IRQ handling}
+
+The device MUST maintain the IRQ masking state independently from other states like
+line direction or line states.
+
+When VIRTIO_GPIO_F_IRQ is negotiated, all irq mask registers need to be cleared,
+so no IRQs can occour, until explicitly enabled.
+
+\subsection{Future versions}\label{sec:Device Types / General Purpose IO / Future versions}
+
+\begin{itemize}
+    \item adding new functionality should be done by adding optional feature bits, if possible --
+          new versions shall only be introduces when feature bits are not sufficient
+    \item future versions must increment the \field{version} value
+    \item the basic data structures (config space, message format) should remain
+          backwards compatible, but may increased in size or use reserved fields
+    \item device needs to support commands in older versions
+    \item CPU should not send commands of newer versions that the device doesn't support
+\end{itemize}
+
+\subsection{Design considerations}\label{sec:Device Types / General Purpose IO / Design considerations}
+
+This specification has been layed out for being easy to implement in \textbf{hardware},
+silicon or FPGA, with a minimum number of required gates, in order to save chip space,
+power consumption and heat emissions, but also being easy to emulate in software, on both
+device and driver side.
+
+For that reason, data structures and signaling protocols have been designed to be very
+simple (e.g. fixed buffer layouts), and features that may not be always present or needed
+(e.g. IRQs) are explicitly negotiated by optional feature bits.
+
+Signal flow has been designed to use a simple (in-order) request-response scheme with
+separated tx/rx queues for all driver initiated transactions, that the device replies
+by just flipping a few bits and sending the packet back.
+
+The subset of virtio used here can also be easily implemented by very low level means
+like serial lines.
-- 
2.20.1


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/