All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-15 17:49 ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-15 17:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: corbet, linus.walleij, bgolaszewski, info, mst, jasowang,
	keescook, anton, ccross, tony.luck, linux-doc, linux-gpio,
	virtualization, linux-riscv

Introduce new GPIO driver for virtual GPIO devices via virtio.

The driver implements the virtio-gpio protocol (ID 41), which can be
used by either VM guests (e.g. bridging virtual gpios from the guest
to real gpios in the host or attaching simulators for automatic
application testing), as well as virtio-gpio hardware devices.

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

---

Status:
    * this driver is now field tested for about 6 month
      (against KVM+Qemu as well as some HW/FPGA implementation)
    * virtio device ID officially allocated
    * virtio spec has been submitted to virtio TC

Changes v4:
    * fixed spelling and formatting as pointed out by Randy
    * fixed virtio terminology: device/CPU instead of host/guest
    * spec: add clarification on versions and concurrency
    * driver: add endianess conversions
    * driver: rebased on mainline and fixed some little breaks
    * uapi: fixed device ID to now offically allocated: #41

Changes v3:
    * spec: fixed type names
    * spec: replace "host"/"guest" by "device"/"cpu"
    * spec: change terminology from "events" to "messages"
    * driver: fixed missing can_sleep flag
    * driver: select VIRTIO instead of depends on
    * driver: drop references to qemu in Kconfig
    * driver: fixed incomplete error handling and possible deadlock
              in case of sending buf failed
    * driver: dropped unneeded WARN_ON
    * driver: fixed retval of virtio_gpio_xmit()
    * driver: dynamically allocate virtio buffers
    * driver: added locking on gpio operations
    * driver: added irq_chip functions

Changes v2:
    * uapi: fixed header license
    * driver: sorted include's
    * driver: fixed formatting
    * driver: fixed unneeded devm allocation - plain kzalloc/kfree is enough
    * driver: fixed missing devm_kzalloc fail check
    * driver: use devm_kcalloc() for array allocation
    * spec: added virtio-gpio protocol specification
---
 Documentation/gpio/virtio-gpio.rst | 195 +++++++++++++++
 MAINTAINERS                        |   6 +
 drivers/gpio/Kconfig               |   7 +
 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpio-virtio.c         | 384 +++++++++++++++++++++++++++++
 include/uapi/linux/virtio_gpio.h   |  39 +++
 include/uapi/linux/virtio_ids.h    |   1 +
 7 files changed, 633 insertions(+)
 create mode 100644 Documentation/gpio/virtio-gpio.rst
 create mode 100644 drivers/gpio/gpio-virtio.c
 create mode 100644 include/uapi/linux/virtio_gpio.h

diff --git a/Documentation/gpio/virtio-gpio.rst b/Documentation/gpio/virtio-gpio.rst
new file mode 100644
index 000000000000..5fcc93f51174
--- /dev/null
+++ b/Documentation/gpio/virtio-gpio.rst
@@ -0,0 +1,195 @@
+===================
+Virtio-GPIO protocol specification
+===================
+...........
+Specification for virtio-based GPIO devices
+...........
+
++------------
++Version_ 1.0
++------------
+
+General
+===================
+
+The virtio-gpio protocol provides access to general purpose IO devices via
+virtio interfaces, used by many virtual machine monitors as well as hardware
+fabrics. In VM setups, these GPIOs could be either provided by some simulator
+(e.g. virtual HIL), routed to some external device or routed to real GPIOs on
+the host (e.g. virtualized embedded applications).
+
+Instead of simulating some existing real GPIO chip within an VMM, this
+protocol provides a hardware independent interface between CPU and device
+that solely relies on an active virtio connection (no matter which transport
+actually used), no other buses or additional platform driver logic required.
+
+At the same time, this protocol be implemented directly in virtio attached
+hardware, FPGAs or tiny MCUs.
+
+Protocol layout
+===================
+
+Configuration space
+----------------------
+
++--------+----------+-------------------------------+
+| Offset | Type     | Description                   |
++========+==========+===============================+
+| 0x00   | u8       | version                       |
++--------+----------+-------------------------------+
+| 0x02   | u16      | number of GPIO lines          |
++--------+----------+-------------------------------+
+| 0x04   | u32      | size of gpio name block       |
++--------+----------+-------------------------------+
+| 0x20   | char[32] | device name (0-terminated)    |
++--------+----------+-------------------------------+
+| 0x40   | char[]   | line names block              |
++--------+----------+-------------------------------+
+
+- for version field currently only value 1 supported.
+- the line names block holds a stream of zero-terminated strings,
+  containing the individual line names in ASCII. line names must unique.
+- unspecified fields are reserved for future use and should be zero.
+
+Virtqueues and messages:
+------------------------
+
+- Queue #0: transmission from device to CPU
+- Queue #1: transmission from CPU to device
+
+The queues transport messages of the struct virtio_gpio_msg:
+
+Message format:
+~~~~~~~~~~~~~~~
+
++--------+----------+---------------+
+| Offset | Type     | Description   |
++========+==========+===============+
+| 0x00   | uint16   | message type  |
++--------+----------+---------------+
+| 0x02   | uint16   | line id       |
++--------+----------+---------------+
+| 0x04   | uint32   | value         |
++--------+----------+---------------+
+
+Message types:
+~~~~~~~~~~~~~~
+
++---------+----------------------------------------+-----------------------------+
+| Code    | Symbol                                 |                             |
++=========+========================================+=============================+
+| 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           | state changed (device->CPU) |
++---------+----------------------------------------+-----------------------------+
+| 0x8000  | VIRTIO_GPIO_MSG_REPLY                  | device reply mask           |
++---------+----------------------------------------+-----------------------------+
+
+Data flow:
+----------------------
+
+- all operations, except ``VIRTIO_GPIO_MSG_DEVICE_LEVEL``, are initiated by CPU
+- device replies with the orinal ``type`` value OR'ed with ``VIRTIO_GPIO_MSG_REPLY``
+- ``VIRTIO_GPIO_MSG_DEVICE_LEVEL`` is only sent asynchronously from device to CPU
+- in replies, a negative ``value`` field denotes an Unix-style / POSIX errno code
+- valid direction values are:
+  * 0 = output
+  * 1 = input
+- valid line state values are:
+  * 0 = inactive
+  * 1 = active
+
+VIRTIO_GPIO_MSG_CPU_REQUEST
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- notify the device that given line# is going to be used
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: errno code (0 = success)
+
+VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line line direction to input
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply: value field holds errno
+  * ``value`` field: errno code (0 = success)
+
+VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line direction to output and given line state
+- request:
+  * ``line`` field: line number
+  * ``value`` field: output state (0=inactive, 1=active)
+- reply:
+  * ``value`` field: holds errno
+
+VIRTIO_GPIO_MSG_CPU_GET_DIRECTION
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- retrieve line direction
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: direction (0=output, 1=input) or errno code
+
+VIRTIO_GPIO_MSG_CPU_GET_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- retrieve line state value
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: line state (0=inactive, 1=active) or errno code
+
+VIRTIO_GPIO_MSG_CPU_SET_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line state value (output only)
+- request:
+  * ``line`` field: line number
+  * ``value`` field: line state (0=inactive, 1=active)
+- reply:
+  * ``value`` field: new line state or errno code
+
+VIRTIO_GPIO_MSG_DEVICE_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- async notification from device to CPU: line state changed
+- ``line`` field: line number
+- ``value`` field: new line state (0=inactive, 1=active)
+
+Request concurrency
+===================
+
+- CPU may send multiple request in serial, as long as the virtio queue
+  is not exceeded
+- device replies must be sent in the same order than the CPU requests
+- CPU should process asynchronous messages from device as soon as possible,
+  in order to avoid missing messages due to queue overrun
+
+Future versions
+===================
+
+- future versions must increment the ``version`` value
+- the basic data structures (config space, message format) should remain
+  backwards compatible, but may increased in size or use reserved fields
+- device needs to support commands in older versions
+- CPU should not send commands of newer versions that the device doesn't support
diff --git a/MAINTAINERS b/MAINTAINERS
index bc0ceef87b73..1189ae5b442b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19397,6 +19397,12 @@ F:	Documentation/filesystems/virtiofs.rst
 F:	fs/fuse/virtio_fs.c
 F:	include/uapi/linux/virtio_fs.h
 
+VIRTIO GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/gpio/gpio-virtio.c
+F:	include/uapi/linux/virtio_gpio.h
+
 VIRTIO GPU DRIVER
 M:	David Airlie <airlied@linux.ie>
 M:	Gerd Hoffmann <kraxel@redhat.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1dd0ec6727fd..b9871e5b3c74 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1655,6 +1655,13 @@ config GPIO_MOCKUP
 	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
 	  it.
 
+config GPIO_VIRTIO
+	tristate "VirtIO GPIO support"
+	select VIRTIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y here to enable guest support for virtio-based GPIOs.
+
 endmenu
 
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d7c81e1611a4..ba42e6549c87 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_GPIO_TWL4030)		+= gpio-twl4030.o
 obj-$(CONFIG_GPIO_TWL6040)		+= gpio-twl6040.o
 obj-$(CONFIG_GPIO_UCB1400)		+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_UNIPHIER)		+= gpio-uniphier.o
+obj-$(CONFIG_GPIO_VIRTIO)		+= gpio-virtio.o
 obj-$(CONFIG_GPIO_VF610)		+= gpio-vf610.o
 obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
 obj-$(CONFIG_GPIO_VISCONTI)		+= gpio-visconti.o
diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
new file mode 100644
index 000000000000..4938cd0350ff
--- /dev/null
+++ b/drivers/gpio/gpio-virtio.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * GPIO driver for virtio-based virtual GPIOs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt, metux IT consult <info@metux.net>
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_gpio.h>
+
+#define CONFIG_VIRTIO_GPIO_MAX_IRQ		256
+
+#define MSG_BUF_SZ	(sizeof(struct virtio_gpio_msg))
+
+struct virtio_gpio_priv {
+	struct gpio_chip gc;
+	spinlock_t vq_lock;
+	struct virtio_device *vdev;
+	int num_gpios;
+	char *name;
+	struct virtqueue *vq_rx;
+	struct virtqueue *vq_tx;
+	struct virtio_gpio_msg last;
+	wait_queue_head_t waitq;
+	unsigned long reply_wait;
+	struct irq_chip irq_chip;
+	DECLARE_BITMAP(irq_mask, CONFIG_VIRTIO_GPIO_MAX_IRQ);
+	unsigned int irq_parents;
+	struct mutex rpc_mutex;
+};
+
+static int virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
+{
+	struct scatterlist rcv_sg;
+	struct virtio_gpio_msg *buf;
+
+	buf = devm_kzalloc(&priv->vdev->dev, MSG_BUF_SZ, GFP_KERNEL);
+	if (!buf) {
+		dev_err(&priv->vdev->dev, "failed to allocate input buffer\n");
+		return -ENOMEM;
+	}
+
+	sg_init_one(&rcv_sg, buf, sizeof(struct virtio_gpio_priv));
+	virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, buf, GFP_KERNEL);
+	virtqueue_kick(priv->vq_rx);
+
+	return 0;
+}
+
+static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
+			    int pin, int value, struct virtio_gpio_msg *ev)
+{
+	struct scatterlist xmit_sg;
+	int ret;
+	unsigned long flags;
+
+	ev->type = cpu_to_le16(type);
+	ev->pin = cpu_to_le16(pin);
+	ev->value = cpu_to_le32(value);
+
+	sg_init_one(&xmit_sg, ev, MSG_BUF_SZ);
+	spin_lock_irqsave(&priv->vq_lock, flags);
+	ret = virtqueue_add_outbuf(priv->vq_tx, &xmit_sg, 1, priv, GFP_KERNEL);
+	if (ret < 0) {
+		dev_err(&priv->vdev->dev,
+			"virtqueue_add_outbuf() failed: %d\n", ret);
+		goto out;
+	}
+	virtqueue_kick(priv->vq_tx);
+
+out:
+	spin_unlock_irqrestore(&priv->vq_lock, flags);
+	return ret;
+}
+
+static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
+{
+	set_bit(id, &priv->reply_wait);
+}
+
+static inline int check_event(struct virtio_gpio_priv *priv, int id)
+{
+	return test_bit(id, &priv->reply_wait);
+}
+
+static inline void clear_event(struct virtio_gpio_priv *priv, int id)
+{
+	clear_bit(id, &priv->reply_wait);
+}
+
+static int virtio_gpio_rpc(struct virtio_gpio_priv *priv, int type,
+			   int pin, int value)
+{
+	int ret;
+	struct virtio_gpio_msg *buf = kzalloc(MSG_BUF_SZ, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+
+	mutex_lock(&priv->rpc_mutex);
+	virtio_gpio_prepare_inbuf(priv);
+	clear_event(priv, type);
+
+	ret = virtio_gpio_xmit(priv, type, pin, value, buf);
+	if (ret)
+		goto out;
+
+	wait_event_interruptible(priv->waitq, check_event(priv, type));
+	ret = priv->last.value;
+
+out:
+	mutex_unlock(&priv->rpc_mutex);
+	kfree(buf);
+	return ret;
+}
+
+static int virtio_gpio_direction_input(struct gpio_chip *gc,
+				       unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT,
+			       pin, 0);
+}
+
+static int virtio_gpio_direction_output(struct gpio_chip *gc,
+					unsigned int pin, int value)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT,
+			       pin, value);
+}
+
+static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_GET_DIRECTION,
+			       pin, 0);
+}
+
+static void virtio_gpio_set(struct gpio_chip *gc,
+			    unsigned int pin, int value)
+{
+	virtio_gpio_rpc(gpiochip_get_data(gc),
+			VIRTIO_GPIO_MSG_CPU_SET_LEVEL, pin, value);
+}
+
+static int virtio_gpio_get(struct gpio_chip *gc,
+			   unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_GET_LEVEL, pin, 0);
+}
+
+static int virtio_gpio_request(struct gpio_chip *gc,
+			       unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_REQUEST, pin, 0);
+}
+
+static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
+			       int pin, int value)
+{
+	int mapped_irq = irq_find_mapping(priv->gc.irq.domain, pin);
+
+	if ((pin < priv->num_gpios) && test_bit(pin, priv->irq_mask))
+		generic_handle_irq(mapped_irq);
+}
+
+static void virtio_gpio_data_rx(struct virtqueue *vq)
+{
+	struct virtio_gpio_priv *priv = vq->vdev->priv;
+	void *data;
+	unsigned int len;
+	struct virtio_gpio_msg *ev;
+
+	data = virtqueue_get_buf(priv->vq_rx, &len);
+	if (!data || !len) {
+		dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
+		return;
+	}
+
+	ev = data;
+
+	memcpy(&priv->last, data, MSG_BUF_SZ);
+
+	ev->type  = le16_to_cpu(ev->type);
+	ev->pin   = le16_to_cpu(ev->pin);
+	ev->value = le32_to_cpu(ev->value);
+
+	switch (ev->type) {
+	case VIRTIO_GPIO_MSG_DEVICE_LEVEL:
+		virtio_gpio_prepare_inbuf(priv);
+		virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
+		break;
+	default:
+		wakeup_event(priv, ev->type & ~VIRTIO_GPIO_MSG_REPLY);
+		break;
+	}
+
+	wake_up_all(&priv->waitq);
+
+	devm_kfree(&priv->vdev->dev, data);
+}
+
+static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
+{
+	struct virtqueue *vqs[2];
+	vq_callback_t *cbs[] = {
+		NULL,
+		virtio_gpio_data_rx,
+	};
+	static const char * const names[] = { "in", "out", };
+	int ret;
+
+	ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
+		return ret;
+	}
+
+	priv->vq_rx = vqs[0];
+	priv->vq_tx = vqs[1];
+
+	ret = virtio_gpio_prepare_inbuf(priv);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "preparing inbuf failed\n");
+		return ret;
+	}
+
+	virtqueue_enable_cb(priv->vq_rx);
+	virtio_device_ready(priv->vdev);
+
+	return 0;
+}
+
+static void virtio_gpio_irq_unmask(struct irq_data *irq)
+{
+	int hwirq = irqd_to_hwirq(irq);
+	struct virtio_gpio_priv *priv
+		= gpiochip_get_data(irq_data_get_irq_chip_data(irq));
+	if (hwirq < CONFIG_VIRTIO_GPIO_MAX_IRQ)
+		set_bit(hwirq, priv->irq_mask);
+}
+
+static void virtio_gpio_irq_mask(struct irq_data *irq)
+{
+	int hwirq = irqd_to_hwirq(irq);
+	struct virtio_gpio_priv *priv
+		= gpiochip_get_data(irq_data_get_irq_chip_data(irq));
+	if (hwirq < CONFIG_VIRTIO_GPIO_MAX_IRQ)
+		clear_bit(hwirq, priv->irq_mask);
+}
+
+static int virtio_gpio_probe(struct virtio_device *vdev)
+{
+	struct virtio_gpio_priv *priv;
+	struct virtio_gpio_config cf = {};
+	char *name_buffer;
+	const char **gpio_names = NULL;
+	struct device *dev = &vdev->dev;
+	struct gpio_irq_chip *girq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
+	if (!priv->name)
+		return -ENOMEM;
+
+	spin_lock_init(&priv->vq_lock);
+	mutex_init(&priv->rpc_mutex);
+
+	virtio_cread_le(vdev, struct virtio_gpio_config, version, &cf.version);
+	virtio_cread_le(vdev, struct virtio_gpio_config, num_gpios,
+			&cf.num_gpios);
+	virtio_cread_le(vdev, struct virtio_gpio_config, names_size,
+			&cf.names_size);
+	virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
+			   priv->name, sizeof(cf.name));
+
+	if (cf.version != 1) {
+		dev_err(dev, "unsupported interface version %d\n", cf.version);
+		return -EINVAL;
+	}
+
+	priv->num_gpios = cf.num_gpios;
+
+	if (cf.names_size) {
+		char *bufwalk;
+		int idx = 0;
+
+		name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
+					   GFP_KERNEL)+1;
+		virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
+				   name_buffer, cf.names_size);
+		name_buffer[cf.names_size] = 0;
+
+		gpio_names = devm_kcalloc(dev, priv->num_gpios, sizeof(char *),
+					  GFP_KERNEL);
+		bufwalk = name_buffer;
+
+		while (idx < priv->num_gpios &&
+		       bufwalk < (name_buffer+cf.names_size)) {
+			gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
+			bufwalk += strlen(bufwalk)+1;
+			idx++;
+		}
+	}
+
+	priv->vdev			= vdev;
+	vdev->priv = priv;
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= dev;
+	priv->gc.label			= (priv->name[0] ? priv->name
+							 : dev_name(dev));
+	priv->gc.ngpio			= priv->num_gpios;
+	priv->gc.names			= gpio_names;
+	priv->gc.base			= -1;
+	priv->gc.request		= virtio_gpio_request;
+	priv->gc.direction_input	= virtio_gpio_direction_input;
+	priv->gc.direction_output	= virtio_gpio_direction_output;
+	priv->gc.get_direction		= virtio_gpio_get_direction;
+	priv->gc.get			= virtio_gpio_get;
+	priv->gc.set			= virtio_gpio_set;
+	priv->gc.can_sleep		= true;
+
+	priv->irq_chip.name		= "virtio-gpio-irq";
+	priv->irq_chip.irq_mask		= virtio_gpio_irq_mask;
+	priv->irq_chip.irq_unmask	= virtio_gpio_irq_unmask;
+
+	girq = &priv->gc.irq;
+
+	priv->gc.irq.chip		= &priv->irq_chip;
+	priv->gc.irq.num_parents	= 1;
+	priv->gc.irq.default_type	= IRQ_TYPE_NONE;
+	priv->gc.irq.handler		= handle_simple_irq;
+	priv->gc.irq.parents		= &priv->irq_parents;
+	priv->irq_parents		= 0;
+
+	init_waitqueue_head(&priv->waitq);
+
+	priv->reply_wait = 0;
+
+	virtio_gpio_alloc_vq(priv);
+
+	return devm_gpiochip_add_data(dev, &priv->gc, priv);
+}
+
+static void virtio_gpio_remove(struct virtio_device *vdev)
+{
+	/* just dummy, virtio subsys can't cope w/ NULL vector */
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_gpio_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.driver.owner	= THIS_MODULE,
+	.id_table	= id_table,
+	.probe		= virtio_gpio_probe,
+	.remove		= virtio_gpio_remove,
+};
+
+module_virtio_driver(virtio_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+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..5b90acae6c85
--- /dev/null
+++ b/include/uapi/linux/virtio_gpio.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _LINUX_VIRTIO_GPIO_H
+#define _LINUX_VIRTIO_GPIO_H
+
+#include <linux/types.h>
+
+enum virtio_gpio_msg_type {
+	// requests from cpu to device
+	VIRTIO_GPIO_MSG_CPU_REQUEST		= 0x01,
+	VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT	= 0x02,
+	VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT	= 0x03,
+	VIRTIO_GPIO_MSG_CPU_GET_DIRECTION	= 0x04,
+	VIRTIO_GPIO_MSG_CPU_GET_LEVEL		= 0x05,
+	VIRTIO_GPIO_MSG_CPU_SET_LEVEL		= 0x06,
+
+	// messages from host to guest
+	VIRTIO_GPIO_MSG_DEVICE_LEVEL		= 0x11,	// gpio state changed
+
+	/* mask bit set on host->guest reply */
+	VIRTIO_GPIO_MSG_REPLY			= 0x8000,
+};
+
+struct virtio_gpio_config {
+	__u8    version;
+	__u8    reserved0;
+	__u16   num_gpios;
+	__u32   names_size;
+	__u8    reserved1[24];
+	__u8    name[32];
+};
+
+struct virtio_gpio_msg {
+	__le16 type;
+	__le16 pin;
+	__le32 value;
+};
+
+#endif /* _LINUX_VIRTIO_GPIO_H */
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 4fe842c3a3a9..0c9bac389ce0 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -56,5 +56,6 @@
 #define VIRTIO_ID_PMEM			27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM	29 /* virtio mac80211-hwsim */
 #define VIRTIO_ID_BT			40 /* virtio bluetooth */
+#define VIRTIO_ID_GPIO			41 /* virtio GPIO */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-15 17:49 ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-15 17:49 UTC (permalink / raw)
  To: linux-kernel
  Cc: corbet, linus.walleij, bgolaszewski, info, mst, jasowang,
	keescook, anton, ccross, tony.luck, linux-doc, linux-gpio,
	virtualization, linux-riscv

Introduce new GPIO driver for virtual GPIO devices via virtio.

The driver implements the virtio-gpio protocol (ID 41), which can be
used by either VM guests (e.g. bridging virtual gpios from the guest
to real gpios in the host or attaching simulators for automatic
application testing), as well as virtio-gpio hardware devices.

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

---

Status:
    * this driver is now field tested for about 6 month
      (against KVM+Qemu as well as some HW/FPGA implementation)
    * virtio device ID officially allocated
    * virtio spec has been submitted to virtio TC

Changes v4:
    * fixed spelling and formatting as pointed out by Randy
    * fixed virtio terminology: device/CPU instead of host/guest
    * spec: add clarification on versions and concurrency
    * driver: add endianess conversions
    * driver: rebased on mainline and fixed some little breaks
    * uapi: fixed device ID to now offically allocated: #41

Changes v3:
    * spec: fixed type names
    * spec: replace "host"/"guest" by "device"/"cpu"
    * spec: change terminology from "events" to "messages"
    * driver: fixed missing can_sleep flag
    * driver: select VIRTIO instead of depends on
    * driver: drop references to qemu in Kconfig
    * driver: fixed incomplete error handling and possible deadlock
              in case of sending buf failed
    * driver: dropped unneeded WARN_ON
    * driver: fixed retval of virtio_gpio_xmit()
    * driver: dynamically allocate virtio buffers
    * driver: added locking on gpio operations
    * driver: added irq_chip functions

Changes v2:
    * uapi: fixed header license
    * driver: sorted include's
    * driver: fixed formatting
    * driver: fixed unneeded devm allocation - plain kzalloc/kfree is enough
    * driver: fixed missing devm_kzalloc fail check
    * driver: use devm_kcalloc() for array allocation
    * spec: added virtio-gpio protocol specification
---
 Documentation/gpio/virtio-gpio.rst | 195 +++++++++++++++
 MAINTAINERS                        |   6 +
 drivers/gpio/Kconfig               |   7 +
 drivers/gpio/Makefile              |   1 +
 drivers/gpio/gpio-virtio.c         | 384 +++++++++++++++++++++++++++++
 include/uapi/linux/virtio_gpio.h   |  39 +++
 include/uapi/linux/virtio_ids.h    |   1 +
 7 files changed, 633 insertions(+)
 create mode 100644 Documentation/gpio/virtio-gpio.rst
 create mode 100644 drivers/gpio/gpio-virtio.c
 create mode 100644 include/uapi/linux/virtio_gpio.h

diff --git a/Documentation/gpio/virtio-gpio.rst b/Documentation/gpio/virtio-gpio.rst
new file mode 100644
index 000000000000..5fcc93f51174
--- /dev/null
+++ b/Documentation/gpio/virtio-gpio.rst
@@ -0,0 +1,195 @@
+===================
+Virtio-GPIO protocol specification
+===================
+...........
+Specification for virtio-based GPIO devices
+...........
+
++------------
++Version_ 1.0
++------------
+
+General
+===================
+
+The virtio-gpio protocol provides access to general purpose IO devices via
+virtio interfaces, used by many virtual machine monitors as well as hardware
+fabrics. In VM setups, these GPIOs could be either provided by some simulator
+(e.g. virtual HIL), routed to some external device or routed to real GPIOs on
+the host (e.g. virtualized embedded applications).
+
+Instead of simulating some existing real GPIO chip within an VMM, this
+protocol provides a hardware independent interface between CPU and device
+that solely relies on an active virtio connection (no matter which transport
+actually used), no other buses or additional platform driver logic required.
+
+At the same time, this protocol be implemented directly in virtio attached
+hardware, FPGAs or tiny MCUs.
+
+Protocol layout
+===================
+
+Configuration space
+----------------------
+
++--------+----------+-------------------------------+
+| Offset | Type     | Description                   |
++========+==========+===============================+
+| 0x00   | u8       | version                       |
++--------+----------+-------------------------------+
+| 0x02   | u16      | number of GPIO lines          |
++--------+----------+-------------------------------+
+| 0x04   | u32      | size of gpio name block       |
++--------+----------+-------------------------------+
+| 0x20   | char[32] | device name (0-terminated)    |
++--------+----------+-------------------------------+
+| 0x40   | char[]   | line names block              |
++--------+----------+-------------------------------+
+
+- for version field currently only value 1 supported.
+- the line names block holds a stream of zero-terminated strings,
+  containing the individual line names in ASCII. line names must unique.
+- unspecified fields are reserved for future use and should be zero.
+
+Virtqueues and messages:
+------------------------
+
+- Queue #0: transmission from device to CPU
+- Queue #1: transmission from CPU to device
+
+The queues transport messages of the struct virtio_gpio_msg:
+
+Message format:
+~~~~~~~~~~~~~~~
+
++--------+----------+---------------+
+| Offset | Type     | Description   |
++========+==========+===============+
+| 0x00   | uint16   | message type  |
++--------+----------+---------------+
+| 0x02   | uint16   | line id       |
++--------+----------+---------------+
+| 0x04   | uint32   | value         |
++--------+----------+---------------+
+
+Message types:
+~~~~~~~~~~~~~~
+
++---------+----------------------------------------+-----------------------------+
+| Code    | Symbol                                 |                             |
++=========+========================================+=============================+
+| 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           | state changed (device->CPU) |
++---------+----------------------------------------+-----------------------------+
+| 0x8000  | VIRTIO_GPIO_MSG_REPLY                  | device reply mask           |
++---------+----------------------------------------+-----------------------------+
+
+Data flow:
+----------------------
+
+- all operations, except ``VIRTIO_GPIO_MSG_DEVICE_LEVEL``, are initiated by CPU
+- device replies with the orinal ``type`` value OR'ed with ``VIRTIO_GPIO_MSG_REPLY``
+- ``VIRTIO_GPIO_MSG_DEVICE_LEVEL`` is only sent asynchronously from device to CPU
+- in replies, a negative ``value`` field denotes an Unix-style / POSIX errno code
+- valid direction values are:
+  * 0 = output
+  * 1 = input
+- valid line state values are:
+  * 0 = inactive
+  * 1 = active
+
+VIRTIO_GPIO_MSG_CPU_REQUEST
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- notify the device that given line# is going to be used
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: errno code (0 = success)
+
+VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line line direction to input
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply: value field holds errno
+  * ``value`` field: errno code (0 = success)
+
+VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line direction to output and given line state
+- request:
+  * ``line`` field: line number
+  * ``value`` field: output state (0=inactive, 1=active)
+- reply:
+  * ``value`` field: holds errno
+
+VIRTIO_GPIO_MSG_CPU_GET_DIRECTION
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- retrieve line direction
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: direction (0=output, 1=input) or errno code
+
+VIRTIO_GPIO_MSG_CPU_GET_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- retrieve line state value
+- request:
+  * ``line`` field: line number
+  * ``value`` field: unused
+- reply:
+  * ``value`` field: line state (0=inactive, 1=active) or errno code
+
+VIRTIO_GPIO_MSG_CPU_SET_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- set line state value (output only)
+- request:
+  * ``line`` field: line number
+  * ``value`` field: line state (0=inactive, 1=active)
+- reply:
+  * ``value`` field: new line state or errno code
+
+VIRTIO_GPIO_MSG_DEVICE_LEVEL
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+- async notification from device to CPU: line state changed
+- ``line`` field: line number
+- ``value`` field: new line state (0=inactive, 1=active)
+
+Request concurrency
+===================
+
+- CPU may send multiple request in serial, as long as the virtio queue
+  is not exceeded
+- device replies must be sent in the same order than the CPU requests
+- CPU should process asynchronous messages from device as soon as possible,
+  in order to avoid missing messages due to queue overrun
+
+Future versions
+===================
+
+- future versions must increment the ``version`` value
+- the basic data structures (config space, message format) should remain
+  backwards compatible, but may increased in size or use reserved fields
+- device needs to support commands in older versions
+- CPU should not send commands of newer versions that the device doesn't support
diff --git a/MAINTAINERS b/MAINTAINERS
index bc0ceef87b73..1189ae5b442b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19397,6 +19397,12 @@ F:	Documentation/filesystems/virtiofs.rst
 F:	fs/fuse/virtio_fs.c
 F:	include/uapi/linux/virtio_fs.h
 
+VIRTIO GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/gpio/gpio-virtio.c
+F:	include/uapi/linux/virtio_gpio.h
+
 VIRTIO GPU DRIVER
 M:	David Airlie <airlied@linux.ie>
 M:	Gerd Hoffmann <kraxel@redhat.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 1dd0ec6727fd..b9871e5b3c74 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1655,6 +1655,13 @@ config GPIO_MOCKUP
 	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
 	  it.
 
+config GPIO_VIRTIO
+	tristate "VirtIO GPIO support"
+	select VIRTIO
+	select GPIOLIB_IRQCHIP
+	help
+	  Say Y here to enable guest support for virtio-based GPIOs.
+
 endmenu
 
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d7c81e1611a4..ba42e6549c87 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -162,6 +162,7 @@ obj-$(CONFIG_GPIO_TWL4030)		+= gpio-twl4030.o
 obj-$(CONFIG_GPIO_TWL6040)		+= gpio-twl6040.o
 obj-$(CONFIG_GPIO_UCB1400)		+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_UNIPHIER)		+= gpio-uniphier.o
