thr3ads.net - Virtualization - [PATCH V3 2/3] gpio: virtio: Add IRQ support [Jun 2021]

If this information is useful, please help other people find it:
Share via:

Viresh Kumar

2021-Jun-10 12:16 UTC

[PATCH V3 2/3] gpio: virtio: Add IRQ support

This patch adds IRQ support for the virtio GPIO driver. Note that this
uses the irq_bus_lock/unlock() callbacks since the operations over
virtio can sleep.

Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
---
 drivers/gpio/gpio-virtio.c       | 256 ++++++++++++++++++++++++++++++-
 include/uapi/linux/virtio_gpio.h |  15 ++
 2 files changed, 263 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
index 1ba8ddf4693a..8bc4b1876961 100644
--- a/drivers/gpio/gpio-virtio.c
+++ b/drivers/gpio/gpio-virtio.c
@@ -20,6 +20,13 @@
 #include <uapi/linux/virtio_gpio.h>
 #include <uapi/linux/virtio_ids.h>
 
+struct vgpio_line {
+	u8 irq_type;
+	bool irq_type_pending;
+	bool masked;
+	bool masked_pending;
+};
+
 struct virtio_gpio {
 	struct virtio_device *vdev;
 	struct mutex lock;
@@ -30,14 +37,20 @@ struct virtio_gpio {
 	struct virtio_gpio_request creq;
 	struct virtio_gpio_response cres;
 
+	struct irq_chip *ic;
+	struct vgpio_line *lines;
+	struct virtqueue *interrupt_vq;
+	struct virtio_gpio_request ireq;
+
 	/* This must be kept as the last entry here, hint: gpio_names[0] */
 	struct virtio_gpio_config config;
 };
 
 #define gpio_chip_to_vgpio(chip) container_of(chip, struct virtio_gpio, gc)
+#define irq_data_to_gpio_chip(d) (d->domain->host_data)
 
-static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,
-			   u8 txdata, u8 *rxdata)
+static int virtio_gpio_req_unlocked(struct virtio_gpio *vgpio, u16 type,
+				    u16 gpio, u8 txdata, u8 *rxdata)
 {
 	struct virtio_gpio_response *res = &vgpio->cres;
 	struct virtio_gpio_request *req = &vgpio->creq;
@@ -56,11 +69,10 @@ static int virtio_gpio_req(struct virtio_gpio *vgpio, u16
type, u16 gpio,
 	sgs[0] = &req_sg;
 	sgs[1] = &res_sg;
 
-	mutex_lock(&vgpio->lock);
 	ret = virtqueue_add_sgs(vgpio->command_vq, sgs, 1, 1, res, GFP_KERNEL);
 	if (ret) {
 		dev_err(dev, "failed to add request to vq\n");
-		goto out;
+		return ret;
 	}
 
 	reinit_completion(&vgpio->completion);
@@ -81,7 +93,16 @@ static int virtio_gpio_req(struct virtio_gpio *vgpio, u16
type, u16 gpio,
 	if (rxdata)
 		*rxdata = res->data;
 
-out:
+	return ret;
+}
+
+static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,
+			   u8 txdata, u8 *rxdata)
+{
+	int ret;
+
+	mutex_lock(&vgpio->lock);
+	ret = virtio_gpio_req_unlocked(vgpio, type, gpio, txdata, rxdata);
 	mutex_unlock(&vgpio->lock);
 
 	return ret;
@@ -152,6 +173,183 @@ static void virtio_gpio_set(struct gpio_chip *gc, unsigned
int gpio, int value)
 	virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_SET_VALUE, gpio, value, NULL);
 }
 
