Re: [Qemu-devel] [PATCH v10 16/26] intel_iommu: add support for split irqchip

[Paolo Bonzini]

On 21/06/2016 09:47, Peter Xu wrote:
> @@ -3323,6 +3325,31 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t 
> dev_id)
>  int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>                               uint64_t address, uint32_t data, PCIDevice *dev)
>  {
> +    X86IOMMUState *iommu = x86_iommu_get_default();
> +
> +    if (iommu) {
> +        int ret;
> +        MSIMessage src, dst;
> +        X86IOMMUClass *class = X86_IOMMU_GET_CLASS(iommu);
> +
> +        src.address = route->u.msi.address_hi;
> +        src.address <<= VTD_MSI_ADDR_HI_SHIFT;
> +        src.address |= route->u.msi.address_lo;
> +        src.data = route->u.msi.data;
> +
> +        ret = class->int_remap(iommu, &src, &dst, dev ? \
> +                               pci_requester_id(dev) : \
> +                               X86_IOMMU_SID_INVALID);
> +        if (ret) {
> +            trace_kvm_x86_fixup_msi_error(route->gsi);
> +            return 1;
> +        }
> +
> +        route->u.msi.address_hi = dst.address >> VTD_MSI_ADDR_HI_SHIFT;
> +        route->u.msi.address_lo = dst.address & VTD_MSI_ADDR_LO_MASK;
> +        route->u.msi.data = dst.data;
> +    }
> +
>      return 0;
>  }

I don't like this particularly.  Instead, I think the X86 IOMMU class
should implement a new interface "MSIRemapper", and PCIBus should have a
pointer to MSIRemapper*.  Then this can become:

    if (dev) {
        PCIBus *bus = PCI_BUS(qdev_get_parent_bus(DEVICE(dev));
        if (bus->remapper) {
            msi_remapper_fixup_route(bus->remapper, &src, &dst,
                                     pci_requester_id(dev));
        }
    }

That said, I'm okay with the patch as is because the issue is not with
the x86 implementation but with kvm_arch_fixup_msi_route.  S390 should
be able to do the same, by implementing MSIRemapper in S390pciState (I
think).

Paolo