+obj-$(CONFIG_GPIO_VIRTIO)		+= gpio-virtio.o
 obj-$(CONFIG_GPIO_VF610)		+= gpio-vf610.o
 obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
 obj-$(CONFIG_GPIO_VISCONTI)		+= gpio-visconti.o
diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
new file mode 100644
index 000000000000..4938cd0350ff
--- /dev/null
+++ b/drivers/gpio/gpio-virtio.c
@@ -0,0 +1,384 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * GPIO driver for virtio-based virtual GPIOs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt, metux IT consult <info@metux.net>
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_gpio.h>
+
+#define CONFIG_VIRTIO_GPIO_MAX_IRQ		256
+
+#define MSG_BUF_SZ	(sizeof(struct virtio_gpio_msg))
+
+struct virtio_gpio_priv {
+	struct gpio_chip gc;
+	spinlock_t vq_lock;
+	struct virtio_device *vdev;
+	int num_gpios;
+	char *name;
+	struct virtqueue *vq_rx;
+	struct virtqueue *vq_tx;
+	struct virtio_gpio_msg last;
+	wait_queue_head_t waitq;
+	unsigned long reply_wait;
+	struct irq_chip irq_chip;
+	DECLARE_BITMAP(irq_mask, CONFIG_VIRTIO_GPIO_MAX_IRQ);
+	unsigned int irq_parents;
+	struct mutex rpc_mutex;
+};
+
+static int virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
+{
+	struct scatterlist rcv_sg;
+	struct virtio_gpio_msg *buf;
+
+	buf = devm_kzalloc(&priv->vdev->dev, MSG_BUF_SZ, GFP_KERNEL);
+	if (!buf) {
+		dev_err(&priv->vdev->dev, "failed to allocate input buffer\n");
+		return -ENOMEM;
+	}
+
+	sg_init_one(&rcv_sg, buf, sizeof(struct virtio_gpio_priv));
+	virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, buf, GFP_KERNEL);
+	virtqueue_kick(priv->vq_rx);
+
+	return 0;
+}
+
+static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
+			    int pin, int value, struct virtio_gpio_msg *ev)
+{
+	struct scatterlist xmit_sg;
+	int ret;
+	unsigned long flags;
+
+	ev->type = cpu_to_le16(type);
+	ev->pin = cpu_to_le16(pin);
+	ev->value = cpu_to_le32(value);
+
+	sg_init_one(&xmit_sg, ev, MSG_BUF_SZ);
+	spin_lock_irqsave(&priv->vq_lock, flags);
+	ret = virtqueue_add_outbuf(priv->vq_tx, &xmit_sg, 1, priv, GFP_KERNEL);
+	if (ret < 0) {
+		dev_err(&priv->vdev->dev,
+			"virtqueue_add_outbuf() failed: %d\n", ret);
+		goto out;
+	}
+	virtqueue_kick(priv->vq_tx);
+
+out:
+	spin_unlock_irqrestore(&priv->vq_lock, flags);
+	return ret;
+}
+
+static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
+{
+	set_bit(id, &priv->reply_wait);
+}
+
+static inline int check_event(struct virtio_gpio_priv *priv, int id)
+{
+	return test_bit(id, &priv->reply_wait);
+}
+
+static inline void clear_event(struct virtio_gpio_priv *priv, int id)
+{
+	clear_bit(id, &priv->reply_wait);
+}
+
+static int virtio_gpio_rpc(struct virtio_gpio_priv *priv, int type,
+			   int pin, int value)
+{
+	int ret;
+	struct virtio_gpio_msg *buf = kzalloc(MSG_BUF_SZ, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+
+	mutex_lock(&priv->rpc_mutex);
+	virtio_gpio_prepare_inbuf(priv);
+	clear_event(priv, type);
+
+	ret = virtio_gpio_xmit(priv, type, pin, value, buf);
+	if (ret)
+		goto out;
+
+	wait_event_interruptible(priv->waitq, check_event(priv, type));
+	ret = priv->last.value;
+
+out:
+	mutex_unlock(&priv->rpc_mutex);
+	kfree(buf);
+	return ret;
+}
+
+static int virtio_gpio_direction_input(struct gpio_chip *gc,
+				       unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT,
+			       pin, 0);
+}
+
+static int virtio_gpio_direction_output(struct gpio_chip *gc,
+					unsigned int pin, int value)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT,
+			       pin, value);
+}
+
+static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_GET_DIRECTION,
+			       pin, 0);
+}
+
+static void virtio_gpio_set(struct gpio_chip *gc,
+			    unsigned int pin, int value)
+{
+	virtio_gpio_rpc(gpiochip_get_data(gc),
+			VIRTIO_GPIO_MSG_CPU_SET_LEVEL, pin, value);
+}
+
+static int virtio_gpio_get(struct gpio_chip *gc,
+			   unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_GET_LEVEL, pin, 0);
+}
+
+static int virtio_gpio_request(struct gpio_chip *gc,
+			       unsigned int pin)
+{
+	return virtio_gpio_rpc(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_MSG_CPU_REQUEST, pin, 0);
+}
+
+static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
+			       int pin, int value)
+{
+	int mapped_irq = irq_find_mapping(priv->gc.irq.domain, pin);
+
+	if ((pin < priv->num_gpios) && test_bit(pin, priv->irq_mask))
+		generic_handle_irq(mapped_irq);
+}
+
+static void virtio_gpio_data_rx(struct virtqueue *vq)
+{
+	struct virtio_gpio_priv *priv = vq->vdev->priv;
+	void *data;
+	unsigned int len;
+	struct virtio_gpio_msg *ev;
+
+	data = virtqueue_get_buf(priv->vq_rx, &len);
+	if (!data || !len) {
+		dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
+		return;
+	}
+
+	ev = data;
+
+	memcpy(&priv->last, data, MSG_BUF_SZ);
+
+	ev->type  = le16_to_cpu(ev->type);
+	ev->pin   = le16_to_cpu(ev->pin);
+	ev->value = le32_to_cpu(ev->value);
+
+	switch (ev->type) {
+	case VIRTIO_GPIO_MSG_DEVICE_LEVEL:
+		virtio_gpio_prepare_inbuf(priv);
+		virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
+		break;
+	default:
+		wakeup_event(priv, ev->type & ~VIRTIO_GPIO_MSG_REPLY);
+		break;
+	}
+
+	wake_up_all(&priv->waitq);
+
+	devm_kfree(&priv->vdev->dev, data);
+}
+
+static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
+{
+	struct virtqueue *vqs[2];
+	vq_callback_t *cbs[] = {
+		NULL,
+		virtio_gpio_data_rx,
+	};
+	static const char * const names[] = { "in", "out", };
+	int ret;
+
+	ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
+		return ret;
+	}
+
+	priv->vq_rx = vqs[0];
+	priv->vq_tx = vqs[1];
+
+	ret = virtio_gpio_prepare_inbuf(priv);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "preparing inbuf failed\n");
+		return ret;
+	}
+
+	virtqueue_enable_cb(priv->vq_rx);
+	virtio_device_ready(priv->vdev);
+
+	return 0;
+}
+
+static void virtio_gpio_irq_unmask(struct irq_data *irq)
+{
+	int hwirq = irqd_to_hwirq(irq);
+	struct virtio_gpio_priv *priv
+		= gpiochip_get_data(irq_data_get_irq_chip_data(irq));
+	if (hwirq < CONFIG_VIRTIO_GPIO_MAX_IRQ)
+		set_bit(hwirq, priv->irq_mask);
+}
+
+static void virtio_gpio_irq_mask(struct irq_data *irq)
+{
+	int hwirq = irqd_to_hwirq(irq);
+	struct virtio_gpio_priv *priv
+		= gpiochip_get_data(irq_data_get_irq_chip_data(irq));
+	if (hwirq < CONFIG_VIRTIO_GPIO_MAX_IRQ)
+		clear_bit(hwirq, priv->irq_mask);
+}
+
+static int virtio_gpio_probe(struct virtio_device *vdev)
+{
+	struct virtio_gpio_priv *priv;
+	struct virtio_gpio_config cf = {};
+	char *name_buffer;
+	const char **gpio_names = NULL;
+	struct device *dev = &vdev->dev;
+	struct gpio_irq_chip *girq;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
+	if (!priv->name)
+		return -ENOMEM;
+
+	spin_lock_init(&priv->vq_lock);
+	mutex_init(&priv->rpc_mutex);
+
+	virtio_cread_le(vdev, struct virtio_gpio_config, version, &cf.version);
+	virtio_cread_le(vdev, struct virtio_gpio_config, num_gpios,
+			&cf.num_gpios);
+	virtio_cread_le(vdev, struct virtio_gpio_config, names_size,
+			&cf.names_size);
+	virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
+			   priv->name, sizeof(cf.name));
+
+	if (cf.version != 1) {
+		dev_err(dev, "unsupported interface version %d\n", cf.version);
+		return -EINVAL;
+	}
+
+	priv->num_gpios = cf.num_gpios;
+
+	if (cf.names_size) {
+		char *bufwalk;
+		int idx = 0;
+
+		name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
+					   GFP_KERNEL)+1;
+		virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
+				   name_buffer, cf.names_size);
+		name_buffer[cf.names_size] = 0;
+
+		gpio_names = devm_kcalloc(dev, priv->num_gpios, sizeof(char *),
+					  GFP_KERNEL);
+		bufwalk = name_buffer;
+
+		while (idx < priv->num_gpios &&
+		       bufwalk < (name_buffer+cf.names_size)) {
+			gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
+			bufwalk += strlen(bufwalk)+1;
+			idx++;
+		}
+	}
+
+	priv->vdev			= vdev;
+	vdev->priv = priv;
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= dev;
+	priv->gc.label			= (priv->name[0] ? priv->name
+							 : dev_name(dev));
+	priv->gc.ngpio			= priv->num_gpios;
+	priv->gc.names			= gpio_names;
+	priv->gc.base			= -1;
+	priv->gc.request		= virtio_gpio_request;
+	priv->gc.direction_input	= virtio_gpio_direction_input;
+	priv->gc.direction_output	= virtio_gpio_direction_output;
+	priv->gc.get_direction		= virtio_gpio_get_direction;
+	priv->gc.get			= virtio_gpio_get;
+	priv->gc.set			= virtio_gpio_set;
+	priv->gc.can_sleep		= true;
+
+	priv->irq_chip.name		= "virtio-gpio-irq";
+	priv->irq_chip.irq_mask		= virtio_gpio_irq_mask;
+	priv->irq_chip.irq_unmask	= virtio_gpio_irq_unmask;
+
+	girq = &priv->gc.irq;
+
+	priv->gc.irq.chip		= &priv->irq_chip;
+	priv->gc.irq.num_parents	= 1;
+	priv->gc.irq.default_type	= IRQ_TYPE_NONE;
+	priv->gc.irq.handler		= handle_simple_irq;
+	priv->gc.irq.parents		= &priv->irq_parents;
+	priv->irq_parents		= 0;
+
+	init_waitqueue_head(&priv->waitq);
+
+	priv->reply_wait = 0;
+
+	virtio_gpio_alloc_vq(priv);
+
+	return devm_gpiochip_add_data(dev, &priv->gc, priv);
+}
+
+static void virtio_gpio_remove(struct virtio_device *vdev)
+{
+	/* just dummy, virtio subsys can't cope w/ NULL vector */
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_gpio_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.driver.owner	= THIS_MODULE,
+	.id_table	= id_table,
+	.probe		= virtio_gpio_probe,
+	.remove		= virtio_gpio_remove,
+};
+
+module_virtio_driver(virtio_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+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..5b90acae6c85
--- /dev/null
+++ b/include/uapi/linux/virtio_gpio.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+
+#ifndef _LINUX_VIRTIO_GPIO_H
+#define _LINUX_VIRTIO_GPIO_H
+
+#include <linux/types.h>
+
+enum virtio_gpio_msg_type {
+	// requests from cpu to device
+	VIRTIO_GPIO_MSG_CPU_REQUEST		= 0x01,
+	VIRTIO_GPIO_MSG_CPU_DIRECTION_INPUT	= 0x02,
+	VIRTIO_GPIO_MSG_CPU_DIRECTION_OUTPUT	= 0x03,
+	VIRTIO_GPIO_MSG_CPU_GET_DIRECTION	= 0x04,
+	VIRTIO_GPIO_MSG_CPU_GET_LEVEL		= 0x05,
+	VIRTIO_GPIO_MSG_CPU_SET_LEVEL		= 0x06,
+
+	// messages from host to guest
+	VIRTIO_GPIO_MSG_DEVICE_LEVEL		= 0x11,	// gpio state changed
+
+	/* mask bit set on host->guest reply */
+	VIRTIO_GPIO_MSG_REPLY			= 0x8000,
+};
+
+struct virtio_gpio_config {
+	__u8    version;
+	__u8    reserved0;
+	__u16   num_gpios;
+	__u32   names_size;
+	__u8    reserved1[24];
+	__u8    name[32];
+};
+
+struct virtio_gpio_msg {
+	__le16 type;
+	__le16 pin;
+	__le32 value;
+};
+
+#endif /* _LINUX_VIRTIO_GPIO_H */
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 4fe842c3a3a9..0c9bac389ce0 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -56,5 +56,6 @@
 #define VIRTIO_ID_PMEM			27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM	29 /* virtio mac80211-hwsim */
 #define VIRTIO_ID_BT			40 /* virtio bluetooth */
+#define VIRTIO_ID_GPIO			41 /* virtio GPIO */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
2.20.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-15 17:49 ` Enrico Weigelt, metux IT consult
  (?)
@ 2021-06-16  8:31   ` Linus Walleij
  -1 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2021-06-16  8:31 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv

Hi Enrico,

On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> Introduce new GPIO driver for virtual GPIO devices via virtio.
>
> The driver implements the virtio-gpio protocol (ID 41), which can be
> used by either VM guests (e.g. bridging virtual gpios from the guest
> to real gpios in the host or attaching simulators for automatic
> application testing), as well as virtio-gpio hardware devices.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

So now there are two contesting patches for this and that creates a
social problem for us as maintainers. I am not too happy about that.

This situation activates the kernel management style document so
I advise involved parties to familiarize themselves with it:
https://www.kernel.org/doc/html/latest/process/management-style.html

Can we get the discussion down to actual technical points?
We really need a virtio GPIO driver, no doubt, so if everyone could
just work toward that goal and compromise with their specific pet
peeves that would be great.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16  8:31   ` Linus Walleij
  0 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2021-06-16  8:31 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv

Hi Enrico,

On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> Introduce new GPIO driver for virtual GPIO devices via virtio.
>
> The driver implements the virtio-gpio protocol (ID 41), which can be
> used by either VM guests (e.g. bridging virtual gpios from the guest
> to real gpios in the host or attaching simulators for automatic
> application testing), as well as virtio-gpio hardware devices.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

So now there are two contesting patches for this and that creates a
social problem for us as maintainers. I am not too happy about that.

This situation activates the kernel management style document so
I advise involved parties to familiarize themselves with it:
https://www.kernel.org/doc/html/latest/process/management-style.html

Can we get the discussion down to actual technical points?
We really need a virtio GPIO driver, no doubt, so if everyone could
just work toward that goal and compromise with their specific pet
peeves that would be great.

Yours,
Linus Walleij

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16  8:31   ` Linus Walleij
  0 siblings, 0 replies; 54+ messages in thread
From: Linus Walleij @ 2021-06-16  8:31 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: Anton Vorontsov, Kees Cook, Michael S. Tsirkin, Jonathan Corbet,
	Linux Doc Mailing List, linux-kernel, virtualization,
	Bartosz Golaszewski, Tony Luck, open list:GPIO SUBSYSTEM,
	Colin Cross, linux-riscv

Hi Enrico,

On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> Introduce new GPIO driver for virtual GPIO devices via virtio.
>
> The driver implements the virtio-gpio protocol (ID 41), which can be
> used by either VM guests (e.g. bridging virtual gpios from the guest
> to real gpios in the host or attaching simulators for automatic
> application testing), as well as virtio-gpio hardware devices.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

So now there are two contesting patches for this and that creates a
social problem for us as maintainers. I am not too happy about that.

This situation activates the kernel management style document so
I advise involved parties to familiarize themselves with it:
https://www.kernel.org/doc/html/latest/process/management-style.html

Can we get the discussion down to actual technical points?
We really need a virtio GPIO driver, no doubt, so if everyone could
just work toward that goal and compromise with their specific pet
peeves that would be great.

Yours,
Linus Walleij
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-16  8:31   ` Linus Walleij
  (?)
@ 2021-06-16 11:49     ` Viresh Kumar
  -1 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-16 11:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Enrico Weigelt, metux IT consult, linux-kernel, Jonathan Corbet,
	Bartosz Golaszewski, Michael S. Tsirkin, Jason Wang, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	Vincent Guittot, Bill Mills, Alex Bennée

On 16-06-21, 10:31, Linus Walleij wrote:
> Hi Enrico,
> 
> On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
> 
> > Introduce new GPIO driver for virtual GPIO devices via virtio.
> >
> > The driver implements the virtio-gpio protocol (ID 41), which can be
> > used by either VM guests (e.g. bridging virtual gpios from the guest
> > to real gpios in the host or attaching simulators for automatic
> > application testing), as well as virtio-gpio hardware devices.
> >
> > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> 
> So now there are two contesting patches for this and that creates a
> social problem for us as maintainers. I am not too happy about that.
> 
> This situation activates the kernel management style document so
> I advise involved parties to familiarize themselves with it:
> https://www.kernel.org/doc/html/latest/process/management-style.html
> 
> Can we get the discussion down to actual technical points?

+1

I can not agree more to this.

> We really need a virtio GPIO driver, no doubt, so if everyone could
> just work toward that goal and compromise with their specific pet
> peeves that would be great.

Enrico,

I am not looking to get any credits for the code or spec here. I don't
really care about that. For the very same reason I kept you as the
author of the 1st patch in the kernel series, so git keeps showing you
as the original author.

All I wanted to work on was the backend (in rust). This is what
happened for I2C for example, Jie Deng (Intel) worked on the spec and
Linux driver and I helped review it, make him fix a thing or two and
that's all. I worked on the rust implementation for the backend then.

You only ever sent 1 real versions of the Linux driver, that too
"6-months-back", there were no real blockers anywhere and you never
attempted to upstream anything.

Similarly, you "never" sent the specification properly to the virtio
lists for review. You sent it once as an attachment to an email, which
no one ever received.

When I tried to move this forward, invested a lot of time into making
it better from specification to code, reviews started happening, you
decided to start blocking it again.

You should be rather happy that something that you worked on is making
progress, specially when you didn't get time to do the same.

You wrote this in your patch:

> > Status:
> >     * this driver is now field tested for about 6 month
> >       (against KVM+Qemu as well as some HW/FPGA implementation)

Linux upstream doesn't really care about this, you can ask any Linux
Maintainer about this. If your code and specification isn't doing the
right thing, and isn't good enough, you will be asked to update it
upon reviews.

YOU JUST CAN'T SAY I WON'T because I have products based on this
version.

This is not how any open source project works. The code and
specification here doesn't belong to a single person or company. It is
for everyone's use.

> >     * virtio device ID officially allocated

Correct.

> >     * virtio spec has been submitted to virtio TC

Which specification are you talking about here ? The only
specification I can see on virtio lists is the one I sent.

And the driver you tried to send isn't aligned to that for sure, and
it takes us back from all the improvements I tried to do.

I am not saying that my version of the specification is the best and
there is no flaw in it. There surely is, but that can't be fixed by
sending another version of it. You need to make a technical point
about it and convince people that what you are saying is correct and
it is required for your use-case (not existing downstream solution).

There is no point going backwards now after making so much of
progress. Even if you try to send your version, it will slowly and
slowly reach very close to my latest version of code and spec. Because
your version of the code and spec weren't good enough for everyone. It
doesn't matter if you have real products on your earlier version, you
can keep using that in your downstream solution, but Linux kernel and
specification are going to get an improved version (from yours or
mine, but that doesn't matter here). You need to accept that changes
to that are inevitable since there are many users of gpio-virtio, not
just you and me, but many more (Like Bjorn expressed his interest in
this today for Qcom stuff).

Again, it would be better if you can discuss further on technical
merits or demerits in the currently circulated specification and give
your invaluable suggestions on the same.

Else we will end up spending few more months with just this and it
won't get us anywhere.

Thanks.

-- 
viresh

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16 11:49     ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-16 11:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Enrico Weigelt, metux IT consult, linux-kernel, Jonathan Corbet,
	Bartosz Golaszewski, Michael S. Tsirkin, Jason Wang, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	Vincent Guittot, Bill Mills, Alex Bennée

On 16-06-21, 10:31, Linus Walleij wrote:
> Hi Enrico,
> 
> On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
> 
> > Introduce new GPIO driver for virtual GPIO devices via virtio.
> >
> > The driver implements the virtio-gpio protocol (ID 41), which can be
> > used by either VM guests (e.g. bridging virtual gpios from the guest
> > to real gpios in the host or attaching simulators for automatic
> > application testing), as well as virtio-gpio hardware devices.
> >
> > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> 
> So now there are two contesting patches for this and that creates a
> social problem for us as maintainers. I am not too happy about that.
> 
> This situation activates the kernel management style document so
> I advise involved parties to familiarize themselves with it:
> https://www.kernel.org/doc/html/latest/process/management-style.html
> 
> Can we get the discussion down to actual technical points?

+1

I can not agree more to this.

> We really need a virtio GPIO driver, no doubt, so if everyone could
> just work toward that goal and compromise with their specific pet
> peeves that would be great.

Enrico,

I am not looking to get any credits for the code or spec here. I don't
really care about that. For the very same reason I kept you as the
author of the 1st patch in the kernel series, so git keeps showing you
as the original author.

All I wanted to work on was the backend (in rust). This is what
happened for I2C for example, Jie Deng (Intel) worked on the spec and
Linux driver and I helped review it, make him fix a thing or two and
that's all. I worked on the rust implementation for the backend then.

You only ever sent 1 real versions of the Linux driver, that too
"6-months-back", there were no real blockers anywhere and you never
attempted to upstream anything.

Similarly, you "never" sent the specification properly to the virtio
lists for review. You sent it once as an attachment to an email, which
no one ever received.

When I tried to move this forward, invested a lot of time into making
it better from specification to code, reviews started happening, you
decided to start blocking it again.

You should be rather happy that something that you worked on is making
progress, specially when you didn't get time to do the same.

You wrote this in your patch:

> > Status:
> >     * this driver is now field tested for about 6 month
> >       (against KVM+Qemu as well as some HW/FPGA implementation)

Linux upstream doesn't really care about this, you can ask any Linux
Maintainer about this. If your code and specification isn't doing the
right thing, and isn't good enough, you will be asked to update it
upon reviews.

YOU JUST CAN'T SAY I WON'T because I have products based on this
version.

This is not how any open source project works. The code and
specification here doesn't belong to a single person or company. It is
for everyone's use.

> >     * virtio device ID officially allocated

Correct.

> >     * virtio spec has been submitted to virtio TC

Which specification are you talking about here ? The only
specification I can see on virtio lists is the one I sent.

And the driver you tried to send isn't aligned to that for sure, and
it takes us back from all the improvements I tried to do.

I am not saying that my version of the specification is the best and
there is no flaw in it. There surely is, but that can't be fixed by
sending another version of it. You need to make a technical point
about it and convince people that what you are saying is correct and
it is required for your use-case (not existing downstream solution).

There is no point going backwards now after making so much of
progress. Even if you try to send your version, it will slowly and
slowly reach very close to my latest version of code and spec. Because
your version of the code and spec weren't good enough for everyone. It
doesn't matter if you have real products on your earlier version, you
can keep using that in your downstream solution, but Linux kernel and
specification are going to get an improved version (from yours or
mine, but that doesn't matter here). You need to accept that changes
to that are inevitable since there are many users of gpio-virtio, not
just you and me, but many more (Like Bjorn expressed his interest in
this today for Qcom stuff).

Again, it would be better if you can discuss further on technical
merits or demerits in the currently circulated specification and give
your invaluable suggestions on the same.

Else we will end up spending few more months with just this and it
won't get us anywhere.

Thanks.

-- 
viresh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16 11:49     ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-16 11:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Anton Vorontsov, Vincent Guittot, Kees Cook, Michael S. Tsirkin,
	Jonathan Corbet, Linux Doc Mailing List, linux-kernel,
	virtualization, Bartosz Golaszewski, Tony Luck,
	open list:GPIO SUBSYSTEM, Colin Cross, linux-riscv,
	Enrico Weigelt, metux IT consult, Bill Mills

On 16-06-21, 10:31, Linus Walleij wrote:
> Hi Enrico,
> 
> On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
> 
> > Introduce new GPIO driver for virtual GPIO devices via virtio.
> >
> > The driver implements the virtio-gpio protocol (ID 41), which can be
> > used by either VM guests (e.g. bridging virtual gpios from the guest
> > to real gpios in the host or attaching simulators for automatic
> > application testing), as well as virtio-gpio hardware devices.
> >
> > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> 
> So now there are two contesting patches for this and that creates a
> social problem for us as maintainers. I am not too happy about that.
> 
> This situation activates the kernel management style document so
> I advise involved parties to familiarize themselves with it:
> https://www.kernel.org/doc/html/latest/process/management-style.html
> 
> Can we get the discussion down to actual technical points?

+1

I can not agree more to this.

> We really need a virtio GPIO driver, no doubt, so if everyone could
> just work toward that goal and compromise with their specific pet
> peeves that would be great.

Enrico,

I am not looking to get any credits for the code or spec here. I don't
really care about that. For the very same reason I kept you as the
author of the 1st patch in the kernel series, so git keeps showing you
as the original author.

All I wanted to work on was the backend (in rust). This is what
happened for I2C for example, Jie Deng (Intel) worked on the spec and
Linux driver and I helped review it, make him fix a thing or two and
that's all. I worked on the rust implementation for the backend then.

You only ever sent 1 real versions of the Linux driver, that too
"6-months-back", there were no real blockers anywhere and you never
attempted to upstream anything.

Similarly, you "never" sent the specification properly to the virtio
lists for review. You sent it once as an attachment to an email, which
no one ever received.

When I tried to move this forward, invested a lot of time into making
it better from specification to code, reviews started happening, you
decided to start blocking it again.

You should be rather happy that something that you worked on is making
progress, specially when you didn't get time to do the same.

You wrote this in your patch:

> > Status:
> >     * this driver is now field tested for about 6 month
> >       (against KVM+Qemu as well as some HW/FPGA implementation)

Linux upstream doesn't really care about this, you can ask any Linux
Maintainer about this. If your code and specification isn't doing the
right thing, and isn't good enough, you will be asked to update it
upon reviews.

YOU JUST CAN'T SAY I WON'T because I have products based on this
version.

This is not how any open source project works. The code and
specification here doesn't belong to a single person or company. It is
for everyone's use.

> >     * virtio device ID officially allocated

Correct.

> >     * virtio spec has been submitted to virtio TC

Which specification are you talking about here ? The only
specification I can see on virtio lists is the one I sent.

And the driver you tried to send isn't aligned to that for sure, and
it takes us back from all the improvements I tried to do.

I am not saying that my version of the specification is the best and
there is no flaw in it. There surely is, but that can't be fixed by
sending another version of it. You need to make a technical point
about it and convince people that what you are saying is correct and
it is required for your use-case (not existing downstream solution).

There is no point going backwards now after making so much of
progress. Even if you try to send your version, it will slowly and
slowly reach very close to my latest version of code and spec. Because
your version of the code and spec weren't good enough for everyone. It
doesn't matter if you have real products on your earlier version, you
can keep using that in your downstream solution, but Linux kernel and
specification are going to get an improved version (from yours or
mine, but that doesn't matter here). You need to accept that changes
to that are inevitable since there are many users of gpio-virtio, not
just you and me, but many more (Like Bjorn expressed his interest in
this today for Qcom stuff).

Again, it would be better if you can discuss further on technical
merits or demerits in the currently circulated specification and give
your invaluable suggestions on the same.

Else we will end up spending few more months with just this and it
won't get us anywhere.

Thanks.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-16  8:31   ` Linus Walleij
@ 2021-06-16 14:41     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 14:41 UTC (permalink / raw)
  To: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv

On 16.06.21 10:31, Linus Walleij wrote:
 > Hi Enrico,

 > So now there are two contesting patches for this and that creates a
 > social problem for us as maintainers. I am not too happy about that.

note that this is a polished up of a repost of my original driver
from last year.

 > Can we get the discussion down to actual technical points?

Sure. Perhaps you recall or discussions from late 2020. The missing
point there was (besides a few wording issues) the missing formal
specification process w/ virtio TC. (spec was already included in this
driver as well as the corresponding qemu patches).

My spec was not just meant for VM applications but also actual silicon
(as already mentioned, some folks of my client also implemented it in
FPGAs - don't ask me about details, they just mentioned it was quite
easy for them).

This is why it is so trimmed on things like fixed packet size,
unidirectional queues, mirroring packets w/ thus a few bits changed,
etc. In constrast, a more network-like approach might have been looking
nicer to traditional computer programmers, but much more complex to do
in pure logic and eat up *lots of* more gates (think of actual memory
management instead of hardwired latches, more complex decoding, etc).

Meanwhile it played out working nicely in several HIL installations

If I wanted to have a simple and CPU-only approach (just for VMs), I
would have just mounted some sysfs pieces via 9P :p

Several weeks ago, Viresh just wanted to continue the missing pieces
(which was: tex'ifying the spec and submitting to virtio TC), but then
unfortunately he invented something entirely different also put my name
on it.

Easy to imagine that I'm not amused at all.


--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16 14:41     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 14:41 UTC (permalink / raw)
  To: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv

On 16.06.21 10:31, Linus Walleij wrote:
 > Hi Enrico,

 > So now there are two contesting patches for this and that creates a
 > social problem for us as maintainers. I am not too happy about that.

note that this is a polished up of a repost of my original driver
from last year.

 > Can we get the discussion down to actual technical points?

Sure. Perhaps you recall or discussions from late 2020. The missing
point there was (besides a few wording issues) the missing formal
specification process w/ virtio TC. (spec was already included in this
driver as well as the corresponding qemu patches).

My spec was not just meant for VM applications but also actual silicon
(as already mentioned, some folks of my client also implemented it in
FPGAs - don't ask me about details, they just mentioned it was quite
easy for them).

This is why it is so trimmed on things like fixed packet size,
unidirectional queues, mirroring packets w/ thus a few bits changed,
etc. In constrast, a more network-like approach might have been looking
nicer to traditional computer programmers, but much more complex to do
in pure logic and eat up *lots of* more gates (think of actual memory
management instead of hardwired latches, more complex decoding, etc).

Meanwhile it played out working nicely in several HIL installations

If I wanted to have a simple and CPU-only approach (just for VMs), I
would have just mounted some sysfs pieces via 9P :p

Several weeks ago, Viresh just wanted to continue the missing pieces
(which was: tex'ifying the spec and submitting to virtio TC), but then
unfortunately he invented something entirely different also put my name
on it.

Easy to imagine that I'm not amused at all.


--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-16 11:49     ` Viresh Kumar
@ 2021-06-16 15:04       ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 15:04 UTC (permalink / raw)
  To: Viresh Kumar, Linus Walleij
  Cc: Enrico Weigelt, metux IT consult, linux-kernel, Jonathan Corbet,
	Bartosz Golaszewski, Michael S. Tsirkin, Jason Wang, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	Vincent Guittot, Bill Mills, Alex Bennée

On 16.06.21 13:49, Viresh Kumar wrote:

> I am not looking to get any credits for the code or spec here. I don't
> really care about that. For the very same reason I kept you as the
> author of the 1st patch in the kernel series, so git keeps showing you
> as the original author.

Fine, and I'm not intenting to fight over credits. I'm just interested
in having a solid solution.

> All I wanted to work on was the backend (in rust). 

Okay, but why did you change the whole thing into something completely
different ?

> You only ever sent 1 real versions of the Linux driver, that too
> "6-months-back", there were no real blockers anywhere and you never
> attempted to upstream anything.

The backlog was in the maillist archive. It was just lingering because
of yet missing formal registering w/ vitio TC and I've been to busy with
more pressing tasks. (for example I've got keep public infrastructure
stuff up and running while folks are in lockdown).

> Similarly, you "never" sent the specification properly to the virtio
> lists for review. You sent it once as an attachment to an email, which
> no one ever received.

Half correct: I sent it to the list, but this wasn't tex'ified yet.

> When I tried to move this forward, invested a lot of time into making
> it better from specification to code, reviews started happening, you
> decided to start blocking it again.

When we had an email conversation about this, it was about submitting
the existing spec in a formal correct way. Don't get me wrong: I
apreciate that somebody's doing the beaurocratic work. But still have
no idea why you changed it completely, so there's quite nothing left
but the name and that it somehow does gpio via virtio.

> Linux upstream doesn't really care about this, you can ask any Linux
> Maintainer about this. 

I happen to be a maintainer myself, and we do actually care about
whether some thing is well tested.

> If your code and specification isn't doing the
> right thing, and isn't good enough, you will be asked to update it
> upon reviews.

Okay, please tell me, where exactly isn't right and why so.

> YOU JUST CAN'T SAY I WON'T because I have products based on this
> version.

Most of driver development goes pretty much like this. Yes, we would
prefer HW and FW vendors to talk with us first, but that rarely happens.

>>>      * virtio spec has been submitted to virtio TC
> 
> Which specification are you talking about here ? The only
> specification I can see on virtio lists is the one I sent.

The one I've resent (now texified) a few days ago. It had been submitted
in ascii form last year. The answer from virtio TC folks whas that there
are some formal steps to be done and it needs to be patched int their
tex document.

> And the driver you tried to send isn't aligned to that for sure, and
> it takes us back from all the improvements I tried to do.

Which improvements, exactly ? Unfortunately, you never talked to me
what exactly you've been missing.

I really have no idea, why you just

> I am not saying that my version of the specification is the best and
> there is no flaw in it. 

I did outline several problems of your spec on virtio list. Things that
already had already incorporated in my original one.

Really, I have no idea why you've never talked to me on specific issues,
but instead threw away, made something completely different and even
claim it would be just kinda "minor upgrade" of mine. WTF ?!

> There is no point going backwards now after making so much of
> progress. Even if you try to send your version, it will slowly and
> slowly reach very close to my latest version of code and spec. 

Or the other way round, as soon as silicon guys come in and see how
complicated and expensive your approach becomes.

> Because your version of the code and spec weren't good enough for everyone.

Again, please tell me what exactly was not "good enought" and who
exactly is "everyone".

> You need to accept that changes to that are inevitable since there 

You sound like a politician that tries to push an hidden agenda,
made by some secret interest group in the back room, against the
people - like "resistance is futile".


Finally, please tell me what exactly you're missing so hard in my spec,
that really needs a completely different implementation, which is hard
and expensive to implement in hardware.



--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-16 15:04       ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 15:04 UTC (permalink / raw)
  To: Viresh Kumar, Linus Walleij
  Cc: Enrico Weigelt, metux IT consult, linux-kernel, Jonathan Corbet,
	Bartosz Golaszewski, Michael S. Tsirkin, Jason Wang, Kees Cook,
	Anton Vorontsov, Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	Vincent Guittot, Bill Mills, Alex Bennée

On 16.06.21 13:49, Viresh Kumar wrote:

> I am not looking to get any credits for the code or spec here. I don't
> really care about that. For the very same reason I kept you as the
> author of the 1st patch in the kernel series, so git keeps showing you
> as the original author.

Fine, and I'm not intenting to fight over credits. I'm just interested
in having a solid solution.

> All I wanted to work on was the backend (in rust). 

Okay, but why did you change the whole thing into something completely
different ?

> You only ever sent 1 real versions of the Linux driver, that too
> "6-months-back", there were no real blockers anywhere and you never
> attempted to upstream anything.

The backlog was in the maillist archive. It was just lingering because
of yet missing formal registering w/ vitio TC and I've been to busy with
more pressing tasks. (for example I've got keep public infrastructure
stuff up and running while folks are in lockdown).

> Similarly, you "never" sent the specification properly to the virtio
> lists for review. You sent it once as an attachment to an email, which
> no one ever received.

Half correct: I sent it to the list, but this wasn't tex'ified yet.

> When I tried to move this forward, invested a lot of time into making
> it better from specification to code, reviews started happening, you
> decided to start blocking it again.

When we had an email conversation about this, it was about submitting
the existing spec in a formal correct way. Don't get me wrong: I
apreciate that somebody's doing the beaurocratic work. But still have
no idea why you changed it completely, so there's quite nothing left
but the name and that it somehow does gpio via virtio.

> Linux upstream doesn't really care about this, you can ask any Linux
> Maintainer about this. 

I happen to be a maintainer myself, and we do actually care about
whether some thing is well tested.

> If your code and specification isn't doing the
> right thing, and isn't good enough, you will be asked to update it
> upon reviews.

Okay, please tell me, where exactly isn't right and why so.

> YOU JUST CAN'T SAY I WON'T because I have products based on this
> version.

Most of driver development goes pretty much like this. Yes, we would
prefer HW and FW vendors to talk with us first, but that rarely happens.

>>>      * virtio spec has been submitted to virtio TC
> 
> Which specification are you talking about here ? The only
> specification I can see on virtio lists is the one I sent.

The one I've resent (now texified) a few days ago. It had been submitted
in ascii form last year. The answer from virtio TC folks whas that there
are some formal steps to be done and it needs to be patched int their
tex document.

> And the driver you tried to send isn't aligned to that for sure, and
> it takes us back from all the improvements I tried to do.

Which improvements, exactly ? Unfortunately, you never talked to me
what exactly you've been missing.

I really have no idea, why you just

> I am not saying that my version of the specification is the best and
> there is no flaw in it. 

I did outline several problems of your spec on virtio list. Things that
already had already incorporated in my original one.

Really, I have no idea why you've never talked to me on specific issues,
but instead threw away, made something completely different and even
claim it would be just kinda "minor upgrade" of mine. WTF ?!

> There is no point going backwards now after making so much of
> progress. Even if you try to send your version, it will slowly and
> slowly reach very close to my latest version of code and spec. 

Or the other way round, as soon as silicon guys come in and see how
complicated and expensive your approach becomes.

> Because your version of the code and spec weren't good enough for everyone.

Again, please tell me what exactly was not "good enought" and who
exactly is "everyone".

> You need to accept that changes to that are inevitable since there 

You sound like a politician that tries to push an hidden agenda,
made by some secret interest group in the back room, against the
people - like "resistance is futile".


Finally, please tell me what exactly you're missing so hard in my spec,
that really needs a completely different implementation, which is hard
and expensive to implement in hardware.



--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver]
  2021-06-16  8:31   ` Linus Walleij
  (?)
@ 2021-06-16 18:47     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 18:47 UTC (permalink / raw)
  To: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	virtio-dev

On 16.06.21 10:31, Linus Walleij wrote:

Hi folks,


<snip>

interesting: trying to post my tex'fied spec to virtio-dev and
virtio-comment quite some time now, but always being blocked. What's 
going on there ?


--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver]
@ 2021-06-16 18:47     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 18:47 UTC (permalink / raw)
  To: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	virtio-dev

On 16.06.21 10:31, Linus Walleij wrote:

Hi folks,


<snip>

interesting: trying to post my tex'fied spec to virtio-dev and
virtio-comment quite some time now, but always being blocked. What's 
going on there ?


--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [virtio-dev] banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver]
@ 2021-06-16 18:47     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-16 18:47 UTC (permalink / raw)
  To: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar
  Cc: linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	virtio-dev

On 16.06.21 10:31, Linus Walleij wrote:

Hi folks,


<snip>

interesting: trying to post my tex'fied spec to virtio-dev and
virtio-comment quite some time now, but always being blocked. What's 
going on there ?


--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

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [virtio-dev] banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver]
  2021-06-16 18:47     ` Enrico Weigelt, metux IT consult
@ 2021-06-16 21:18       ` Chet Ensign
  -1 siblings, 0 replies; 54+ messages in thread
From: Chet Ensign @ 2021-06-16 21:18 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: virtio-dev, Anton Vorontsov, Kees Cook, Jonathan Corbet,
	Viresh Kumar, Linus Walleij, Michael S. Tsirkin,
	Linux Doc Mailing List, linux-kernel, virtualization,
	Bartosz Golaszewski, Tony Luck, open list:GPIO SUBSYSTEM,
	Colin Cross, linux-riscv, Enrico Weigelt, metux IT consult


[-- Attachment #1.1: Type: text/plain, Size: 1514 bytes --]

Hi mtx -

I checked our logs and I don't see you as subscribed to either list. That
should explain why you're being blocked.

To subscribe to virtio-comment, follow the instructions at
https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio

To subscribe to virtio-dev, you can send a blank email to
virtio-dev-subscribe@lists.oasis-open.org

If those don't resolve the problem, let me know and I will investigate
further.

/chet

On Wed, Jun 16, 2021 at 2:47 PM Enrico Weigelt, metux IT consult <
lkml@metux.net> wrote:

> On 16.06.21 10:31, Linus Walleij wrote:
>
> Hi folks,
>
>
> <snip>
>
> interesting: trying to post my tex'fied spec to virtio-dev and
> virtio-comment quite some time now, but always being blocked. What's
> going on there ?
>
>
> --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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
>

-- 
Chet Ensign

Chief Technical Community Steward

OASIS Open

+1 201-341-1393 <+1+201-341-1393>
chet.ensign@oasis-open.org
www.oasis-open.org

[-- Attachment #1.2: Type: text/html, Size: 5116 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [virtio-dev] banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver]
@ 2021-06-16 21:18       ` Chet Ensign
  0 siblings, 0 replies; 54+ messages in thread
From: Chet Ensign @ 2021-06-16 21:18 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, Viresh Kumar,
	linux-kernel, Jonathan Corbet, Bartosz Golaszewski,
	Michael S. Tsirkin, Jason Wang, Kees Cook, Anton Vorontsov,
	Colin Cross, Tony Luck, Linux Doc Mailing List,
	open list:GPIO SUBSYSTEM, virtualization, linux-riscv,
	virtio-dev

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

Hi mtx -

I checked our logs and I don't see you as subscribed to either list. That
should explain why you're being blocked.

To subscribe to virtio-comment, follow the instructions at
https://www.oasis-open.org/committees/comments/index.php?wg_abbrev=virtio

To subscribe to virtio-dev, you can send a blank email to
virtio-dev-subscribe@lists.oasis-open.org

If those don't resolve the problem, let me know and I will investigate
further.

/chet

On Wed, Jun 16, 2021 at 2:47 PM Enrico Weigelt, metux IT consult <
lkml@metux.net> wrote:

> On 16.06.21 10:31, Linus Walleij wrote:
>
> Hi folks,
>
>
> <snip>
>
> interesting: trying to post my tex'fied spec to virtio-dev and
> virtio-comment quite some time now, but always being blocked. What's
> going on there ?
>
>
> --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
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
>

-- 
Chet Ensign

Chief Technical Community Steward

OASIS Open

+1 201-341-1393 <+1+201-341-1393>
chet.ensign@oasis-open.org
www.oasis-open.org

[-- Attachment #2: Type: text/html, Size: 5116 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-16 15:04       ` Enrico Weigelt, metux IT consult
  (?)
@ 2021-06-17  3:59         ` Viresh Kumar
  -1 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17  3:59 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 16-06-21, 17:04, Enrico Weigelt, metux IT consult wrote:
> Half correct: I sent it to the list, but this wasn't tex'ified yet.
> 
> When we had an email conversation about this, it was about submitting
> the existing spec in a formal correct way. Don't get me wrong: I
> apreciate that somebody's doing the beaurocratic work. But still have
> no idea why you changed it completely, so there's quite nothing left
> but the name and that it somehow does gpio via virtio.

> The one I've resent (now texified) a few days ago. It had been submitted
> in ascii form last year. The answer from virtio TC folks whas that there
> are some formal steps to be done and it needs to be patched int their
> tex document.

Okay, we figured out now that you _haven't_ subscribed to virtio lists
and so your stuff never landed in anyone's inbox. But you did send
something and didn't completely went away.

Since you started this all and still want to do it, I will take my
patches back and let you finish with what you started. I will help
review them.

Please start with specification first, and resend them as soon as
possible. So we can start with reviews there.

Also please cc relevant people directly, like GPIO maintainers in
kernel and few more from CC list of this email, as most of these
people aren't subscribed to virtio lists, they will never get your
patches otherwise. Lets get over this once and for all.

> You sound like a politician that tries to push an hidden agenda,
> made by some secret interest group in the back room, against the
> people - like "resistance is futile".

:)

-- 
viresh

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-17  3:59         ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17  3:59 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 16-06-21, 17:04, Enrico Weigelt, metux IT consult wrote:
> Half correct: I sent it to the list, but this wasn't tex'ified yet.
> 
> When we had an email conversation about this, it was about submitting
> the existing spec in a formal correct way. Don't get me wrong: I
> apreciate that somebody's doing the beaurocratic work. But still have
> no idea why you changed it completely, so there's quite nothing left
> but the name and that it somehow does gpio via virtio.

> The one I've resent (now texified) a few days ago. It had been submitted
> in ascii form last year. The answer from virtio TC folks whas that there
> are some formal steps to be done and it needs to be patched int their
> tex document.

Okay, we figured out now that you _haven't_ subscribed to virtio lists
and so your stuff never landed in anyone's inbox. But you did send
something and didn't completely went away.

Since you started this all and still want to do it, I will take my
patches back and let you finish with what you started. I will help
review them.

Please start with specification first, and resend them as soon as
possible. So we can start with reviews there.

Also please cc relevant people directly, like GPIO maintainers in
kernel and few more from CC list of this email, as most of these
people aren't subscribed to virtio lists, they will never get your
patches otherwise. Lets get over this once and for all.

> You sound like a politician that tries to push an hidden agenda,
> made by some secret interest group in the back room, against the
> people - like "resistance is futile".

:)

-- 
viresh

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-17  3:59         ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17  3:59 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Anton Vorontsov, Vincent Guittot, Kees Cook, Michael S. Tsirkin,
	Linus Walleij, Jonathan Corbet, Linux Doc Mailing List,
	linux-kernel, virtualization, Bartosz Golaszewski, Tony Luck,
	open list:GPIO SUBSYSTEM, Colin Cross, linux-riscv,
	Enrico Weigelt, metux IT consult, Bill Mills

On 16-06-21, 17:04, Enrico Weigelt, metux IT consult wrote:
> Half correct: I sent it to the list, but this wasn't tex'ified yet.
> 
> When we had an email conversation about this, it was about submitting
> the existing spec in a formal correct way. Don't get me wrong: I
> apreciate that somebody's doing the beaurocratic work. But still have
> no idea why you changed it completely, so there's quite nothing left
> but the name and that it somehow does gpio via virtio.

> The one I've resent (now texified) a few days ago. It had been submitted
> in ascii form last year. The answer from virtio TC folks whas that there
> are some formal steps to be done and it needs to be patched int their
> tex document.

Okay, we figured out now that you _haven't_ subscribed to virtio lists
and so your stuff never landed in anyone's inbox. But you did send
something and didn't completely went away.

Since you started this all and still want to do it, I will take my
patches back and let you finish with what you started. I will help
review them.

Please start with specification first, and resend them as soon as
possible. So we can start with reviews there.

Also please cc relevant people directly, like GPIO maintainers in
kernel and few more from CC list of this email, as most of these
people aren't subscribed to virtio lists, they will never get your
patches otherwise. Lets get over this once and for all.

> You sound like a politician that tries to push an hidden agenda,
> made by some secret interest group in the back room, against the
> people - like "resistance is futile".

:)

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-17  3:59         ` Viresh Kumar
@ 2021-06-17  9:54           ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-17  9:54 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 17.06.21 05:59, Viresh Kumar wrote:

 > Okay, we figured out now that you _haven't_ subscribed to virtio lists
 > and so your stuff never landed in anyone's inbox. But you did send
 > something and didn't completely went away.

Actually, I am subscribed in the list. We already had debates on it,
including on your postings (but also other things). And the ascii
version of the spec actually landed on the list last year, we had
discussions about it there.

I've just had the problem that my patches didn't go through, which is
very strange, since I actually am on the list and other mails of mine
went through all the time. I'm now suspecting it's triggered by some
subtle difference between my regular mail clients and git send-email.

 > Since you started this all and still want to do it, I will take my
 > patches back and let you finish with what you started. I will help
 > review them.

Thank you very much.

Please don't me wrong, I really don't wanna any kind of power play, just
wanna get an technically good solution. If there had been any mis-
understandings at that point, I'm officially saying sorry here.

Let's be friends.

You mentioned you've been missing with my spec. Please come foreward and
tell us what exactly you're missing and what your use cases are.

Note that I've intentionally left off certain "more sophisticated"
functionality we find on *some* gpio controllers, eg. per-line irq
masking, pinmux settings for several reasons, e.g.:

* those are only implemented by some hardware
* often implemented in or at least need to be coordinated with other
   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
   device)
* it shall be possible to support even the most simple devices and
   have the more sophisticated things totally optional. minium
   requirements for silicon implementations should be the lowest possible
   (IOW: minimal number of logic gates)

 >> You sound like a politician that tries to push an hidden agenda,
 >> made by some secret interest group in the back room, against the
 >> people - like "resistance is futile".
 >
 > :)

Perhaps I've been a bit overreacting at that point. But: this is really
that kind of talking we hear from politicians and corporate leaders
since many years, whenever they wanna push something through that we the
people don't want. Politicians use that as a social engineering tool for
demotivating any resistance. Over heare in Germany this even had become
a meme, and folks from CCC made a radio show about and named by that
(the German word is "alternativlos" - in english: without any
alternative). No idea about other countries, maybe it's a cultural
issue, but over here, those kind of talking had become a red light.

Of course, I never intended to accuse you of being one of these people.
Sorry if there's been misunderstanding.


Let's get back to your implementation: you've mentioned you're routing
raw virtio traffic into userland, to some other process (outside VMMs
like qemu) - how exactly are you doing that ?

That could be interesting for completely different scenarios. For
example, I'm currently exploring how to get VirGL running between 
separate processes running under the same kernel instance (fow now we
only have the driver side inside VM and the device outside it), means
driver and device are running as separate processes.

The primary use case are containers that shall have really GPU generic
drivers, not knowing anything about the actual hardware on the host.
Currently, container workloads wanting to use a GPU need to have special
drivers for exactly the HW the host happens to have. This makes generic,
portable container images a tuff problem.

I haven't digged deeply into the matter, but some virtio-tap transport
could be an relatively easy (probably not the most efficient) way to
solve this problem. In that scanario it would like this:

* we have a "virgl server" (could be some X or wayland application, or
   completely own compositor) opens up the device-end of an "virtio-tap"
   transport and attaches its virtio-gpio device emulation on it.
* "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
   instance on this (also leading to a new DRI device)
* container runtime picks the new DRI device and maps it into the
   container(s)
   [ yet open question, whether one DRI device for many containers
     is enough ]
* container application sees that virtio-gpu DRI device and speaks to
   it (mesa->virgl backend)
* the "virgl-server" receives buffers and commands from via virtio and
   sends them to the host's GL or Gallium API.

Once we're already there, we might think whether it could make sense
putting virtio routing into kvm itself, instead of letting qemu catch
page faults and virtual irqs. Yet have to see whether that's a good
idea, but I can imagine some performance improvements here.



--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-17  9:54           ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2021-06-17  9:54 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 17.06.21 05:59, Viresh Kumar wrote:

 > Okay, we figured out now that you _haven't_ subscribed to virtio lists
 > and so your stuff never landed in anyone's inbox. But you did send
 > something and didn't completely went away.

Actually, I am subscribed in the list. We already had debates on it,
including on your postings (but also other things). And the ascii
version of the spec actually landed on the list last year, we had
discussions about it there.

I've just had the problem that my patches didn't go through, which is
very strange, since I actually am on the list and other mails of mine
went through all the time. I'm now suspecting it's triggered by some
subtle difference between my regular mail clients and git send-email.

 > Since you started this all and still want to do it, I will take my
 > patches back and let you finish with what you started. I will help
 > review them.

Thank you very much.

Please don't me wrong, I really don't wanna any kind of power play, just
wanna get an technically good solution. If there had been any mis-
understandings at that point, I'm officially saying sorry here.

Let's be friends.

You mentioned you've been missing with my spec. Please come foreward and
tell us what exactly you're missing and what your use cases are.

Note that I've intentionally left off certain "more sophisticated"
functionality we find on *some* gpio controllers, eg. per-line irq
masking, pinmux settings for several reasons, e.g.:

* those are only implemented by some hardware
* often implemented in or at least need to be coordinated with other
   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
   device)
* it shall be possible to support even the most simple devices and
   have the more sophisticated things totally optional. minium
   requirements for silicon implementations should be the lowest possible
   (IOW: minimal number of logic gates)

 >> You sound like a politician that tries to push an hidden agenda,
 >> made by some secret interest group in the back room, against the
 >> people - like "resistance is futile".
 >
 > :)