+/* Interrupt handling */
+static void vgpio_irq_mask(struct virtio_gpio *vgpio, u16 gpio)
+{
+	int ret;
+
+	ret = virtio_gpio_req_unlocked(vgpio, VIRTIO_GPIO_REQ_IRQ_MASK, gpio, 0,
+				       NULL);
+	if (ret)
+		dev_err(&vgpio->vdev->dev, "failed to mask irq: %d\n",
ret);
+}
+
+static void vgpio_irq_unmask(struct virtio_gpio *vgpio, u16 gpio)
+{
+	int ret;
+
+	ret = virtio_gpio_req_unlocked(vgpio, VIRTIO_GPIO_REQ_IRQ_UNMASK, gpio,
+				       0, NULL);
+	if (ret)
+		dev_err(&vgpio->vdev->dev, "failed to unmask irq: %d\n",
ret);
+}
+
+static void vgpio_irq_set_type(struct virtio_gpio *vgpio, u16 gpio, u8 type)
+{
+	int ret;
+
+	ret = virtio_gpio_req_unlocked(vgpio, VIRTIO_GPIO_REQ_IRQ_TYPE, gpio,
+				       type, NULL);
+	if (ret)
+		dev_err(&vgpio->vdev->dev, "failed to set irq type:
%d\n", ret);
+}
+
+static void virtio_gpio_irq_mask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_to_gpio_chip(d);
+	struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
+	struct vgpio_line *line = &vgpio->lines[d->hwirq];
+
+	line->masked = true;
+	line->masked_pending = true;
+}
+
+static void virtio_gpio_irq_unmask(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_to_gpio_chip(d);
+	struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
+	struct vgpio_line *line = &vgpio->lines[d->hwirq];
+
+	line->masked = false;
+	line->masked_pending = true;
+}
+
+static int virtio_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	struct gpio_chip *gc = irq_data_to_gpio_chip(d);
+	struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
+	struct vgpio_line *line = &vgpio->lines[d->hwirq];
+	u8 irq_type;
+
+	switch (type) {
+	case IRQ_TYPE_NONE:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_NONE;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING;
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW;
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		irq_type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH;
+		break;
+	default:
+		dev_err(&vgpio->vdev->dev, "unsupported irq type: %u\n",
type);
+		return -EINVAL;
+	}
+
+	line->irq_type = irq_type;
+	line->irq_type_pending = true;
+
+	return 0;
+}
+
+static void virtio_gpio_irq_bus_lock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_to_gpio_chip(d);
+	struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
+
+	mutex_lock(&vgpio->lock);
+}
+
+static void virtio_gpio_irq_bus_sync_unlock(struct irq_data *d)
+{
+	struct gpio_chip *gc = irq_data_to_gpio_chip(d);
+	struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
+	struct vgpio_line *line = &vgpio->lines[d->hwirq];
+
+	if (line->irq_type_pending) {
+		vgpio_irq_set_type(vgpio, d->hwirq, line->irq_type);
+		line->irq_type_pending = false;
+	}
+
+	if (line->masked_pending) {
+		if (line->masked)
+			vgpio_irq_mask(vgpio, d->hwirq);
+		else
+			vgpio_irq_unmask(vgpio, d->hwirq);
+		line->masked_pending = false;
+	}
+
+	mutex_unlock(&vgpio->lock);
+}
+
+static void virtio_gpio_interrupt_prepare(struct virtio_gpio *vgpio)
+{
+	struct scatterlist sg[1];
+
+	/* Clear request buffer */
+	vgpio->ireq.type = 0;
+	vgpio->ireq.gpio = vgpio->config.ngpio;
+
+	sg_init_one(sg, &vgpio->ireq, sizeof(vgpio->ireq));
+	virtqueue_add_inbuf(vgpio->interrupt_vq, sg, 1, vgpio, GFP_KERNEL);
+	virtqueue_kick(vgpio->interrupt_vq);
+}
+
+static void virtio_gpio_interrupt(struct virtqueue *vq)
+{
+	struct virtio_gpio *vgpio = vq->vdev->priv;
+	struct device *dev = &vq->vdev->dev;
+	unsigned int len;
+	int irq, gpio, ret;
+	void *data;
+
+	data = virtqueue_get_buf(vgpio->interrupt_vq, &len);
+	if (!data || !len) {
+		dev_warn(dev, "No data received on interrupt\n");
+		return;
+	}
+
+	if (WARN_ON(data != vgpio))
+		return;
+
+	if (le16_to_cpu(vgpio->ireq.type) != VIRTIO_GPIO_REQ_IRQ_EVENT) {
+		dev_warn(dev, "Invalid irq-type: %d\n", vgpio->ireq.type);
+		goto out;
+	}
+
+	gpio = le16_to_cpu(vgpio->ireq.gpio);
+
+	if (gpio >= vgpio->config.ngpio) {
+		dev_warn(dev, "Invalid GPIO number: %d\n", gpio);
+		goto out;
+	}
+
+	irq = irq_find_mapping(vgpio->gc.irq.domain, gpio);
+	if (!irq) {
+		dev_err(dev, "Failed to find IRQ for gpio: %d\n", gpio);
+		goto out;
+	}
+
+	local_irq_disable();
+	ret = generic_handle_irq(irq);
+	local_irq_enable();
+
+	if (ret)
+		dev_err(dev, "failed to invoke irq handler\n");
+
+out:
+	virtio_gpio_interrupt_prepare(vgpio);
+}
+
 static void virtio_gpio_command(struct virtqueue *vq)
 {
 	struct virtio_gpio *vgpio = vq->vdev->priv;
@@ -162,14 +360,15 @@ static void virtio_gpio_command(struct virtqueue *vq)
 static int virtio_gpio_alloc_vqs(struct virtio_device *vdev)
 {
 	struct virtio_gpio *vgpio = vdev->priv;
-	const char * const names[] = { "command" };
+	const char * const names[] = { "command", "interrupt" };
 	vq_callback_t *cbs[] = {
 		virtio_gpio_command,
+		virtio_gpio_interrupt,
 	};
-	struct virtqueue *vqs[1] = {NULL};
+	struct virtqueue *vqs[2] = {NULL};
 	int ret;
 
-	ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL);
+	ret = virtio_find_vqs(vdev, vgpio->ic ? 2 : 1, vqs, cbs, names, NULL);
 	if (ret) {
 		dev_err(&vdev->dev, "failed to allocate vqs: %d\n", ret);
 		return ret;
@@ -177,6 +376,13 @@ static int virtio_gpio_alloc_vqs(struct virtio_device
*vdev)
 
 	vgpio->command_vq = vqs[0];
 
+	if (vgpio->ic) {
+		vgpio->interrupt_vq = vqs[1];
+
+		virtio_gpio_interrupt_prepare(vgpio);
+		virtqueue_enable_cb(vgpio->interrupt_vq);
+	}
+
 	/* Mark the device ready to perform operations from within probe() */
 	virtio_device_ready(vgpio->vdev);
 	return 0;
@@ -278,6 +484,34 @@ static int virtio_gpio_probe(struct virtio_device *vdev)
 	if (IS_ERR(vgpio->gc.names))
 		return PTR_ERR(vgpio->gc.names);
 
+	if (virtio_has_feature(vdev, VIRTIO_GPIO_F_IRQ)) {
+		vgpio->lines = devm_kcalloc(dev, config->ngpio,
+					    sizeof(*vgpio->lines), GFP_KERNEL);
+		if (!vgpio->lines)
+			return -ENOMEM;
+
+		vgpio->ic = devm_kzalloc(dev, sizeof(*vgpio->ic), GFP_KERNEL);
+		if (!vgpio->ic)
+			return -ENOMEM;
+
+		vgpio->gc.irq.chip		= vgpio->ic;
+		vgpio->ic->name			= vgpio->gc.label;
+		vgpio->ic->irq_mask		= virtio_gpio_irq_mask;
+		vgpio->ic->irq_unmask		= virtio_gpio_irq_unmask;
+		vgpio->ic->irq_set_type		= virtio_gpio_irq_set_type;
+
+		/* These are required to implement irqchip for slow busses */
+		vgpio->ic->irq_bus_lock		= virtio_gpio_irq_bus_lock;
+		vgpio->ic->irq_bus_sync_unlock	= virtio_gpio_irq_bus_sync_unlock;
+
+		/* The event comes from the outside so no parent handler */
+		vgpio->gc.irq.parent_handler	= NULL;
+		vgpio->gc.irq.num_parents	= 0;
+		vgpio->gc.irq.parents		= NULL;
+		vgpio->gc.irq.default_type	= IRQ_TYPE_NONE;
+		vgpio->gc.irq.handler		= handle_level_irq;
+	}
+
 	vdev->priv = vgpio;
 	mutex_init(&vgpio->lock);
 	init_completion(&vgpio->completion);
@@ -309,7 +543,13 @@ static const struct virtio_device_id id_table[] = {
 };
 MODULE_DEVICE_TABLE(virtio, id_table);
 
+static const unsigned int features[] = {
+	VIRTIO_GPIO_F_IRQ,
+};
+
 static struct virtio_driver virtio_gpio_driver = {
+	.feature_table		= features,
+	.feature_table_size	= ARRAY_SIZE(features),
 	.id_table		= id_table,
 	.probe			= virtio_gpio_probe,
 	.remove			= virtio_gpio_remove,
diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h
index e805e33a79cb..533d70d77d2c 100644
--- a/include/uapi/linux/virtio_gpio.h
+++ b/include/uapi/linux/virtio_gpio.h
@@ -5,6 +5,9 @@
 
 #include <linux/types.h>
 
+/* Virtio GPIO Feature bits */
+#define VIRTIO_GPIO_F_IRQ			0
+
 /* Virtio GPIO request types */
 #define VIRTIO_GPIO_REQ_ACTIVATE		0x01
 #define VIRTIO_GPIO_REQ_DEACTIVATE		0x02
@@ -13,6 +16,18 @@
 #define VIRTIO_GPIO_REQ_DIRECTION_OUT		0x05
 #define VIRTIO_GPIO_REQ_GET_VALUE		0x06
 #define VIRTIO_GPIO_REQ_SET_VALUE		0x07
+#define VIRTIO_GPIO_REQ_IRQ_TYPE		0x08
+#define VIRTIO_GPIO_REQ_IRQ_MASK		0x09
+#define VIRTIO_GPIO_REQ_IRQ_UNMASK		0x0a
+#define VIRTIO_GPIO_REQ_IRQ_EVENT		0x0b
+
+/* Virtio GPIO IRQ types */
+#define VIRTIO_GPIO_IRQ_TYPE_NONE		0x00
+#define VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING	0x01
+#define VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING	0x02
+#define VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH		0x03
+#define VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH		0x04
+#define VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW		0x08
 
 /* Possible values of the status field */
 #define VIRTIO_GPIO_STATUS_OK			0x0
-- 
2.31.1.272.g89b43f80a514

Linus Walleij

2021-Jun-10 21:30 UTC

head link

[PATCH V3 2/3] gpio: virtio: Add IRQ support

Hi Viresh!

thanks for this interesting patch!

On Thu, Jun 10, 2021 at 2:16 PM Viresh Kumar <viresh.kumar at linaro.org>
wrote:
> This patch adds IRQ support for the virtio GPIO driver. Note that this
> uses the irq_bus_lock/unlock() callbacks since the operations over
> virtio can sleep.
>
> Signed-off-by: Viresh Kumar <viresh.kumar at linaro.org>
>  drivers/gpio/gpio-virtio.c       | 256 ++++++++++++++++++++++++++++++-
>  include/uapi/linux/virtio_gpio.h |  15 ++
You also need to
select GPIOLIB_IRQCHIP
in the Kconfig entry for the gpio-virtio driver, because you make
use of it.
> +static void virtio_gpio_irq_mask(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_to_gpio_chip(d);
> +       struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
> +       struct vgpio_line *line = &vgpio->lines[d->hwirq];
> +
> +       line->masked = true;
> +       line->masked_pending = true;
> +}
> +
> +static void virtio_gpio_irq_unmask(struct irq_data *d)
> +{
> +       struct gpio_chip *gc = irq_data_to_gpio_chip(d);
> +       struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc);
> +       struct vgpio_line *line = &vgpio->lines[d->hwirq];
> +
> +       line->masked = false;
> +       line->masked_pending = true;
> +}
This looks dangerous in combination with this:
> +static void virtio_gpio_interrupt(struct virtqueue *vq)
> +{
(...)> +       local_irq_disable();
> +       ret = generic_handle_irq(irq);
> +       local_irq_enable();
Nominally slow IRQs like those being marshalled over
virtio should be nested, handle_nested_irq(irq);
but are they? Or are they just quite slow not super slow?

If a threaded IRQF_ONESHOT was requested the
IRQ core will kick the thread and *MASK* this IRQ,
which means it will call back to your .irq_mask() function
and expect it to be masked from this
point.

But the IRQ will not actually be masked until you issue
your callbacks in the .irq_bus_sync_unlock() callback
right?

So from this point until .irq_bus_sync_unlock()
get called and actually mask the IRQ, it could be
fired again? I suppose the IRQ handler is reentrant?
This would violate the API.

I would say that from this point and until you sync
you need a spinlock or other locking primitive to
stop this IRQ from fireing again, and a spinlock will
imply local_irq_disable() so this gets really complex.

I would say only using nesting IRQs or guarantee this
some other way, one way would be to specify that
whatever is at the other side of virtio cannot send another
GPIO IRQ message before the last one is handled,
so you would need to send a specific (new)
VIRTIO_GPIO_REQ_IRQ_ACK after all other messages
have been sent in .irq_bus_sync_unlock()
so that the next GPIO IRQ can be dispatched after that.

(Is this how messaged signalled interrupts work? No idea.
When in doubt ask the IRQ maintainers.)

Thanks,
Linus Walleij

Virtualization - Jun 2021 - [PATCH V3 2/3] gpio: virtio: Add IRQ support

[PATCH V3 2/3] gpio: virtio: Add IRQ support

[PATCH V3 2/3] gpio: virtio: Add IRQ support