Hi Peter,
On 12/03/18 12:59, Peter Maydell wrote:
> 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
ARM SMMU translates MSIs whereas Intel/AMD IOMMU do not translate them.
Hence this implementation
> * the code here needs to know specifically that this is an SMMU
> and not some other kind of IOMMU
Yes when introducing virtio-iommu we will need to get this fixed. We
need a way to retrieve the iommu mr from the as. I will work on this
concurrently.
>
> 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.
OK
Thanks
Eric
>
> thanks
> -- PMM
>