Perhaps I've been a bit overreacting at that point. But: this is really
that kind of talking we hear from politicians and corporate leaders
since many years, whenever they wanna push something through that we the
people don't want. Politicians use that as a social engineering tool for
demotivating any resistance. Over heare in Germany this even had become
a meme, and folks from CCC made a radio show about and named by that
(the German word is "alternativlos" - in english: without any
alternative). No idea about other countries, maybe it's a cultural
issue, but over here, those kind of talking had become a red light.

Of course, I never intended to accuse you of being one of these people.
Sorry if there's been misunderstanding.


Let's get back to your implementation: you've mentioned you're routing
raw virtio traffic into userland, to some other process (outside VMMs
like qemu) - how exactly are you doing that ?

That could be interesting for completely different scenarios. For
example, I'm currently exploring how to get VirGL running between 
separate processes running under the same kernel instance (fow now we
only have the driver side inside VM and the device outside it), means
driver and device are running as separate processes.

The primary use case are containers that shall have really GPU generic
drivers, not knowing anything about the actual hardware on the host.
Currently, container workloads wanting to use a GPU need to have special
drivers for exactly the HW the host happens to have. This makes generic,
portable container images a tuff problem.

I haven't digged deeply into the matter, but some virtio-tap transport
could be an relatively easy (probably not the most efficient) way to
solve this problem. In that scanario it would like this:

