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

[Paolo Bonzini]

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);