[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_ro
From: |
Longpeng (Mike, Cloud Infrastructure Service Product Dept.) |
Subject: |
RE: [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route |
Date: |
Sat, 13 Nov 2021 09:21:39 +0000 |
> -----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);
> >
- [PATCH v5 0/6] optimize the downtime for vfio migration, Longpeng(Mike), 2021/11/03
- [PATCH v5 2/6] vfio: move re-enabling INTX out of the common helper, Longpeng(Mike), 2021/11/03
- [PATCH v5 1/6] vfio: simplify the conditional statements in vfio_msi_enable, Longpeng(Mike), 2021/11/03
- [PATCH v5 3/6] vfio: simplify the failure path in vfio_msi_enable, Longpeng(Mike), 2021/11/03
- [PATCH v5 5/6] Revert "vfio: Avoid disabling and enabling vectors repeatedly in VFIO migration", Longpeng(Mike), 2021/11/03
- [PATCH v5 4/6] kvm: irqchip: extract kvm_irqchip_add_deferred_msi_route, Longpeng(Mike), 2021/11/03
- [PATCH v5 6/6] vfio: defer to commit kvm irq routing when enable msi/msix, Longpeng(Mike), 2021/11/03
- Re: [PATCH v5 0/6] optimize the downtime for vfio migration, Alex Williamson, 2021/11/03