* we have a "virgl server" (could be some X or wayland application, or
   completely own compositor) opens up the device-end of an "virtio-tap"
   transport and attaches its virtio-gpio device emulation on it.
* "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
   instance on this (also leading to a new DRI device)
* container runtime picks the new DRI device and maps it into the
   container(s)
   [ yet open question, whether one DRI device for many containers
     is enough ]
* container application sees that virtio-gpu DRI device and speaks to
   it (mesa->virgl backend)
* the "virgl-server" receives buffers and commands from via virtio and
   sends them to the host's GL or Gallium API.

Once we're already there, we might think whether it could make sense
putting virtio routing into kvm itself, instead of letting qemu catch
page faults and virtual irqs. Yet have to see whether that's a good
idea, but I can imagine some performance improvements here.



--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2021-06-17  9:54           ` Enrico Weigelt, metux IT consult
  (?)
@ 2021-06-17 11:47             ` Viresh Kumar
  -1 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17 11:47 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 17-06-21, 11:54, Enrico Weigelt, metux IT consult wrote:
> Actually, I am subscribed in the list. We already had debates on it,
> including on your postings (but also other things).

Right.

> And the ascii
> version of the spec actually landed on the list last year, we had
> discussions about it there.

I tried to search for it earlier, but never found anything on virtio
list.  Maybe I missed it then.

> I've just had the problem that my patches didn't go through, which is
> very strange, since I actually am on the list and other mails of mine
> went through all the time. I'm now suspecting it's triggered by some
> subtle difference between my regular mail clients and git send-email.
> 
> > Since you started this all and still want to do it, I will take my
> > patches back and let you finish with what you started. I will help
> > review them.
> 
> Thank you very much.
> 
> Please don't me wrong, I really don't wanna any kind of power play, just
> wanna get an technically good solution. If there had been any mis-
> understandings at that point, I'm officially saying sorry here.

Its okay, we are both trying to make things better here :)

> Let's be friends.
> 
> You mentioned you've been missing with my spec. Please come foreward and
> tell us what exactly you're missing and what your use cases are.

I have sent a detailed review of your spec patch, lets do it there
point by point :)

> Note that I've intentionally left off certain "more sophisticated"
> functionality we find on *some* gpio controllers, eg. per-line irq
> masking, pinmux settings for several reasons, e.g.:
> 
> * those are only implemented by some hardware
> * often implemented in or at least need to be coordinated with other
>   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
>   device)
> * it shall be possible to support even the most simple devices and
>   have the more sophisticated things totally optional. minium
>   requirements for silicon implementations should be the lowest possible
>   (IOW: minimal number of logic gates)
> 
> >> You sound like a politician that tries to push an hidden agenda,
> >> made by some secret interest group in the back room, against the
> >> people - like "resistance is futile".
> >
> > :)
> 
> Perhaps I've been a bit overreacting at that point. But: this is really
> that kind of talking we hear from politicians and corporate leaders
> since many years, whenever they wanna push something through that we the
> people don't want. Politicians use that as a social engineering tool for
> demotivating any resistance. Over heare in Germany this even had become
> a meme, and folks from CCC made a radio show about and named by that
> (the German word is "alternativlos" - in english: without any
> alternative). No idea about other countries, maybe it's a cultural
> issue, but over here, those kind of talking had become a red light.
> 
> Of course, I never intended to accuse you of being one of these people.
> Sorry if there's been misunderstanding.

It sounded strange yesterday to be honest, but I have gone past it
already :)
 
> Let's get back to your implementation: you've mentioned you're routing
> raw virtio traffic into userland, to some other process (outside VMMs
> like qemu) - how exactly are you doing that ?
> 
> That could be interesting for completely different scenarios. For
> example, I'm currently exploring how to get VirGL running between separate
> processes running under the same kernel instance (fow now we
> only have the driver side inside VM and the device outside it), means
> driver and device are running as separate processes.
> 
> The primary use case are containers that shall have really GPU generic
> drivers, not knowing anything about the actual hardware on the host.
> Currently, container workloads wanting to use a GPU need to have special
> drivers for exactly the HW the host happens to have. This makes generic,
> portable container images a tuff problem.
> 
> I haven't digged deeply into the matter, but some virtio-tap transport
> could be an relatively easy (probably not the most efficient) way to
> solve this problem. In that scanario it would like this:
> 
> * we have a "virgl server" (could be some X or wayland application, or
>   completely own compositor) opens up the device-end of an "virtio-tap"
>   transport and attaches its virtio-gpio device emulation on it.
> * "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
>   instance on this (also leading to a new DRI device)
> * container runtime picks the new DRI device and maps it into the
>   container(s)
>   [ yet open question, whether one DRI device for many containers
>     is enough ]
> * container application sees that virtio-gpu DRI device and speaks to
>   it (mesa->virgl backend)
> * the "virgl-server" receives buffers and commands from via virtio and
>   sends them to the host's GL or Gallium API.
> 
> Once we're already there, we might think whether it could make sense
> putting virtio routing into kvm itself, instead of letting qemu catch
> page faults and virtual irqs. Yet have to see whether that's a good
> idea, but I can imagine some performance improvements here.

We (at Linaro) work on software enablement normally and not end
products (rarely that happen though), like framework level work in the
kernel which can later be used by everyone to build their drivers on.

There are many companies like Qualcomm, ST Micro, etc, who want to use
Virtio in general for Automotive or other applications / solution. The
purpose of Project Stratos [1], an initiative of Linaro, is working
towards developing hypervisor agnostic Virtio interfaces and
standards. The end products and applications will be worked on by the
members directly and we need to add basic minimum support, with all
the generally required APIs or interfaces.

-- 
viresh

[1] https://linaro.atlassian.net/wiki/spaces/STR/overview

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-17 11:47             ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17 11:47 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Linus Walleij, Enrico Weigelt, metux IT consult, linux-kernel,
	Jonathan Corbet, Bartosz Golaszewski, Michael S. Tsirkin,
	Jason Wang, Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	Linux Doc Mailing List, open list:GPIO SUBSYSTEM, virtualization,
	linux-riscv, Vincent Guittot, Bill Mills, Alex Bennée

On 17-06-21, 11:54, Enrico Weigelt, metux IT consult wrote:
> Actually, I am subscribed in the list. We already had debates on it,
> including on your postings (but also other things).

Right.

> And the ascii
> version of the spec actually landed on the list last year, we had
> discussions about it there.

I tried to search for it earlier, but never found anything on virtio
list.  Maybe I missed it then.

> I've just had the problem that my patches didn't go through, which is
> very strange, since I actually am on the list and other mails of mine
> went through all the time. I'm now suspecting it's triggered by some
> subtle difference between my regular mail clients and git send-email.
> 
> > Since you started this all and still want to do it, I will take my
> > patches back and let you finish with what you started. I will help
> > review them.
> 
> Thank you very much.
> 
> Please don't me wrong, I really don't wanna any kind of power play, just
> wanna get an technically good solution. If there had been any mis-
> understandings at that point, I'm officially saying sorry here.

Its okay, we are both trying to make things better here :)

> Let's be friends.
> 
> You mentioned you've been missing with my spec. Please come foreward and
> tell us what exactly you're missing and what your use cases are.

I have sent a detailed review of your spec patch, lets do it there
point by point :)

> Note that I've intentionally left off certain "more sophisticated"
> functionality we find on *some* gpio controllers, eg. per-line irq
> masking, pinmux settings for several reasons, e.g.:
> 
> * those are only implemented by some hardware
> * often implemented in or at least need to be coordinated with other
>   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
>   device)
> * it shall be possible to support even the most simple devices and
>   have the more sophisticated things totally optional. minium
>   requirements for silicon implementations should be the lowest possible
>   (IOW: minimal number of logic gates)
> 
> >> You sound like a politician that tries to push an hidden agenda,
> >> made by some secret interest group in the back room, against the
> >> people - like "resistance is futile".
> >
> > :)
> 
> Perhaps I've been a bit overreacting at that point. But: this is really
> that kind of talking we hear from politicians and corporate leaders
> since many years, whenever they wanna push something through that we the
> people don't want. Politicians use that as a social engineering tool for
> demotivating any resistance. Over heare in Germany this even had become
> a meme, and folks from CCC made a radio show about and named by that
> (the German word is "alternativlos" - in english: without any
> alternative). No idea about other countries, maybe it's a cultural
> issue, but over here, those kind of talking had become a red light.
> 
> Of course, I never intended to accuse you of being one of these people.
> Sorry if there's been misunderstanding.

It sounded strange yesterday to be honest, but I have gone past it
already :)
 
> Let's get back to your implementation: you've mentioned you're routing
> raw virtio traffic into userland, to some other process (outside VMMs
> like qemu) - how exactly are you doing that ?
> 
> That could be interesting for completely different scenarios. For
> example, I'm currently exploring how to get VirGL running between separate
> processes running under the same kernel instance (fow now we
> only have the driver side inside VM and the device outside it), means
> driver and device are running as separate processes.
> 
> The primary use case are containers that shall have really GPU generic
> drivers, not knowing anything about the actual hardware on the host.
> Currently, container workloads wanting to use a GPU need to have special
> drivers for exactly the HW the host happens to have. This makes generic,
> portable container images a tuff problem.
> 
> I haven't digged deeply into the matter, but some virtio-tap transport
> could be an relatively easy (probably not the most efficient) way to
> solve this problem. In that scanario it would like this:
> 
> * we have a "virgl server" (could be some X or wayland application, or
>   completely own compositor) opens up the device-end of an "virtio-tap"
>   transport and attaches its virtio-gpio device emulation on it.
> * "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
>   instance on this (also leading to a new DRI device)
> * container runtime picks the new DRI device and maps it into the
>   container(s)
>   [ yet open question, whether one DRI device for many containers
>     is enough ]
> * container application sees that virtio-gpu DRI device and speaks to
>   it (mesa->virgl backend)
> * the "virgl-server" receives buffers and commands from via virtio and
>   sends them to the host's GL or Gallium API.
> 
> Once we're already there, we might think whether it could make sense
> putting virtio routing into kvm itself, instead of letting qemu catch
> page faults and virtual irqs. Yet have to see whether that's a good
> idea, but I can imagine some performance improvements here.

We (at Linaro) work on software enablement normally and not end
products (rarely that happen though), like framework level work in the
kernel which can later be used by everyone to build their drivers on.

There are many companies like Qualcomm, ST Micro, etc, who want to use
Virtio in general for Automotive or other applications / solution. The
purpose of Project Stratos [1], an initiative of Linaro, is working
towards developing hypervisor agnostic Virtio interfaces and
standards. The end products and applications will be worked on by the
members directly and we need to add basic minimum support, with all
the generally required APIs or interfaces.

-- 
viresh

