Re: [Qemu-devel] [PATCH 4/6] kvm: Don't assume irqchip-in-kernel implies irqfds

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2012-07-25 15:24, Peter Maydell wrote:
> Instead of assuming that we can use irqfds if and only if
> kvm_irqchip_in_kernel(), add a bool to the KVMState which
> indicates this, and is set only on x86 and only if the
> irqchip is in the kernel.
> 
> The kernel documentation implies that the only thing
> you need to use KVM_IRQFD is that KVM_CAP_IRQFD is
> advertised, but this seems to be untrue. In particular
> the kernel does not (alas) return a sensible error if you
> try to set up an irqfd when you haven't created an irqchip.
> If it did we could remove all this nonsense and let the
> kernel return the error code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  kvm-all.c         |    3 ++-
>  kvm-stub.c        |    1 +
>  kvm.h             |   10 ++++++++++
>  target-i386/kvm.c |    4 ++++
>  4 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8e21d81..a88b8ad 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -101,6 +101,7 @@ struct KVMState
>  KVMState *kvm_state;
>  bool kvm_kernel_irqchip;
>  bool kvm_async_interrupt_injection;
> +bool kvm_irqfds_allowed;

Why allowed vs enabled? You only have kvm_async_interrupt_injection as well.

>  
>  static const KVMCapabilityInfo kvm_required_capabilites[] = {
>      KVM_CAP_INFO(USER_MEMORY),
> @@ -1126,7 +1127,7 @@ static int kvm_irqchip_assign_irqfd(KVMState *s, int fd, int virq, bool assign)
>          .flags = assign ? 0 : KVM_IRQFD_FLAG_DEASSIGN,
>      };
>  
> -    if (!kvm_irqchip_in_kernel()) {
> +    if (!kvm_irqfds_enabled()) {
>          return -ENOSYS;
>      }
>  
> diff --git a/kvm-stub.c b/kvm-stub.c
> index af1cb5e..179e5de 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -20,6 +20,7 @@
>  KVMState *kvm_state;
>  bool kvm_kernel_irqchip;
>  bool kvm_async_interrupt_injection;
> +bool kvm_irqfds_allowed;
>  
>  int kvm_init_vcpu(CPUArchState *env)
>  {
> diff --git a/kvm.h b/kvm.h
> index e6cbf12..2337eb0 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -25,6 +25,7 @@
>  extern int kvm_allowed;
>  extern bool kvm_kernel_irqchip;
>  extern bool kvm_async_interrupt_injection;
> +extern bool kvm_irqfds_allowed;
>  
>  #if defined CONFIG_KVM || !defined NEED_CPU_H
>  #define kvm_enabled()           (kvm_allowed)
> @@ -38,10 +39,19 @@ extern bool kvm_async_interrupt_injection;
>   * (where the vcpu must be stopped at a suitable point first).
>   */
>  #define kvm_async_interrupt_injection() (kvm_async_interrupt_injection)
> +/**
> + * kvm_irqfds_enabled:
> + *
> + * Returns: true if we can use irqfds to inject interrupts into
> + * a KVM CPU (ie the kernel supports irqfds and we are running
> + * with a configuration where it is meaningful to use them).
> + */
> +#define kvm_irqfds_enabled() (kvm_irqfds_allowed)
>  #else
>  #define kvm_enabled()           (0)
>  #define kvm_irqchip_in_kernel() (false)
>  #define kvm_async_interrupt_injection() (false)
> +#define kvm_irqfds_enabled() (false)
>  #endif
>  
>  struct kvm_run;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 503abeb..8e19a4d 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2045,4 +2045,8 @@ void kvm_arch_init_irq_routing(KVMState *s)
>           */
>          no_hpet = 1;
>      }
> +    /* We know at this point that we're using the in-kernel
> +     * irqchip, so we can use irqfds.
> +     */
> +    kvm_irqfds_allowed = true;
>  }
> 

Otherwise:

Acked-by: Jan Kiszka <jan.kiszka@siemens.com>

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux