1. Patchew
  2. QEMU
  3. [PATCH v5 0/6] optimize the downtime for vfio migration
  4. View patch

[PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route

Longpeng(Mike) posted 6 patches 4 years, 3 months ago
Maintainers: Alex Williamson <alex.williamson@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route
Posted by Longpeng(Mike) 4 years, 3 months ago 
Extract a common helper that add MSI route for specific vector
but does not commit immediately.

Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
---
 accel/kvm/kvm-all.c  | 15 +++++++++++++--
 include/sysemu/kvm.h |  6 ++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index db8d83b..8627f7c 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1953,7 +1953,7 @@ 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_deferred_msi_route(KVMState *s, int vector, PCIDevice *dev)
 {
     struct kvm_irq_routing_entry kroute = {};
     int virq;
@@ -1996,7 +1996,18 @@ 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);
+
+    return virq;
+}
+
+int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
+{
+    int virq;
+
+    virq = kvm_irqchip_add_deferred_msi_route(s, vector, dev);
+    if (virq >= 0) {
+        kvm_irqchip_commit_routes(s);
+    }
 
     return virq;
 }
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee..8de0d9a 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -476,6 +476,12 @@ void kvm_init_cpu_signals(CPUState *cpu);
  * @return: virq (>=0) when success, errno (<0) when failed.
  */
 int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
+/**
+ * Add MSI route for specific vector but does not commit to KVM
+ * immediately
+ */
+int kvm_irqchip_add_deferred_msi_route(KVMState *s, int vector,
+                                       PCIDevice *dev);
 int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
                                  PCIDevice *dev);
 void kvm_irqchip_commit_routes(KVMState *s);