[1] https://linaro.atlassian.net/wiki/spaces/STR/overview

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2021-06-17 11:47             ` Viresh Kumar
  0 siblings, 0 replies; 54+ messages in thread
From: Viresh Kumar @ 2021-06-17 11:47 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Anton Vorontsov, Vincent Guittot, Kees Cook, Michael S. Tsirkin,
	Linus Walleij, Jonathan Corbet, Linux Doc Mailing List,
	linux-kernel, virtualization, Bartosz Golaszewski, Tony Luck,
	open list:GPIO SUBSYSTEM, Colin Cross, linux-riscv,
	Enrico Weigelt, metux IT consult, Bill Mills

On 17-06-21, 11:54, Enrico Weigelt, metux IT consult wrote:
> Actually, I am subscribed in the list. We already had debates on it,
> including on your postings (but also other things).

Right.

> And the ascii
> version of the spec actually landed on the list last year, we had
> discussions about it there.

I tried to search for it earlier, but never found anything on virtio
list.  Maybe I missed it then.

> I've just had the problem that my patches didn't go through, which is
> very strange, since I actually am on the list and other mails of mine
> went through all the time. I'm now suspecting it's triggered by some
> subtle difference between my regular mail clients and git send-email.
> 
> > Since you started this all and still want to do it, I will take my
> > patches back and let you finish with what you started. I will help
> > review them.
> 
> Thank you very much.
> 
> Please don't me wrong, I really don't wanna any kind of power play, just
> wanna get an technically good solution. If there had been any mis-
> understandings at that point, I'm officially saying sorry here.

Its okay, we are both trying to make things better here :)

> Let's be friends.
> 
> You mentioned you've been missing with my spec. Please come foreward and
> tell us what exactly you're missing and what your use cases are.

I have sent a detailed review of your spec patch, lets do it there
point by point :)

> Note that I've intentionally left off certain "more sophisticated"
> functionality we find on *some* gpio controllers, eg. per-line irq
> masking, pinmux settings for several reasons, e.g.:
> 
> * those are only implemented by some hardware
> * often implemented in or at least need to be coordinated with other
>   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
>   device)
> * it shall be possible to support even the most simple devices and
>   have the more sophisticated things totally optional. minium
>   requirements for silicon implementations should be the lowest possible
>   (IOW: minimal number of logic gates)
> 
> >> You sound like a politician that tries to push an hidden agenda,
> >> made by some secret interest group in the back room, against the
> >> people - like "resistance is futile".
> >
> > :)
> 
> Perhaps I've been a bit overreacting at that point. But: this is really
> that kind of talking we hear from politicians and corporate leaders
> since many years, whenever they wanna push something through that we the
> people don't want. Politicians use that as a social engineering tool for
> demotivating any resistance. Over heare in Germany this even had become
> a meme, and folks from CCC made a radio show about and named by that
> (the German word is "alternativlos" - in english: without any
> alternative). No idea about other countries, maybe it's a cultural
> issue, but over here, those kind of talking had become a red light.
> 
> Of course, I never intended to accuse you of being one of these people.
> Sorry if there's been misunderstanding.

It sounded strange yesterday to be honest, but I have gone past it
already :)
 
> Let's get back to your implementation: you've mentioned you're routing
> raw virtio traffic into userland, to some other process (outside VMMs
> like qemu) - how exactly are you doing that ?
> 
> That could be interesting for completely different scenarios. For
> example, I'm currently exploring how to get VirGL running between separate
> processes running under the same kernel instance (fow now we
> only have the driver side inside VM and the device outside it), means
> driver and device are running as separate processes.
> 
> The primary use case are containers that shall have really GPU generic
> drivers, not knowing anything about the actual hardware on the host.
> Currently, container workloads wanting to use a GPU need to have special
> drivers for exactly the HW the host happens to have. This makes generic,
> portable container images a tuff problem.
> 
> I haven't digged deeply into the matter, but some virtio-tap transport
> could be an relatively easy (probably not the most efficient) way to
> solve this problem. In that scanario it would like this:
> 
> * we have a "virgl server" (could be some X or wayland application, or
>   completely own compositor) opens up the device-end of an "virtio-tap"
>   transport and attaches its virtio-gpio device emulation on it.
> * "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
>   instance on this (also leading to a new DRI device)
> * container runtime picks the new DRI device and maps it into the
>   container(s)
>   [ yet open question, whether one DRI device for many containers
>     is enough ]
> * container application sees that virtio-gpu DRI device and speaks to
>   it (mesa->virgl backend)
> * the "virgl-server" receives buffers and commands from via virtio and
>   sends them to the host's GL or Gallium API.
> 
> Once we're already there, we might think whether it could make sense
> putting virtio routing into kvm itself, instead of letting qemu catch
> page faults and virtual irqs. Yet have to see whether that's a good
> idea, but I can imagine some performance improvements here.

We (at Linaro) work on software enablement normally and not end
products (rarely that happen though), like framework level work in the
kernel which can later be used by everyone to build their drivers on.

There are many companies like Qualcomm, ST Micro, etc, who want to use
Virtio in general for Automotive or other applications / solution. The
purpose of Project Stratos [1], an initiative of Linaro, is working
towards developing hypervisor agnostic Virtio interfaces and
standards. The end products and applications will be worked on by the
members directly and we need to add basic minimum support, with all
the generally required APIs or interfaces.

-- 
viresh

[1] https://linaro.atlassian.net/wiki/spaces/STR/overview
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-12-04  8:28         ` Enrico Weigelt, metux IT consult
@ 2020-12-04  9:06           ` Michael Walle
  -1 siblings, 0 replies; 54+ messages in thread
From: Michael Walle @ 2020-12-04  9:06 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Enrico Weigelt, metux IT consult, Bartosz Golaszewski,
	Michael S. Tsirkin, LKML, Linus Walleij, Jason Wang, linux-gpio,
	virtualization, linux-riscv

Am 2020-12-04 09:28, schrieb Enrico Weigelt, metux IT consult:
> On 03.12.20 23:35, Michael Walle wrote:
>> Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
>>> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>>>> +               bufwalk = name_buffer;
>>>>> +
>>>>> +               while (idx < priv->num_gpios &&
>>>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>>>> +                       gpio_names[idx] = (strlen(bufwalk) ? 
>>>>> bufwalk
>>>>> : NULL);
>>>>> +                       bufwalk += strlen(bufwalk)+1;
>>>>> +                       idx++;
>>>> 
>>>> 
>>>> Something's wrong with indentation here.
>>> 
>>> i dont think so: the "bufwalk ..." line belongs to the while 
>>> expression
>>> and is right under the "idx", as it should be. I didn't want to break 
>>> up
>>> at the "<" operator. shall i do this instead ?
>> 
>> Or don't break the lines at all. Both lines don't add up to more than
>> 100 chars,
>> right?
> 
> IIRC checkpatch complains at 80 chars. Has that been changed ?

yes,
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144

-michael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-04  9:06           ` Michael Walle
  0 siblings, 0 replies; 54+ messages in thread
From: Michael Walle @ 2020-12-04  9:06 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Michael S. Tsirkin, Jason Wang, LKML, virtualization,
	Bartosz Golaszewski, Enrico Weigelt, metux IT consult,
	linux-gpio, linux-riscv, Linus Walleij

Am 2020-12-04 09:28, schrieb Enrico Weigelt, metux IT consult:
> On 03.12.20 23:35, Michael Walle wrote:
>> Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
>>> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>>>> +               bufwalk = name_buffer;
>>>>> +
>>>>> +               while (idx < priv->num_gpios &&
>>>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>>>> +                       gpio_names[idx] = (strlen(bufwalk) ? 
>>>>> bufwalk
>>>>> : NULL);
>>>>> +                       bufwalk += strlen(bufwalk)+1;
>>>>> +                       idx++;
>>>> 
>>>> 
>>>> Something's wrong with indentation here.
>>> 
>>> i dont think so: the "bufwalk ..." line belongs to the while 
>>> expression
>>> and is right under the "idx", as it should be. I didn't want to break 
>>> up
>>> at the "<" operator. shall i do this instead ?
>> 
>> Or don't break the lines at all. Both lines don't add up to more than
>> 100 chars,
>> right?
> 
> IIRC checkpatch complains at 80 chars. Has that been changed ?

yes,
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144

-michael

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-12-03 22:35       ` Michael Walle
@ 2020-12-04  8:28         ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-04  8:28 UTC (permalink / raw)
  To: Michael Walle, Enrico Weigelt, metux IT consult
  Cc: Bartosz Golaszewski, Michael S. Tsirkin, LKML, Linus Walleij,
	Jason Wang, linux-gpio, virtualization, linux-riscv

On 03.12.20 23:35, Michael Walle wrote:
> Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
>> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>>> +               bufwalk = name_buffer;
>>>> +
>>>> +               while (idx < priv->num_gpios &&
>>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk
>>>> : NULL);
>>>> +                       bufwalk += strlen(bufwalk)+1;
>>>> +                       idx++;
>>>
>>>
>>> Something's wrong with indentation here.
>>
>> i dont think so: the "bufwalk ..." line belongs to the while expression
>> and is right under the "idx", as it should be. I didn't want to break up
>> at the "<" operator. shall i do this instead ?
> 
> Or don't break the lines at all. Both lines don't add up to more than
> 100 chars,
> right?

IIRC checkpatch complains at 80 chars. Has that been changed ?


--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-04  8:28         ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-04  8:28 UTC (permalink / raw)
  To: Michael Walle, Enrico Weigelt, metux IT consult
  Cc: linux-gpio, Jason Wang, Michael S. Tsirkin, LKML, virtualization,
	Bartosz Golaszewski, linux-riscv, Linus Walleij

On 03.12.20 23:35, Michael Walle wrote:
> Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
>> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>>> +               bufwalk = name_buffer;
>>>> +
>>>> +               while (idx < priv->num_gpios &&
>>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk
>>>> : NULL);
>>>> +                       bufwalk += strlen(bufwalk)+1;
>>>> +                       idx++;
>>>
>>>
>>> Something's wrong with indentation here.
>>
>> i dont think so: the "bufwalk ..." line belongs to the while expression
>> and is right under the "idx", as it should be. I didn't want to break up
>> at the "<" operator. shall i do this instead ?
> 
> Or don't break the lines at all. Both lines don't add up to more than
> 100 chars,
> right?

IIRC checkpatch complains at 80 chars. Has that been changed ?


--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-12-03 19:00     ` Enrico Weigelt, metux IT consult
@ 2020-12-03 22:35       ` Michael Walle
  -1 siblings, 0 replies; 54+ messages in thread
From: Michael Walle @ 2020-12-03 22:35 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Bartosz Golaszewski, Enrico Weigelt, metux IT consult,
	Michael S. Tsirkin, LKML, Linus Walleij, Jason Wang, linux-gpio,
	virtualization, linux-riscv

Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>> +               bufwalk = name_buffer;
>>> +
>>> +               while (idx < priv->num_gpios &&
>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk 
>>> : NULL);
>>> +                       bufwalk += strlen(bufwalk)+1;
>>> +                       idx++;
>> 
>> 
>> Something's wrong with indentation here.
> 
> i dont think so: the "bufwalk ..." line belongs to the while expression
> and is right under the "idx", as it should be. I didn't want to break 
> up
> at the "<" operator. shall i do this instead ?

Or don't break the lines at all. Both lines don't add up to more than 
100 chars,
right?

-michael

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-03 22:35       ` Michael Walle
  0 siblings, 0 replies; 54+ messages in thread
From: Michael Walle @ 2020-12-03 22:35 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Michael S. Tsirkin, Jason Wang, LKML, virtualization,
	Bartosz Golaszewski, linux-gpio, linux-riscv, Enrico Weigelt,
	metux IT consult, Linus Walleij

Am 2020-12-03 20:00, schrieb Enrico Weigelt, metux IT consult:
> On 02.12.20 15:15, Bartosz Golaszewski wrote:
>>> +               bufwalk = name_buffer;
>>> +
>>> +               while (idx < priv->num_gpios &&
>>> +                      bufwalk < (name_buffer+cf.names_size)) {
>>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk 
>>> : NULL);
>>> +                       bufwalk += strlen(bufwalk)+1;
>>> +                       idx++;
>> 
>> 
>> Something's wrong with indentation here.
> 
> i dont think so: the "bufwalk ..." line belongs to the while expression
> and is right under the "idx", as it should be. I didn't want to break 
> up
> at the "<" operator. shall i do this instead ?

Or don't break the lines at all. Both lines don't add up to more than 
100 chars,
right?

-michael

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-29 22:10   ` Jonathan Neuschäfer
@ 2020-12-03 19:12     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:12 UTC (permalink / raw)
  To: Jonathan Neuschäfer, Enrico Weigelt, metux IT consult
  Cc: linux-kernel, linux-gpio, linus.walleij, mst, virtualization,
	bgolaszewski, linux-riscv, jasowang

On 29.11.20 23:10, Jonathan Neuschäfer wrote:
> Hi,
>
> On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT
consult wrote:
>> Introducing new gpio driver for virtual GPIO devices via virtio.
>>
>> The driver allows routing gpio control into VM guests, eg. brigding
>> virtual gpios to specific host gpios, or attaching simulators for
>> automatic application testing.
>>
>> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
>
> is there a spec of the virtio-gpio protocol available? If so, linking it
> in the commit message and/or driver/Kconfig would be nice.


added it to Documentation/gpio/ in patch version 2.

--mtx

>
>
> Best regards,
> Jonathan Neuschäfer
>
-- 
---
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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-03 19:12     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:12 UTC (permalink / raw)
  To: Jonathan Neuschäfer, Enrico Weigelt, metux IT consult
  Cc: bgolaszewski, linus.walleij, mst, linux-kernel, virtualization,
	linux-gpio, linux-riscv, jasowang

On 29.11.20 23:10, Jonathan Neuschäfer wrote:
> Hi,
>
> On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT
consult wrote:
>> Introducing new gpio driver for virtual GPIO devices via virtio.
>>
>> The driver allows routing gpio control into VM guests, eg. brigding
>> virtual gpios to specific host gpios, or attaching simulators for
>> automatic application testing.
>>
>> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
>
> is there a spec of the virtio-gpio protocol available? If so, linking it
> in the commit message and/or driver/Kconfig would be nice.


added it to Documentation/gpio/ in patch version 2.

--mtx

>
>
> Best regards,
> Jonathan Neuschäfer
>
-- 
---
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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:45   ` Randy Dunlap
@ 2020-12-03 19:01     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:01 UTC (permalink / raw)
  To: Randy Dunlap, Enrico Weigelt, metux IT consult, linux-kernel
  Cc: linus.walleij, bgolaszewski, mst, jasowang, linux-gpio,
	virtualization, linux-riscv

On 27.11.20 19:45, Randy Dunlap wrote:
> On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 5d4de5cd6759..e8414d82cf75 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>>  	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>>  	  it.
>>  
>> +config GPIO_VIRTIO
>> +	tristate "VirtIO GPIO support"
>> +	depends on VIRTIO
>> +	help
>> +	  Say Y here to enable guest support for virtio based GPIOs.
> 
> 	                                         virtio-based
> 
>> +
>> +	  These virtual gpios can be routed to real GPIOs or attached to
> 
> 	                GPIOs

thx. fixed in v2.


> 
>> +	  simulators on the host (qemu).
>> +
>>  endif
> 
> 

-- 
---
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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-03 19:01     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:01 UTC (permalink / raw)
  To: Randy Dunlap, Enrico Weigelt, metux IT consult, linux-kernel
  Cc: linux-gpio, linus.walleij, mst, virtualization, bgolaszewski,
	linux-riscv, jasowang

On 27.11.20 19:45, Randy Dunlap wrote:
> On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 5d4de5cd6759..e8414d82cf75 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>>  	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>>  	  it.
>>  
>> +config GPIO_VIRTIO
>> +	tristate "VirtIO GPIO support"
>> +	depends on VIRTIO
>> +	help
>> +	  Say Y here to enable guest support for virtio based GPIOs.
> 
> 	                                         virtio-based
> 
>> +
>> +	  These virtual gpios can be routed to real GPIOs or attached to
> 
> 	                GPIOs

thx. fixed in v2.


> 
>> +	  simulators on the host (qemu).
>> +
>>  endif
> 
> 

-- 
---
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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-12-02 14:15   ` Bartosz Golaszewski
@ 2020-12-03 19:00     ` Enrico Weigelt, metux IT consult
  -1 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:00 UTC (permalink / raw)
  To: Bartosz Golaszewski, Enrico Weigelt, metux IT consult,
	Michael S. Tsirkin
  Cc: LKML, Linus Walleij, Jason Wang, linux-gpio, virtualization, linux-riscv

On 02.12.20 15:15, Bartosz Golaszewski wrote:

Hi,

<snip>

> With a third entry (after gpio-mockup and gpio-aggregator) I think
> these deserve a separate submenu for "virtual GPIO drivers" or
> something like that.

just sent a separate patch.

> 
> Please keep headers ordered alphabetically.

fixed in v2.

> Don't use tabs here - not only doesn't it improve readability but
> you're not even consistent.

fixed in v2.

> 
>> +
>> +static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
>> +{
>> +       struct scatterlist rcv_sg;
>> +
>> +       sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
>> +       virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
>> +                           GFP_KERNEL);
>> +       virtqueue_kick(priv->vq_rx);
>> +}
>> +
>> +static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
>> +                           int pin, int value, struct virtio_gpio_event *ev)
>> +{
>> +       struct scatterlist sg[1];
>> +       int ret;
>> +       unsigned long flags;
>> +
>> +       WARN_ON(!ev);
>> +
>> +       ev->type = type;
>> +       ev->pin = pin;
>> +       ev->value = value;
>> +
>> +       sg_init_table(sg, 1);
>> +       sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
>> +
>> +       spin_lock_irqsave(&priv->vq_lock, flags);
>> +       ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
>> +                                  priv, GFP_KERNEL);
>> +       if (ret < 0) {
>> +               dev_err(&priv->vdev->dev,
>> +                       "virtqueue_add_outbuf() failed: %d\n", ret);
>> +               goto out;
>> +       }
>> +       virtqueue_kick(priv->vq_tx);
>> +
>> +out:
>> +       spin_unlock_irqrestore(&priv->vq_lock, flags);
>> +       return 0;
>> +}
>> +
>> +static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       set_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static inline int check_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       return test_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static inline void clear_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       clear_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
>> +                          int pin, int value)
>> +{
>> +       struct virtio_gpio_event *ev
>> +               = devm_kzalloc(&priv->vdev->dev,
>> +                              sizeof(struct virtio_gpio_event), GFP_KERNEL);
> 
> Just move the allocation to a separate line because this is super ugly.

done.

> 
>> +
>> +       if (!ev)
>> +               return -ENOMEM;
>> +
>> +       clear_event(priv, type);
>> +       virtio_gpio_xmit(priv, type, pin, value, ev);
>> +       wait_event_interruptible(priv->waitq, check_event(priv, type));
>> +
>> +       devm_kfree(&priv->vdev->dev, ev);
> 
> In fact you don't need the managed variant if you're freeing it in the
> same function. Managed resources should live as long as a device is
> attached to a driver.

thx, this was a left from originally global buffer.

>> +static void virtio_gpio_data_rx(struct virtqueue *vq)
>> +{
>> +       struct virtio_gpio_priv *priv = vq->vdev->priv;
>> +       void *data;
>> +       unsigned int len;
>> +       struct virtio_gpio_event *ev;
>> +
>> +       data = virtqueue_get_buf(priv->vq_rx, &len);
>> +       if (!data || !len) {
>> +               dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
>> +               return;
>> +       }
>> +
>> +       ev = data;
>> +       WARN_ON(data != &priv->rcv_buf);
> 
> Is it fine to proceed if this is the case?

Good question :o

Actually, it should never happen - if it does, we might have a bug in
either this driver or even virtio subsystem. But still it *should*
return a valid buffer (unless virtio is broken)

>> +
>> +       memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
>> +
>> +       switch (ev->type) {
>> +       case VIRTIO_GPIO_EV_HOST_LEVEL:
>> +               virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
>> +       break;
> 
> Break should be one tab farther.

fixed in v2

>> +       default:
>> +               wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
>> +       break;
>> +       }
>> +       virtio_gpio_prepare_inbuf(priv);
>> +       wake_up_all(&priv->waitq);
>> +}
>> +
>> +static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
>> +{
>> +       struct virtqueue *vqs[2];
>> +       vq_callback_t *cbs[] = {
>> +               NULL,
>> +               virtio_gpio_data_rx,
>> +       };
>> +       static const char * const names[] = { "in", "out", };
>> +       int ret;
>> +
>> +       ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
>> +       if (ret) {
>> +               dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       priv->vq_rx = vqs[0];
>> +       priv->vq_tx = vqs[1];
>> +
>> +       virtio_gpio_prepare_inbuf(priv);
>> +
>> +       virtio_config_enable(priv->vdev);
>> +       virtqueue_enable_cb(priv->vq_rx);
>> +       virtio_device_ready(priv->vdev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int virtio_gpio_probe(struct virtio_device *vdev)
>> +{
>> +       struct virtio_gpio_priv *priv;
>> +       struct virtio_gpio_config cf = {};
>> +       char *name_buffer;
>> +       const char **gpio_names = NULL;
>> +       struct device *dev = &vdev->dev;
>> +
>> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +       if (!priv)
>> +               return -ENOMEM;
>> +
>> +       priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
> 
> This can fail.

ups!

>> +       spin_lock_init(&priv->vq_lock);
>> +       spin_lock_init(&priv->op_lock);
>> +
>> +       virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
>> +       virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
>> +                    &cf.num_gpios);
>> +       virtio_cread(vdev, struct virtio_gpio_config, names_size,
>> +                    &cf.names_size);
>> +       virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
>> +                          priv->name, sizeof(cf.name));
>> +
>> +       if (cf.version != 1) {
>> +               dev_err(dev, "unsupported interface version %d\n", cf.version);
>> +               return -EINVAL;
>> +       }
>> +
>> +       priv->num_gpios = cf.num_gpios;
>> +
>> +       if (cf.names_size) {
>> +               char *bufwalk;
>> +               int idx = 0;
>> +
>> +               name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
>> +                                          GFP_KERNEL)+1;
>> +               virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
>> +                                  name_buffer, cf.names_size);
>> +               name_buffer[cf.names_size] = 0;
>> +
>> +               gpio_names = devm_kzalloc(dev,
>> +                                         sizeof(char *) * priv->num_gpios,
>> +                                         GFP_KERNEL);
> 
> Use devm_kcalloc() for arrays.

ok.

> 
>> +               bufwalk = name_buffer;
>> +
>> +               while (idx < priv->num_gpios &&
>> +                      bufwalk < (name_buffer+cf.names_size)) {
>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
>> +                       bufwalk += strlen(bufwalk)+1;
>> +                       idx++;
> 
> 
> Something's wrong with indentation here.

i dont think so: the "bufwalk ..." line belongs to the while expression
and is right under the "idx", as it should be. I didn't want to break up
at the "<" operator. shall i do this instead ?

>> +               }
>> +       }
>> +
>> +       priv->gc.owner                  = THIS_MODULE;
>> +       priv->gc.parent                 = dev;
>> +       priv->gc.label                  = (priv->name[0] ? priv->name
>> +                                                        : dev_name(dev));
>> +       priv->gc.ngpio                  = priv->num_gpios;
>> +       priv->gc.names                  = gpio_names;
>> +       priv->gc.base                   = -1;
>> +       priv->gc.request                = virtio_gpio_request;
>> +       priv->gc.direction_input        = virtio_gpio_direction_input;
>> +       priv->gc.direction_output       = virtio_gpio_direction_output;
>> +       priv->gc.get_direction          = virtio_gpio_get_direction;
>> +       priv->gc.get                    = virtio_gpio_get;
>> +       priv->gc.set                    = virtio_gpio_set;
>> +
>> +       priv->vdev                      = vdev;
>> +       vdev->priv = priv;
>> +
>> +       priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
>> +                                             NUMA_NO_NODE);
>> +       if (priv->irq_base < 0) {
>> +               dev_err(&vdev->dev, "failed to alloc irqs\n");
>> +               priv->irq_base = -1;
>> +               return -ENOMEM;
>> +       }
>> +
>> +       init_waitqueue_head(&priv->waitq);
>> +
>> +       priv->reply_wait = 0;
>> +
>> +       virtio_gpio_alloc_vq(priv);
>> +
>> +       return devm_gpiochip_add_data(dev, &priv->gc, priv);
>> +}
> 
> I don't know virtio at all - Michael: could you take a look at the
> code here and see if it looks good to you?
> 
>> +
>> +static void virtio_gpio_remove(struct virtio_device *vdev)
>> +{
>> +}
> 
> Just don't implement it. Or does virtio actually require the remove() callback?

yes, it does, unfortunately -- see: virtio_dev_remove()

I'll propose a patch for virtio for fixing that.


--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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-03 19:00     ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-12-03 19:00 UTC (permalink / raw)
  To: Bartosz Golaszewski, Enrico Weigelt, metux IT consult,
	Michael S. Tsirkin
  Cc: Jason Wang, LKML, virtualization, linux-gpio, linux-riscv, Linus Walleij

On 02.12.20 15:15, Bartosz Golaszewski wrote:

Hi,

<snip>

> With a third entry (after gpio-mockup and gpio-aggregator) I think
> these deserve a separate submenu for "virtual GPIO drivers" or
> something like that.

just sent a separate patch.

> 
> Please keep headers ordered alphabetically.

fixed in v2.

> Don't use tabs here - not only doesn't it improve readability but
> you're not even consistent.

fixed in v2.

