Re: [Qemu-devel] [PATCH 2/4] kvm: Introduce kvm_arch_irqchip_create

[Peter Maydell]

On 23 August 2013 20:41, Christoffer Dall <address@hidden> wrote:
> Introduce kvm_arch_irqchip_create an arch-specific hook in preparation
> for architecture-specific use of the device control API to create IRQ
> chips.
>
> Following patches will implement the ARM irqchip create method to prefer
> the device control API over the older KVM_CREATE_IRQCHIP API.
>
> Signed-off-by: Christoffer Dall <address@hidden>
> ---
>  include/sysemu/kvm.h |    4 ++++
>  kvm-all.c            |   11 +++++++++--
>  target-arm/kvm.c     |    5 +++++
>  target-i386/kvm.c    |    5 +++++
>  target-ppc/kvm.c     |    5 +++++
>  target-s390x/kvm.c   |    5 +++++
>  6 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index de74411..1e5847e 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -224,6 +224,10 @@ void kvm_arch_reset_vcpu(CPUState *cpu);
>  int kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
>  int kvm_arch_on_sigbus(int code, void *addr);
>
> +/* return negative error on error, 0 if irq chip was not created, and 
> positive
> + * number if it was created */
> +int kvm_arch_irqchip_create(KVMState *s);

Could you follow the doc-comment coding style for this,
please? (The extract/deposit ops in include/qemu/bitops.h
are the template I usually crib from.)

> +
>  void kvm_arch_init_irq_routing(KVMState *s);
>
>  int kvm_set_irq(KVMState *s, int irq, int level);
> diff --git a/kvm-all.c b/kvm-all.c
> index 716860f..fe64f3b 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1295,10 +1295,17 @@ static int kvm_irqchip_create(KVMState *s)
>          return 0;
>      }
>
> -    ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
> +    /* First probe and see if there's a arch-specific hook to create the
> +     * in-kernel irqchip for us */
> +    ret = kvm_arch_irqchip_create(s);
>      if (ret < 0) {
> -        fprintf(stderr, "Create kernel irqchip failed\n");
>          return ret;
> +    } else if (ret == 0) {
> +        ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
> +        if (ret < 0) {
> +            fprintf(stderr, "Create kernel irqchip failed\n");
> +            return ret;
> +        }
>      }
>
>      kvm_kernel_irqchip = true;
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index b92e00d..2484d90 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -647,3 +647,8 @@ void kvm_arch_remove_all_hw_breakpoints(void)
>  void kvm_arch_init_irq_routing(KVMState *s)
>  {
>  }
> +
> +int kvm_arch_irqchip_create(KVMState *s)
> +{
> +    return 0;
> +}

[ditto in s390/ppc/i386]

We can avoid all these identical stub versions of
this function by using our stubs mechanism:
create a file stubs/kvm.c, put the dummy function
definition in it, and add a line to stubs/Makefile.obj
saying "stub-obj-$(CONFIG_KVM) += kvm.o"

Then only the targets which actually need a real
implementation of this function have to provide one;
everybody else gets the stub version automatically.

In fact you could have the stub version's implementation
be to call kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP), which
would then simplify the kvm-all.c code a little.

(There are probably a few other kvm target functions
we could provide stubs for, but that's a later cleanup.)

-- PMM