On 17 February 2018 at 18:46, Eric Auger <eric.au...@redhat.com> wrote:
> In case the MSI is translated by an IOMMU we need to fixup the
> MSI route with the translated address.
>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> ---
>
> v5 -> v6:
> - use IOMMUMemoryRegionClass API
>
> It is still unclear to me if we need to register an IOMMUNotifier
> to handle any change in the MSI doorbell which would occur behind
> the scene and would not lead to any call to kvm_arch_fixup_msi_route().

Paolo, do you know the answer to this question ?

> ---
>  target/arm/kvm.c        | 27 +++++++++++++++++++++++++++
>  target/arm/trace-events |  3 +++
>  2 files changed, 30 insertions(+)
>
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 1219d00..9f5976a 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -20,8 +20,13 @@
>  #include "sysemu/kvm.h"
>  #include "kvm_arm.h"
>  #include "cpu.h"
> +#include "trace.h"
>  #include "internals.h"
>  #include "hw/arm/arm.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/msi.h"
> +#include "hw/arm/smmu-common.h"
> +#include "hw/arm/smmuv3.h"
>  #include "exec/memattrs.h"
>  #include "exec/address-spaces.h"
>  #include "hw/boards.h"
> @@ -666,6 +671,28 @@ int kvm_arm_vgic_probe(void)
>  int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>                               uint64_t address, uint32_t data, PCIDevice *dev)
>  {
> +    AddressSpace *as = pci_device_iommu_address_space(dev);
> +    IOMMUMemoryRegionClass *imrc;
> +    IOMMUTLBEntry entry;
> +    SMMUDevice *sdev;
> +
> +    if (as == &address_space_memory) {
> +        return 0;
> +    }
> +
> +    /* MSI doorbell address is translated by an IOMMU */
> +    sdev = container_of(as, SMMUDevice, as);
> +    imrc = IOMMU_MEMORY_REGION_GET_CLASS(&sdev->iommu);
> +
> +    entry = imrc->translate(&sdev->iommu, address, IOMMU_WO);
> +
> +    route->u.msi.address_lo = entry.translated_addr;
> +    route->u.msi.address_hi = entry.translated_addr >> 32;
> +
> +    trace_kvm_arm_fixup_msi_route(address, sdev->devfn,
> +                                  sdev->iommu.parent_obj.name,
> +                                  entry.translated_addr);
> +
>      return 0;
>  }

It seems a bit odd that:
 * the code for arm for "PCI devices behind IOMMU need to have
   the MSI doorbell writes go through the IOMMU" looks rather
   different from the code for x86 for the same thing
 * the code here needs to know specifically that this is an SMMU
   and not some other kind of IOMMU

I would have expected this to be more generic-to-all-IOMMU
APIs and maybe even implemented in the generic KVM support code...

The x86 code seems to be similarly x86-specific though, so
this is more of a "perhaps there is a cleanup opportunity here"
observation I guess.

thanks
-- PMM

Reply via email to