Re: [Qemu-devel] [PATCH 3/3 RPC] kvm: extend kvm_irqchip_add_msi_route to work on s390

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Wed, 22 Oct 2014 17:12:00 +0200
Frank Blaschka <blaschka@linux.vnet.ibm.com> wrote:

> From: Frank Blaschka <frank.blaschka@de.ibm.com>
> 
> on s390 MSI-X irqs are presented as thin or adapter interrupts
> for this we have to reorganize the routing entry to contain
> valid information for the adapter interrupt code on s390.
> To minimize impact on existing code we introduce an architecture
> function to fixup the routing entry.
> 
> Signed-off-by: Frank Blaschka <frank.blaschka@de.ibm.com>
> ---
>  include/sysemu/kvm.h |  4 ++++
>  kvm-all.c            |  2 ++
>  target-arm/kvm.c     |  5 +++++
>  target-i386/kvm.c    |  5 +++++
>  target-mips/kvm.c    |  5 +++++
>  target-ppc/kvm.c     |  5 +++++
>  target-s390x/kvm.c   | 25 +++++++++++++++++++++++++
>  7 files changed, 51 insertions(+)
> 

> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 9280132..c3b3b0f 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -43,6 +43,7 @@
>  #include "trace.h"
>  #include "qapi-event.h"
>  #include "pci_ic.h"
> +#include "hw/s390x/s390-pci-bus.h"
> 
>  /* #define DEBUG_KVM */
> 
> @@ -1639,3 +1640,27 @@ int kvm_s390_get_machine_props(S390MachineProps *prop)
>  {
>      return cpu_model_get(KVM_S390_VM_CPU_MACHINE, (uint64_t) prop);
>  }
> +
> +void kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
> +                              uint64_t address, uint32_t data)
> +{
> +    S390PCIBusDevice *pbdev;
> +    uint32_t fid = data >> ZPCI_MSI_VEC_BITS;
> +    uint32_t vec = data & ZPCI_MSI_VEC_MASK;

I think this needs a comment so it is easier to understand why you're
mucking around with PCI devices and I/O adapters here without referring
to git blame :)

> +
> +    pbdev = s390_pci_find_dev_by_fid(fid);
> +    if (!pbdev) {
> +        DPRINTF("add_msi_route no dev\n");
> +        return;

This feels wrong. Shouldn't this function fail if you can't find the
pci device? The calling function can probably just release the virq
again and return an error.

> +    }
> +
> +    pbdev->routes.adapter.ind_offset = vec;
> +
> +    route->type = KVM_IRQ_ROUTING_S390_ADAPTER;
> +    route->flags = 0;
> +    route->u.adapter.summary_addr = pbdev->routes.adapter.summary_addr;
> +    route->u.adapter.ind_addr = pbdev->routes.adapter.ind_addr;
> +    route->u.adapter.summary_offset = pbdev->routes.adapter.summary_offset;
> +    route->u.adapter.ind_offset = pbdev->routes.adapter.ind_offset;
> +    route->u.adapter.adapter_id = pbdev->routes.adapter.adapter_id;
> +}