Re: [Qemu-devel] [RFC][PATCH 05/11] kvm: Introduce kvm_irqchip_add_msi_route

[Alex Williamson]

On Mon, 2012-05-14 at 18:07 -0300, Jan Kiszka wrote:
> Add a service that establishes a static route from a virtual IRQ line to
> an MSI message. Will be used for IRQFD and device assignment. As we will
> use this service outside of CONFIG_KVM protected code, stub it properly.
> 
> Signed-off-by: Jan Kiszka <address@hidden>
> ---
>  kvm-all.c  |   31 +++++++++++++++++++++++++++++++
>  kvm-stub.c |    8 ++++++++
>  kvm.h      |   10 ++++++----
>  3 files changed, 45 insertions(+), 4 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 8ab83db..f45b852 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1080,6 +1080,32 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>      return kvm_irqchip_set_irq(s, route->kroute.gsi, 1);
>  }
>  
> +int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
> +{
> +    struct kvm_irq_routing_entry kroute;
> +    int gsi;
> +
> +    if (!kvm_irqchip_in_kernel()) {
> +        return -ENOSYS;
> +    }
> +
> +    gsi = kvm_get_pseudo_gsi(s);
> +    if (gsi < 0) {
> +        return gsi;
> +    }
> +
> +    kroute.gsi = gsi;
> +    kroute.type = KVM_IRQ_ROUTING_MSI;
> +    kroute.flags = 0;
> +    kroute.u.msi.address_lo = (uint32_t)msg.address;
> +    kroute.u.msi.address_hi = msg.address >> 32;
> +    kroute.u.msi.data = msg.data;
> +
> +    kvm_add_routing_entry(s, &kroute);
> +
> +    return gsi;
> +}
> +
>  #else /* !KVM_CAP_IRQ_ROUTING */
>  
>  static void kvm_init_irq_routing(KVMState *s)
> @@ -1090,6 +1116,11 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>  {
>      abort();
>  }
> +
> +int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
> +{
> +    abort();
> +}
>  #endif /* !KVM_CAP_IRQ_ROUTING */


Jan,

Could we perhaps return a sane error value here?  This seems to be the
initial function used to setup direct MSI injection and if we have code
like:

virq = kvm_irqchip_add_msi_route(...)
if (virq < 0) {
    slow path...
} else {
    fast path...
}

It works on x86 w/ or w/o irqchip, works with kvm disabled, but gives an
abort on !x86.  I really don't want to have to surround the above in a
#define or in-kernel ioapic test.  Thanks,

Alex

>  static int kvm_irqchip_create(KVMState *s)
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 47c573d..db3a7dc 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -12,10 +12,13 @@
>  
>  #include "qemu-common.h"
>  #include "hw/hw.h"
> +#include "hw/msi.h"
>  #include "cpu.h"
>  #include "gdbstub.h"
>  #include "kvm.h"
>  
> +KVMState *kvm_state;
> +
>  int kvm_init_vcpu(CPUArchState *env)
>  {
>      return -ENOSYS;
> @@ -128,3 +131,8 @@ int kvm_on_sigbus(int code, void *addr)
>  {
>      return 1;
>  }
> +
> +int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
> +{
> +    return -ENOSYS;
> +}
> diff --git a/kvm.h b/kvm.h
> index 8b061bd..67df1f1 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -44,6 +44,10 @@ typedef struct KVMCapabilityInfo {
>  #define KVM_CAP_INFO(CAP) { "KVM_CAP_" stringify(CAP), KVM_CAP_##CAP }
>  #define KVM_CAP_LAST_INFO { NULL, 0 }
>  
> +struct KVMState;
> +typedef struct KVMState KVMState;
> +extern KVMState *kvm_state;
> +
>  /* external API */
>  
>  int kvm_init(void);
> @@ -88,10 +92,6 @@ int kvm_on_sigbus(int code, void *addr);
>  
>  /* internal API */
>  
> -struct KVMState;
> -typedef struct KVMState KVMState;
> -extern KVMState *kvm_state;
> -
>  int kvm_ioctl(KVMState *s, int type, ...);
>  
>  int kvm_vm_ioctl(KVMState *s, int type, ...);
> @@ -213,4 +213,6 @@ int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t 
> val, bool assign,
>                             uint32_t size);
>  
>  int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool 
> assign);
> +
> +int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg);
>  #endif