> 
>> +
>> +static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
>> +{
>> +       struct scatterlist rcv_sg;
>> +
>> +       sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
>> +       virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
>> +                           GFP_KERNEL);
>> +       virtqueue_kick(priv->vq_rx);
>> +}
>> +
>> +static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
>> +                           int pin, int value, struct virtio_gpio_event *ev)
>> +{
>> +       struct scatterlist sg[1];
>> +       int ret;
>> +       unsigned long flags;
>> +
>> +       WARN_ON(!ev);
>> +
>> +       ev->type = type;
>> +       ev->pin = pin;
>> +       ev->value = value;
>> +
>> +       sg_init_table(sg, 1);
>> +       sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
>> +
>> +       spin_lock_irqsave(&priv->vq_lock, flags);
>> +       ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
>> +                                  priv, GFP_KERNEL);
>> +       if (ret < 0) {
>> +               dev_err(&priv->vdev->dev,
>> +                       "virtqueue_add_outbuf() failed: %d\n", ret);
>> +               goto out;
>> +       }
>> +       virtqueue_kick(priv->vq_tx);
>> +
>> +out:
>> +       spin_unlock_irqrestore(&priv->vq_lock, flags);
>> +       return 0;
>> +}
>> +
>> +static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       set_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static inline int check_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       return test_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static inline void clear_event(struct virtio_gpio_priv *priv, int id)
>> +{
>> +       clear_bit(id, &priv->reply_wait);
>> +}
>> +
>> +static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
>> +                          int pin, int value)
>> +{
>> +       struct virtio_gpio_event *ev
>> +               = devm_kzalloc(&priv->vdev->dev,
>> +                              sizeof(struct virtio_gpio_event), GFP_KERNEL);
> 
> Just move the allocation to a separate line because this is super ugly.

done.

> 
>> +
>> +       if (!ev)
>> +               return -ENOMEM;
>> +
>> +       clear_event(priv, type);
>> +       virtio_gpio_xmit(priv, type, pin, value, ev);
>> +       wait_event_interruptible(priv->waitq, check_event(priv, type));
>> +
>> +       devm_kfree(&priv->vdev->dev, ev);
> 
> In fact you don't need the managed variant if you're freeing it in the
> same function. Managed resources should live as long as a device is
> attached to a driver.

thx, this was a left from originally global buffer.

>> +static void virtio_gpio_data_rx(struct virtqueue *vq)
>> +{
>> +       struct virtio_gpio_priv *priv = vq->vdev->priv;
>> +       void *data;
>> +       unsigned int len;
>> +       struct virtio_gpio_event *ev;
>> +
>> +       data = virtqueue_get_buf(priv->vq_rx, &len);
>> +       if (!data || !len) {
>> +               dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
>> +               return;
>> +       }
>> +
>> +       ev = data;
>> +       WARN_ON(data != &priv->rcv_buf);
> 
> Is it fine to proceed if this is the case?

Good question :o

Actually, it should never happen - if it does, we might have a bug in
either this driver or even virtio subsystem. But still it *should*
return a valid buffer (unless virtio is broken)

>> +
>> +       memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
>> +
>> +       switch (ev->type) {
>> +       case VIRTIO_GPIO_EV_HOST_LEVEL:
>> +               virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
>> +       break;
> 
> Break should be one tab farther.

fixed in v2

>> +       default:
>> +               wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
>> +       break;
>> +       }
>> +       virtio_gpio_prepare_inbuf(priv);
>> +       wake_up_all(&priv->waitq);
>> +}
>> +
>> +static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
>> +{
>> +       struct virtqueue *vqs[2];
>> +       vq_callback_t *cbs[] = {
>> +               NULL,
>> +               virtio_gpio_data_rx,
>> +       };
>> +       static const char * const names[] = { "in", "out", };
>> +       int ret;
>> +
>> +       ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
>> +       if (ret) {
>> +               dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
>> +               return ret;
>> +       }
>> +
>> +       priv->vq_rx = vqs[0];
>> +       priv->vq_tx = vqs[1];
>> +
>> +       virtio_gpio_prepare_inbuf(priv);
>> +
>> +       virtio_config_enable(priv->vdev);
>> +       virtqueue_enable_cb(priv->vq_rx);
>> +       virtio_device_ready(priv->vdev);
>> +
>> +       return 0;
>> +}
>> +
>> +static int virtio_gpio_probe(struct virtio_device *vdev)
>> +{
>> +       struct virtio_gpio_priv *priv;
>> +       struct virtio_gpio_config cf = {};
>> +       char *name_buffer;
>> +       const char **gpio_names = NULL;
>> +       struct device *dev = &vdev->dev;
>> +
>> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +       if (!priv)
>> +               return -ENOMEM;
>> +
>> +       priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
> 
> This can fail.

ups!

>> +       spin_lock_init(&priv->vq_lock);
>> +       spin_lock_init(&priv->op_lock);
>> +
>> +       virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
>> +       virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
>> +                    &cf.num_gpios);
>> +       virtio_cread(vdev, struct virtio_gpio_config, names_size,
>> +                    &cf.names_size);
>> +       virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
>> +                          priv->name, sizeof(cf.name));
>> +
>> +       if (cf.version != 1) {
>> +               dev_err(dev, "unsupported interface version %d\n", cf.version);
>> +               return -EINVAL;
>> +       }
>> +
>> +       priv->num_gpios = cf.num_gpios;
>> +
>> +       if (cf.names_size) {
>> +               char *bufwalk;
>> +               int idx = 0;
>> +
>> +               name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
>> +                                          GFP_KERNEL)+1;
>> +               virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
>> +                                  name_buffer, cf.names_size);
>> +               name_buffer[cf.names_size] = 0;
>> +
>> +               gpio_names = devm_kzalloc(dev,
>> +                                         sizeof(char *) * priv->num_gpios,
>> +                                         GFP_KERNEL);
> 
> Use devm_kcalloc() for arrays.

ok.

> 
>> +               bufwalk = name_buffer;
>> +
>> +               while (idx < priv->num_gpios &&
>> +                      bufwalk < (name_buffer+cf.names_size)) {
>> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
>> +                       bufwalk += strlen(bufwalk)+1;
>> +                       idx++;
> 
> 
> Something's wrong with indentation here.

i dont think so: the "bufwalk ..." line belongs to the while expression
and is right under the "idx", as it should be. I didn't want to break up
at the "<" operator. shall i do this instead ?

>> +               }
>> +       }
>> +
>> +       priv->gc.owner                  = THIS_MODULE;
>> +       priv->gc.parent                 = dev;
>> +       priv->gc.label                  = (priv->name[0] ? priv->name
>> +                                                        : dev_name(dev));
>> +       priv->gc.ngpio                  = priv->num_gpios;
>> +       priv->gc.names                  = gpio_names;
>> +       priv->gc.base                   = -1;
>> +       priv->gc.request                = virtio_gpio_request;
>> +       priv->gc.direction_input        = virtio_gpio_direction_input;
>> +       priv->gc.direction_output       = virtio_gpio_direction_output;
>> +       priv->gc.get_direction          = virtio_gpio_get_direction;
>> +       priv->gc.get                    = virtio_gpio_get;
>> +       priv->gc.set                    = virtio_gpio_set;
>> +
>> +       priv->vdev                      = vdev;
>> +       vdev->priv = priv;
>> +
>> +       priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
>> +                                             NUMA_NO_NODE);
>> +       if (priv->irq_base < 0) {
>> +               dev_err(&vdev->dev, "failed to alloc irqs\n");
>> +               priv->irq_base = -1;
>> +               return -ENOMEM;
>> +       }
>> +
>> +       init_waitqueue_head(&priv->waitq);
>> +
>> +       priv->reply_wait = 0;
>> +
>> +       virtio_gpio_alloc_vq(priv);
>> +
>> +       return devm_gpiochip_add_data(dev, &priv->gc, priv);
>> +}
> 
> I don't know virtio at all - Michael: could you take a look at the
> code here and see if it looks good to you?
> 
>> +
>> +static void virtio_gpio_remove(struct virtio_device *vdev)
>> +{
>> +}
> 
> Just don't implement it. Or does virtio actually require the remove() callback?

yes, it does, unfortunately -- see: virtio_dev_remove()

I'll propose a patch for virtio for fixing that.


--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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
@ 2020-12-02 14:15   ` Bartosz Golaszewski
  -1 siblings, 0 replies; 54+ messages in thread
From: Bartosz Golaszewski @ 2020-12-02 14:15 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Michael S. Tsirkin
  Cc: LKML, Linus Walleij, Jason Wang, linux-gpio, virtualization, linux-riscv

On Fri, Nov 27, 2020 at 7:30 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Introducing new gpio driver for virtual GPIO devices via virtio.
>
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  MAINTAINERS                      |   6 +
>  drivers/gpio/Kconfig             |   9 ++
>  drivers/gpio/Makefile            |   1 +
>  drivers/gpio/gpio-virtio.c       | 332 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/virtio_gpio.h |  39 +++++
>  include/uapi/linux/virtio_ids.h  |   1 +
>  6 files changed, 388 insertions(+)
>  create mode 100644 drivers/gpio/gpio-virtio.c
>  create mode 100644 include/uapi/linux/virtio_gpio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a008b70f3c16..dfb8bfe09bbe 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18588,6 +18588,12 @@ F:     Documentation/filesystems/virtiofs.rst
>  F:     fs/fuse/virtio_fs.c
>  F:     include/uapi/linux/virtio_fs.h
>
> +VIRTIO GPIO DRIVER
> +M:     Enrico Weigelt, metux IT consult <info@metux.net>
> +S:     Maintained
> +F:     drivers/gpio/gpio-virtio.c
> +F:     include/uapi/linux/virtio_gpio.h
> +
>  VIRTIO GPU DRIVER
>  M:     David Airlie <airlied@linux.ie>
>  M:     Gerd Hoffmann <kraxel@redhat.com>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>           tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>           it.
>
> +config GPIO_VIRTIO
> +       tristate "VirtIO GPIO support"
> +       depends on VIRTIO
> +       help
> +         Say Y here to enable guest support for virtio based GPIOs.
> +
> +         These virtual gpios can be routed to real GPIOs or attached to
> +         simulators on the host (qemu).
> +

With a third entry (after gpio-mockup and gpio-aggregator) I think
these deserve a separate submenu for "virtual GPIO drivers" or
something like that.

>  endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 09dada80ac34..2b12e75af123 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_GPIO_TWL4030)          += gpio-twl4030.o
>  obj-$(CONFIG_GPIO_TWL6040)             += gpio-twl6040.o
>  obj-$(CONFIG_GPIO_UCB1400)             += gpio-ucb1400.o
>  obj-$(CONFIG_GPIO_UNIPHIER)            += gpio-uniphier.o
> +obj-$(CONFIG_GPIO_VIRTIO)              += gpio-virtio.o
>  obj-$(CONFIG_GPIO_VF610)               += gpio-vf610.o
>  obj-$(CONFIG_GPIO_VIPERBOARD)          += gpio-viperboard.o
>  obj-$(CONFIG_GPIO_VR41XX)              += gpio-vr41xx.o
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> new file mode 100644
> index 000000000000..61e183cc26bf
> --- /dev/null
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -0,0 +1,332 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * GPIO driver for virtio-based virtual GPIOs
> + *
> + * Copyright (C) 2018 metux IT consult
> + * Author: Enrico Weigelt, metux IT consult <info@metux.net>
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/spinlock.h>
> +#include <linux/virtio_config.h>
> +#include <uapi/linux/virtio_ids.h>
> +#include <uapi/linux/virtio_gpio.h>
> +

Please keep headers ordered alphabetically.

> +struct virtio_gpio_priv {
> +       struct gpio_chip                gc;
> +       spinlock_t                      vq_lock;
> +       spinlock_t                      op_lock;
> +       struct virtio_device            *vdev;
> +       int                             num_gpios;
> +       char                            *name;
> +       struct virtqueue                *vq_rx;
> +       struct virtqueue                *vq_tx;
> +       struct virtio_gpio_event        rcv_buf;
> +       struct virtio_gpio_event        last;
> +       int                             irq_base;
> +       wait_queue_head_t               waitq;
> +
> +       unsigned long reply_wait;
> +};

Don't use tabs here - not only doesn't it improve readability but
you're not even consistent.

> +
> +static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
> +{
> +       struct scatterlist rcv_sg;
> +
> +       sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
> +       virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
> +                           GFP_KERNEL);
> +       virtqueue_kick(priv->vq_rx);
> +}
> +
> +static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
> +                           int pin, int value, struct virtio_gpio_event *ev)
> +{
> +       struct scatterlist sg[1];
> +       int ret;
> +       unsigned long flags;
> +
> +       WARN_ON(!ev);
> +
> +       ev->type = type;
> +       ev->pin = pin;
> +       ev->value = value;
> +
> +       sg_init_table(sg, 1);
> +       sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
> +
> +       spin_lock_irqsave(&priv->vq_lock, flags);
> +       ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
> +                                  priv, GFP_KERNEL);
> +       if (ret < 0) {
> +               dev_err(&priv->vdev->dev,
> +                       "virtqueue_add_outbuf() failed: %d\n", ret);
> +               goto out;
> +       }
> +       virtqueue_kick(priv->vq_tx);
> +
> +out:
> +       spin_unlock_irqrestore(&priv->vq_lock, flags);
> +       return 0;
> +}
> +
> +static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       set_bit(id, &priv->reply_wait);
> +}
> +
> +static inline int check_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       return test_bit(id, &priv->reply_wait);
> +}
> +
> +static inline void clear_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       clear_bit(id, &priv->reply_wait);
> +}
> +
> +static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
> +                          int pin, int value)
> +{
> +       struct virtio_gpio_event *ev
> +               = devm_kzalloc(&priv->vdev->dev,
> +                              sizeof(struct virtio_gpio_event), GFP_KERNEL);

Just move the allocation to a separate line because this is super ugly.

> +
> +       if (!ev)
> +               return -ENOMEM;
> +
> +       clear_event(priv, type);
> +       virtio_gpio_xmit(priv, type, pin, value, ev);
> +       wait_event_interruptible(priv->waitq, check_event(priv, type));
> +
> +       devm_kfree(&priv->vdev->dev, ev);

In fact you don't need the managed variant if you're freeing it in the
same function. Managed resources should live as long as a device is
attached to a driver.

> +
> +       return priv->last.value;
> +}
> +
> +static int virtio_gpio_direction_input(struct gpio_chip *gc,
> +                                      unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT,
> +                              pin, 0);
> +}
> +
> +static int virtio_gpio_direction_output(struct gpio_chip *gc,
> +                                       unsigned int pin, int value)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT,
> +                              pin, value);
> +}
> +
> +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_GET_DIRECTION,
> +                              pin, 0);
> +}
> +
> +static void virtio_gpio_set(struct gpio_chip *gc,
> +                           unsigned int pin, int value)
> +{
> +       virtio_gpio_req(gpiochip_get_data(gc),
> +                       VIRTIO_GPIO_EV_GUEST_SET_VALUE, pin, value);
> +}
> +
> +static int virtio_gpio_get(struct gpio_chip *gc,
> +                          unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_GET_VALUE, pin, 0);
> +}
> +
> +static int virtio_gpio_request(struct gpio_chip *gc,
> +                              unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_REQUEST, pin, 0);
> +}
> +
> +static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
> +                             int pin, int value)
> +{
> +       if (pin < priv->num_gpios)
> +               generic_handle_irq(priv->irq_base + pin);
> +}
> +
> +static void virtio_gpio_data_rx(struct virtqueue *vq)
> +{
> +       struct virtio_gpio_priv *priv = vq->vdev->priv;
> +       void *data;
> +       unsigned int len;
> +       struct virtio_gpio_event *ev;
> +
> +       data = virtqueue_get_buf(priv->vq_rx, &len);
> +       if (!data || !len) {
> +               dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
> +               return;
> +       }
> +
> +       ev = data;
> +       WARN_ON(data != &priv->rcv_buf);

Is it fine to proceed if this is the case?

> +
> +       memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
> +
> +       switch (ev->type) {
> +       case VIRTIO_GPIO_EV_HOST_LEVEL:
> +               virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
> +       break;

Break should be one tab farther.

> +       default:
> +               wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
> +       break;
> +       }
> +       virtio_gpio_prepare_inbuf(priv);
> +       wake_up_all(&priv->waitq);
> +}
> +
> +static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
> +{
> +       struct virtqueue *vqs[2];
> +       vq_callback_t *cbs[] = {
> +               NULL,
> +               virtio_gpio_data_rx,
> +       };
> +       static const char * const names[] = { "in", "out", };
> +       int ret;
> +
> +       ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
> +       if (ret) {
> +               dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
> +               return ret;
> +       }
> +
> +       priv->vq_rx = vqs[0];
> +       priv->vq_tx = vqs[1];
> +
> +       virtio_gpio_prepare_inbuf(priv);
> +
> +       virtio_config_enable(priv->vdev);
> +       virtqueue_enable_cb(priv->vq_rx);
> +       virtio_device_ready(priv->vdev);
> +
> +       return 0;
> +}
> +
> +static int virtio_gpio_probe(struct virtio_device *vdev)
> +{
> +       struct virtio_gpio_priv *priv;
> +       struct virtio_gpio_config cf = {};
> +       char *name_buffer;
> +       const char **gpio_names = NULL;
> +       struct device *dev = &vdev->dev;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);

This can fail.

> +
> +       spin_lock_init(&priv->vq_lock);
> +       spin_lock_init(&priv->op_lock);
> +
> +       virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
> +       virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
> +                    &cf.num_gpios);
> +       virtio_cread(vdev, struct virtio_gpio_config, names_size,
> +                    &cf.names_size);
> +       virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
> +                          priv->name, sizeof(cf.name));
> +
> +       if (cf.version != 1) {
> +               dev_err(dev, "unsupported interface version %d\n", cf.version);
> +               return -EINVAL;
> +       }
> +
> +       priv->num_gpios = cf.num_gpios;
> +
> +       if (cf.names_size) {
> +               char *bufwalk;
> +               int idx = 0;
> +
> +               name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
> +                                          GFP_KERNEL)+1;
> +               virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
> +                                  name_buffer, cf.names_size);
> +               name_buffer[cf.names_size] = 0;
> +
> +               gpio_names = devm_kzalloc(dev,
> +                                         sizeof(char *) * priv->num_gpios,
> +                                         GFP_KERNEL);

Use devm_kcalloc() for arrays.

> +               bufwalk = name_buffer;
> +
> +               while (idx < priv->num_gpios &&
> +                      bufwalk < (name_buffer+cf.names_size)) {
> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
> +                       bufwalk += strlen(bufwalk)+1;
> +                       idx++;


Something's wrong with indentation here.

> +               }
> +       }
> +
> +       priv->gc.owner                  = THIS_MODULE;
> +       priv->gc.parent                 = dev;
> +       priv->gc.label                  = (priv->name[0] ? priv->name
> +                                                        : dev_name(dev));
> +       priv->gc.ngpio                  = priv->num_gpios;
> +       priv->gc.names                  = gpio_names;
> +       priv->gc.base                   = -1;
> +       priv->gc.request                = virtio_gpio_request;
> +       priv->gc.direction_input        = virtio_gpio_direction_input;
> +       priv->gc.direction_output       = virtio_gpio_direction_output;
> +       priv->gc.get_direction          = virtio_gpio_get_direction;
> +       priv->gc.get                    = virtio_gpio_get;
> +       priv->gc.set                    = virtio_gpio_set;
> +
> +       priv->vdev                      = vdev;
> +       vdev->priv = priv;
> +
> +       priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
> +                                             NUMA_NO_NODE);
> +       if (priv->irq_base < 0) {
> +               dev_err(&vdev->dev, "failed to alloc irqs\n");
> +               priv->irq_base = -1;
> +               return -ENOMEM;
> +       }
> +
> +       init_waitqueue_head(&priv->waitq);
> +
> +       priv->reply_wait = 0;
> +
> +       virtio_gpio_alloc_vq(priv);
> +
> +       return devm_gpiochip_add_data(dev, &priv->gc, priv);
> +}

I don't know virtio at all - Michael: could you take a look at the
code here and see if it looks good to you?

> +
> +static void virtio_gpio_remove(struct virtio_device *vdev)
> +{
> +}

Just don't implement it. Or does virtio actually require the remove() callback?

Bartosz

> +
> +static const struct virtio_device_id id_table[] = {
> +       { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
> +       { 0 },
> +};
> +
> +static struct virtio_driver virtio_gpio_driver = {
> +       .driver.name    = KBUILD_MODNAME,
> +       .driver.owner   = THIS_MODULE,
> +       .id_table       = id_table,
> +       .probe          = virtio_gpio_probe,
> +       .remove         = virtio_gpio_remove,
> +};
> +
> +module_virtio_driver(virtio_gpio_driver);
> +
> +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
> +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..5a48b975bc7a
> --- /dev/null
> +++ b/include/uapi/linux/virtio_gpio.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _LINUX_VIRTIO_GPIO_H
> +#define _LINUX_VIRTIO_GPIO_H
> +
> +#include <linux/types.h>
> +
> +enum virtio_gpio_event_type {
> +       // requests from quest to host
> +       VIRTIO_GPIO_EV_GUEST_REQUEST            = 0x01, // ->request()
> +       VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT    = 0x02, // ->direction_input()
> +       VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT   = 0x03, // ->direction_output()
> +       VIRTIO_GPIO_EV_GUEST_GET_DIRECTION      = 0x04, // ->get_direction()
> +       VIRTIO_GPIO_EV_GUEST_GET_VALUE          = 0x05, // ->get_value()
> +       VIRTIO_GPIO_EV_GUEST_SET_VALUE          = 0x06, // ->set_value()
> +
> +       // messages from host to guest
> +       VIRTIO_GPIO_EV_HOST_LEVEL               = 0x11, // gpio state changed
> +
> +       /* mask bit set on host->guest reply */
> +       VIRTIO_GPIO_EV_REPLY                    = 0xF000,
> +};
> +
> +struct virtio_gpio_config {
> +       __u8    version;
> +       __u8    reserved0;
> +       __u16   num_gpios;
> +       __u32   names_size;
> +       __u8    reserved1[24];
> +       __u8    name[32];
> +};
> +
> +struct virtio_gpio_event {
> +       __le16 type;
> +       __le16 pin;
> +       __le32 value;
> +};
> +
> +#endif /* _LINUX_VIRTIO_GPIO_H */
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */
>
>  #endif /* _LINUX_VIRTIO_IDS_H */
> --
> 2.11.0
>

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-12-02 14:15   ` Bartosz Golaszewski
  0 siblings, 0 replies; 54+ messages in thread
From: Bartosz Golaszewski @ 2020-12-02 14:15 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, Michael S. Tsirkin
  Cc: Jason Wang, LKML, virtualization, linux-gpio, linux-riscv, Linus Walleij

On Fri, Nov 27, 2020 at 7:30 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Introducing new gpio driver for virtual GPIO devices via virtio.
>
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  MAINTAINERS                      |   6 +
>  drivers/gpio/Kconfig             |   9 ++
>  drivers/gpio/Makefile            |   1 +
>  drivers/gpio/gpio-virtio.c       | 332 +++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/virtio_gpio.h |  39 +++++
>  include/uapi/linux/virtio_ids.h  |   1 +
>  6 files changed, 388 insertions(+)
>  create mode 100644 drivers/gpio/gpio-virtio.c
>  create mode 100644 include/uapi/linux/virtio_gpio.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a008b70f3c16..dfb8bfe09bbe 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18588,6 +18588,12 @@ F:     Documentation/filesystems/virtiofs.rst
>  F:     fs/fuse/virtio_fs.c
>  F:     include/uapi/linux/virtio_fs.h
>
> +VIRTIO GPIO DRIVER
> +M:     Enrico Weigelt, metux IT consult <info@metux.net>
> +S:     Maintained
> +F:     drivers/gpio/gpio-virtio.c
> +F:     include/uapi/linux/virtio_gpio.h
> +
>  VIRTIO GPU DRIVER
>  M:     David Airlie <airlied@linux.ie>
>  M:     Gerd Hoffmann <kraxel@redhat.com>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>           tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>           it.
>
> +config GPIO_VIRTIO
> +       tristate "VirtIO GPIO support"
> +       depends on VIRTIO
> +       help
> +         Say Y here to enable guest support for virtio based GPIOs.
> +
> +         These virtual gpios can be routed to real GPIOs or attached to
> +         simulators on the host (qemu).
> +

With a third entry (after gpio-mockup and gpio-aggregator) I think
these deserve a separate submenu for "virtual GPIO drivers" or
something like that.

>  endif
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 09dada80ac34..2b12e75af123 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -160,6 +160,7 @@ obj-$(CONFIG_GPIO_TWL4030)          += gpio-twl4030.o
>  obj-$(CONFIG_GPIO_TWL6040)             += gpio-twl6040.o
>  obj-$(CONFIG_GPIO_UCB1400)             += gpio-ucb1400.o
>  obj-$(CONFIG_GPIO_UNIPHIER)            += gpio-uniphier.o
> +obj-$(CONFIG_GPIO_VIRTIO)              += gpio-virtio.o
>  obj-$(CONFIG_GPIO_VF610)               += gpio-vf610.o
>  obj-$(CONFIG_GPIO_VIPERBOARD)          += gpio-viperboard.o
>  obj-$(CONFIG_GPIO_VR41XX)              += gpio-vr41xx.o
> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
> new file mode 100644
> index 000000000000..61e183cc26bf
> --- /dev/null
> +++ b/drivers/gpio/gpio-virtio.c
> @@ -0,0 +1,332 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * GPIO driver for virtio-based virtual GPIOs
> + *
> + * Copyright (C) 2018 metux IT consult
> + * Author: Enrico Weigelt, metux IT consult <info@metux.net>
> + *
> + */
> +
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/spinlock.h>
> +#include <linux/virtio_config.h>
> +#include <uapi/linux/virtio_ids.h>
> +#include <uapi/linux/virtio_gpio.h>
> +

Please keep headers ordered alphabetically.

> +struct virtio_gpio_priv {
> +       struct gpio_chip                gc;
> +       spinlock_t                      vq_lock;
> +       spinlock_t                      op_lock;
> +       struct virtio_device            *vdev;
> +       int                             num_gpios;
> +       char                            *name;
> +       struct virtqueue                *vq_rx;
> +       struct virtqueue                *vq_tx;
> +       struct virtio_gpio_event        rcv_buf;
> +       struct virtio_gpio_event        last;
> +       int                             irq_base;
> +       wait_queue_head_t               waitq;
> +
> +       unsigned long reply_wait;
> +};

Don't use tabs here - not only doesn't it improve readability but
you're not even consistent.

