* [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors @ 2019-07-19 13:31 Fei Li 2019-07-19 13:31 ` [PATCH 1/2] virtio-mmio: Process vrings more proactively Fei Li ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: Fei Li @ 2019-07-19 13:31 UTC (permalink / raw) To: linux-kernel Cc: Jason Wang, Pawel Moll, Michael S . Tsirkin, Suzuki K Poulose, Fam Zheng, Fei Li 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] virtio-mmio: Process vrings more proactively 2019-07-19 13:31 [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors Fei Li @ 2019-07-19 13:31 ` Fei Li 2019-07-19 15:17 ` Michael S. Tsirkin 2019-07-19 13:31 ` [PATCH 2/2] virtio-mmio: support multiple interrupt vectors Fei Li ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: Fei Li @ 2019-07-19 13:31 UTC (permalink / raw) To: linux-kernel Cc: Jason Wang, Pawel Moll, Michael S . Tsirkin, Suzuki K Poulose, Fam Zheng From: Fam Zheng <zhengfeiran@bytedance.com> 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 <zhengfeiran@bytedance.com> --- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] virtio-mmio: Process vrings more proactively 2019-07-19 13:31 ` [PATCH 1/2] virtio-mmio: Process vrings more proactively Fei Li @ 2019-07-19 15:17 ` Michael S. Tsirkin 2019-07-22 2:28 ` [External Email] " Fam Zheng 0 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2019-07-19 15:17 UTC (permalink / raw) To: Fei Li; +Cc: linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng On Fri, Jul 19, 2019 at 09:31:34PM +0800, Fei Li wrote: > From: Fam Zheng <zhengfeiran@bytedance.com> > > 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 <zhengfeiran@bytedance.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH 1/2] virtio-mmio: Process vrings more proactively 2019-07-19 15:17 ` Michael S. Tsirkin @ 2019-07-22 2:28 ` Fam Zheng 2019-07-25 14:33 ` Michael S. Tsirkin 0 siblings, 1 reply; 14+ messages in thread From: Fam Zheng @ 2019-07-22 2:28 UTC (permalink / raw) To: Michael S. Tsirkin, Fei Li Cc: linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose 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 <zhengfeiran@bytedance.com> >> >> 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 <zhengfeiran@bytedance.com> > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH 1/2] virtio-mmio: Process vrings more proactively 2019-07-22 2:28 ` [External Email] " Fam Zheng @ 2019-07-25 14:33 ` Michael S. Tsirkin 0 siblings, 0 replies; 14+ messages in thread From: Michael S. Tsirkin @ 2019-07-25 14:33 UTC (permalink / raw) To: Fam Zheng; +Cc: Fei Li, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose 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 <zhengfeiran@bytedance.com> > > > > > > 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 <zhengfeiran@bytedance.com> > > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] virtio-mmio: support multiple interrupt vectors 2019-07-19 13:31 [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors Fei Li 2019-07-19 13:31 ` [PATCH 1/2] virtio-mmio: Process vrings more proactively Fei Li @ 2019-07-19 13:31 ` Fei Li 2019-07-19 15:14 ` [PATCH v1 0/2] " Michael S. Tsirkin 2019-07-31 4:10 ` 李菲 3 siblings, 0 replies; 14+ messages in thread From: Fei Li @ 2019-07-19 13:31 UTC (permalink / raw) To: linux-kernel Cc: Jason Wang, Pawel Moll, Michael S . Tsirkin, Suzuki K Poulose, Fam Zheng, Fei Li Rework vm_find_vqs() to support multiple interrupt vectors for virtio-mmio device. Considering without msi/msi-x only limited irq routing entries (only 24) are allocated, to support more interrupts for device, esp. virtio-net device with multi rx/tx queue pairs, this patch requests one vector for the config change and one vector for two continuous vqs (e.g. each rx/tx queue pair). If failing to request multiple interrupt vectors, fall back to the old style: request only one irq for both the config and all vqs. Add irq_first & irq_last to store the irq information when processing the device command line. Signed-off-by: Fei Li <lifei.shirley@bytedance.com> --- drivers/virtio/virtio_mmio.c | 237 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 194 insertions(+), 43 deletions(-) diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 9b42502b2204..92d16c86ea8f 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -75,7 +75,7 @@ * Currently hardcoded to the page size. */ #define VIRTIO_MMIO_VRING_ALIGN PAGE_SIZE - +#define VQ_NAME_LEN 20 #define to_virtio_mmio_device(_plat_dev) \ container_of(_plat_dev, struct virtio_mmio_device, vdev) @@ -90,6 +90,9 @@ struct virtio_mmio_device { /* a list of queues so we can dispatch IRQs */ spinlock_t lock; struct list_head virtqueues; + + /* Add name for each virtqueue, can be used for the callback. */ + char *vq_names; }; struct virtio_mmio_vq_info { @@ -279,7 +282,16 @@ static bool vm_notify(struct virtqueue *vq) return true; } -/* Notify all virtqueues on an interrupt. */ +/* Only handle the config change, then return. */ +static irqreturn_t vm_config_changed(int irq, void *opaque) +{ + struct virtio_mmio_device *vm_dev = opaque; + + virtio_config_changed(&vm_dev->vdev); + return IRQ_HANDLED; +} + +/* For old style: only one interrupt for both the config and all virtqueues. */ static irqreturn_t vm_interrupt(int irq, void *opaque) { struct virtio_mmio_device *vm_dev = opaque; @@ -336,11 +348,31 @@ static void vm_del_vqs(struct virtio_device *vdev) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); struct virtqueue *vq, *n; + unsigned int start; + int i = 0, shared = 0; + struct resource *res = platform_get_resource(vm_dev->pdev, + IORESOURCE_IRQ, 0); - list_for_each_entry_safe(vq, n, &vdev->vqs, list) - vm_del_vq(vq); + if (res == NULL) + return; + start = res->start; + if (res->end == start) { + free_irq(start, vm_dev); + } else { + /* Try to free_irq for vq[i] */ + list_for_each_entry_safe(vq, n, &vdev->vqs, list) { + free_irq(start + shared + 1, vq); + if (i % 2 != 0) + shared++; + vm_del_vq(vq); + i++; + } + /* Try to free_irq for config */ + free_irq(start, vdev); + } - free_irq(platform_get_irq(vm_dev->pdev, 0), vm_dev); + kfree(vm_dev->vq_names); + vm_dev->vq_names = NULL; } static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, @@ -453,26 +485,66 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, unsigned index, return ERR_PTR(err); } -static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, - struct virtqueue *vqs[], - vq_callback_t *callbacks[], - const char * const names[], - const bool *ctx, - struct irq_affinity *desc) +static int vm_request_multi_vectors(struct virtio_device *vdev, + unsigned int start, int nvectors) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); - int irq = platform_get_irq(vm_dev->pdev, 0); - int i, err, queue_idx = 0; + int err = -ENOMEM; - if (irq < 0) { - dev_err(&vdev->dev, "Cannot get IRQ resource\n"); - return irq; - } + vm_dev->vq_names = kmalloc(nvectors * VQ_NAME_LEN, GFP_KERNEL); + if (!vm_dev->vq_names) + goto error; - err = request_irq(irq, vm_interrupt, IRQF_SHARED, - dev_name(&vdev->dev), vm_dev); + /* Firstly, request one irq vector for config */ + snprintf(vm_dev->vq_names, VQ_NAME_LEN, + "%s-config", dev_name(&vdev->dev)); + err = request_irq(start, vm_config_changed, 0, + vm_dev->vq_names, vm_dev); if (err) - return err; + goto error; + + return 0; +error: + vm_del_vqs(vdev); + return err; +} + +static int vm_try_to_find_vqs(struct virtio_device *vdev, unsigned int nvqs, + struct virtqueue *vqs[], + vq_callback_t *callbacks[], + const char * const names[], + const bool *ctx, + struct irq_affinity *desc, + bool multi_vectors) +{ + struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); + unsigned int start, shared = 0; + int i, err = 0, nvectors = 0, queue_idx = 0; + struct resource *res = platform_get_resource(vm_dev->pdev, + IORESOURCE_IRQ, 0); + + if (res == NULL) + return -EINVAL; + + start = res->start; + if (!multi_vectors) { + /* Old style: only one interrupt for config and all vqs. */ + err = request_irq(start, vm_interrupt, IRQF_SHARED, + dev_name(&vdev->dev), vm_dev); + if (err) + goto error_request; + res->end = start; + } else { + /* Optimizing: one for config change, one per vq pair. */ + nvectors = 1; + for (i = 0; i < nvqs; i++) + if (callbacks[i]) + ++nvectors; + + err = vm_request_multi_vectors(vdev, start, nvectors); + if (err) + goto error_request; + } for (i = 0; i < nvqs; ++i) { if (!names[i]) { @@ -483,14 +555,65 @@ static int vm_find_vqs(struct virtio_device *vdev, unsigned nvqs, vqs[i] = vm_setup_vq(vdev, queue_idx++, callbacks[i], names[i], ctx ? ctx[i] : false); if (IS_ERR(vqs[i])) { - vm_del_vqs(vdev); - return PTR_ERR(vqs[i]); + err = PTR_ERR(vqs[i]); + goto error_vq; } + + /* Do not request_irq for vq without a callback, e.i. control */ + if (!callbacks[i]) + continue; + + /* If multi-vectors not supported: don't request more vectors */ + if (start == res->end) + break; + + /* For multi-vectors: choose vq as the dev_id for request_irq */ + snprintf(vm_dev->vq_names + (i + 1) * VQ_NAME_LEN, VQ_NAME_LEN, + "%s-%s", dev_name(&vdev->dev), names[i]); + err = request_irq(start + shared + 1, vring_interrupt, + IRQF_SHARED, + vm_dev->vq_names + (i + 1) * VQ_NAME_LEN, + vqs[i]); + if (err) + goto error_vq; + + if (i % 2 != 0) + shared += 1; } + return err; +error_vq: + vm_del_vqs(vdev); +error_request: + return err; +} - return 0; +static int vm_find_vqs(struct virtio_device *vdev, unsigned int nvqs, + struct virtqueue *vqs[], + vq_callback_t *callbacks[], + const char * const names[], + const bool *ctx, + struct irq_affinity *desc) +{ + struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); + struct resource *res = platform_get_resource(vm_dev->pdev, + IORESOURCE_IRQ, 0); + + if (res == NULL) + return -EINVAL; + /* If supports multi-vectors: request more vectors for config and vqs */ + if (res->start < res->end) + if (!vm_try_to_find_vqs(vdev, nvqs, vqs, callbacks, + names, ctx, desc, true)) + return 0; + /* Only request one vector for both the config and all vqs: + * 1. Handle for devices not supporting multi vectors; + * 2. Fallback to the old style in case requesting multi-vectors failed + */ + return vm_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names, + ctx, desc, false); } + static const char *vm_bus_name(struct virtio_device *vdev) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); @@ -637,32 +760,42 @@ static int vm_cmdline_set(const char *device, struct resource resources[2] = {}; char *str; long long int base, size; - unsigned int irq; + unsigned int irq_first, irq_last; int processed, consumed = 0; struct platform_device *pdev; /* Consume "size" part of the command line parameter */ size = memparse(device, &str); - /* Get "@<base>:<irq>[:<id>]" chunks */ - processed = sscanf(str, "@%lli:%u%n:%d%n", - &base, &irq, &consumed, - &vm_cmdline_id, &consumed); - - /* - * sscanf() must processes at least 2 chunks; also there - * must be no extra characters after the last chunk, so - * str[consumed] must be '\0' - */ - if (processed < 2 || str[consumed]) - return -EINVAL; + if (strchr(str, '[') == NULL && strchr(str, ']') == NULL) { + /* For old style: parse as "@<base>:<irq>[:<id>]" chunks */ + processed = sscanf(str, "@%lli:%u%n:%d%n", + &base, &irq_first, &consumed, + &vm_cmdline_id, &consumed); + /* + * sscanf() must processes at least 2 chunks; also there + * must be no extra characters after the last chunk, so + * str[consumed] must be '\0' + */ + if (processed < 2 || str[consumed]) + return -EINVAL; + irq_last = irq_first; + } else { + /* For multi-vectors: @<base>:[<irq_first>-<irq_last>][:<id>] */ + processed = sscanf(str, "@%lli:[%u-%u]%n:%d%n", + &base, &irq_first, &irq_last, &consumed, + &vm_cmdline_id, &consumed); + if (processed < 3 || str[consumed]) + return -EINVAL; + } resources[0].flags = IORESOURCE_MEM; resources[0].start = base; resources[0].end = base + size - 1; resources[1].flags = IORESOURCE_IRQ; - resources[1].start = resources[1].end = irq; + resources[1].start = irq_first; + resources[1].end = irq_last; if (!vm_cmdline_parent_registered) { err = device_register(&vm_cmdline_parent); @@ -673,11 +806,19 @@ static int vm_cmdline_set(const char *device, vm_cmdline_parent_registered = 1; } - pr_info("Registering device virtio-mmio.%d at 0x%llx-0x%llx, IRQ %d.\n", - vm_cmdline_id, - (unsigned long long)resources[0].start, - (unsigned long long)resources[0].end, - (int)resources[1].start); + if (resources[1].end > resources[1].start) + pr_info("Registering device virtio-mmio.%d at 0x%llx-0x%llx, IRQs %d-%d.\n", + vm_cmdline_id, + (unsigned long long)resources[0].start, + (unsigned long long)resources[0].end, + (int)resources[1].start, + (int)resources[1].end); + else + pr_info("Registering device virtio-mmio.%d at 0x%llx-0x%llx, IRQ %d.\n", + vm_cmdline_id, + (unsigned long long)resources[0].start, + (unsigned long long)resources[0].end, + (int)resources[1].start); pdev = platform_device_register_resndata(&vm_cmdline_parent, "virtio-mmio", vm_cmdline_id++, @@ -692,7 +833,17 @@ static int vm_cmdline_get_device(struct device *dev, void *data) unsigned int len = strlen(buffer); struct platform_device *pdev = to_platform_device(dev); - snprintf(buffer + len, PAGE_SIZE - len, "0x%llx@0x%llx:%llu:%d\n", + if (pdev->resource[1].end > pdev->resource[1].start) + snprintf(buffer + len, PAGE_SIZE - len, + "0x%llx@0x%llx:[%llu-%llu]:%d\n", + pdev->resource[0].end - pdev->resource[0].start + 1ULL, + (unsigned long long)pdev->resource[0].start, + (unsigned long long)pdev->resource[1].start, + (unsigned long long)pdev->resource[1].end, + pdev->id); + else + snprintf(buffer + len, PAGE_SIZE - len, + "0x%llx@0x%llx:%llu:%d\n", pdev->resource[0].end - pdev->resource[0].start + 1ULL, (unsigned long long)pdev->resource[0].start, (unsigned long long)pdev->resource[1].start, -- 2.11.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-19 13:31 [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors Fei Li 2019-07-19 13:31 ` [PATCH 1/2] virtio-mmio: Process vrings more proactively Fei Li 2019-07-19 13:31 ` [PATCH 2/2] virtio-mmio: support multiple interrupt vectors Fei Li @ 2019-07-19 15:14 ` Michael S. Tsirkin 2019-07-22 3:22 ` [External Email] " 李菲 2019-07-31 4:10 ` 李菲 3 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2019-07-19 15:14 UTC (permalink / raw) To: Fei Li; +Cc: linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-19 15:14 ` [PATCH v1 0/2] " Michael S. Tsirkin @ 2019-07-22 3:22 ` 李菲 2019-07-22 8:39 ` Michael S. Tsirkin 0 siblings, 1 reply; 14+ messages in thread From: 李菲 @ 2019-07-22 3:22 UTC (permalink / raw) To: Michael S. Tsirkin, virtio-dev Cc: linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-22 3:22 ` [External Email] " 李菲 @ 2019-07-22 8:39 ` Michael S. Tsirkin 2019-07-22 13:43 ` 李菲 0 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2019-07-22 8:39 UTC (permalink / raw) To: 李菲 Cc: virtio-dev, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote: > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-22 8:39 ` Michael S. Tsirkin @ 2019-07-22 13:43 ` 李菲 2019-07-30 20:26 ` Michael S. Tsirkin 0 siblings, 1 reply; 14+ messages in thread From: 李菲 @ 2019-07-22 13:43 UTC (permalink / raw) To: Michael S. Tsirkin Cc: virtio-dev, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote: > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-22 13:43 ` 李菲 @ 2019-07-30 20:26 ` Michael S. Tsirkin 2019-07-31 4:04 ` 李菲 0 siblings, 1 reply; 14+ messages in thread From: Michael S. Tsirkin @ 2019-07-30 20:26 UTC (permalink / raw) To: 李菲 Cc: virtio-dev, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote: > On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote: > > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-30 20:26 ` Michael S. Tsirkin @ 2019-07-31 4:04 ` 李菲 2019-08-02 11:01 ` Sergio Lopez 0 siblings, 1 reply; 14+ messages in thread From: 李菲 @ 2019-07-31 4:04 UTC (permalink / raw) To: Michael S. Tsirkin, Sergio Lopez Cc: virtio-dev, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng, Xiongchun Duan On Wed, Jul 31, 2019 at 4:26 AM Michael S. Tsirkin <mst@redhat.com> wrote: > > On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote: > > On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <mst@redhat.com> wrote: > > > > > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote: > > > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [External Email] Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-31 4:04 ` 李菲 @ 2019-08-02 11:01 ` Sergio Lopez 0 siblings, 0 replies; 14+ messages in thread From: Sergio Lopez @ 2019-08-02 11:01 UTC (permalink / raw) To: 李菲 Cc: Michael S. Tsirkin, virtio-dev, linux-kernel, Jason Wang, Pawel Moll, Suzuki K Poulose, Fam Zheng, Xiongchun Duan [-- Attachment #1: Type: text/plain, Size: 6019 bytes --] 李菲 <lifei.shirley@bytedance.com> writes: > On Wed, Jul 31, 2019 at 4:26 AM Michael S. Tsirkin <mst@redhat.com> wrote: >> >> On Mon, Jul 22, 2019 at 09:43:18PM +0800, 李菲 wrote: >> > On Mon, Jul 22, 2019 at 4:39 PM Michael S. Tsirkin <mst@redhat.com> wrote: >> > > >> > > On Mon, Jul 22, 2019 at 11:22:02AM +0800, 李菲 wrote: >> > > > On Fri, Jul 19, 2019 at 11:14 PM Michael S. Tsirkin <mst@redhat.com> 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors 2019-07-19 13:31 [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors Fei Li ` (2 preceding siblings ...) 2019-07-19 15:14 ` [PATCH v1 0/2] " Michael S. Tsirkin @ 2019-07-31 4:10 ` 李菲 3 siblings, 0 replies; 14+ messages in thread From: 李菲 @ 2019-07-31 4:10 UTC (permalink / raw) To: Sergio Lopez Cc: Jason Wang, Pawel Moll, Michael S . Tsirkin, Suzuki K Poulose, Fam Zheng, linux-kernel, Xiongchun Duan 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 <lifei.shirley@bytedance.com> 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-08-02 11:01 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-19 13:31 [PATCH v1 0/2] virtio-mmio: support multiple interrupt vectors Fei Li 2019-07-19 13:31 ` [PATCH 1/2] virtio-mmio: Process vrings more proactively Fei Li 2019-07-19 15:17 ` Michael S. Tsirkin 2019-07-22 2:28 ` [External Email] " Fam Zheng 2019-07-25 14:33 ` Michael S. Tsirkin 2019-07-19 13:31 ` [PATCH 2/2] virtio-mmio: support multiple interrupt vectors Fei Li 2019-07-19 15:14 ` [PATCH v1 0/2] " Michael S. Tsirkin 2019-07-22 3:22 ` [External Email] " 李菲 2019-07-22 8:39 ` Michael S. Tsirkin 2019-07-22 13:43 ` 李菲 2019-07-30 20:26 ` Michael S. Tsirkin 2019-07-31 4:04 ` 李菲 2019-08-02 11:01 ` Sergio Lopez 2019-07-31 4:10 ` 李菲
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).