June 2021 - Stratos-dev - op-lists.linaro.org

[PATCH v2 0/5] virtio: Add vhost-user based RNG

by Mathieu Poirier

This sets adds a vhost-user based random number generator (RNG), similar to what has been done for i2c and virtiofsd, with the implementation following the patterns already set forth in those. Applies cleanly to git://git.qemu.org/qemu.git master(1ea06abceec6). Thanks, Mathieu ----- New for V2: - Replaced "antropy" for "entropy" (Joakim). Mathieu Poirier (5): vhost-user-rng: Add vhost-user-rng implementation vhost-user-rng-pci: Add vhost-user-rng-pci implementation vhost-user-rng: backend: Add RNG vhost-user daemon implementation docs: Add documentation for vhost based RNG implementation MAINTAINERS: Add maintainer for vhost-user RNG implementation MAINTAINERS | 9 + docs/tools/vhost-user-rng.rst | 74 +++++ hw/virtio/Kconfig | 5 + hw/virtio/meson.build | 2 + hw/virtio/vhost-user-rng-pci.c | 79 +++++ hw/virtio/vhost-user-rng.c | 294 +++++++++++++++++ include/hw/virtio/vhost-user-rng.h | 33 ++ tools/meson.build | 8 + tools/vhost-user-rng/50-qemu-rng.json.in | 5 + tools/vhost-user-rng/main.c | 403 +++++++++++++++++++++++ tools/vhost-user-rng/meson.build | 10 + 11 files changed, 922 insertions(+) create mode 100644 docs/tools/vhost-user-rng.rst create mode 100644 hw/virtio/vhost-user-rng-pci.c create mode 100644 hw/virtio/vhost-user-rng.c create mode 100644 include/hw/virtio/vhost-user-rng.h create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in create mode 100644 tools/vhost-user-rng/main.c create mode 100644 tools/vhost-user-rng/meson.build -- 2.25.1

3 years, 4 months

2
8
0 0

[PATCH V3 0/3] gpio: Add virtio based driver

by Viresh Kumar

Hello, This adds a virtio based GPIO driver based on the proposed specification [1]. The first two versions [2] were sent by Enrico earlier and here is a newer version. I have retained the authorship of the work done by Enrico (1st patch) to make sure we don't loose his credits. I have tested all basic operations of the patchset (with Qemu guest) with the libgpiod utility. I have also tested the handling of interrupts on the guest side. All works as expected. I will now be working towards a Rust based hypervisor agnostic backend to provide a generic solution for that. This should be merged only after the specifications are merged, I will keep everyone posted for the same. I am not adding a version history here as a lot of changes have been made, from protocol to code and this really needs a full review from scratch. -- Viresh [1] https://lists.oasis-open.org/archives/virtio-comment/202106/msg00022.html [2] https://lore.kernel.org/linux-gpio/20201203191135.21576-2-info@metux.net/ Enrico Weigelt, metux IT consult (1): gpio: Add virtio-gpio driver Viresh Kumar (2): gpio: virtio: Add IRQ support MAINTAINERS: Add entry for Virtio-gpio MAINTAINERS | 7 + drivers/gpio/Kconfig | 9 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-virtio.c | 566 +++++++++++++++++++++++++++++++ include/uapi/linux/virtio_gpio.h | 56 +++ include/uapi/linux/virtio_ids.h | 1 + 6 files changed, 640 insertions(+) create mode 100644 drivers/gpio/gpio-virtio.c create mode 100644 include/uapi/linux/virtio_gpio.h -- 2.31.1.272.g89b43f80a514

3 years, 4 months

8
34
0 0

Stratos June 24th meeting canceled

by Mike Holmes

Hi All The meeting on June 24th is cancelled, the majority of the participants in Linaro will be attending Rust Training. Mike https://linaro.atlassian.net/wiki/spaces/STR/overview -- Mike Holmes (he / him / his) | Director, Foundation Technologies, Linaro Mike.Holmes(a)linaro.org <mike.holmes(a)linaro.org> "Work should be fun and collaborative; the rest follows."

3 years, 4 months

1
0
0 0

Re: [Stratos-dev] [PATCH V3 1/3] gpio: Add virtio-gpio driver

by Arnd Bergmann