> +
> +static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
> +{
> +       struct scatterlist rcv_sg;
> +
> +       sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
> +       virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
> +                           GFP_KERNEL);
> +       virtqueue_kick(priv->vq_rx);
> +}
> +
> +static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
> +                           int pin, int value, struct virtio_gpio_event *ev)
> +{
> +       struct scatterlist sg[1];
> +       int ret;
> +       unsigned long flags;
> +
> +       WARN_ON(!ev);
> +
> +       ev->type = type;
> +       ev->pin = pin;
> +       ev->value = value;
> +
> +       sg_init_table(sg, 1);
> +       sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
> +
> +       spin_lock_irqsave(&priv->vq_lock, flags);
> +       ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
> +                                  priv, GFP_KERNEL);
> +       if (ret < 0) {
> +               dev_err(&priv->vdev->dev,
> +                       "virtqueue_add_outbuf() failed: %d\n", ret);
> +               goto out;
> +       }
> +       virtqueue_kick(priv->vq_tx);
> +
> +out:
> +       spin_unlock_irqrestore(&priv->vq_lock, flags);
> +       return 0;
> +}
> +
> +static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       set_bit(id, &priv->reply_wait);
> +}
> +
> +static inline int check_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       return test_bit(id, &priv->reply_wait);
> +}
> +
> +static inline void clear_event(struct virtio_gpio_priv *priv, int id)
> +{
> +       clear_bit(id, &priv->reply_wait);
> +}
> +
> +static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
> +                          int pin, int value)
> +{
> +       struct virtio_gpio_event *ev
> +               = devm_kzalloc(&priv->vdev->dev,
> +                              sizeof(struct virtio_gpio_event), GFP_KERNEL);

Just move the allocation to a separate line because this is super ugly.

> +
> +       if (!ev)
> +               return -ENOMEM;
> +
> +       clear_event(priv, type);
> +       virtio_gpio_xmit(priv, type, pin, value, ev);
> +       wait_event_interruptible(priv->waitq, check_event(priv, type));
> +
> +       devm_kfree(&priv->vdev->dev, ev);

In fact you don't need the managed variant if you're freeing it in the
same function. Managed resources should live as long as a device is
attached to a driver.

> +
> +       return priv->last.value;
> +}
> +
> +static int virtio_gpio_direction_input(struct gpio_chip *gc,
> +                                      unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT,
> +                              pin, 0);
> +}
> +
> +static int virtio_gpio_direction_output(struct gpio_chip *gc,
> +                                       unsigned int pin, int value)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT,
> +                              pin, value);
> +}
> +
> +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_GET_DIRECTION,
> +                              pin, 0);
> +}
> +
> +static void virtio_gpio_set(struct gpio_chip *gc,
> +                           unsigned int pin, int value)
> +{
> +       virtio_gpio_req(gpiochip_get_data(gc),
> +                       VIRTIO_GPIO_EV_GUEST_SET_VALUE, pin, value);
> +}
> +
> +static int virtio_gpio_get(struct gpio_chip *gc,
> +                          unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_GET_VALUE, pin, 0);
> +}
> +
> +static int virtio_gpio_request(struct gpio_chip *gc,
> +                              unsigned int pin)
> +{
> +       return virtio_gpio_req(gpiochip_get_data(gc),
> +                              VIRTIO_GPIO_EV_GUEST_REQUEST, pin, 0);
> +}
> +
> +static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
> +                             int pin, int value)
> +{
> +       if (pin < priv->num_gpios)
> +               generic_handle_irq(priv->irq_base + pin);
> +}
> +
> +static void virtio_gpio_data_rx(struct virtqueue *vq)
> +{
> +       struct virtio_gpio_priv *priv = vq->vdev->priv;
> +       void *data;
> +       unsigned int len;
> +       struct virtio_gpio_event *ev;
> +
> +       data = virtqueue_get_buf(priv->vq_rx, &len);
> +       if (!data || !len) {
> +               dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
> +               return;
> +       }
> +
> +       ev = data;
> +       WARN_ON(data != &priv->rcv_buf);

Is it fine to proceed if this is the case?

> +
> +       memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
> +
> +       switch (ev->type) {
> +       case VIRTIO_GPIO_EV_HOST_LEVEL:
> +               virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
> +       break;

Break should be one tab farther.

> +       default:
> +               wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
> +       break;
> +       }
> +       virtio_gpio_prepare_inbuf(priv);
> +       wake_up_all(&priv->waitq);
> +}
> +
> +static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
> +{
> +       struct virtqueue *vqs[2];
> +       vq_callback_t *cbs[] = {
> +               NULL,
> +               virtio_gpio_data_rx,
> +       };
> +       static const char * const names[] = { "in", "out", };
> +       int ret;
> +
> +       ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
> +       if (ret) {
> +               dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
> +               return ret;
> +       }
> +
> +       priv->vq_rx = vqs[0];
> +       priv->vq_tx = vqs[1];
> +
> +       virtio_gpio_prepare_inbuf(priv);
> +
> +       virtio_config_enable(priv->vdev);
> +       virtqueue_enable_cb(priv->vq_rx);
> +       virtio_device_ready(priv->vdev);
> +
> +       return 0;
> +}
> +
> +static int virtio_gpio_probe(struct virtio_device *vdev)
> +{
> +       struct virtio_gpio_priv *priv;
> +       struct virtio_gpio_config cf = {};
> +       char *name_buffer;
> +       const char **gpio_names = NULL;
> +       struct device *dev = &vdev->dev;
> +
> +       priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);

This can fail.

> +
> +       spin_lock_init(&priv->vq_lock);
> +       spin_lock_init(&priv->op_lock);
> +
> +       virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
> +       virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
> +                    &cf.num_gpios);
> +       virtio_cread(vdev, struct virtio_gpio_config, names_size,
> +                    &cf.names_size);
> +       virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
> +                          priv->name, sizeof(cf.name));
> +
> +       if (cf.version != 1) {
> +               dev_err(dev, "unsupported interface version %d\n", cf.version);
> +               return -EINVAL;
> +       }
> +
> +       priv->num_gpios = cf.num_gpios;
> +
> +       if (cf.names_size) {
> +               char *bufwalk;
> +               int idx = 0;
> +
> +               name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
> +                                          GFP_KERNEL)+1;
> +               virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
> +                                  name_buffer, cf.names_size);
> +               name_buffer[cf.names_size] = 0;
> +
> +               gpio_names = devm_kzalloc(dev,
> +                                         sizeof(char *) * priv->num_gpios,
> +                                         GFP_KERNEL);

Use devm_kcalloc() for arrays.

> +               bufwalk = name_buffer;
> +
> +               while (idx < priv->num_gpios &&
> +                      bufwalk < (name_buffer+cf.names_size)) {
> +                       gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
> +                       bufwalk += strlen(bufwalk)+1;
> +                       idx++;


Something's wrong with indentation here.

> +               }
> +       }
> +
> +       priv->gc.owner                  = THIS_MODULE;
> +       priv->gc.parent                 = dev;
> +       priv->gc.label                  = (priv->name[0] ? priv->name
> +                                                        : dev_name(dev));
> +       priv->gc.ngpio                  = priv->num_gpios;
> +       priv->gc.names                  = gpio_names;
> +       priv->gc.base                   = -1;
> +       priv->gc.request                = virtio_gpio_request;
> +       priv->gc.direction_input        = virtio_gpio_direction_input;
> +       priv->gc.direction_output       = virtio_gpio_direction_output;
> +       priv->gc.get_direction          = virtio_gpio_get_direction;
> +       priv->gc.get                    = virtio_gpio_get;
> +       priv->gc.set                    = virtio_gpio_set;
> +
> +       priv->vdev                      = vdev;
> +       vdev->priv = priv;
> +
> +       priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
> +                                             NUMA_NO_NODE);
> +       if (priv->irq_base < 0) {
> +               dev_err(&vdev->dev, "failed to alloc irqs\n");
> +               priv->irq_base = -1;
> +               return -ENOMEM;
> +       }
> +
> +       init_waitqueue_head(&priv->waitq);
> +
> +       priv->reply_wait = 0;
> +
> +       virtio_gpio_alloc_vq(priv);
> +
> +       return devm_gpiochip_add_data(dev, &priv->gc, priv);
> +}

I don't know virtio at all - Michael: could you take a look at the
code here and see if it looks good to you?

> +
> +static void virtio_gpio_remove(struct virtio_device *vdev)
> +{
> +}

Just don't implement it. Or does virtio actually require the remove() callback?

Bartosz

> +
> +static const struct virtio_device_id id_table[] = {
> +       { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
> +       { 0 },
> +};
> +
> +static struct virtio_driver virtio_gpio_driver = {
> +       .driver.name    = KBUILD_MODNAME,
> +       .driver.owner   = THIS_MODULE,
> +       .id_table       = id_table,
> +       .probe          = virtio_gpio_probe,
> +       .remove         = virtio_gpio_remove,
> +};
> +
> +module_virtio_driver(virtio_gpio_driver);
> +
> +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
> +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..5a48b975bc7a
> --- /dev/null
> +++ b/include/uapi/linux/virtio_gpio.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef _LINUX_VIRTIO_GPIO_H
> +#define _LINUX_VIRTIO_GPIO_H
> +
> +#include <linux/types.h>
> +
> +enum virtio_gpio_event_type {
> +       // requests from quest to host
> +       VIRTIO_GPIO_EV_GUEST_REQUEST            = 0x01, // ->request()
> +       VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT    = 0x02, // ->direction_input()
> +       VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT   = 0x03, // ->direction_output()
> +       VIRTIO_GPIO_EV_GUEST_GET_DIRECTION      = 0x04, // ->get_direction()
> +       VIRTIO_GPIO_EV_GUEST_GET_VALUE          = 0x05, // ->get_value()
> +       VIRTIO_GPIO_EV_GUEST_SET_VALUE          = 0x06, // ->set_value()
> +
> +       // messages from host to guest
> +       VIRTIO_GPIO_EV_HOST_LEVEL               = 0x11, // gpio state changed
> +
> +       /* mask bit set on host->guest reply */
> +       VIRTIO_GPIO_EV_REPLY                    = 0xF000,
> +};
> +
> +struct virtio_gpio_config {
> +       __u8    version;
> +       __u8    reserved0;
> +       __u16   num_gpios;
> +       __u32   names_size;
> +       __u8    reserved1[24];
> +       __u8    name[32];
> +};
> +
> +struct virtio_gpio_event {
> +       __le16 type;
> +       __le16 pin;
> +       __le32 value;
> +};
> +
> +#endif /* _LINUX_VIRTIO_GPIO_H */
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */
>
>  #endif /* _LINUX_VIRTIO_IDS_H */
> --
> 2.11.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  (?)
@ 2020-11-29 22:10   ` Jonathan Neuschäfer
  -1 siblings, 0 replies; 54+ messages in thread
From: Jonathan Neuschäfer @ 2020-11-29 22:10 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, linux-gpio, linus.walleij, mst, virtualization,
	bgolaszewski, linux-riscv, jasowang

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

Hi,

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> Introducing new gpio driver for virtual GPIO devices via virtio.
> 
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

is there a spec of the virtio-gpio protocol available? If so, linking it
in the commit message and/or driver/Kconfig would be nice.


Best regards,
Jonathan Neuschäfer

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

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-29 22:10   ` Jonathan Neuschäfer
  0 siblings, 0 replies; 54+ messages in thread
From: Jonathan Neuschäfer @ 2020-11-29 22:10 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: bgolaszewski, linus.walleij, mst, linux-kernel, virtualization,
	linux-gpio, linux-riscv, jasowang


[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]

Hi,

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> Introducing new gpio driver for virtual GPIO devices via virtio.
> 
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

is there a spec of the virtio-gpio protocol available? If so, linking it
in the commit message and/or driver/Kconfig would be nice.


Best regards,
Jonathan Neuschäfer

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-29 22:10   ` Jonathan Neuschäfer
  0 siblings, 0 replies; 54+ messages in thread
From: Jonathan Neuschäfer @ 2020-11-29 22:10 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: bgolaszewski, linus.walleij, mst, linux-kernel, virtualization,
	linux-gpio, linux-riscv


[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]

Hi,

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> Introducing new gpio driver for virtual GPIO devices via virtio.
> 
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

is there a spec of the virtio-gpio protocol available? If so, linking it
in the commit message and/or driver/Kconfig would be nice.


Best regards,
Jonathan Neuschäfer

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

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  (?)
@ 2020-11-29 20:11   ` Michael S. Tsirkin
  -1 siblings, 0 replies; 54+ messages in thread
From: Michael S. Tsirkin @ 2020-11-29 20:11 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, linus.walleij, bgolaszewski, jasowang, linux-gpio,
	virtualization, linux-riscv

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */


Pls remember to reserve the ID with the virtio TC
before using it in the driver. Thanks!

>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 2.11.0


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-29 20:11   ` Michael S. Tsirkin
  0 siblings, 0 replies; 54+ messages in thread
From: Michael S. Tsirkin @ 2020-11-29 20:11 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-gpio, jasowang, linux-kernel, virtualization, bgolaszewski,
	linux-riscv, linus.walleij

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */


Pls remember to reserve the ID with the virtio TC
before using it in the driver. Thanks!

>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 2.11.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-29 20:11   ` Michael S. Tsirkin
  0 siblings, 0 replies; 54+ messages in thread
From: Michael S. Tsirkin @ 2020-11-29 20:11 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-gpio, linux-kernel, virtualization, bgolaszewski,
	linux-riscv, linus.walleij

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */


Pls remember to reserve the ID with the virtio TC
before using it in the driver. Thanks!

>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 2.11.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  (?)
  (?)
@ 2020-11-27 19:33   ` kernel test robot
  -1 siblings, 0 replies; 54+ messages in thread
From: kernel test robot @ 2020-11-27 19:33 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: kbuild-all, linus.walleij, bgolaszewski, info, mst, jasowang,
	linux-gpio, virtualization, linux-riscv

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

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on linux/master linus/master v5.10-rc5 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: c6x-randconfig-p002-20201128 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a134ec4827b0ffb7edd6a292238bd93fb377f127
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
        git checkout a134ec4827b0ffb7edd6a292238bd93fb377f127
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_gpio.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_gpio.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1288: headers] Error 2
   arch/c6x/kernel/asm-offsets.c:14:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      14 | void foo(void)
         |      ^~~
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25224 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 19:33   ` kernel test robot
  0 siblings, 0 replies; 54+ messages in thread
From: kernel test robot @ 2020-11-27 19:33 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: kbuild-all, mst, linus.walleij, virtualization, bgolaszewski,
	linux-gpio, linux-riscv, info, jasowang

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

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on linux/master linus/master v5.10-rc5 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: c6x-randconfig-p002-20201128 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a134ec4827b0ffb7edd6a292238bd93fb377f127
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
        git checkout a134ec4827b0ffb7edd6a292238bd93fb377f127
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_gpio.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_gpio.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1288: headers] Error 2
   arch/c6x/kernel/asm-offsets.c:14:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      14 | void foo(void)
         |      ^~~
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25224 bytes --]

[-- Attachment #3: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 19:33   ` kernel test robot
  0 siblings, 0 replies; 54+ messages in thread
From: kernel test robot @ 2020-11-27 19:33 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: kbuild-all, mst, linus.walleij, virtualization, bgolaszewski,
	linux-gpio, linux-riscv, info

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

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on linux/master linus/master v5.10-rc5 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: c6x-randconfig-p002-20201128 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a134ec4827b0ffb7edd6a292238bd93fb377f127
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
        git checkout a134ec4827b0ffb7edd6a292238bd93fb377f127
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_gpio.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_gpio.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1288: headers] Error 2
   arch/c6x/kernel/asm-offsets.c:14:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      14 | void foo(void)
         |      ^~~
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25224 bytes --]

[-- Attachment #3: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 19:33   ` kernel test robot
  0 siblings, 0 replies; 54+ messages in thread
From: kernel test robot @ 2020-11-27 19:33 UTC (permalink / raw)
  To: kbuild-all

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

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on linux/master linus/master v5.10-rc5 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: c6x-randconfig-p002-20201128 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a134ec4827b0ffb7edd6a292238bd93fb377f127
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
        git checkout a134ec4827b0ffb7edd6a292238bd93fb377f127
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_gpio.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_gpio.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1288: headers] Error 2
   arch/c6x/kernel/asm-offsets.c:14:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      14 | void foo(void)
         |      ^~~
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 25224 bytes --]

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
  2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  (?)
@ 2020-11-27 18:45   ` Randy Dunlap
  -1 siblings, 0 replies; 54+ messages in thread
From: Randy Dunlap @ 2020-11-27 18:45 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: linus.walleij, bgolaszewski, mst, jasowang, linux-gpio,
	virtualization, linux-riscv

On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>  	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>  	  it.
>  
> +config GPIO_VIRTIO
> +	tristate "VirtIO GPIO support"
> +	depends on VIRTIO
> +	help
> +	  Say Y here to enable guest support for virtio based GPIOs.

	                                         virtio-based

> +
> +	  These virtual gpios can be routed to real GPIOs or attached to

	                GPIOs

> +	  simulators on the host (qemu).
> +
>  endif


-- 
~Randy


^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 18:45   ` Randy Dunlap
  0 siblings, 0 replies; 54+ messages in thread
From: Randy Dunlap @ 2020-11-27 18:45 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: linux-gpio, linus.walleij, mst, virtualization, bgolaszewski,
	linux-riscv, jasowang

On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>  	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>  	  it.
>  
> +config GPIO_VIRTIO
> +	tristate "VirtIO GPIO support"
> +	depends on VIRTIO
> +	help
> +	  Say Y here to enable guest support for virtio based GPIOs.

	                                         virtio-based

> +
> +	  These virtual gpios can be routed to real GPIOs or attached to

	                GPIOs

> +	  simulators on the host (qemu).
> +
>  endif


-- 
~Randy


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 54+ messages in thread

