diff mbox series

drivers: gpio: add virtio-gpio guest driver

Message ID 20201127183003.2849-1-info@metux.net (mailing list archive)
State New, archived
Headers show
Series drivers: gpio: add virtio-gpio guest driver | expand

Commit Message

Enrico Weigelt, metux IT consult Nov. 27, 2020, 6:30 p.m. UTC
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

Comments

Randy Dunlap Nov. 27, 2020, 6:45 p.m. UTC | #1
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
kernel test robot Nov. 27, 2020, 7:33 p.m. UTC | #2
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
Michael S. Tsirkin Nov. 29, 2020, 8:11 p.m. UTC | #3
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
J. Neuschäfer Nov. 29, 2020, 10:10 p.m. UTC | #4
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
Bartosz Golaszewski Dec. 2, 2020, 2:15 p.m. UTC | #5
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
>
Enrico Weigelt, metux IT consult Dec. 3, 2020, 7 p.m. UTC | #6
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
Enrico Weigelt, metux IT consult Dec. 3, 2020, 7:01 p.m. UTC | #7
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
> 
>
Enrico Weigelt, metux IT consult Dec. 3, 2020, 7:12 p.m. UTC | #8
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
>
Michael Walle Dec. 3, 2020, 10:35 p.m. UTC | #9
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
Enrico Weigelt, metux IT consult Dec. 4, 2020, 8:28 a.m. UTC | #10
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
Michael Walle Dec. 4, 2020, 9:06 a.m. UTC | #11
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
diff mbox series

Patch

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