On Mon, Jun 14, 2021 at 2:50 PM Vincent Guittot via Stratos-dev <stratos-dev(a)op-lists.linaro.org> wrote:> > On Mon, 14 Jun 2021 at 14:33, Arnd Bergmann <arnd(a)kernel.org> wrote: > > On Mon, Jun 14, 2021 at 12:23 PM Viresh Kumar <viresh.kumar(a)linaro.org> wrote: > > > > I think most importantly we need a DT binding to describe what device > > nodes are supposed to look like underneath a virtio-mmio or > > virtio-pci device in order for a hypervisor to pass down the > > information to a guest OS in a generic way. We can probably borrow > > the USB naming, and replace compatible="usbVID,PID" with > > compatible="virtioDID", with the device ID in hexadecimal digits, > > such as "virtio22" for I2C (virtio device ID 34 == 0x22) if we decide > > to have a sub-node under the device, or we just point dev->of_node > > of the virtio device to the platform/pci device that is its parent > > in Linux. > > > > Adding the Linux guest code to the virtio layer should be fairly > > straightforward, and I suppose it could be mostly copied from the > > corresponding code that added this for mmc in commit 25185f3f31c9 > > ("mmc: Add SDIO function devicetree subnode parsing") and for USB > > in commit 69bec7259853 ("USB: core: let USB device know device > > node") and 1a7e3948cb9f ("USB: add device-tree support for > > interfaces"). > > And something similar is also done with SCMI protocols which are > defined in a SCMI node. A typical example: > > cpu@0 { > ... > clocks = <&scmi_dvfs 0>; > ... > }; > > deviceX: deviceX@YYYYYYY { > ... > clocks = <&scmi_clk 0>; > ... > }; > > scmi: scmi { > compatible = "arm,scmi-virtio"; > #address-cells = <1>; > #size-cells = <0>; > > scmi_devpd: protocol@11 { > reg = <0x11>; > #power-domain-cells = <1>; > }; > > scmi_clk: protocol@14 { > reg = <0x14>; > #clock-cells = <1>; > }; > > scmi_sensors: protocol@15 { > reg = <0x15>; > #thermal-sensor-cells = <1>; > }; > > scmi_dvfs: protocol@13 { > reg = <0x13>; > #clock-cells = <1>; > }; > }; But this example seem to be completely different from the ones I mentioned: The scmi node that you have here looks like it shows up under the root of the device tree, not below the virtio device that implements the scmi transport. Arnd

3 years, 4 months

2
3
0 0

[PATCH V4] virtio-gpio: Add the device specification

by Viresh Kumar

virtio-gpio is a virtual GPIO controller. It provides a way to flexibly communicate with the host GPIO controllers from the guest. This patch adds the specification for it. Based on the initial work done by: "Enrico Weigelt, metux IT consult" <lkml(a)metux.net>. Fixes: https://github.com/oasis-tcs/virtio-spec/issues/110 Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- V3 -> V4: - The GPIO line names must be unique within a device. - The gpio_names[0] field is dropped and gpio_names_offset field is added in place of the 2 bytes of padding. - New interrupts must not be initiated by the device, without a response for the previous one. V2 -> V3: - Unused char in name string should be marked 0 now. - s/host/device/ and s/guest/driver/ - Added a new feature for IRQ mode, VIRTIO_GPIO_F_IRQ. - A new feature should be added for Version information if required later on. V1 -> V2: - gpio_names_size is 32 bit. - gpio field is 16 bit. - padding added 16 bit. - Added packed attribute to few structures - Add the missing 'type' field to the request - Dropped to _nodata request/responses to simplify a bit, updated related text. --- conformance.tex | 26 +++- content.tex | 1 + virtio-gpio.tex | 309 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 332 insertions(+), 4 deletions(-) create mode 100644 virtio-gpio.tex diff --git a/conformance.tex b/conformance.tex index a164cbb69093..5341abe096c2 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 / GPIO 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 / GPIO Device Conformance}. \item Clause \ref{sec:Conformance / Legacy Interface: Transitional Device and Transitional Driver Conformance}. \end{itemize} @@ -288,6 +290,14 @@ \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}{GPIO Driver Conformance}\label{sec:Conformance / Driver Conformance / GPIO Driver Conformance} + +An GPIO driver MUST conform to the following normative statements: + +\begin{itemize} +\item \ref{drivernormative:Device Types / GPIO Device / Device Operation} +\end{itemize} + \conformance{\section}{Device Conformance}\label{sec:Conformance / Device Conformance} A device MUST conform to the following normative statements: @@ -527,6 +537,14 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \item \ref{devicenormative:Device Types / SCMI Device / Device Operation / Shared Memory Operation} \end{itemize} +\conformance{\subsection}{GPIO Device Conformance}\label{sec:Conformance / Device Conformance / GPIO Device Conformance} + +An GPIO device MUST conform to the following normative statements: + +\begin{itemize} +\item \ref{devicenormative:Device Types / GPIO Device / Device Operation} +\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 d9913d056317..e572ac3bb6c0 100644 --- a/content.tex +++ b/content.tex @@ -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{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 000000000000..7582993f3fd9 --- /dev/null +++ b/virtio-gpio.tex @@ -0,0 +1,309 @@ +\section{GPIO Device}\label{sec:Device Types / GPIO Device} + +virtio-gpio is a virtual general purpose IO controller device. It provides a way +to access the host GPIO devices from the guest. This device provides a hardware +independent interface between the host and the guests. It allows the host to +club together GPIO lines from otherwise independent GPIO controllers and present +them as a single GPIO controller device at the guest. + +\subsection{Device ID}\label{sec:Device Types / GPIO Device / Device ID} +41 + +\subsection{Virtqueues}\label{sec:Device Types / GPIO Device / Virtqueues} + +\begin{description} +\item[0] command +\item[1] interrupt +\end{description} + +The \field{interrupt} virtqueue is only used if the \field{VIRTIO_GPIO_F_IRQ} +feature is supported by the device. + +\subsection{Feature bits}\label{sec:Device Types / GPIO Device / Feature bits} + +\begin{description} +\item[VIRTIO_GPIO_F_IRQ (0)] The device supports interrupts on GPIO lines. +\end{description} + + +\subsection{Device configuration layout}\label{sec:Device Types / GPIO Device / Device configuration layout} + +All fields of this configuration are always available and are read-only for the driver. + +\begin{lstlisting} +struct virtio_gpio_config { + char name[32]; + u16 ngpio; + u16 gpio_names_offset; + u32 gpio_names_size; +} __attribute__((packed)); +\end{lstlisting} + +\begin{description} +\item[\field{name}] is a null-terminated string that represents the name of the + GPIO controller. The unused characters in the string must be initialized to + zero by the device. + +\item[\field{ngpio}] is the total number of GPIO lines provided by the + controller. + +\item[\field{gpio_names_offset}] is the offset of the "gpio_names" memory block + from the base of the \field{struct virtio_gpio_config}. The "gpio names" + block contains a stream of \field{ngpio} null-terminated strings. Each and + every GPIO line name MUST be unique within a GPIO Device and the name MUST + not be NULL for any of the GPIO line, unless the \field{gpio_names_size} is + set to zero and this feature isn't used by the device. + + The \field{gpio_names_offset} field can only be used if the + \field{gpio_names_size} is set to a non-zero value. When used, its value + must be greater than or equal to the size of the \field{struct + virtio_gpio_config} (which is 40 bytes in the current version of the spec). + + This field is added, instead of fixing the offset of the "gpio_names" block + to the end of the structure, in order to allow the configuration structure + to be extended in future if need arises to add more fields to it. + +\item[\field{gpio_names_size}] is the size of the "gpio_names" memory block. + This field must be set to 0, if the "gpio_names" block is not used by the + device. +\end{description} + + +\subsection{Device Initialization}\label{sec:Device Types / GPIO Device / Device Initialization} + +\begin{enumerate} +\item The virtqueue is initialized. +\end{enumerate} + +\subsection{Device Operation}\label{sec:Device Types / GPIO Device / Device Operation} + +The operations of a virtio GPIO device are almost always driven by the driver. +The driver initiates one of the requests from \field{VIRTIO_GPIO_REQ_*} on the +\field{command} virtqueue and the device responds synchronously on the same +virtqueue with a response message. The only time a request is initiated by the +device is when it needs to report detection of an interrupt on a GPIO line and +the \field{VIRTIO_GPIO_F_IRQ} feature is supported by the device. This is done +by the device by sending the (\field{VIRTIO_GPIO_IRQ_EVENT}) request on the +\field{interrupt} virtqueue. + +\begin{lstlisting} +/* GPIO request types */ +#define VIRTIO_GPIO_REQ_ACTIVATE 0x01 +#define VIRTIO_GPIO_REQ_DEACTIVATE 0x02 +#define VIRTIO_GPIO_REQ_GET_DIRECTION 0x03 +#define VIRTIO_GPIO_REQ_DIRECTION_IN 0x04 +#define VIRTIO_GPIO_REQ_DIRECTION_OUT 0x05 +#define VIRTIO_GPIO_REQ_GET_VALUE 0x06 +#define VIRTIO_GPIO_REQ_SET_VALUE 0x07 +#define VIRTIO_GPIO_REQ_IRQ_TYPE 0x08 +#define VIRTIO_GPIO_REQ_IRQ_MASK 0x09 +#define VIRTIO_GPIO_REQ_IRQ_UNMASK 0x0a +#define VIRTIO_GPIO_IRQ_EVENT 0x0b +\end{lstlisting} + +\subsubsection{Device Operation: Request Queue}\label{sec:Device Types / GPIO Device / Device Operation: Request Queue} + +The communication between the device and the driver take place using a pair of +request and response messages. The virtio GPIO specification defines two request +and two response layouts. The particular request/response pair used for each +GPIO request type is specific later in the request specific sections. + +Supported request and response types: + +\begin{lstlisting} +/* Virtio GPIO Request */ +struct virtio_gpio_request { + u16 type; + u16 gpio; + u8 data; +} __attribute__((packed)); + +/* Virtio GPIO Response */ +struct virtio_gpio_response { + u8 status; + u8 data; +}; +\end{lstlisting} + +\begin{description} +\item[\field{type}] GPIO request type. +\item[\field{gpio}] GPIO line number. +\item[\field{data}] Request/Response specific data, not used by all + request/response messages. +\item[\field{status}] Status of the request, success or failure. +\end{description} + +Here is the list of different values these fields can contain based on the +specific request type. + +\begin{lstlisting} +/* GPIO line direction */ +#define VIRTIO_GPIO_DIRECTION_OUT 0x0 +#define VIRTIO_GPIO_DIRECTION_IN 0x1 + +/* GPIO line interrupt type */ +#define VIRTIO_GPIO_IRQ_TYPE_NONE 0x00 +#define VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING 0x01 +#define VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING 0x02 +#define VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH 0x03 +#define VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH 0x04 +#define VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW 0x08 + +/* Possible values of the status field */ +#define VIRTIO_GPIO_STATUS_OK 0x0 +#define VIRTIO_GPIO_STATUS_ERR 0x1 +\end{lstlisting} + +\subsubsection{Device Operation: Activate}\label{sec:Device Types / GPIO Device / Device Operation / Activate } + +The \field{VIRTIO_GPIO_REQ_ACTIVATE} request is initiated by the driver to +request the device to activate one of the GPIO lines for use. The driver sends +the \field{struct virtio_gpio_request} to the device, and the device responds +with \field{struct virtio_gpio_response}. Neither the request, nor the response +uses the \field{data} field. + +\subsubsection{Device Operation: Deactivate}\label{sec:Device Types / GPIO Device / Device Operation / Deactivate } + +The \field{VIRTIO_GPIO_REQ_DEACTIVATE} request is initiated by the driver to +request the device to deactivate one of the GPIO lines driver was previously +using. The driver sends the \field{struct virtio_gpio_request} to the device, +and the device responds with \field{struct virtio_gpio_response}. Neither the +request, nor the response uses the \field{data} field. + +\subsubsection{Device Operation: Get Direction}\label{sec:Device Types / GPIO Device / Device Operation / Get Direction } + +The \field{VIRTIO_GPIO_REQ_GET_DIRECTION} request is initiated by the driver to +request the device to return a GPIO line's configured direction. The driver +sends the \field{struct virtio_gpio_request} to the device, and the device +responds with \field{struct virtio_gpio_response}. The device shall set the +\field{data} field in the response to \field{VIRTIO_GPIO_DIRECTION_OUT} or +\field{VIRTIO_GPIO_DIRECTION_IN}. The request doesn't use the \field{data} +field. + +\subsubsection{Device Operation: Direction In}\label{sec:Device Types / GPIO Device / Device Operation / Direction In } + +The \field{VIRTIO_GPIO_REQ_DIRECTION_IN} request is initiated by the driver to +request the device to configure a GPIO line for input direction. The driver +sends the \field{struct virtio_gpio_request} to the device, and the device +responds with \field{struct virtio_gpio_response}. Neither the request, nor the +response uses the \field{data} field. + +\subsubsection{Device Operation: Direction Out}\label{sec:Device Types / GPIO Device / Device Operation / Direction Out } + +The \field{VIRTIO_GPIO_REQ_DIRECTION_OUT} request is initiated by the driver to +request the device to configure a GPIO line for output direction with an initial +output value (0 for low, 1 for high). The driver sends the \field{struct +virtio_gpio_request} to the device with initial output value set in the +\field{data} field, and the device responds with \field{struct +virtio_gpio_response}. The responds doesn't use the \field{data} field. + +\subsubsection{Device Operation: Get Value}\label{sec:Device Types / GPIO Device / Device Operation / Get Value } + +The \field{VIRTIO_GPIO_REQ_GET_VALUE} request is initiated by the driver to +request the device to return the current value sensed on a GPIO line (0 for low, +1 for high) configured for input. The driver sends the \field{struct +virtio_gpio_request} to the device, and the device responds responds with +\field{struct virtio_gpio_response} with its \field{data} field set to GPIO's +value. The request doesn't use the \field{data} field. + +\subsubsection{Device Operation: Set Value}\label{sec:Device Types / GPIO Device / Device Operation / Set Value } + +The \field{VIRTIO_GPIO_REQ_SET_VALUE} request is initiated by the driver to +request the device to set the value (0 for low, 1 for high) for a GPIO line +configured for output. The driver sends the \field{struct virtio_gpio_request} +to the device, with the output value set in the \field{data} field, and the +device responds with \field{struct virtio_gpio_response}. The responds doesn't +use the \field{data} field. + +\subsubsection{Device Operation: IRQ Type}\label{sec:Device Types / GPIO Device / Device Operation / IRQ Type } + +The \field{VIRTIO_GPIO_REQ_IRQ_TYPE} request is initiated by the driver to +request the device to set the IRQ trigger type (one of +\field{VIRTIO_GPIO_IRQ_TYPE_*}) for a GPIO line configured for input. The driver +sends the \field{struct virtio_gpio_request} to the device, with the IRQ trigger +type set in the \field{data} field, and the device responds with \field{struct +virtio_gpio_response}. The responds doesn't use the \field{data} field. + +This request is only supported if the \field{VIRTIO_GPIO_F_IRQ} feature is +supported by the device. + +\subsubsection{Device Operation: IRQ Mask}\label{sec:Device Types / GPIO Device / Device Operation / IRQ Mask } + +The \field{VIRTIO_GPIO_REQ_IRQ_MASK} request is initiated by the driver to +request the device to mask the specified GPIO line for interrupts. The driver +sends the \field{struct virtio_gpio_request} to the device, and the device +responds with \field{struct virtio_gpio_response}. Neither the request, nor the +response uses the \field{data} field. + +This request is only supported if the \field{VIRTIO_GPIO_F_IRQ} feature is +supported by the device. + +\subsubsection{Device Operation: IRQ Unmask}\label{sec:Device Types / GPIO Device / Device Operation / IRQ Unmask } + +The \field{VIRTIO_GPIO_REQ_IRQ_UNMASK} request is initiated by the driver to +request the device to unmask the specified GPIO line for interrupts. The driver +sends the \field{struct virtio_gpio_request} to the device, and the device +responds with \field{struct virtio_gpio_response}. Neither the request, nor the +response uses the \field{data} field. + +This request is only supported if the \field{VIRTIO_GPIO_F_IRQ} feature is +supported by the device. + +\subsubsection{Device Operation: IRQ Event}\label{sec:Device Types / GPIO Device / Device Operation / IRQ Event } + +The \field{VIRTIO_GPIO_IRQ_EVENT} request is the only request initiated by the +device to inform the driver that an interrupt is detected on one of the GPIO +lines configured for input. The device sends the \field{struct +virtio_gpio_request} to the driver, and the driver responds with \field{struct +virtio_gpio_response}. This is the only request that uses the \field{interrupt} +virtqueue, while all other requests use the \field{command} virtqueue. Neither +the request, nor the response uses the \field{data} field. The device shouldn't +initiate a new \field{VIRTIO_GPIO_IRQ_EVENT} request on the \field{interrupt} +virtqueue until the time it has received a response from the driver for the +previous \field{VIRTIO_GPIO_IRQ_EVENT} request. + +This request is only supported if the \field{VIRTIO_GPIO_F_IRQ} feature is +supported by the device. + + +\drivernormative{\subsubsection}{Device Operation}{Device Types / GPIO Device / Device Operation} + +A driver MUST set all the fields of the \field{struct virtio_gpio_request} +before sending the request, except for the requests where the \field{data} field +isn't used by the request type. + +A driver MUST NOT use the \field{data} field in the response message if the +\field{status} returned from the device is \field{VIRTIO_GPIO_STATUS_ERR}. + +A driver MUST NOT try to set value of a GPIO line configured for input. + +A driver MUST NOT try to get value of a GPIO line configured for output. + +A driver MUST NOT send IRQ related requests for a GPIO line configured for +output. + +A driver MUST NOT initiate IRQ related requests if the \field{VIRTIO_GPIO_F_IRQ} +feature is not supported by the device. + +A driver MUST queue only one request at a time and wait for its response before +queuing the next request. + +\devicenormative{\subsubsection}{Device Operation}{Device Types / GPIO Device / Device Operation} + +A device MUST set the \field{status} field to \field{VIRTIO_GPIO_STATUS_OK} for +all successful requests. + +A device MUST set all the fields of the \field{struct virtio_gpio_response} +before sending the response, except for the response where the \field{data} +field isn't used by the request type, unless an error has occurred and it has +set the \field{status} field to \field{VIRTIO_GPIO_STATUS_ERR}. + +A device MUST add a unique and valid string in the \field{gpio_names} field of +the \field{struct virtio_gpio_config} for every supported GPIO line, if the +\field{gpio_names_size} is set to a non-zero value. + +A device MUST NOT send a request on the \field{interrupt} virtqueue if the +\field{VIRTIO_GPIO_F_IRQ} feature is not supported by it. + +A device MUST NOT send a new request on the \field{interrupt} virtqueue if the +previous request is not responded to yet, by the driver. -- 2.31.1.272.g89b43f80a514

3 years, 5 months

1
0
0 0

Re: [Stratos-dev] [PATCH V3 1/3] gpio: Add virtio-gpio driver

by Arnd Bergmann

On Fri, Jun 11, 2021 at 5:39 AM Viresh Kumar via Stratos-dev <stratos-dev(a)op-lists.linaro.org> wrote: > On 10-06-21, 19:40, Jean-Philippe Brucker wrote: > > On Thu, Jun 10, 2021 at 12:16:46PM +0000, Viresh Kumar via Stratos-dev wrote: > > > +} __packed; > > > > No need for __packed, because the fields are naturally aligned (as > > required by the virtio spec) > > Yeah, I know, but I tend to add that for structures which aren't very > simple (like the request/response ones), just to avoid human errors > and hours of debugging someone need to go through. __packed won't harm > at least :) Extraneous __packed annotations do cause real problems: - On architectures without hardware unaligned accesses, the compiler is forced to emit byte load/store instructions, which is slower and breaks atomic updates to shared variables - If a function takes a pointer of a packed struct member, and passes that pointer to a function that expects a regular aligned pointer, you get undefined behavior. Newer compilers produce a warning if you do that (we currently shut up that warning because there are many false positives in the kernel), but you can also run into CPU exceptions or broken code even on CPUs that do support unaligned accesses when the variable ends up being actually unaligned (as you just told the compiler that it is allowed to do). Arnd

