Re: [PATCH 4/5] kvm: irqchip: support defer to commit the route

[Alex Williamson <alex.williamson@redhat.com>]

On Wed, 25 Aug 2021 15:56:19 +0800
"Longpeng(Mike)" <longpeng2@huawei.com> wrote:

> The kvm_irqchip_commit_routes() is relatively expensive, so
> provide the users a choice to commit the route immediately
> or not when they add msi/msix route.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  accel/kvm/kvm-all.c    | 10 +++++++---
>  accel/stubs/kvm-stub.c |  3 ++-
>  hw/misc/ivshmem.c      |  2 +-
>  hw/vfio/pci.c          |  2 +-
>  hw/virtio/virtio-pci.c |  2 +-
>  include/sysemu/kvm.h   |  4 +++-
>  target/i386/kvm/kvm.c  |  2 +-
>  7 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 0125c17..1f788a2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1950,7 +1950,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>      return kvm_set_irq(s, route->kroute.gsi, 1);
>  }
>  
> -int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev,
> +                              bool defer_commit)
>  {
>      struct kvm_irq_routing_entry kroute = {};
>      int virq;
> @@ -1993,7 +1994,9 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
>  
>      kvm_add_routing_entry(s, &kroute);
>      kvm_arch_add_msi_route_post(&kroute, vector, dev);
> -    kvm_irqchip_commit_routes(s);
> +    if (!defer_commit) {
> +        kvm_irqchip_commit_routes(s);
> +    }


Personally I'd rather rename the function to
kvm_irqchip_add_deferred_msi_route() and kvm_irqchip_add_msi_route()
becomes a wrapper appending kvm_irqchip_commit_routes() to that.
Thanks,

Alex

>  
>      return virq;
>  }
> @@ -2151,7 +2154,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>      abort();
>  }
>  
> -int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev,
> +                              bool defer_commit)
>  {
>      return -ENOSYS;
>  }
> diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
> index 5b1d00a..d5caaca 100644
> --- a/accel/stubs/kvm-stub.c
> +++ b/accel/stubs/kvm-stub.c
> @@ -81,7 +81,8 @@ int kvm_on_sigbus(int code, void *addr)
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> -int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev,
> +                              bool defer_commit)
>  {
>      return -ENOSYS;
>  }
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 1ba4a98..98b14cc 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -429,7 +429,7 @@ static void ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector,
>      IVSHMEM_DPRINTF("ivshmem_add_kvm_msi_virq vector:%d\n", vector);
>      assert(!s->msi_vectors[vector].pdev);
>  
> -    ret = kvm_irqchip_add_msi_route(kvm_state, vector, pdev);
> +    ret = kvm_irqchip_add_msi_route(kvm_state, vector, pdev, false);
>      if (ret < 0) {
>          error_setg(errp, "kvm_irqchip_add_msi_route failed");
>          return;
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ca37fb7..3ab67d6 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -427,7 +427,7 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
>          return;
>      }
>  
> -    virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev);
> +    virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev, false);
>      if (virq < 0) {
>          event_notifier_cleanup(&vector->kvm_interrupt);
>          return;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 433060a..7e2d021 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -684,7 +684,7 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
>      int ret;
>  
>      if (irqfd->users == 0) {
> -        ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
> +        ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev, false);
>          if (ret < 0) {
>              return ret;
>          }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index a1ab1ee..1932dc0 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -473,9 +473,11 @@ void kvm_init_cpu_signals(CPUState *cpu);
>   *          message.
>   * @dev:    Owner PCI device to add the route. If @dev is specified
>   *          as @NULL, an empty MSI message will be inited.
> + * @defer_commit:   Defer to commit new route to the KVM core.
>   * @return: virq (>=0) when success, errno (<0) when failed.
>   */
> -int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev,
> +                              bool defer_commit);
>  int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
>                                   PCIDevice *dev);
>  void kvm_irqchip_commit_routes(KVMState *s);
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index e69abe4..896406b 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -4724,7 +4724,7 @@ void kvm_arch_init_irq_routing(KVMState *s)
>          /* If the ioapic is in QEMU and the lapics are in KVM, reserve
>             MSI routes for signaling interrupts to the local apics. */
>          for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> -            if (kvm_irqchip_add_msi_route(s, 0, NULL) < 0) {
> +            if (kvm_irqchip_add_msi_route(s, 0, NULL, false) < 0) {
>                  error_report("Could not enable split IRQ mode.");
>                  exit(1);
>              }