* Re: [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 18:45   ` Randy Dunlap
  0 siblings, 0 replies; 54+ messages in thread
From: Randy Dunlap @ 2020-11-27 18:45 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: linux-gpio, linus.walleij, mst, virtualization, bgolaszewski,
	linux-riscv

On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
>  	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
>  	  it.
>  
> +config GPIO_VIRTIO
> +	tristate "VirtIO GPIO support"
> +	depends on VIRTIO
> +	help
> +	  Say Y here to enable guest support for virtio based GPIOs.

	                                         virtio-based

> +
> +	  These virtual gpios can be routed to real GPIOs or attached to

	                GPIOs

> +	  simulators on the host (qemu).
> +
>  endif


-- 
~Randy

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 54+ messages in thread

* [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-11-27 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: linus.walleij, bgolaszewski, info, mst, jasowang, linux-gpio,
	virtualization, linux-riscv

Introducing new gpio driver for virtual GPIO devices via virtio.

The driver allows routing gpio control into VM guests, eg. brigding
virtual gpios to specific host gpios, or attaching simulators for
automatic application testing.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                      |   6 +
 drivers/gpio/Kconfig             |   9 ++
 drivers/gpio/Makefile            |   1 +
 drivers/gpio/gpio-virtio.c       | 332 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_gpio.h |  39 +++++
 include/uapi/linux/virtio_ids.h  |   1 +
 6 files changed, 388 insertions(+)
 create mode 100644 drivers/gpio/gpio-virtio.c
 create mode 100644 include/uapi/linux/virtio_gpio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a008b70f3c16..dfb8bfe09bbe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18588,6 +18588,12 @@ F:	Documentation/filesystems/virtiofs.rst
 F:	fs/fuse/virtio_fs.c
 F:	include/uapi/linux/virtio_fs.h
 
+VIRTIO GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/gpio/gpio-virtio.c
+F:	include/uapi/linux/virtio_gpio.h
+
 VIRTIO GPU DRIVER
 M:	David Airlie <airlied@linux.ie>
 M:	Gerd Hoffmann <kraxel@redhat.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5d4de5cd6759..e8414d82cf75 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
 	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
 	  it.
 
+config GPIO_VIRTIO
+	tristate "VirtIO GPIO support"
+	depends on VIRTIO
+	help
+	  Say Y here to enable guest support for virtio based GPIOs.
+
+	  These virtual gpios can be routed to real GPIOs or attached to
+	  simulators on the host (qemu).
+
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 09dada80ac34..2b12e75af123 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_GPIO_TWL4030)		+= gpio-twl4030.o
 obj-$(CONFIG_GPIO_TWL6040)		+= gpio-twl6040.o
 obj-$(CONFIG_GPIO_UCB1400)		+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_UNIPHIER)		+= gpio-uniphier.o
+obj-$(CONFIG_GPIO_VIRTIO)		+= gpio-virtio.o
 obj-$(CONFIG_GPIO_VF610)		+= gpio-vf610.o
 obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
 obj-$(CONFIG_GPIO_VR41XX)		+= gpio-vr41xx.o
diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
new file mode 100644
index 000000000000..61e183cc26bf
--- /dev/null
+++ b/drivers/gpio/gpio-virtio.c
@@ -0,0 +1,332 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * GPIO driver for virtio-based virtual GPIOs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt, metux IT consult <info@metux.net>
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio/driver.h>
+#include <linux/spinlock.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_gpio.h>
+
+struct virtio_gpio_priv {
+	struct gpio_chip		gc;
+	spinlock_t			vq_lock;
+	spinlock_t			op_lock;
+	struct virtio_device		*vdev;
+	int				num_gpios;
+	char				*name;
+	struct virtqueue		*vq_rx;
+	struct virtqueue		*vq_tx;
+	struct virtio_gpio_event	rcv_buf;
+	struct virtio_gpio_event	last;
+	int				irq_base;
+	wait_queue_head_t		waitq;
+
+	unsigned long reply_wait;
+};
+
+static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
+{
+	struct scatterlist rcv_sg;
+
+	sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
+	virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
+			    GFP_KERNEL);
+	virtqueue_kick(priv->vq_rx);
+}
+
+static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
+			    int pin, int value, struct virtio_gpio_event *ev)
+{
+	struct scatterlist sg[1];
+	int ret;
+	unsigned long flags;
+
+	WARN_ON(!ev);
+
+	ev->type = type;
+	ev->pin = pin;
+	ev->value = value;
+
+	sg_init_table(sg, 1);
+	sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
+
+	spin_lock_irqsave(&priv->vq_lock, flags);
+	ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
+				   priv, GFP_KERNEL);
+	if (ret < 0) {
+		dev_err(&priv->vdev->dev,
+			"virtqueue_add_outbuf() failed: %d\n", ret);
+		goto out;
+	}
+	virtqueue_kick(priv->vq_tx);
+
+out:
+	spin_unlock_irqrestore(&priv->vq_lock, flags);
+	return 0;
+}
+
+static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
+{
+	set_bit(id, &priv->reply_wait);
+}
+
+static inline int check_event(struct virtio_gpio_priv *priv, int id)
+{
+	return test_bit(id, &priv->reply_wait);
+}
+
+static inline void clear_event(struct virtio_gpio_priv *priv, int id)
+{
+	clear_bit(id, &priv->reply_wait);
+}
+
+static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
+			   int pin, int value)
+{
+	struct virtio_gpio_event *ev
+		= devm_kzalloc(&priv->vdev->dev,
+			       sizeof(struct virtio_gpio_event), GFP_KERNEL);
+
+	if (!ev)
+		return -ENOMEM;
+
+	clear_event(priv, type);
+	virtio_gpio_xmit(priv, type, pin, value, ev);
+	wait_event_interruptible(priv->waitq, check_event(priv, type));
+
+	devm_kfree(&priv->vdev->dev, ev);
+
+	return priv->last.value;
+}
+
+static int virtio_gpio_direction_input(struct gpio_chip *gc,
+				       unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT,
+			       pin, 0);
+}
+
+static int virtio_gpio_direction_output(struct gpio_chip *gc,
+					unsigned int pin, int value)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT,
+			       pin, value);
+}
+
+static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_GET_DIRECTION,
+			       pin, 0);
+}
+
+static void virtio_gpio_set(struct gpio_chip *gc,
+			    unsigned int pin, int value)
+{
+	virtio_gpio_req(gpiochip_get_data(gc),
+			VIRTIO_GPIO_EV_GUEST_SET_VALUE, pin, value);
+}
+
+static int virtio_gpio_get(struct gpio_chip *gc,
+			   unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_GET_VALUE, pin, 0);
+}
+
+static int virtio_gpio_request(struct gpio_chip *gc,
+			       unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_REQUEST, pin, 0);
+}
+
+static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
+			      int pin, int value)
+{
+	if (pin < priv->num_gpios)
+		generic_handle_irq(priv->irq_base + pin);
+}
+
+static void virtio_gpio_data_rx(struct virtqueue *vq)
+{
+	struct virtio_gpio_priv *priv = vq->vdev->priv;
+	void *data;
+	unsigned int len;
+	struct virtio_gpio_event *ev;
+
+	data = virtqueue_get_buf(priv->vq_rx, &len);
+	if (!data || !len) {
+		dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
+		return;
+	}
+
+	ev = data;
+	WARN_ON(data != &priv->rcv_buf);
+
+	memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
+
+	switch (ev->type) {
+	case VIRTIO_GPIO_EV_HOST_LEVEL:
+		virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
+	break;
+	default:
+		wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
+	break;
+	}
+	virtio_gpio_prepare_inbuf(priv);
+	wake_up_all(&priv->waitq);
+}
+
+static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
+{
+	struct virtqueue *vqs[2];
+	vq_callback_t *cbs[] = {
+		NULL,
+		virtio_gpio_data_rx,
+	};
+	static const char * const names[] = { "in", "out", };
+	int ret;
+
+	ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
+		return ret;
+	}
+
+	priv->vq_rx = vqs[0];
+	priv->vq_tx = vqs[1];
+
+	virtio_gpio_prepare_inbuf(priv);
+
+	virtio_config_enable(priv->vdev);
+	virtqueue_enable_cb(priv->vq_rx);
+	virtio_device_ready(priv->vdev);
+
+	return 0;
+}
+
+static int virtio_gpio_probe(struct virtio_device *vdev)
+{
+	struct virtio_gpio_priv *priv;
+	struct virtio_gpio_config cf = {};
+	char *name_buffer;
+	const char **gpio_names = NULL;
+	struct device *dev = &vdev->dev;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
+
+	spin_lock_init(&priv->vq_lock);
+	spin_lock_init(&priv->op_lock);
+
+	virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
+	virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
+		     &cf.num_gpios);
+	virtio_cread(vdev, struct virtio_gpio_config, names_size,
+		     &cf.names_size);
+	virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
+			   priv->name, sizeof(cf.name));
+
+	if (cf.version != 1) {
+		dev_err(dev, "unsupported interface version %d\n", cf.version);
+		return -EINVAL;
+	}
+
+	priv->num_gpios = cf.num_gpios;
+
+	if (cf.names_size) {
+		char *bufwalk;
+		int idx = 0;
+
+		name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
+					   GFP_KERNEL)+1;
+		virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
+				   name_buffer, cf.names_size);
+		name_buffer[cf.names_size] = 0;
+
+		gpio_names = devm_kzalloc(dev,
+					  sizeof(char *) * priv->num_gpios,
+					  GFP_KERNEL);
+		bufwalk = name_buffer;
+
+		while (idx < priv->num_gpios &&
+		       bufwalk < (name_buffer+cf.names_size)) {
+			gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
+			bufwalk += strlen(bufwalk)+1;
+			idx++;
+		}
+	}
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= dev;
+	priv->gc.label			= (priv->name[0] ? priv->name
+							 : dev_name(dev));
+	priv->gc.ngpio			= priv->num_gpios;
+	priv->gc.names			= gpio_names;
+	priv->gc.base			= -1;
+	priv->gc.request		= virtio_gpio_request;
+	priv->gc.direction_input	= virtio_gpio_direction_input;
+	priv->gc.direction_output	= virtio_gpio_direction_output;
+	priv->gc.get_direction		= virtio_gpio_get_direction;
+	priv->gc.get			= virtio_gpio_get;
+	priv->gc.set			= virtio_gpio_set;
+
+	priv->vdev			= vdev;
+	vdev->priv = priv;
+
+	priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
+					      NUMA_NO_NODE);
+	if (priv->irq_base < 0) {
+		dev_err(&vdev->dev, "failed to alloc irqs\n");
+		priv->irq_base = -1;
+		return -ENOMEM;
+	}
+
+	init_waitqueue_head(&priv->waitq);
+
+	priv->reply_wait = 0;
+
+	virtio_gpio_alloc_vq(priv);
+
+	return devm_gpiochip_add_data(dev, &priv->gc, priv);
+}
+
+static void virtio_gpio_remove(struct virtio_device *vdev)
+{
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_gpio_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.driver.owner	= THIS_MODULE,
+	.id_table	= id_table,
+	.probe		= virtio_gpio_probe,
+	.remove		= virtio_gpio_remove,
+};
+
+module_virtio_driver(virtio_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+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..5a48b975bc7a
--- /dev/null
+++ b/include/uapi/linux/virtio_gpio.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _LINUX_VIRTIO_GPIO_H
+#define _LINUX_VIRTIO_GPIO_H
+
+#include <linux/types.h>
+
+enum virtio_gpio_event_type {
+	// requests from quest to host
+	VIRTIO_GPIO_EV_GUEST_REQUEST		= 0x01,	// ->request()
+	VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT	= 0x02,	// ->direction_input()
+	VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT	= 0x03,	// ->direction_output()
+	VIRTIO_GPIO_EV_GUEST_GET_DIRECTION	= 0x04,	// ->get_direction()
+	VIRTIO_GPIO_EV_GUEST_GET_VALUE		= 0x05,	// ->get_value()
+	VIRTIO_GPIO_EV_GUEST_SET_VALUE		= 0x06,	// ->set_value()
+
+	// messages from host to guest
+	VIRTIO_GPIO_EV_HOST_LEVEL		= 0x11,	// gpio state changed
+
+	/* mask bit set on host->guest reply */
+	VIRTIO_GPIO_EV_REPLY			= 0xF000,
+};
+
+struct virtio_gpio_config {
+	__u8    version;
+	__u8    reserved0;
+	__u16   num_gpios;
+	__u32   names_size;
+	__u8    reserved1[24];
+	__u8    name[32];
+};
+
+struct virtio_gpio_event {
+	__le16 type;
+	__le16 pin;
+	__le32 value;
+};
+
+#endif /* _LINUX_VIRTIO_GPIO_H */
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index b052355ac7a3..85772c0bcb4b 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -48,5 +48,6 @@
 #define VIRTIO_ID_FS           26 /* virtio filesystem */
 #define VIRTIO_ID_PMEM         27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_GPIO           30 /* virtio GPIO */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 54+ messages in thread

* [PATCH] drivers: gpio: add virtio-gpio guest driver
@ 2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
  0 siblings, 0 replies; 54+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2020-11-27 18:30 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-gpio, linus.walleij, mst, virtualization, bgolaszewski,
	linux-riscv, info, jasowang

Introducing new gpio driver for virtual GPIO devices via virtio.

The driver allows routing gpio control into VM guests, eg. brigding
virtual gpios to specific host gpios, or attaching simulators for
automatic application testing.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 MAINTAINERS                      |   6 +
 drivers/gpio/Kconfig             |   9 ++
 drivers/gpio/Makefile            |   1 +
 drivers/gpio/gpio-virtio.c       | 332 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_gpio.h |  39 +++++
 include/uapi/linux/virtio_ids.h  |   1 +
 6 files changed, 388 insertions(+)
 create mode 100644 drivers/gpio/gpio-virtio.c
 create mode 100644 include/uapi/linux/virtio_gpio.h

diff --git a/MAINTAINERS b/MAINTAINERS
index a008b70f3c16..dfb8bfe09bbe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18588,6 +18588,12 @@ F:	Documentation/filesystems/virtiofs.rst
 F:	fs/fuse/virtio_fs.c
 F:	include/uapi/linux/virtio_fs.h
 
+VIRTIO GPIO DRIVER
+M:	Enrico Weigelt, metux IT consult <info@metux.net>
+S:	Maintained
+F:	drivers/gpio/gpio-virtio.c
+F:	include/uapi/linux/virtio_gpio.h
+
 VIRTIO GPU DRIVER
 M:	David Airlie <airlied@linux.ie>
 M:	Gerd Hoffmann <kraxel@redhat.com>
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 5d4de5cd6759..e8414d82cf75 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1613,4 +1613,13 @@ config GPIO_MOCKUP
 	  tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in
 	  it.
 
+config GPIO_VIRTIO
+	tristate "VirtIO GPIO support"
+	depends on VIRTIO
+	help
+	  Say Y here to enable guest support for virtio based GPIOs.
+
+	  These virtual gpios can be routed to real GPIOs or attached to
+	  simulators on the host (qemu).
+
 endif
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 09dada80ac34..2b12e75af123 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_GPIO_TWL4030)		+= gpio-twl4030.o
 obj-$(CONFIG_GPIO_TWL6040)		+= gpio-twl6040.o
 obj-$(CONFIG_GPIO_UCB1400)		+= gpio-ucb1400.o
 obj-$(CONFIG_GPIO_UNIPHIER)		+= gpio-uniphier.o
+obj-$(CONFIG_GPIO_VIRTIO)		+= gpio-virtio.o
 obj-$(CONFIG_GPIO_VF610)		+= gpio-vf610.o
 obj-$(CONFIG_GPIO_VIPERBOARD)		+= gpio-viperboard.o
 obj-$(CONFIG_GPIO_VR41XX)		+= gpio-vr41xx.o
diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
new file mode 100644
index 000000000000..61e183cc26bf
--- /dev/null
+++ b/drivers/gpio/gpio-virtio.c
@@ -0,0 +1,332 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * GPIO driver for virtio-based virtual GPIOs
+ *
+ * Copyright (C) 2018 metux IT consult
+ * Author: Enrico Weigelt, metux IT consult <info@metux.net>
+ *
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/gpio/driver.h>
+#include <linux/spinlock.h>
+#include <linux/virtio_config.h>
+#include <uapi/linux/virtio_ids.h>
+#include <uapi/linux/virtio_gpio.h>
+
+struct virtio_gpio_priv {
+	struct gpio_chip		gc;
+	spinlock_t			vq_lock;
+	spinlock_t			op_lock;
+	struct virtio_device		*vdev;
+	int				num_gpios;
+	char				*name;
+	struct virtqueue		*vq_rx;
+	struct virtqueue		*vq_tx;
+	struct virtio_gpio_event	rcv_buf;
+	struct virtio_gpio_event	last;
+	int				irq_base;
+	wait_queue_head_t		waitq;
+
+	unsigned long reply_wait;
+};
+
+static void virtio_gpio_prepare_inbuf(struct virtio_gpio_priv *priv)
+{
+	struct scatterlist rcv_sg;
+
+	sg_init_one(&rcv_sg, &priv->rcv_buf, sizeof(priv->rcv_buf));
+	virtqueue_add_inbuf(priv->vq_rx, &rcv_sg, 1, &priv->rcv_buf,
+			    GFP_KERNEL);
+	virtqueue_kick(priv->vq_rx);
+}
+
+static int virtio_gpio_xmit(struct virtio_gpio_priv *priv, int type,
+			    int pin, int value, struct virtio_gpio_event *ev)
+{
+	struct scatterlist sg[1];
+	int ret;
+	unsigned long flags;
+
+	WARN_ON(!ev);
+
+	ev->type = type;
+	ev->pin = pin;
+	ev->value = value;
+
+	sg_init_table(sg, 1);
+	sg_set_buf(&sg[0], ev, sizeof(struct virtio_gpio_event));
+
+	spin_lock_irqsave(&priv->vq_lock, flags);
+	ret = virtqueue_add_outbuf(priv->vq_tx, sg, ARRAY_SIZE(sg),
+				   priv, GFP_KERNEL);
+	if (ret < 0) {
+		dev_err(&priv->vdev->dev,
+			"virtqueue_add_outbuf() failed: %d\n", ret);
+		goto out;
+	}
+	virtqueue_kick(priv->vq_tx);
+
+out:
+	spin_unlock_irqrestore(&priv->vq_lock, flags);
+	return 0;
+}
+
+static inline void wakeup_event(struct virtio_gpio_priv *priv, int id)
+{
+	set_bit(id, &priv->reply_wait);
+}
+
+static inline int check_event(struct virtio_gpio_priv *priv, int id)
+{
+	return test_bit(id, &priv->reply_wait);
+}
+
+static inline void clear_event(struct virtio_gpio_priv *priv, int id)
+{
+	clear_bit(id, &priv->reply_wait);
+}
+
+static int virtio_gpio_req(struct virtio_gpio_priv *priv, int type,
+			   int pin, int value)
+{
+	struct virtio_gpio_event *ev
+		= devm_kzalloc(&priv->vdev->dev,
+			       sizeof(struct virtio_gpio_event), GFP_KERNEL);
+
+	if (!ev)
+		return -ENOMEM;
+
+	clear_event(priv, type);
+	virtio_gpio_xmit(priv, type, pin, value, ev);
+	wait_event_interruptible(priv->waitq, check_event(priv, type));
+
+	devm_kfree(&priv->vdev->dev, ev);
+
+	return priv->last.value;
+}
+
+static int virtio_gpio_direction_input(struct gpio_chip *gc,
+				       unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT,
+			       pin, 0);
+}
+
+static int virtio_gpio_direction_output(struct gpio_chip *gc,
+					unsigned int pin, int value)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT,
+			       pin, value);
+}
+
+static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_GET_DIRECTION,
+			       pin, 0);
+}
+
+static void virtio_gpio_set(struct gpio_chip *gc,
+			    unsigned int pin, int value)
+{
+	virtio_gpio_req(gpiochip_get_data(gc),
+			VIRTIO_GPIO_EV_GUEST_SET_VALUE, pin, value);
+}
+
+static int virtio_gpio_get(struct gpio_chip *gc,
+			   unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_GET_VALUE, pin, 0);
+}
+
+static int virtio_gpio_request(struct gpio_chip *gc,
+			       unsigned int pin)
+{
+	return virtio_gpio_req(gpiochip_get_data(gc),
+			       VIRTIO_GPIO_EV_GUEST_REQUEST, pin, 0);
+}
+
+static void virtio_gpio_signal(struct virtio_gpio_priv *priv, int event,
+			      int pin, int value)
+{
+	if (pin < priv->num_gpios)
+		generic_handle_irq(priv->irq_base + pin);
+}
+
+static void virtio_gpio_data_rx(struct virtqueue *vq)
+{
+	struct virtio_gpio_priv *priv = vq->vdev->priv;
+	void *data;
+	unsigned int len;
+	struct virtio_gpio_event *ev;
+
+	data = virtqueue_get_buf(priv->vq_rx, &len);
+	if (!data || !len) {
+		dev_warn(&vq->vdev->dev, "RX received no data ! %d\n", len);
+		return;
+	}
+
+	ev = data;
+	WARN_ON(data != &priv->rcv_buf);
+
+	memcpy(&priv->last, &priv->rcv_buf, sizeof(struct virtio_gpio_event));
+
+	switch (ev->type) {
+	case VIRTIO_GPIO_EV_HOST_LEVEL:
+		virtio_gpio_signal(priv, ev->type, ev->pin, ev->value);
+	break;
+	default:
+		wakeup_event(priv, ev->type & ~VIRTIO_GPIO_EV_REPLY);
+	break;
+	}
+	virtio_gpio_prepare_inbuf(priv);
+	wake_up_all(&priv->waitq);
+}
+
+static int virtio_gpio_alloc_vq(struct virtio_gpio_priv *priv)
+{
+	struct virtqueue *vqs[2];
+	vq_callback_t *cbs[] = {
+		NULL,
+		virtio_gpio_data_rx,
+	};
+	static const char * const names[] = { "in", "out", };
+	int ret;
+
+	ret = virtio_find_vqs(priv->vdev, 2, vqs, cbs, names, NULL);
+	if (ret) {
+		dev_err(&priv->vdev->dev, "failed to alloc vqs: %d\n", ret);
+		return ret;
+	}
+
+	priv->vq_rx = vqs[0];
+	priv->vq_tx = vqs[1];
+
+	virtio_gpio_prepare_inbuf(priv);
+
+	virtio_config_enable(priv->vdev);
+	virtqueue_enable_cb(priv->vq_rx);
+	virtio_device_ready(priv->vdev);
+
+	return 0;
+}
+
+static int virtio_gpio_probe(struct virtio_device *vdev)
+{
+	struct virtio_gpio_priv *priv;
+	struct virtio_gpio_config cf = {};
+	char *name_buffer;
+	const char **gpio_names = NULL;
+	struct device *dev = &vdev->dev;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->name = devm_kzalloc(dev, sizeof(cf.name)+1, GFP_KERNEL);
+
+	spin_lock_init(&priv->vq_lock);
+	spin_lock_init(&priv->op_lock);
+
+	virtio_cread(vdev, struct virtio_gpio_config, version, &cf.version);
+	virtio_cread(vdev, struct virtio_gpio_config, num_gpios,
+		     &cf.num_gpios);
+	virtio_cread(vdev, struct virtio_gpio_config, names_size,
+		     &cf.names_size);
+	virtio_cread_bytes(vdev, offsetof(struct virtio_gpio_config, name),
+			   priv->name, sizeof(cf.name));
+
+	if (cf.version != 1) {
+		dev_err(dev, "unsupported interface version %d\n", cf.version);
+		return -EINVAL;
+	}
+
+	priv->num_gpios = cf.num_gpios;
+
+	if (cf.names_size) {
+		char *bufwalk;
+		int idx = 0;
+
+		name_buffer = devm_kzalloc(&vdev->dev, cf.names_size,
+					   GFP_KERNEL)+1;
+		virtio_cread_bytes(vdev, sizeof(struct virtio_gpio_config),
+				   name_buffer, cf.names_size);
+		name_buffer[cf.names_size] = 0;
+
+		gpio_names = devm_kzalloc(dev,
+					  sizeof(char *) * priv->num_gpios,
+					  GFP_KERNEL);
+		bufwalk = name_buffer;
+
+		while (idx < priv->num_gpios &&
+		       bufwalk < (name_buffer+cf.names_size)) {
+			gpio_names[idx] = (strlen(bufwalk) ? bufwalk : NULL);
+			bufwalk += strlen(bufwalk)+1;
+			idx++;
+		}
+	}
+
+	priv->gc.owner			= THIS_MODULE;
+	priv->gc.parent			= dev;
+	priv->gc.label			= (priv->name[0] ? priv->name
+							 : dev_name(dev));
+	priv->gc.ngpio			= priv->num_gpios;
+	priv->gc.names			= gpio_names;
+	priv->gc.base			= -1;
+	priv->gc.request		= virtio_gpio_request;
+	priv->gc.direction_input	= virtio_gpio_direction_input;
+	priv->gc.direction_output	= virtio_gpio_direction_output;
+	priv->gc.get_direction		= virtio_gpio_get_direction;
+	priv->gc.get			= virtio_gpio_get;
+	priv->gc.set			= virtio_gpio_set;
+
+	priv->vdev			= vdev;
+	vdev->priv = priv;
+
+	priv->irq_base = devm_irq_alloc_descs(dev, -1, 0, priv->num_gpios,
+					      NUMA_NO_NODE);
+	if (priv->irq_base < 0) {
+		dev_err(&vdev->dev, "failed to alloc irqs\n");
+		priv->irq_base = -1;
+		return -ENOMEM;
+	}
+
+	init_waitqueue_head(&priv->waitq);
+
+	priv->reply_wait = 0;
+
+	virtio_gpio_alloc_vq(priv);
+
+	return devm_gpiochip_add_data(dev, &priv->gc, priv);
+}
+
+static void virtio_gpio_remove(struct virtio_device *vdev)
+{
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID },
+	{ 0 },
+};
+
+static struct virtio_driver virtio_gpio_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.driver.owner	= THIS_MODULE,
+	.id_table	= id_table,
+	.probe		= virtio_gpio_probe,
+	.remove		= virtio_gpio_remove,
+};
+
+module_virtio_driver(virtio_gpio_driver);
+
+MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
+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..5a48b975bc7a
--- /dev/null
+++ b/include/uapi/linux/virtio_gpio.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#ifndef _LINUX_VIRTIO_GPIO_H
+#define _LINUX_VIRTIO_GPIO_H
+
+#include <linux/types.h>
+
+enum virtio_gpio_event_type {
+	// requests from quest to host
+	VIRTIO_GPIO_EV_GUEST_REQUEST		= 0x01,	// ->request()
+	VIRTIO_GPIO_EV_GUEST_DIRECTION_INPUT	= 0x02,	// ->direction_input()
+	VIRTIO_GPIO_EV_GUEST_DIRECTION_OUTPUT	= 0x03,	// ->direction_output()
+	VIRTIO_GPIO_EV_GUEST_GET_DIRECTION	= 0x04,	// ->get_direction()
+	VIRTIO_GPIO_EV_GUEST_GET_VALUE		= 0x05,	// ->get_value()
+	VIRTIO_GPIO_EV_GUEST_SET_VALUE		= 0x06,	// ->set_value()
+
+	// messages from host to guest
+	VIRTIO_GPIO_EV_HOST_LEVEL		= 0x11,	// gpio state changed
+
+	/* mask bit set on host->guest reply */
+	VIRTIO_GPIO_EV_REPLY			= 0xF000,
+};
+
+struct virtio_gpio_config {
+	__u8    version;
+	__u8    reserved0;
+	__u16   num_gpios;
+	__u32   names_size;
+	__u8    reserved1[24];
+	__u8    name[32];
+};
+
+struct virtio_gpio_event {
+	__le16 type;
+	__le16 pin;
+	__le32 value;
+};
+
+#endif /* _LINUX_VIRTIO_GPIO_H */
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index b052355ac7a3..85772c0bcb4b 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -48,5 +48,6 @@
 #define VIRTIO_ID_FS           26 /* virtio filesystem */
 #define VIRTIO_ID_PMEM         27 /* virtio pmem */
 #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
+#define VIRTIO_ID_GPIO           30 /* virtio GPIO */
 
 #endif /* _LINUX_VIRTIO_IDS_H */
-- 
2.11.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 54+ messages in thread

end of thread, other threads:[~2021-06-17 11:48 UTC | newest]

Thread overview: 54+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-15 17:49 [PATCH] drivers: gpio: add virtio-gpio guest driver Enrico Weigelt, metux IT consult
2021-06-15 17:49 ` Enrico Weigelt, metux IT consult
2021-06-16  8:31 ` Linus Walleij
2021-06-16  8:31   ` Linus Walleij
2021-06-16  8:31   ` Linus Walleij
2021-06-16 11:49   ` Viresh Kumar
2021-06-16 11:49     ` Viresh Kumar
2021-06-16 11:49     ` Viresh Kumar
2021-06-16 15:04     ` Enrico Weigelt, metux IT consult
2021-06-16 15:04       ` Enrico Weigelt, metux IT consult
2021-06-17  3:59       ` Viresh Kumar
2021-06-17  3:59         ` Viresh Kumar
2021-06-17  3:59         ` Viresh Kumar
2021-06-17  9:54         ` Enrico Weigelt, metux IT consult
2021-06-17  9:54           ` Enrico Weigelt, metux IT consult
2021-06-17 11:47           ` Viresh Kumar
2021-06-17 11:47             ` Viresh Kumar
2021-06-17 11:47             ` Viresh Kumar
2021-06-16 14:41   ` Enrico Weigelt, metux IT consult
2021-06-16 14:41     ` Enrico Weigelt, metux IT consult
2021-06-16 18:47   ` banned on virtio list ? [Re: [PATCH] drivers: gpio: add virtio-gpio guest driver] Enrico Weigelt, metux IT consult
2021-06-16 18:47     ` [virtio-dev] " Enrico Weigelt, metux IT consult
2021-06-16 18:47     ` Enrico Weigelt, metux IT consult
2021-06-16 21:18     ` [virtio-dev] " Chet Ensign
2021-06-16 21:18       ` Chet Ensign
  -- strict thread matches above, loose matches on Subject: below --
2020-11-27 18:30 [PATCH] drivers: gpio: add virtio-gpio guest driver Enrico Weigelt, metux IT consult
2020-11-27 18:30 ` Enrico Weigelt, metux IT consult
2020-11-27 18:45 ` Randy Dunlap
2020-11-27 18:45   ` Randy Dunlap
2020-11-27 18:45   ` Randy Dunlap
2020-12-03 19:01   ` Enrico Weigelt, metux IT consult
2020-12-03 19:01     ` Enrico Weigelt, metux IT consult
2020-11-27 19:33 ` kernel test robot
2020-11-27 19:33   ` kernel test robot
2020-11-27 19:33   ` kernel test robot
2020-11-27 19:33   ` kernel test robot
2020-11-29 20:11 ` Michael S. Tsirkin
2020-11-29 20:11   ` Michael S. Tsirkin
2020-11-29 20:11   ` Michael S. Tsirkin
2020-11-29 22:10 ` Jonathan Neuschäfer
2020-11-29 22:10   ` Jonathan Neuschäfer
2020-11-29 22:10   ` Jonathan Neuschäfer
2020-12-03 19:12   ` Enrico Weigelt, metux IT consult
2020-12-03 19:12     ` Enrico Weigelt, metux IT consult
2020-12-02 14:15 ` Bartosz Golaszewski
2020-12-02 14:15   ` Bartosz Golaszewski
2020-12-03 19:00   ` Enrico Weigelt, metux IT consult
2020-12-03 19:00     ` Enrico Weigelt, metux IT consult
2020-12-03 22:35     ` Michael Walle
2020-12-03 22:35       ` Michael Walle
2020-12-04  8:28       ` Enrico Weigelt, metux IT consult
2020-12-04  8:28         ` Enrico Weigelt, metux IT consult
2020-12-04  9:06         ` Michael Walle
2020-12-04  9:06           ` Michael Walle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.