3 years, 5 months

2
1
0 0

Re: [Stratos-dev] [PATCH V3 1/3] gpio: Add virtio-gpio driver

by Jean-Philippe Brucker

Hi, Not being very familiar with GPIO, I just have a few general comments and one on the config space layout On Thu, Jun 10, 2021 at 12:16:46PM +0000, Viresh Kumar via Stratos-dev wrote: > +static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio, > + u8 txdata, u8 *rxdata) > +{ > + struct virtio_gpio_response *res = &vgpio->cres; > + struct virtio_gpio_request *req = &vgpio->creq; > + struct scatterlist *sgs[2], req_sg, res_sg; > + struct device *dev = &vgpio->vdev->dev; > + unsigned long time_left; > + unsigned int len; > + int ret; > + > + req->type = cpu_to_le16(type); > + req->gpio = cpu_to_le16(gpio); > + req->data = txdata; > + > + sg_init_one(&req_sg, req, sizeof(*req)); > + sg_init_one(&res_sg, res, sizeof(*res)); > + sgs[0] = &req_sg; > + sgs[1] = &res_sg; > + > + mutex_lock(&vgpio->lock); > + ret = virtqueue_add_sgs(vgpio->command_vq, sgs, 1, 1, res, GFP_KERNEL); > + if (ret) { > + dev_err(dev, "failed to add request to vq\n"); > + goto out; > + } > + > + reinit_completion(&vgpio->completion); > + virtqueue_kick(vgpio->command_vq); > + > + time_left = wait_for_completion_timeout(&vgpio->completion, HZ / 10); > + if (!time_left) { > + dev_err(dev, "virtio GPIO backend timeout\n"); > + return -ETIMEDOUT; mutex is still held > + } > + > + WARN_ON(res != virtqueue_get_buf(vgpio->command_vq, &len)); > + if (unlikely(res->status != VIRTIO_GPIO_STATUS_OK)) { > + dev_err(dev, "virtio GPIO request failed: %d\n", gpio); > + return -EINVAL; and here > + } > + > + if (rxdata) > + *rxdata = res->data; > + > +out: > + mutex_unlock(&vgpio->lock); > + > + return ret; > +} > + > +static int virtio_gpio_request(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_ACTIVATE, gpio, 0, NULL); > +} > + > +static void virtio_gpio_free(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DEACTIVATE, gpio, 0, NULL); > +} > + > +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 direction; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_DIRECTION, gpio, 0, > + &direction); > + if (ret) > + return ret; > + > + return direction; > +} > + > +static int virtio_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_IN, gpio, 0, > + NULL); > +} > + > +static int virtio_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio, > + int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_OUT, gpio, (u8) (that dangling cast looks a bit odd to me) > + value, NULL); > +} > + > +static int virtio_gpio_get(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 value; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_VALUE, gpio, 0, &value); > + if (ret) > + return ret; > + > + return value; > +} > + > +static void virtio_gpio_set(struct gpio_chip *gc, unsigned int gpio, int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_SET_VALUE, gpio, value, NULL); > +} > + > +static void virtio_gpio_command(struct virtqueue *vq) > +{ > + struct virtio_gpio *vgpio = vq->vdev->priv; > + > + complete(&vgpio->completion); > +} > + > +static int virtio_gpio_alloc_vqs(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + const char * const names[] = { "command" }; > + vq_callback_t *cbs[] = { > + virtio_gpio_command, > + }; > + struct virtqueue *vqs[1] = {NULL}; > + int ret; > + > + ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL); > + if (ret) { > + dev_err(&vdev->dev, "failed to allocate vqs: %d\n", ret); > + return ret; > + } > + > + vgpio->command_vq = vqs[0]; > + > + /* Mark the device ready to perform operations from within probe() */ > + virtio_device_ready(vgpio->vdev); May fit better in the parent function > + return 0; > +} > + > +static void virtio_gpio_free_vqs(struct virtio_device *vdev) > +{ > + vdev->config->reset(vdev); > + vdev->config->del_vqs(vdev); > +} > + > +static const char **parse_gpio_names(struct virtio_device *vdev, > + struct virtio_gpio_config *config) > +{ > + struct device *dev = &vdev->dev; > + char *str = config->gpio_names; > + const char **names; > + int i, len; > + > + if (!config->gpio_names_size) > + return NULL; > + > + names = devm_kcalloc(dev, config->ngpio, sizeof(names), GFP_KERNEL); > + if (!names) > + return ERR_PTR(-ENOMEM); > + > + /* NULL terminate the string instead of checking it */ > + config->gpio_names[config->gpio_names_size - 1] = '\0'; > + > + for (i = 0; i < config->ngpio; i++) { > + /* > + * We have already marked the last byte with '\0' > + * earlier, this shall be enough here. > + */ > + if (str == config->gpio_names + config->gpio_names_size) { > + dev_err(dev, "Invalid gpio_names\n"); > + return ERR_PTR(-EINVAL); > + } > + > + len = strlen(str); > + if (!len) { > + dev_err(dev, "Missing GPIO name: %d\n", i); > + return ERR_PTR(-EINVAL); > + } > + > + names[i] = str; > + str += len + 1; > + } > + > + return names; > +} > + > +static int virtio_gpio_probe(struct virtio_device *vdev) > +{ > + struct virtio_gpio_config *config; > + struct device *dev = &vdev->dev; > + struct virtio_gpio *vgpio; > + u32 size; > + int ret; > + > + virtio_cread(vdev, struct virtio_gpio_config, gpio_names_size, &size); > + size = cpu_to_le32(size); le32_to_cpu()? > + > + vgpio = devm_kzalloc(dev, sizeof(*vgpio) + size, GFP_KERNEL); > + if (!vgpio) > + return -ENOMEM; > + > + config = &vgpio->config; > + > + /* Read configuration */ > + virtio_cread_bytes(vdev, 0, config, sizeof(*config) + size); > + > + /* NULL terminate the string instead of checking it */ > + config->name[sizeof(config->name) - 1] = '\0'; > + config->ngpio = cpu_to_le16(config->ngpio); > + config->gpio_names_size = cpu_to_le32(config->gpio_names_size); leXX_to_cpu()? > + > + if (!config->ngpio) { > + dev_err(dev, "Number of GPIOs can't be zero\n"); > + return -EINVAL; > + } > + > + vgpio->vdev = vdev; > + vgpio->gc.request = virtio_gpio_request; > + vgpio->gc.free = virtio_gpio_free; > + vgpio->gc.get_direction = virtio_gpio_get_direction; > + vgpio->gc.direction_input = virtio_gpio_direction_input; > + vgpio->gc.direction_output = virtio_gpio_direction_output; > + vgpio->gc.get = virtio_gpio_get; > + vgpio->gc.set = virtio_gpio_set; > + vgpio->gc.ngpio = config->ngpio; > + vgpio->gc.base = -1; /* Allocate base dynamically */ > + vgpio->gc.label = config->name[0] ? > + config->name : dev_name(dev); > + vgpio->gc.parent = dev; > + vgpio->gc.owner = THIS_MODULE; > + vgpio->gc.can_sleep = true; > + vgpio->gc.names = parse_gpio_names(vdev, config); > + if (IS_ERR(vgpio->gc.names)) > + return PTR_ERR(vgpio->gc.names); > + > + vdev->priv = vgpio; > + mutex_init(&vgpio->lock); > + init_completion(&vgpio->completion); > + > + ret = virtio_gpio_alloc_vqs(vdev); > + if (ret) > + return ret; > + > + ret = gpiochip_add(&vgpio->gc); > + if (ret) { > + virtio_gpio_free_vqs(vdev); > + dev_err(dev, "Failed to add virtio-gpio controller\n"); > + } > + > + return ret; > +} > + > +static void virtio_gpio_remove(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + > + gpiochip_remove(&vgpio->gc); > + virtio_gpio_free_vqs(vdev); > +} > + > +static const struct virtio_device_id id_table[] = { > + { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID }, > + {}, > +}; > +MODULE_DEVICE_TABLE(virtio, id_table); > + > +static struct virtio_driver virtio_gpio_driver = { > + .id_table = id_table, > + .probe = virtio_gpio_probe, > + .remove = virtio_gpio_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .owner = THIS_MODULE, > + }, > +}; > +module_virtio_driver(virtio_gpio_driver); > + > +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info(a)metux.net>"); > +MODULE_AUTHOR("Viresh Kumar <viresh.kumar(a)linaro.org>"); > +MODULE_DESCRIPTION("VirtIO GPIO driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h > new file mode 100644 > index 000000000000..e805e33a79cb > --- /dev/null > +++ b/include/uapi/linux/virtio_gpio.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > + > +#ifndef _LINUX_VIRTIO_GPIO_H > +#define _LINUX_VIRTIO_GPIO_H > + > +#include <linux/types.h> > + > +/* Virtio GPIO request types */ > +#define VIRTIO_GPIO_REQ_ACTIVATE 0x01 > +#define VIRTIO_GPIO_REQ_DEACTIVATE 0x02 > +#define VIRTIO_GPIO_REQ_GET_DIRECTION 0x03 > +#define VIRTIO_GPIO_REQ_DIRECTION_IN 0x04 > +#define VIRTIO_GPIO_REQ_DIRECTION_OUT 0x05 > +#define VIRTIO_GPIO_REQ_GET_VALUE 0x06 > +#define VIRTIO_GPIO_REQ_SET_VALUE 0x07 > + > +/* Possible values of the status field */ > +#define VIRTIO_GPIO_STATUS_OK 0x0 > +#define VIRTIO_GPIO_STATUS_ERR 0x1 > + > +struct virtio_gpio_config { > + char name[32]; > + __u16 ngpio; > + __u16 padding; > + __u32 gpio_names_size; > + char gpio_names[0]; A variable-size array here will make it very difficult to append new fields to virtio_gpio_config, when adding new features to the device. (New fields cannot be inserted in between, since older drivers will expect existing fields at a specific offset.) You could replace it with a reference to the string array, for example "__u16 gpio_names_offset" declaring the offset between the beginning of device-specific config and the string array. The 'name' field could also be indirect to avoid setting a fixed 32-char size, but that's not as important. > +} __packed; No need for __packed, because the fields are naturally aligned (as required by the virtio spec) Thanks, Jean > + > +/* Virtio GPIO Request / Response */ > +struct virtio_gpio_request { > + __u16 type; > + __u16 gpio; > + __u8 data; > +}; > + > +struct virtio_gpio_response { > + __u8 status; > + __u8 data; > +}; > + > +#endif /* _LINUX_VIRTIO_GPIO_H */ > diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h > index b89391dff6c9..0c24e8ae2499 100644 > --- a/include/uapi/linux/virtio_ids.h > +++ b/include/uapi/linux/virtio_ids.h > @@ -55,5 +55,6 @@ > #define VIRTIO_ID_PMEM 27 /* virtio pmem */ > #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ > #define VIRTIO_ID_I2C_ADAPTER 34 /* virtio i2c adapter */ > +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ > > #endif /* _LINUX_VIRTIO_IDS_H */ > -- > 2.31.1.272.g89b43f80a514 > > -- > Stratos-dev mailing list > Stratos-dev(a)op-lists.linaro.org > https://op-lists.linaro.org/mailman/listinfo/stratos-dev

