Hi,
This patch series implements multiple interrupt vectors support for
virtio-mmio device. This is especially useful for multiqueue vhost-net
device when using firecracker micro-vms as the guest.
Test result:
With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
receive 9 times more pps comparing with only one irq:
- 564830.38 rxpck/s for 8 irqs on
- 67665.06 rxpck/s for 1 irq on
Please help to review, thanks!
Have a nice day
Fei
Fam Zheng (1):
virtio-mmio: Process vrings more proactively
Fei Li (1):
virtio-mmio: support multiple interrupt vectors
drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
1 file changed, 196 insertions(+), 42 deletions(-)
--
2.11.0
From: Fam Zheng <[email protected]>
This allows the backend to _not_ trap mmio read of the status register
after injecting IRQ in the data path, which can improve the performance
significantly by avoiding a vmexit for each interrupt.
More importantly it also makes it possible for Firecracker to hook up
virtio-mmio with vhost-net, in which case there isn't a way to implement
proper status register handling.
For a complete backend that does set either INT_CONFIG bit or INT_VRING
bit upon generating irq, what happens hasn't changed.
Signed-off-by: Fam Zheng <[email protected]>
---
drivers/virtio/virtio_mmio.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index e09edb5c5e06..9b42502b2204 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -295,9 +295,7 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
virtio_config_changed(&vm_dev->vdev);
ret = IRQ_HANDLED;
- }
-
- if (likely(status & VIRTIO_MMIO_INT_VRING)) {
+ } else {
spin_lock_irqsave(&vm_dev->lock, flags);
list_for_each_entry(info, &vm_dev->virtqueues, node)
ret |= vring_interrupt(irq, info->vq);
--
2.11.0
On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> Hi,
>
> This patch series implements multiple interrupt vectors support for
> virtio-mmio device. This is especially useful for multiqueue vhost-net
> device when using firecracker micro-vms as the guest.
>
> Test result:
> With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> receive 9 times more pps comparing with only one irq:
> - 564830.38 rxpck/s for 8 irqs on
> - 67665.06 rxpck/s for 1 irq on
>
> Please help to review, thanks!
>
> Have a nice day
> Fei
Interesting. The spec says though:
4.2.3.4
Notifications From The Device
The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
used buffer notification or a configuration change notification to the device.
So I'm guessing we need to change the host/guest interface?
If true pls cc virtio-dev.
Also, do we need to update dt bindings documentation?
>
> Fam Zheng (1):
> virtio-mmio: Process vrings more proactively
>
> Fei Li (1):
> virtio-mmio: support multiple interrupt vectors
>
> drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> 1 file changed, 196 insertions(+), 42 deletions(-)
>
> --
> 2.11.0
On Fri, Jul 19, 2019 at 09:31:34PM +0800, Fei Li wrote:
> From: Fam Zheng <[email protected]>
>
> This allows the backend to _not_ trap mmio read of the status register
> after injecting IRQ in the data path, which can improve the performance
> significantly by avoiding a vmexit for each interrupt.
>
> More importantly it also makes it possible for Firecracker to hook up
> virtio-mmio with vhost-net, in which case there isn't a way to implement
> proper status register handling.
>
> For a complete backend that does set either INT_CONFIG bit or INT_VRING
> bit upon generating irq, what happens hasn't changed.
>
> Signed-off-by: Fam Zheng <[email protected]>
This has a side effect of skipping vring callbacks
if they trigger at the same time with a config
interrupt.
I don't see why this is safe.
> ---
> drivers/virtio/virtio_mmio.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index e09edb5c5e06..9b42502b2204 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -295,9 +295,7 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
> if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
> virtio_config_changed(&vm_dev->vdev);
> ret = IRQ_HANDLED;
> - }
> -
> - if (likely(status & VIRTIO_MMIO_INT_VRING)) {
> + } else {
> spin_lock_irqsave(&vm_dev->lock, flags);
> list_for_each_entry(info, &vm_dev->virtqueues, node)
> ret |= vring_interrupt(irq, info->vq);
> --
> 2.11.0
On 7/19/19 11:17 PM, Michael S. Tsirkin wrote:
> On Fri, Jul 19, 2019 at 09:31:34PM +0800, Fei Li wrote:
>> From: Fam Zheng <[email protected]>
>>
>> This allows the backend to _not_ trap mmio read of the status register
>> after injecting IRQ in the data path, which can improve the performance
>> significantly by avoiding a vmexit for each interrupt.
>>
>> More importantly it also makes it possible for Firecracker to hook up
>> virtio-mmio with vhost-net, in which case there isn't a way to implement
>> proper status register handling.
>>
>> For a complete backend that does set either INT_CONFIG bit or INT_VRING
>> bit upon generating irq, what happens hasn't changed.
>>
>> Signed-off-by: Fam Zheng <[email protected]>
> This has a side effect of skipping vring callbacks
> if they trigger at the same time with a config
> interrupt.
> I don't see why this is safe.
Good point! I think the block can be moved out from the else block and
run unconditionally then.
Fam
>
>
>> ---
>> drivers/virtio/virtio_mmio.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
>> index e09edb5c5e06..9b42502b2204 100644
>> --- a/drivers/virtio/virtio_mmio.c
>> +++ b/drivers/virtio/virtio_mmio.c
>> @@ -295,9 +295,7 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
>> if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
>> virtio_config_changed(&vm_dev->vdev);
>> ret = IRQ_HANDLED;
>> - }
>> -
>> - if (likely(status & VIRTIO_MMIO_INT_VRING)) {
>> + } else {
>> spin_lock_irqsave(&vm_dev->lock, flags);
>> list_for_each_entry(info, &vm_dev->virtqueues, node)
>> ret |= vring_interrupt(irq, info->vq);
>> --
>> 2.11.0
On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> > Hi,
> >
> > This patch series implements multiple interrupt vectors support for
> > virtio-mmio device. This is especially useful for multiqueue vhost-net
> > device when using firecracker micro-vms as the guest.
> >
> > Test result:
> > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> > receive 9 times more pps comparing with only one irq:
> > - 564830.38 rxpck/s for 8 irqs on
> > - 67665.06 rxpck/s for 1 irq on
> >
> > Please help to review, thanks!
> >
> > Have a nice day
> > Fei
>
>
> Interesting. The spec says though:
>
> 4.2.3.4
> Notifications From The Device
> The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
> least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
> used buffer notification or a configuration change notification to the device.
>
Yes, the spec needs to be updated if we want to use mult-irqs.
>
> So I'm guessing we need to change the host/guest interface?
Just to confirm, does the "the host/guest interface" you mentioned mean how to
pass the irq information from the user space tool to guest kernel?
In this patch, we do this by passing the [irq_start, irq_end]
interface via setting guest
kernel command line, that is done in vm_cmdline_set().
Also there is another way to do this: add two new registers describing irq info
(irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
Which one do you prefer?
> If true pls cc virtio-dev.
Sure.
>
> Also, do we need to update dt bindings documentation?
You mean the following doc? Sure. :)
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
Thanks for the review!
Have a nice day
Fei
>
> >
> > Fam Zheng (1):
> > virtio-mmio: Process vrings more proactively
> >
> > Fei Li (1):
> > virtio-mmio: support multiple interrupt vectors
> >
> > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> > 1 file changed, 196 insertions(+), 42 deletions(-)
> >
> > --
> > 2.11.0
On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote:
> On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> > > Hi,
> > >
> > > This patch series implements multiple interrupt vectors support for
> > > virtio-mmio device. This is especially useful for multiqueue vhost-net
> > > device when using firecracker micro-vms as the guest.
> > >
> > > Test result:
> > > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> > > receive 9 times more pps comparing with only one irq:
> > > - 564830.38 rxpck/s for 8 irqs on
> > > - 67665.06 rxpck/s for 1 irq on
> > >
> > > Please help to review, thanks!
> > >
> > > Have a nice day
> > > Fei
> >
> >
> > Interesting. The spec says though:
> >
> > 4.2.3.4
> > Notifications From The Device
> > The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
> > least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
> > used buffer notification or a configuration change notification to the device.
> >
> Yes, the spec needs to be updated if we want to use mult-irqs.
> >
> > So I'm guessing we need to change the host/guest interface?
> Just to confirm, does the "the host/guest interface" you mentioned mean how to
> pass the irq information from the user space tool to guest kernel?
> In this patch, we do this by passing the [irq_start, irq_end]
> interface via setting guest
> kernel command line, that is done in vm_cmdline_set().
> Also there is another way to do this: add two new registers describing irq info
> (irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
>
> Which one do you prefer?
I'm not sure - so far irq was passed on the command line, right?
The first step in implementing any spec change would be to update qemu
code to virtio 1. Which is not a huge project but so far no one
bothered.
> > If true pls cc virtio-dev.
> Sure.
> >
> > Also, do we need to update dt bindings documentation?
> You mean the following doc? Sure. :)
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
>
> Thanks for the review!
>
> Have a nice day
> Fei
>
>
> >
> > >
> > > Fam Zheng (1):
> > > virtio-mmio: Process vrings more proactively
> > >
> > > Fei Li (1):
> > > virtio-mmio: support multiple interrupt vectors
> > >
> > > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> > > 1 file changed, 196 insertions(+), 42 deletions(-)
> > >
> > > --
> > > 2.11.0
On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote:
> > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> > > > Hi,
> > > >
> > > > This patch series implements multiple interrupt vectors support for
> > > > virtio-mmio device. This is especially useful for multiqueue vhost-net
> > > > device when using firecracker micro-vms as the guest.
> > > >
> > > > Test result:
> > > > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> > > > receive 9 times more pps comparing with only one irq:
> > > > - 564830.38 rxpck/s for 8 irqs on
> > > > - 67665.06 rxpck/s for 1 irq on
> > > >
> > > > Please help to review, thanks!
> > > >
> > > > Have a nice day
> > > > Fei
> > >
> > >
> > > Interesting. The spec says though:
> > >
> > > 4.2.3.4
> > > Notifications From The Device
> > > The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
> > > least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
> > > used buffer notification or a configuration change notification to the device.
> > >
> > Yes, the spec needs to be updated if we want to use mult-irqs.
> > >
> > > So I'm guessing we need to change the host/guest interface?
> > Just to confirm, does the "the host/guest interface" you mentioned mean how to
> > pass the irq information from the user space tool to guest kernel?
> > In this patch, we do this by passing the [irq_start, irq_end]
> > interface via setting guest
> > kernel command line, that is done in vm_cmdline_set().
> > Also there is another way to do this: add two new registers describing irq info
> > (irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
> >
> > Which one do you prefer?
>
> I'm not sure - so far irq was passed on the command line, right?
Yes.
>
> The first step in implementing any spec change would be to update qemu
> code to virtio 1. Which is not a huge project but so far no one
> bothered.
Emm, actually I only did the test with using firecracker to start a
micro-vm, but without qemu.
To be honest, the reason why implementing multi-irq on virtio-mmio is
mainly because the
current firecracker using virtio-mmio device and it has no pci thing,
thus no msi/msix to
handle the interruptions.
On the other hand, considering pci is well supported in qemu, I am
wondering whether we
still need this. If needed, we would like to do this. :)
Have a nice day, thanks
Fei
>
>
> > > If true pls cc virtio-dev.
> > Sure.
> > >
> > > Also, do we need to update dt bindings documentation?
> > You mean the following doc? Sure. :)
> > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
> >
> > Thanks for the review!
> >
> > Have a nice day
> > Fei
> >
> >
> > >
> > > >
> > > > Fam Zheng (1):
> > > > virtio-mmio: Process vrings more proactively
> > > >
> > > > Fei Li (1):
> > > > virtio-mmio: support multiple interrupt vectors
> > > >
> > > > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> > > > 1 file changed, 196 insertions(+), 42 deletions(-)
> > > >
> > > > --
> > > > 2.11.0
On Mon, Jul 22, 2019 at 10:28:30AM +0800, Fam Zheng wrote:
>
> On 7/19/19 11:17 PM, Michael S. Tsirkin wrote:
> > On Fri, Jul 19, 2019 at 09:31:34PM +0800, Fei Li wrote:
> > > From: Fam Zheng <[email protected]>
> > >
> > > This allows the backend to _not_ trap mmio read of the status register
> > > after injecting IRQ in the data path, which can improve the performance
> > > significantly by avoiding a vmexit for each interrupt.
> > >
> > > More importantly it also makes it possible for Firecracker to hook up
> > > virtio-mmio with vhost-net, in which case there isn't a way to implement
> > > proper status register handling.
> > >
> > > For a complete backend that does set either INT_CONFIG bit or INT_VRING
> > > bit upon generating irq, what happens hasn't changed.
> > >
> > > Signed-off-by: Fam Zheng <[email protected]>
> > This has a side effect of skipping vring callbacks
> > if they trigger at the same time with a config
> > interrupt.
> > I don't see why this is safe.
>
> Good point! I think the block can be moved out from the else block and run
> unconditionally then.
>
> Fam
Won't same callback run from multiple irq handlers then?
Running multiple vring callbacks at the same time isn't
a good idea either.
>
> >
> >
> > > ---
> > > drivers/virtio/virtio_mmio.c | 4 +---
> > > 1 file changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> > > index e09edb5c5e06..9b42502b2204 100644
> > > --- a/drivers/virtio/virtio_mmio.c
> > > +++ b/drivers/virtio/virtio_mmio.c
> > > @@ -295,9 +295,7 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
> > > if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
> > > virtio_config_changed(&vm_dev->vdev);
> > > ret = IRQ_HANDLED;
> > > - }
> > > -
> > > - if (likely(status & VIRTIO_MMIO_INT_VRING)) {
> > > + } else {
> > > spin_lock_irqsave(&vm_dev->lock, flags);
> > > list_for_each_entry(info, &vm_dev->virtqueues, node)
> > > ret |= vring_interrupt(irq, info->vq);
> > > --
> > > 2.11.0
On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote:
> On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote:
> > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
> > > >
> > > > On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> > > > > Hi,
> > > > >
> > > > > This patch series implements multiple interrupt vectors support for
> > > > > virtio-mmio device. This is especially useful for multiqueue vhost-net
> > > > > device when using firecracker micro-vms as the guest.
> > > > >
> > > > > Test result:
> > > > > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> > > > > receive 9 times more pps comparing with only one irq:
> > > > > - 564830.38 rxpck/s for 8 irqs on
> > > > > - 67665.06 rxpck/s for 1 irq on
> > > > >
> > > > > Please help to review, thanks!
> > > > >
> > > > > Have a nice day
> > > > > Fei
> > > >
> > > >
> > > > Interesting. The spec says though:
> > > >
> > > > 4.2.3.4
> > > > Notifications From The Device
> > > > The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
> > > > least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
> > > > used buffer notification or a configuration change notification to the device.
> > > >
> > > Yes, the spec needs to be updated if we want to use mult-irqs.
> > > >
> > > > So I'm guessing we need to change the host/guest interface?
> > > Just to confirm, does the "the host/guest interface" you mentioned mean how to
> > > pass the irq information from the user space tool to guest kernel?
> > > In this patch, we do this by passing the [irq_start, irq_end]
> > > interface via setting guest
> > > kernel command line, that is done in vm_cmdline_set().
> > > Also there is another way to do this: add two new registers describing irq info
> > > (irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
> > >
> > > Which one do you prefer?
> >
> > I'm not sure - so far irq was passed on the command line, right?
> Yes.
> >
> > The first step in implementing any spec change would be to update qemu
> > code to virtio 1. Which is not a huge project but so far no one
> > bothered.
> Emm, actually I only did the test with using firecracker to start a
> micro-vm, but without qemu.
> To be honest, the reason why implementing multi-irq on virtio-mmio is
> mainly because the
> current firecracker using virtio-mmio device and it has no pci thing,
> thus no msi/msix to
> handle the interruptions.
> On the other hand, considering pci is well supported in qemu, I am
> wondering whether we
> still need this. If needed, we would like to do this. :)
>
> Have a nice day, thanks
> Fei
Sergio Lopez is now working on updating mmio to v1 support in qemu.
Maybe get in touch with him on how he looks at this extension.
Not asking you to work on qemu, but it makes sense
to get an ack for guest bits from another popular hypervisor.
> >
> >
> > > > If true pls cc virtio-dev.
> > > Sure.
> > > >
> > > > Also, do we need to update dt bindings documentation?
> > > You mean the following doc? Sure. :)
> > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
> > >
> > > Thanks for the review!
> > >
> > > Have a nice day
> > > Fei
> > >
> > >
> > > >
> > > > >
> > > > > Fam Zheng (1):
> > > > > virtio-mmio: Process vrings more proactively
> > > > >
> > > > > Fei Li (1):
> > > > > virtio-mmio: support multiple interrupt vectors
> > > > >
> > > > > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> > > > > 1 file changed, 196 insertions(+), 42 deletions(-)
> > > > >
> > > > > --
> > > > > 2.11.0
On Wed, Jul 31, 2019 at 4:26 AM Michael S. Tsirkin <[email protected]> wrote:
>
> On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote:
> > On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote:
> > > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
> > > > >
> > > > > On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
> > > > > > Hi,
> > > > > >
> > > > > > This patch series implements multiple interrupt vectors support for
> > > > > > virtio-mmio device. This is especially useful for multiqueue vhost-net
> > > > > > device when using firecracker micro-vms as the guest.
> > > > > >
> > > > > > Test result:
> > > > > > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> > > > > > receive 9 times more pps comparing with only one irq:
> > > > > > - 564830.38 rxpck/s for 8 irqs on
> > > > > > - 67665.06 rxpck/s for 1 irq on
> > > > > >
> > > > > > Please help to review, thanks!
> > > > > >
> > > > > > Have a nice day
> > > > > > Fei
> > > > >
> > > > >
> > > > > Interesting. The spec says though:
> > > > >
> > > > > 4.2.3.4
> > > > > Notifications From The Device
> > > > > The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
> > > > > least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
> > > > > used buffer notification or a configuration change notification to the device.
> > > > >
> > > > Yes, the spec needs to be updated if we want to use mult-irqs.
> > > > >
> > > > > So I'm guessing we need to change the host/guest interface?
> > > > Just to confirm, does the "the host/guest interface" you mentioned mean how to
> > > > pass the irq information from the user space tool to guest kernel?
> > > > In this patch, we do this by passing the [irq_start, irq_end]
> > > > interface via setting guest
> > > > kernel command line, that is done in vm_cmdline_set().
> > > > Also there is another way to do this: add two new registers describing irq info
> > > > (irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
> > > >
> > > > Which one do you prefer?
> > >
> > > I'm not sure - so far irq was passed on the command line, right?
> > Yes.
> > >
> > > The first step in implementing any spec change would be to update qemu
> > > code to virtio 1. Which is not a huge project but so far no one
> > > bothered.
> > Emm, actually I only did the test with using firecracker to start a
> > micro-vm, but without qemu.
> > To be honest, the reason why implementing multi-irq on virtio-mmio is
> > mainly because the
> > current firecracker using virtio-mmio device and it has no pci thing,
> > thus no msi/msix to
> > handle the interruptions.
> > On the other hand, considering pci is well supported in qemu, I am
> > wondering whether we
> > still need this. If needed, we would like to do this. :)
> >
> > Have a nice day, thanks
> > Fei
>
>
> Sergio Lopez is now working on updating mmio to v1 support in qemu.
> Maybe get in touch with him on how he looks at this extension.
Thanks for the info! :)
Hi Sergio Lopez,
I saw your [virtio-mmio: modern (v2)] patch series in Qemu mailing
list, thanks for moving this on.
And long Story Short, these two kernel patches is to add the multi-irq
support for virtio-mmio driver.
As this involves the spec change and you are now implementing
virtio-mmio v2, could you help to
give some suggestions on this extension?
I will cc you the original patch soon, thanks.
> Not asking you to work on qemu, but it makes sense
> to get an ack for guest bits from another popular hypervisor.
I agree, absolutely right. And I once work on Qemu development, hope
the combined
background could help to move this multi-irq feature forward. :)
Have a nice day, many thanks
Fei
>
>
> > >
> > >
> > > > > If true pls cc virtio-dev.
> > > > Sure.
> > > > >
> > > > > Also, do we need to update dt bindings documentation?
> > > > You mean the following doc? Sure. :)
> > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
> > > >
> > > > Thanks for the review!
> > > >
> > > > Have a nice day
> > > > Fei
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > Fam Zheng (1):
> > > > > > virtio-mmio: Process vrings more proactively
> > > > > >
> > > > > > Fei Li (1):
> > > > > > virtio-mmio: support multiple interrupt vectors
> > > > > >
> > > > > > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> > > > > > 1 file changed, 196 insertions(+), 42 deletions(-)
> > > > > >
> > > > > > --
> > > > > > 2.11.0
Hi Sergio,
Considering your implementing virtio-mmio v2 in Qemu, please help to give some
suggestions on this patch series. Thanks :)
For web, this link:
https://www.spinics.net/lists/kernel/msg3195667.html may help.
Have a nice day
Fei
On Fri, Jul 19, 2019 at 9:31 PM Fei Li <[email protected]> wrote:
>
> Hi,
>
> This patch series implements multiple interrupt vectors support for
> virtio-mmio device. This is especially useful for multiqueue vhost-net
> device when using firecracker micro-vms as the guest.
>
> Test result:
> With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
> receive 9 times more pps comparing with only one irq:
> - 564830.38 rxpck/s for 8 irqs on
> - 67665.06 rxpck/s for 1 irq on
>
> Please help to review, thanks!
>
> Have a nice day
> Fei
>
>
> Fam Zheng (1):
> virtio-mmio: Process vrings more proactively
>
> Fei Li (1):
> virtio-mmio: support multiple interrupt vectors
>
> drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
> 1 file changed, 196 insertions(+), 42 deletions(-)
>
> --
> 2.11.0
>
李菲 <[email protected]> writes:
> On Wed, Jul 31, 2019 at 4:26 AM Michael S. Tsirkin <[email protected]> wrote:
>>
>> On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote:
>> > On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <[email protected]> wrote:
>> > >
>> > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote:
>> > > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <[email protected]> wrote:
>> > > > >
>> > > > > On Fri, Jul 19, 2019 at 09:31:33PM +0800, Fei Li wrote:
>> > > > > > Hi,
>> > > > > >
>> > > > > > This patch series implements multiple interrupt vectors support for
>> > > > > > virtio-mmio device. This is especially useful for multiqueue vhost-net
>> > > > > > device when using firecracker micro-vms as the guest.
>> > > > > >
>> > > > > > Test result:
>> > > > > > With 8 vcpus & 8 net queues set, one vhost-net device with 8 irqs can
>> > > > > > receive 9 times more pps comparing with only one irq:
>> > > > > > - 564830.38 rxpck/s for 8 irqs on
>> > > > > > - 67665.06 rxpck/s for 1 irq on
>> > > > > >
>> > > > > > Please help to review, thanks!
>> > > > > >
>> > > > > > Have a nice day
>> > > > > > Fei
>> > > > >
>> > > > >
>> > > > > Interesting. The spec says though:
>> > > > >
>> > > > > 4.2.3.4
>> > > > > Notifications From The Device
>> > > > > The memory mapped virtio device is using a single, dedicated interrupt signal, which is asserted when at
>> > > > > least one of the bits described in the description of InterruptStatus is set. This is how the device sends a
>> > > > > used buffer notification or a configuration change notification to the device.
>> > > > >
>> > > > Yes, the spec needs to be updated if we want to use mult-irqs.
>> > > > >
>> > > > > So I'm guessing we need to change the host/guest interface?
>> > > > Just to confirm, does the "the host/guest interface" you mentioned mean how to
>> > > > pass the irq information from the user space tool to guest kernel?
>> > > > In this patch, we do this by passing the [irq_start, irq_end]
>> > > > interface via setting guest
>> > > > kernel command line, that is done in vm_cmdline_set().
>> > > > Also there is another way to do this: add two new registers describing irq info
>> > > > (irq_start & irq_end OR irq_start & irq_numbers) to the virtio config space.
>> > > >
>> > > > Which one do you prefer?
>> > >
>> > > I'm not sure - so far irq was passed on the command line, right?
>> > Yes.
>> > >
>> > > The first step in implementing any spec change would be to update qemu
>> > > code to virtio 1. Which is not a huge project but so far no one
>> > > bothered.
>> > Emm, actually I only did the test with using firecracker to start a
>> > micro-vm, but without qemu.
>> > To be honest, the reason why implementing multi-irq on virtio-mmio is
>> > mainly because the
>> > current firecracker using virtio-mmio device and it has no pci thing,
>> > thus no msi/msix to
>> > handle the interruptions.
>> > On the other hand, considering pci is well supported in qemu, I am
>> > wondering whether we
>> > still need this. If needed, we would like to do this. :)
>> >
>> > Have a nice day, thanks
>> > Fei
>>
>>
>> Sergio Lopez is now working on updating mmio to v1 support in qemu.
>> Maybe get in touch with him on how he looks at this extension.
> Thanks for the info! :)
>
> Hi Sergio Lopez,
> I saw your [virtio-mmio: modern (v2)] patch series in Qemu mailing
> list, thanks for moving this on.
> And long Story Short, these two kernel patches is to add the multi-irq
> support for virtio-mmio driver.
> As this involves the spec change and you are now implementing
> virtio-mmio v2, could you help to
> give some suggestions on this extension?
> I will cc you the original patch soon, thanks.
I like having the possibility of using multiple irq lines for a single
virtio-mmio, but I think having to specify an irq range for each device
is a bit inconvenient.
Usually, you want an irq line per each virtqueue. So, ideally, irq
assignment should be somehow linked to the virtqueue initialization. But
this isn't PCI, so the Guest can't simply request some MSI/MSI-X
vectors and tell the device about them, as virtio-pci does.
I think an option could be extending the specification with a new
QueueVector read-only register, which would return either the irq line
to be used for the virtqueue selected by QueueSel, or a value indicating
there isn't a dedicated line for this particular virtqueue.
This way, hypervisors could lazily allocate irq lines on demand when
the QueueVector is read for the first time (or after a reset), and
users (and fdt's) would still only need to specify a single interrupt
for each virtio-mmio definition.
>> Not asking you to work on qemu, but it makes sense
>> to get an ack for guest bits from another popular hypervisor.
> I agree, absolutely right. And I once work on Qemu development, hope
> the combined
> background could help to move this multi-irq feature forward. :)
>
>
> Have a nice day, many thanks
> Fei
>>
>>
>> > >
>> > >
>> > > > > If true pls cc virtio-dev.
>> > > > Sure.
>> > > > >
>> > > > > Also, do we need to update dt bindings documentation?
>> > > > You mean the following doc? Sure. :)
>> > > > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/virtio/mmio.txt
>> > > >
>> > > > Thanks for the review!
>> > > >
>> > > > Have a nice day
>> > > > Fei
>> > > >
>> > > >
>> > > > >
>> > > > > >
>> > > > > > Fam Zheng (1):
>> > > > > > virtio-mmio: Process vrings more proactively
>> > > > > >
>> > > > > > Fei Li (1):
>> > > > > > virtio-mmio: support multiple interrupt vectors
>> > > > > >
>> > > > > > drivers/virtio/virtio_mmio.c | 238 +++++++++++++++++++++++++++++++++++--------
>> > > > > > 1 file changed, 196 insertions(+), 42 deletions(-)
>> > > > > >
>> > > > > > --
>> > > > > > 2.11.0