Re: [RFC PATCH] virtio-mmio: support for multiple irqs
On Thu, Nov 06, 2014 at 05:54:54PM +0800, Shannon Zhao wrote:
> On 2014/11/6 17:34, Michael S. Tsirkin wrote:
> > On Tue, Nov 04, 2014 at 05:35:12PM +0800, Shannon Zhao wrote:
> >> As the current virtio-mmio only support single irq,
> >> so some advanced features such as vhost-net with irqfd
> >> are not supported. And the net performance is not
> >> the best without vhost-net and irqfd supporting.
> >>
> >> This patch support virtio-mmio to request multiple
> >> irqs like virtio-pci. With this patch and qemu assigning
> >> multiple irqs for virtio-mmio device, it's ok to use
> >> vhost-net with irqfd on arm/arm64.
> >>
> >> As arm doesn't support msi-x now, we use GSI for
> >> multiple irq. In this patch we use "vm_try_to_find_vqs"
> >> to check whether multiple irqs are supported like
> >> virtio-pci.
> >>
> >> Is this the right direction? is there other ways to
> >> make virtio-mmio support multiple irq? Hope for feedback.
> >> Thanks.
> >>
> >> Signed-off-by: Shannon Zhao
> >
> >
> > So how does guest discover whether host device supports multiple IRQs?
>
> Guest uses vm_try_to_find_vqs to check whether it can get multiple IRQs
> like virtio-pci uses vp_try_to_find_vqs. And within function
> vm_request_multiple_irqs, guest check whether the number of IRQs host
> device gives is equal to the number we want.
OK but how does host specify the number of IRQs for a device?
for pci this is done through the MSI-X capability register.
> for (i = 0; i < nirqs; i++) {
> irq = platform_get_irq(vm_dev->pdev, i);
> if (irq == -ENXIO)
> goto error;
> }
>
> If we can't get the expected number of IRQs, return error and this try
> fails. Then guest will try two IRQS and single IRQ like virtio-pci.
>
> > Could you please document the new interface?
> > E.g. send a patch for virtio spec.
>
> Ok, I'll send it later. Thank you very much :)
>
> Shannon
>
> > I think this really should be controlled by hypervisor, per device.
> > I'm also tempted to make this a virtio 1.0 only feature.
> >
> >
> >
> >> ---
> >> drivers/virtio/virtio_mmio.c | 234
> >> --
> >> 1 files changed, 203 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> >> index c600ccf..2b7d935 100644
> >> --- a/drivers/virtio/virtio_mmio.c
> >> +++ b/drivers/virtio/virtio_mmio.c
> >> @@ -122,6 +122,15 @@ struct virtio_mmio_device {
> >>/* a list of queues so we can dispatch IRQs */
> >>spinlock_t lock;
> >>struct list_head virtqueues;
> >> +
> >> + /* multiple irq support */
> >> + int single_irq_enabled;
> >> + /* Number of available irqs */
> >> + unsigned num_irqs;
> >> + /* Used number of irqs */
> >> + int used_irqs;
> >> + /* Name strings for interrupts. */
> >> + char (*vm_vq_names)[256];
> >> };
> >>
> >> struct virtio_mmio_vq_info {
> >> @@ -229,33 +238,53 @@ static bool vm_notify(struct virtqueue *vq)
> >>return true;
> >> }
> >>
> >> +/* Handle a configuration change: Tell driver if it wants to know. */
> >> +static irqreturn_t vm_config_changed(int irq, void *opaque)
> >> +{
> >> + struct virtio_mmio_device *vm_dev = opaque;
> >> + struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
> >> + struct virtio_driver, driver);
> >> +
> >> + if (vdrv && vdrv->config_changed)
> >> + vdrv->config_changed(&vm_dev->vdev);
> >> + return IRQ_HANDLED;
> >> +}
> >> +
> >> /* Notify all virtqueues on an interrupt. */
> >> -static irqreturn_t vm_interrupt(int irq, void *opaque)
> >> +static irqreturn_t vm_vring_interrupt(int irq, void *opaque)
> >> {
> >>struct virtio_mmio_device *vm_dev = opaque;
> >>struct virtio_mmio_vq_info *info;
> >> - struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
> >> - struct virtio_driver, driver);
> >> - unsigned long status;
> >> + irqreturn_t ret = IRQ_NONE;
> >>unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&vm_dev->lock, flags);
> >> + list_for_each_entry(info, &vm_dev->virtqueues, node) {
> >> + if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> >> + ret = IRQ_HANDLED;
> >> + }
> >> + spin_unlock_irqrestore(&vm_dev->lock, flags);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +/* Notify all virtqueues and handle a configuration
> >> + * change on an interrupt. */
> >> +static irqreturn_t vm_interrupt(int irq, void *opaque)
> >> +{
> >> + struct virtio_mmio_device *vm_dev = opaque;
> >> + unsigned long status;
> >>irqreturn_t ret = IRQ_NONE;
> >>
> >>/* Read and acknowledge interrupts */
> >>status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
> >>writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
> >>
> >> - if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
> >> - && vdrv && vdrv->config_changed) {
> >> - vdrv->config_changed(&vm_dev->vdev);
Re: [RFC PATCH] virtio-mmio: support for multiple irqs
On Tue, Nov 04, 2014 at 05:35:12PM +0800, Shannon Zhao wrote:
> As the current virtio-mmio only support single irq,
> so some advanced features such as vhost-net with irqfd
> are not supported. And the net performance is not
> the best without vhost-net and irqfd supporting.
>
> This patch support virtio-mmio to request multiple
> irqs like virtio-pci. With this patch and qemu assigning
> multiple irqs for virtio-mmio device, it's ok to use
> vhost-net with irqfd on arm/arm64.
>
> As arm doesn't support msi-x now, we use GSI for
> multiple irq. In this patch we use "vm_try_to_find_vqs"
> to check whether multiple irqs are supported like
> virtio-pci.
>
> Is this the right direction? is there other ways to
> make virtio-mmio support multiple irq? Hope for feedback.
> Thanks.
>
> Signed-off-by: Shannon Zhao
So how does guest discover whether host device supports multiple IRQs?
Could you please document the new interface?
E.g. send a patch for virtio spec.
I think this really should be controlled by hypervisor, per device.
I'm also tempted to make this a virtio 1.0 only feature.
> ---
> drivers/virtio/virtio_mmio.c | 234
> --
> 1 files changed, 203 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index c600ccf..2b7d935 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -122,6 +122,15 @@ struct virtio_mmio_device {
> /* a list of queues so we can dispatch IRQs */
> spinlock_t lock;
> struct list_head virtqueues;
> +
> + /* multiple irq support */
> + int single_irq_enabled;
> + /* Number of available irqs */
> + unsigned num_irqs;
> + /* Used number of irqs */
> + int used_irqs;
> + /* Name strings for interrupts. */
> + char (*vm_vq_names)[256];
> };
>
> struct virtio_mmio_vq_info {
> @@ -229,33 +238,53 @@ static bool vm_notify(struct virtqueue *vq)
> return true;
> }
>
> +/* Handle a configuration change: Tell driver if it wants to know. */
> +static irqreturn_t vm_config_changed(int irq, void *opaque)
> +{
> + struct virtio_mmio_device *vm_dev = opaque;
> + struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
> + struct virtio_driver, driver);
> +
> + if (vdrv && vdrv->config_changed)
> + vdrv->config_changed(&vm_dev->vdev);
> + return IRQ_HANDLED;
> +}
> +
> /* Notify all virtqueues on an interrupt. */
> -static irqreturn_t vm_interrupt(int irq, void *opaque)
> +static irqreturn_t vm_vring_interrupt(int irq, void *opaque)
> {
> struct virtio_mmio_device *vm_dev = opaque;
> struct virtio_mmio_vq_info *info;
> - struct virtio_driver *vdrv = container_of(vm_dev->vdev.dev.driver,
> - struct virtio_driver, driver);
> - unsigned long status;
> + irqreturn_t ret = IRQ_NONE;
> unsigned long flags;
> +
> + spin_lock_irqsave(&vm_dev->lock, flags);
> + list_for_each_entry(info, &vm_dev->virtqueues, node) {
> + if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
> + ret = IRQ_HANDLED;
> + }
> + spin_unlock_irqrestore(&vm_dev->lock, flags);
> +
> + return ret;
> +}
> +
> +/* Notify all virtqueues and handle a configuration
> + * change on an interrupt. */
> +static irqreturn_t vm_interrupt(int irq, void *opaque)
> +{
> + struct virtio_mmio_device *vm_dev = opaque;
> + unsigned long status;
> irqreturn_t ret = IRQ_NONE;
>
> /* Read and acknowledge interrupts */
> status = readl(vm_dev->base + VIRTIO_MMIO_INTERRUPT_STATUS);
> writel(status, vm_dev->base + VIRTIO_MMIO_INTERRUPT_ACK);
>
> - if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)
> - && vdrv && vdrv->config_changed) {
> - vdrv->config_changed(&vm_dev->vdev);
> - ret = IRQ_HANDLED;
> - }
> + if (unlikely(status & VIRTIO_MMIO_INT_CONFIG))
> + return vm_config_changed(irq, opaque);
>
> - if (likely(status & VIRTIO_MMIO_INT_VRING)) {
> - spin_lock_irqsave(&vm_dev->lock, flags);
> - list_for_each_entry(info, &vm_dev->virtqueues, node)
> - ret |= vring_interrupt(irq, info->vq);
> - spin_unlock_irqrestore(&vm_dev->lock, flags);
> - }
> + if (likely(status & VIRTIO_MMIO_INT_VRING))
> + return vm_vring_interrupt(irq, opaque);
>
> return ret;
> }
> @@ -284,18 +313,98 @@ static void vm_del_vq(struct virtqueue *vq)
> kfree(info);
> }
>
> -static void vm_del_vqs(struct virtio_device *vdev)
> +static void vm_free_irqs(struct virtio_device *vdev)
> {
> + int i;
> struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
> +
> + if (vm_dev->single_irq_enabled) {
> + free_irq(platform_get_irq(vm_dev->pdev, 0), vm_dev);
> + vm_dev->single_irq_enabled = 0;
> + }
> +
> +
Re: [Qemu-devel] [RFC PATCH 0/2] virtio-mmio: add irqfd support for vhost-net based on virtio-mmio
On 2014/11/6 9:59, Shannon Zhao wrote: > > > On 2014/11/5 16:43, Eric Auger wrote: >> On 10/27/2014 12:23 PM, Li Liu wrote: >>> >>> >>> On 2014/10/27 17:37, Peter Maydell wrote: On 25 October 2014 09:24, john.liuli wrote: > To get the interrupt reason to support such VIRTIO_NET_F_STATUS > features I add a new register offset VIRTIO_MMIO_ISRMEM which > will help to establish a shared memory region between qemu and > virtio-mmio device. Then the interrupt reason can be accessed by > guest driver through this region. At the same time, the virtio-mmio > dirver check this region to see irqfd is supported or not during > the irq handler registration, and different handler will be assigned. If you want to add a new register you should probably propose an update to the virtio spec. However, it seems to me it would be better to get generic PCI/PCIe working on the ARM virt board instead; then we can let virtio-mmio quietly fade away. This has been on the todo list for ages (and there have been RFC patches posted for plain PCI), it's just nobody's had time to work on it. thanks -- PMM >>> >>> So you mean virtio-mmio will be replaced by PCI/PCIe on ARM at last? >>> If so, let this patch go with the wind:). Thx. >> >> Hi, >> >> As a fix of current situation where ISR is only partially updated when >> vhost-irqfd handles standard IRQ and waiting for PCI emuluation, >> wouldn't it make sense to store ISR content on vhost driver side and >> introduce ioctls to read/write it. When using vhost BE, virtio QEMU >> device would use those ioctl to read/update the ISR content. On top of >> that we would update the ISR in vhost before triggering the irqfd. If I >> do not miss anything this would at least make things functional with irqfd. >> >> As a second step, we could try to introduce in-kernel emulation of >> ISR/ACK to fix the performance issue related to going to user-side each >> time ISR/ACK accesses are done. >> >> Do you think it is worth investigating this direction? >> > Hi, > > About this problem I have a talk with Li Liu. As MST said, we could use > multiple GSI to support vhost-net with irqfd. And we have figured out a way > to solve this problem. The method is as same as virtio-pci which is to assign > multiple irqs for virtio-mmio. Also it can support multiqueue virtio-net on > arm. > > Would you have a look at this method? Thank you very much. > > - virtio-mmio: support for multiple irqs > http://www.spinics.net/lists/kernel/msg1858860.html > > Thanks, > Shannon > Yeah, I think multiple GSI is more compatible with MSI-X. And even virtio-mmio will fade away at last. It still make senses for ARM32 which can't support PCI/PCIe. BTW, this patch is handed over to Shannon and please refer to new patch at http://www.spinics.net/lists/kernel/msg1858860.html. Li. >> Thank you in advance >> >> Best Regards >> >> Eric >> >> >>> >>> Li. . > > > . > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