3 years, 5 months

2
1
0 0

Re: [Stratos-dev] [PATCH V3 1/3] gpio: Add virtio-gpio driver

by Jean-Philippe Brucker

On Thu, Jun 10, 2021 at 04:00:39PM +0000, Enrico Weigelt, metux IT consult via Stratos-dev wrote: > On 10.06.21 15:22, Arnd Bergmann wrote: > > > Can you give an example of how this would be hooked up to other drivers > > using those gpios. Can you give an example of how using the "gpio-keys" or > > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > Connecting between self-probing bus'es and DT is generally tricky. IMHO > we don't have any generic mechanism for that. DT does have a generic description of PCI endpoints, which virtio-iommu relies on to express the relation between IOMMU and endpoint nodes [1]. I think the problem here is similar: the client node needs a phandle to the GPIO controller which may use virtio-pci transport? Note that it mostly works if the device is on the root PCI bus. Behind a bridge the OS may change the device's bus number as needed, so the BDF reference in DT is only valid if the software providing the DT description (VMM or firmware) initializes bus numbers accordingly (and I don't remember if Linux supports this case well). Thanks, Jean [1] Documentation/devicetree/bindings/virtio/iommu.txt > > I've made a few attempts, but nothing practically useful, which would be > accepted by the corresponding maintainers, yet. We'd either need some > very special logic in DT probing or pseudo-bus'es for the mapping. > (DT wants to do those connections via phandle's, which in turn need the > referenced nodes to be present in the DT). > > > From what I can tell, both the mmio and pci variants of virtio can have their > > dev->of_node populated, but I don't see the logic in register_virtio_device() > > that looks up the of_node of the virtio_device that the of_gpio code then > > tries to refer to. > > Have you ever successfully bound a virtio device via DT ? > > > --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(a)metux.net -- +49-151-27565287 > -- > Stratos-dev mailing list > Stratos-dev(a)op-lists.linaro.org > https://op-lists.linaro.org/mailman/listinfo/stratos-dev

