On Wednesday, October 20, 2021, Viresh Kumar <[email protected]> wrote:
> This patch adds IRQ support for the virtio GPIO driver. Note that this > uses the irq_bus_lock/unlock() callbacks, since those operations over > virtio may sleep. > > Reviewed-by: Linus Walleij <[email protected]> > Signed-off-by: Viresh Kumar <[email protected]> > --- > Bartosz, > > The spec changes are close to merging now, I will let you know once the > ballot > is closed and the spec changes are merged. You can then pick this patch for > 5.16. > > V5->V6: > - Sent it separately as the other patches are already merged. > - Queue the buffers only after enabling the irq (as per latest spec). > - Migrate to raw_spinlock_t. > > drivers/gpio/Kconfig | 1 + > drivers/gpio/gpio-virtio.c | 310 ++++++++++++++++++++++++++++++- > include/uapi/linux/virtio_gpio.h | 25 +++ > 3 files changed, 332 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index fae5141251e5..bfa723ff8e7c 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1674,6 +1674,7 @@ config GPIO_MOCKUP > config GPIO_VIRTIO > tristate "VirtIO GPIO support" > depends on VIRTIO > + select GPIOLIB_IRQCHIP > help > Say Y here to enable guest support for virtio-based GPIO > controllers. > > diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c > index d24f1c9264bc..73fbe2eda4b9 100644 > --- a/drivers/gpio/gpio-virtio.c > +++ b/drivers/gpio/gpio-virtio.c > @@ -16,6 +16,7 @@ > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/mutex.h> > +#include <linux/spinlock.h> > #include <linux/virtio_config.h> > #include <uapi/linux/virtio_gpio.h> > #include <uapi/linux/virtio_ids.h> > @@ -28,12 +29,30 @@ struct virtio_gpio_line { > unsigned int rxlen; > }; > > +struct vgpio_irq_line { > + u8 type; > + bool disabled; > + bool masked; > + bool queued; > + bool update_pending; > + bool queue_pending; > + > + struct virtio_gpio_irq_request ireq ____cacheline_aligned; > + struct virtio_gpio_irq_response ires ____cacheline_aligned; > +}; > + > struct virtio_gpio { > struct virtio_device *vdev; > struct mutex lock; /* Protects virtqueue operation */ > struct gpio_chip gc; > struct virtio_gpio_line *lines; > struct virtqueue *request_vq; > + > + /* irq support */ > + struct virtqueue *event_vq; > + struct mutex irq_lock; /* Protects irq operation */ > + raw_spinlock_t eventq_lock; /* Protects queuing of the buffer */ > + struct vgpio_irq_line *irq_lines; > }; > > static int _virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio, > @@ -186,6 +205,244 @@ static void virtio_gpio_set(struct gpio_chip *gc, > unsigned int gpio, int value) > virtio_gpio_req(vgpio, VIRTIO_GPIO_MSG_SET_VALUE, gpio, value, > NULL); > } > > +/* Interrupt handling */ > +static void virtio_gpio_irq_prepare(struct virtio_gpio *vgpio, u16 gpio) > +{ > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[gpio]; > + struct virtio_gpio_irq_request *ireq = &irq_line->ireq; > + struct virtio_gpio_irq_response *ires = &irq_line->ires; > + struct scatterlist *sgs[2], req_sg, res_sg; > + int ret; > + > + if (WARN_ON(irq_line->queued || irq_line->masked || > irq_line->disabled)) > + return; > + > + ireq->gpio = cpu_to_le16(gpio); > + sg_init_one(&req_sg, ireq, sizeof(*ireq)); > + sg_init_one(&res_sg, ires, sizeof(*ires)); > + sgs[0] = &req_sg; > + sgs[1] = &res_sg; > + > + ret = virtqueue_add_sgs(vgpio->event_vq, sgs, 1, 1, irq_line, > GFP_ATOMIC); > + if (ret) { > + dev_err(&vgpio->vdev->dev, "failed to add request to > eventq\n"); > + return; > + } > + > + irq_line->queued = true; > + virtqueue_kick(vgpio->event_vq); > +} > + > +static void virtio_gpio_irq_enable(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + > + raw_spin_lock(&vgpio->eventq_lock); > + irq_line->disabled = false; > + irq_line->masked = false; > + irq_line->queue_pending = true; > + raw_spin_unlock(&vgpio->eventq_lock); > + > + irq_line->update_pending = true; > +} > + > +static void virtio_gpio_irq_disable(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + > + raw_spin_lock(&vgpio->eventq_lock); > + irq_line->disabled = true; > + irq_line->masked = true; > + irq_line->queue_pending = false; > + raw_spin_unlock(&vgpio->eventq_lock); > + > + irq_line->update_pending = true; > +} > + > +static void virtio_gpio_irq_mask(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + > + raw_spin_lock(&vgpio->eventq_lock); > + irq_line->masked = true; > + raw_spin_unlock(&vgpio->eventq_lock); > +} > + > +static void virtio_gpio_irq_unmask(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + > + raw_spin_lock(&vgpio->eventq_lock); > + irq_line->masked = false; > + > + /* Queue the buffer unconditionally on unmask */ > + virtio_gpio_irq_prepare(vgpio, d->hwirq); > + raw_spin_unlock(&vgpio->eventq_lock); > +} > + > +static int virtio_gpio_irq_set_type(struct irq_data *d, unsigned int > type) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + > + switch (type) { > + case IRQ_TYPE_NONE: > + type = VIRTIO_GPIO_IRQ_TYPE_NONE; > + break; IIRC you add dead code. IRQ framework never calls this if type is not set. > + case IRQ_TYPE_EDGE_RISING: > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING; > + break; > + case IRQ_TYPE_EDGE_FALLING: > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING; > + break; > + case IRQ_TYPE_EDGE_BOTH: > + type = VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH; > + break; > + case IRQ_TYPE_LEVEL_LOW: > + type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_LOW; > + break; > + case IRQ_TYPE_LEVEL_HIGH: > + type = VIRTIO_GPIO_IRQ_TYPE_LEVEL_HIGH; > + break; > + default: > + dev_err(&vgpio->vdev->dev, "unsupported irq type: %u\n", > type); > + return -EINVAL; > + } > + > + irq_line->type = type; > + irq_line->update_pending = true; > + > + return 0; > +} > + > +static void virtio_gpio_irq_bus_lock(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + > + mutex_lock(&vgpio->irq_lock); > +} > + > +static void virtio_gpio_irq_bus_sync_unlock(struct irq_data *d) > +{ > + struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > + struct virtio_gpio *vgpio = gpiochip_get_data(gc); > + struct vgpio_irq_line *irq_line = &vgpio->irq_lines[d->hwirq]; > + u8 type = irq_line->disabled ? VIRTIO_GPIO_IRQ_TYPE_NONE : > irq_line->type; > + unsigned long flags; > + > + if (irq_line->update_pending) { > + irq_line->update_pending = false; > + virtio_gpio_req(vgpio, VIRTIO_GPIO_MSG_IRQ_TYPE, d->hwirq, > type, > + NULL); > + > + /* Queue the buffer only after interrupt is enabled */ > + raw_spin_lock_irqsave(&vgpio->eventq_lock, flags); > + if (irq_line->queue_pending) { > + irq_line->queue_pending = false; > + virtio_gpio_irq_prepare(vgpio, d->hwirq); > + } > + raw_spin_unlock_irqrestore(&vgpio->eventq_lock, flags); > + } > + > + mutex_unlock(&vgpio->irq_lock); > +} > + > +static struct irq_chip vgpio_irq_chip = { > + .name = "virtio-gpio", > + .irq_enable = virtio_gpio_irq_enable, > + .irq_disable = virtio_gpio_irq_disable, > + .irq_mask = virtio_gpio_irq_mask, > + .irq_unmask = virtio_gpio_irq_unmask, > + .irq_set_type = virtio_gpio_irq_set_type, > + > + /* These are required to implement irqchip for slow busses */ > + .irq_bus_lock = virtio_gpio_irq_bus_lock, > + .irq_bus_sync_unlock = virtio_gpio_irq_bus_sync_unlock, > +}; > + > +static bool ignore_irq(struct virtio_gpio *vgpio, int gpio, > + struct vgpio_irq_line *irq_line) > +{ > + bool ignore = false; > + > + raw_spin_lock(&vgpio->eventq_lock); > + irq_line->queued = false; > + > + /* Interrupt is disabled currently */ > + if (irq_line->masked || irq_line->disabled) { > + ignore = true; > + goto unlock; > + } > + > + /* > + * Buffer is returned as the interrupt was disabled earlier, but is > + * enabled again now. Requeue the buffers. > + */ > + if (irq_line->ires.status == VIRTIO_GPIO_IRQ_STATUS_INVALID) { > + virtio_gpio_irq_prepare(vgpio, gpio); > + ignore = true; > + goto unlock; > + } > + > + if (WARN_ON(irq_line->ires.status != VIRTIO_GPIO_IRQ_STATUS_VALID)) > + ignore = true; > + > +unlock: > + raw_spin_unlock(&vgpio->eventq_lock); > + > + return ignore; > +} > + > +static void virtio_gpio_event_vq(struct virtqueue *vq) > +{ > + struct virtio_gpio *vgpio = vq->vdev->priv; > + struct device *dev = &vgpio->vdev->dev; > + struct vgpio_irq_line *irq_line; > + int irq, gpio, ret; > + unsigned int len; > + > + while (true) { > + irq_line = virtqueue_get_buf(vgpio->event_vq, &len); > + if (!irq_line) > + break; > + > + if (len != sizeof(irq_line->ires)) { > + dev_err(dev, "irq with incorrect length (%u : > %u)\n", > + len, (unsigned int)sizeof(irq_line->ires)); > + continue; > + } > + > + /* > + * Find GPIO line number from the offset of irq_line > within the > + * irq_lines block. We can also get GPIO number from > + * irq-request, but better not to rely on a buffer > returned by > + * remote. > + */ > + gpio = irq_line - vgpio->irq_lines; > + WARN_ON(gpio >= vgpio->gc.ngpio); > + > + if (unlikely(ignore_irq(vgpio, gpio, irq_line))) > + continue; > + > + irq = irq_find_mapping(vgpio->gc.irq.domain, gpio); > + WARN_ON(!irq); > + > + ret = generic_handle_irq(irq); IIRC there is a new API that basically combines the two above. > + if (ret) > + dev_err(dev, "failed to handle interrupt: %d\n", > ret); > + }; > +} > + > static void virtio_gpio_request_vq(struct virtqueue *vq) > { > struct virtio_gpio_line *line; > @@ -210,14 +467,15 @@ static void virtio_gpio_free_vqs(struct > virtio_device *vdev) > static int virtio_gpio_alloc_vqs(struct virtio_gpio *vgpio, > struct virtio_device *vdev) > { > - const char * const names[] = { "requestq" }; > + const char * const names[] = { "requestq", "eventq" }; > vq_callback_t *cbs[] = { > virtio_gpio_request_vq, > + virtio_gpio_event_vq, > }; > - struct virtqueue *vqs[1] = { NULL }; > + struct virtqueue *vqs[2] = { NULL, NULL }; > int ret; > > - ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL); > + ret = virtio_find_vqs(vdev, vgpio->irq_lines ? 2 : 1, vqs, cbs, > names, NULL); > if (ret) { > dev_err(&vdev->dev, "failed to find vqs: %d\n", ret); > return ret; > @@ -225,11 +483,23 @@ static int virtio_gpio_alloc_vqs(struct virtio_gpio > *vgpio, > > if (!vqs[0]) { > dev_err(&vdev->dev, "failed to find requestq vq\n"); > - return -ENODEV; > + goto out; > } > vgpio->request_vq = vqs[0]; > > + if (vgpio->irq_lines && !vqs[1]) { > + dev_err(&vdev->dev, "failed to find eventq vq\n"); > + goto out; > + } > + vgpio->event_vq = vqs[1]; > + > return 0; > + > +out: > + if (vqs[0] || vqs[1]) > + virtio_gpio_free_vqs(vdev); > + > + return -ENODEV; > } > > static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio, > @@ -325,6 +595,32 @@ static int virtio_gpio_probe(struct virtio_device > *vdev) > vgpio->gc.owner = THIS_MODULE; > vgpio->gc.can_sleep = true; > > + /* Interrupt support */ > + if (virtio_has_feature(vdev, VIRTIO_GPIO_F_IRQ)) { > + vgpio->irq_lines = devm_kcalloc(dev, ngpio, > + sizeof(*vgpio->irq_lines), > + GFP_KERNEL); One line? > + if (!vgpio->irq_lines) > + return -ENOMEM; > + > + /* 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; > + vgpio->gc.irq.chip = &vgpio_irq_chip; > + > + for (i = 0; i < ngpio; i++) { > + vgpio->irq_lines[i].type = > VIRTIO_GPIO_IRQ_TYPE_NONE; > + vgpio->irq_lines[i].disabled = true; > + vgpio->irq_lines[i].masked = true; > + } > + > + mutex_init(&vgpio->irq_lock); > + raw_spin_lock_init(&vgpio->eventq_lock); > + } > + > ret = virtio_gpio_alloc_vqs(vgpio, vdev); > if (ret) > return ret; > @@ -357,7 +653,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 0445f905d8cc..d04af9c5f0de 100644 > --- a/include/uapi/linux/virtio_gpio.h > +++ b/include/uapi/linux/virtio_gpio.h > @@ -5,12 +5,16 @@ > > #include <linux/types.h> > > +/* Virtio GPIO Feature bits */ > +#define VIRTIO_GPIO_F_IRQ 0 > + > /* Virtio GPIO request types */ > #define VIRTIO_GPIO_MSG_GET_NAMES 0x0001 > #define VIRTIO_GPIO_MSG_GET_DIRECTION 0x0002 > #define VIRTIO_GPIO_MSG_SET_DIRECTION 0x0003 > #define VIRTIO_GPIO_MSG_GET_VALUE 0x0004 > #define VIRTIO_GPIO_MSG_SET_VALUE 0x0005 > +#define VIRTIO_GPIO_MSG_IRQ_TYPE 0x0006 > > /* Possible values of the status field */ > #define VIRTIO_GPIO_STATUS_OK 0x0 > @@ -21,6 +25,14 @@ > #define VIRTIO_GPIO_DIRECTION_OUT 0x01 > #define VIRTIO_GPIO_DIRECTION_IN 0x02 > > +/* 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 > + > struct virtio_gpio_config { > __le16 ngpio; > __u8 padding[2]; > @@ -44,4 +56,17 @@ struct virtio_gpio_response_get_names { > __u8 value[]; > }; > > +/* Virtio GPIO IRQ Request / Response */ > +struct virtio_gpio_irq_request { > + __le16 gpio; > +}; > + > +struct virtio_gpio_irq_response { > + __u8 status; > +}; > > I’m wondering if those above should be packed. > +/* Possible values of the interrupt status field */ > +#define VIRTIO_GPIO_IRQ_STATUS_INVALID 0x0 > +#define VIRTIO_GPIO_IRQ_STATUS_VALID 0x1 > + > #endif /* _LINUX_VIRTIO_GPIO_H */ > -- > 2.31.1.272.g89b43f80a514 > > -- With Best Regards, Andy Shevchenko
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