-- 
1.8.3.1
RE: [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route
Posted by Longpeng (Mike, Cloud Infrastructure Service Product Dept.) 4 years, 2 months ago 
Hi Paolo,

Ping...

Do you have any suggestions about this change ? It seems Alex has no
objection on this series now, but we need your ACK, thanks.


> -----Original Message-----
> From: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> Sent: Wednesday, November 3, 2021 4:17 PM
> To: alex.williamson@redhat.com; pbonzini@redhat.com
> Cc: qemu-devel@nongnu.org; kvm@vger.kernel.org; Gonglei (Arei)
> <arei.gonglei@huawei.com>; Longpeng (Mike, Cloud Infrastructure Service
> Product Dept.) <longpeng2@huawei.com>
> Subject: [PATCH v5 4/6] kvm: irqchip: extract
> kvm_irqchip_add_deferred_msi_route
> 
> Extract a common helper that add MSI route for specific vector
> but does not commit immediately.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> ---
>  accel/kvm/kvm-all.c  | 15 +++++++++++++--
>  include/sysemu/kvm.h |  6 ++++++
>  2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index db8d83b..8627f7c 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1953,7 +1953,7 @@ 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_deferred_msi_route(KVMState *s, int vector, PCIDevice
> *dev)
>  {
>      struct kvm_irq_routing_entry kroute = {};
>      int virq;
> @@ -1996,7 +1996,18 @@ 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);
> +
> +    return virq;
> +}
> +
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> +{
> +    int virq;
> +
> +    virq = kvm_irqchip_add_deferred_msi_route(s, vector, dev);
> +    if (virq >= 0) {
> +        kvm_irqchip_commit_routes(s);
> +    }
> 
>      return virq;
>  }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index a1ab1ee..8de0d9a 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -476,6 +476,12 @@ void kvm_init_cpu_signals(CPUState *cpu);
>   * @return: virq (>=0) when success, errno (<0) when failed.
>   */
>  int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
> +/**
> + * Add MSI route for specific vector but does not commit to KVM
> + * immediately
> + */
> +int kvm_irqchip_add_deferred_msi_route(KVMState *s, int vector,
> +                                       PCIDevice *dev);
>  int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
>                                   PCIDevice *dev);
>  void kvm_irqchip_commit_routes(KVMState *s);
> --
> 1.8.3.1
Re: [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route
Posted by Paolo Bonzini 4 years, 2 months ago 
On 11/3/21 09:16, Longpeng(Mike) wrote:
> Extract a common helper that add MSI route for specific vector
> but does not commit immediately.
> 
> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>

I think adding the new function is not necessary; I have no problem 
moving the call to kvm_irqchip_commit_routes to the callers.  Perhaps 
you can have an API like this:

typedef struct KVMRouteChange {
     KVMState *s;
     int changes;
} KVMRouteChange;

KVMRouteChange kvm_irqchip_begin_route_changes(KVMState *s)
{
     return (KVMRouteChange) { .s = s, .changes = 0 };
}

void kvm_irqchip_commit_route_changes(KVMRouteChange *c)
{
     if (c->changes) {
         kvm_irqchip_commit_routes(c->s);
         c->changes = 0;
    }
}

int kvm_irqchip_add_msi_route(KVMRouteChange *c, int vector, PCIDevice *dev)
{
     KVMState *s = c->s;
     ...
     kvm_add_routing_entry(s, &kroute);
     kvm_arch_add_msi_route_post(&kroute, vector, dev);
     c->changes++;

     return virq;
}

so it's harder for the callers to "forget" kvm_irqchip_commit_route_changes.

Paolo

> ---
>   accel/kvm/kvm-all.c  | 15 +++++++++++++--
>   include/sysemu/kvm.h |  6 ++++++
>   2 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index db8d83b..8627f7c 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1953,7 +1953,7 @@ 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_deferred_msi_route(KVMState *s, int vector, PCIDevice *dev)
>   {
>       struct kvm_irq_routing_entry kroute = {};
>       int virq;
> @@ -1996,7 +1996,18 @@ 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);
> +
> +    return virq;
> +}
> +
> +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> +{
> +    int virq;
> +
> +    virq = kvm_irqchip_add_deferred_msi_route(s, vector, dev);
> +    if (virq >= 0) {
> +        kvm_irqchip_commit_routes(s);
> +    }
>   
>       return virq;
>   }
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index a1ab1ee..8de0d9a 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -476,6 +476,12 @@ void kvm_init_cpu_signals(CPUState *cpu);
>    * @return: virq (>=0) when success, errno (<0) when failed.
>    */
>   int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
> +/**
> + * Add MSI route for specific vector but does not commit to KVM
> + * immediately
> + */
> +int kvm_irqchip_add_deferred_msi_route(KVMState *s, int vector,
> +                                       PCIDevice *dev);
>   int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
>                                    PCIDevice *dev);
>   void kvm_irqchip_commit_routes(KVMState *s);
>
RE: [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route
Posted by Longpeng (Mike, Cloud Infrastructure Service Product Dept.) 4 years, 2 months ago 

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> Sent: Friday, November 12, 2021 5:32 PM
> To: Longpeng (Mike, Cloud Infrastructure Service Product Dept.)
> <longpeng2@huawei.com>; alex.williamson@redhat.com
> Cc: qemu-devel@nongnu.org; kvm@vger.kernel.org; Gonglei (Arei)
> <arei.gonglei@huawei.com>
> Subject: Re: [PATCH v5 4/6] kvm: irqchip: extract
> kvm_irqchip_add_deferred_msi_route
> 
> On 11/3/21 09:16, Longpeng(Mike) wrote:
> > Extract a common helper that add MSI route for specific vector
> > but does not commit immediately.
> >
> > Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com>
> 
> I think adding the new function is not necessary; I have no problem
> moving the call to kvm_irqchip_commit_routes to the callers.  Perhaps
> you can have an API like this:
> 
> typedef struct KVMRouteChange {
>      KVMState *s;
>      int changes;
> } KVMRouteChange;
> 
> KVMRouteChange kvm_irqchip_begin_route_changes(KVMState *s)
> {
>      return (KVMRouteChange) { .s = s, .changes = 0 };
> }
> 
> void kvm_irqchip_commit_route_changes(KVMRouteChange *c)
> {
>      if (c->changes) {
>          kvm_irqchip_commit_routes(c->s);
>          c->changes = 0;
>     }
> }
> 
> int kvm_irqchip_add_msi_route(KVMRouteChange *c, int vector, PCIDevice *dev)
> {
>      KVMState *s = c->s;
>      ...
>      kvm_add_routing_entry(s, &kroute);
>      kvm_arch_add_msi_route_post(&kroute, vector, dev);
>      c->changes++;
> 
>      return virq;
> }
> 
> so it's harder for the callers to "forget" kvm_irqchip_commit_route_changes.
> 

Make sense.

We have 4 adding functions currently, the first two trigger committing inside
and the others do not.
 1. kvm_irqchip_add_adapter_route (commit inside)
 2. kvm_irqchip_add_msi_route (commit inside)
 3. kvm_irqchip_add_irq_route (commit outside)
 4. kvm_irqchip_add_hv_sint_route (commit outside)

How about just moving the kvm_irqchip_commit_routes() out of 
kvm_irqchip_add_msi_route() in this series and implement the solution you
suggested in another series ? I think we should apply the solution to
s390_adapter routing type and updating paths.


> Paolo
> 
> > ---
> >   accel/kvm/kvm-all.c  | 15 +++++++++++++--
> >   include/sysemu/kvm.h |  6 ++++++
> >   2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index db8d83b..8627f7c 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -1953,7 +1953,7 @@ 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_deferred_msi_route(KVMState *s, int vector, PCIDevice
> *dev)
> >   {
> >       struct kvm_irq_routing_entry kroute = {};
> >       int virq;
> > @@ -1996,7 +1996,18 @@ 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);
> > +
> > +    return virq;
> > +}
> > +
> > +int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
> > +{
> > +    int virq;
> > +
> > +    virq = kvm_irqchip_add_deferred_msi_route(s, vector, dev);
> > +    if (virq >= 0) {
> > +        kvm_irqchip_commit_routes(s);
> > +    }
> >
> >       return virq;
> >   }
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index a1ab1ee..8de0d9a 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -476,6 +476,12 @@ void kvm_init_cpu_signals(CPUState *cpu);
> >    * @return: virq (>=0) when success, errno (<0) when failed.
> >    */
> >   int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
> > +/**
> > + * Add MSI route for specific vector but does not commit to KVM
> > + * immediately
> > + */
> > +int kvm_irqchip_add_deferred_msi_route(KVMState *s, int vector,
> > +                                       PCIDevice *dev);
> >   int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
> >                                    PCIDevice *dev);
> >   void kvm_irqchip_commit_routes(KVMState *s);
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.