3 years, 5 months

2
1
0 0

Re: [Stratos-dev] [PATCH 0/5] virtio: Add vhost-user based RNG service

by Srivatsa Vaddagiri

* Mathieu Poirier via Stratos-dev <stratos-dev(a)op-lists.linaro.org> [2021-06-01 20:03:14]: > Hi all, > > This sets adds a vhost-user based random number generator (RNG), > similar to what has been done for i2c and virtiofsd. In fact > the implementation for vhost-user-rng and vhost-user-rng-pci > follow what was done for vhost-user-i2c. Is there a Rust language based equivalent that is planned as well (just like what was done for virtio-i2c)? - vatsa -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

3 years, 5 months

2
1
0 0

Question on VirGL/virtio-gpu features

by Alex Bennée

Hi All, I was having a discussion the other day with an external team looking at using virtualised graphics for a production platform. They had some questions about features I couldn't answer due to my imperfect understanding of all the moving parts. Does anyone know of any work being done on the following: Alternate pixel formats ----------------------- I believe this is related to pixel data being expressed in different colour spaces. I see that VIRTIO_GPU_CMD_RESOURCE_CREATE_2D has the notion of different RGB layouts for pixel data. For 3D pixel data is this something that is ever visible to the virtio layer or is this all handled from within the OpenGL interface? I see there is a VIRTIO_GPU_CMD_SET_SCANOUT_BLOB for defining resources that can get referenced but that is still restricted to the same virtio_gpu_formats. Overlay planes -------------- As I understand it these are used for compositing multiple graphics sources. We currently working on a virtualised virtio-video backend and at some point we'll want to consider how you might overlay a 3D view from one VM with a video source from a second VM and either do the compositing in the host system or potentially a 3rd VM. Is anyone aware of any work being done in this direction yet? vsync ----- The existing virtio-gpu spec doesn't mention vsync at all. My last experience with active vsync synchronisation was back in the days of the Atari ST so I'm a little behind on how it works with modern 3D. Is this something that ever needs to be exposed to the guest or is it preserve of the host system to ensure newly drawn frames are swapped out at the right time to avoid tearing? The Khronos pages refer to Swap Interval being a platform specific extension. Does this mean it needs to be something specifically handled by VirGL? DRM --- Probably the least interesting problem from an open source point of view but I guess for media consumers their needs to be a solution for DRM protected streams to leave a VM and be handled "somewhere" else. Are there any proposals for exposing DRM via VirtIO that anyone is aware of? Any pointers gratefully received! Thanks in advance, -- Alex Bennée

3 years, 5 months

2
1
0 0

2024

2023

2022

2021

2020

Stratos-dev June 2021