Re: [Qemu-arm] [PATCH v9 11/14] target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route

[Auger Eric]

Hi Paolo,

On 13/03/18 14:37, Paolo Bonzini wrote:
> On 12/03/2018 16:16, Auger Eric wrote:
>>> 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 ?
> 
> Yes, x86 is wrong in this respect (it wouldn't use an IOMMUNotifier, but
> it still should process Interrupt Entry Cache invalidations).

I am not sure anymore of the issue we want to fix. The concen I had in
mind was: what if the MSI doorbell gets unmapped. We would need to be
notified in some way. As the doorbell lies in MMIO space, no
IOMMUNotifier is attached to it at the moment. On the other hand, if the
doorbell gets unmapped, can't we expect some kind of tear down of the
route? Paolo, is it the use case you are talking about here?
> 
>>> 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
> 
> More precisely, Intel IOMMU implements interrupt remapping through an
> MMIO region instead of an IOMMU region, because on x86 interrupt
> remapping can also change the MSI value and not just the address.
thank you for the clarification. Yes I too much focused on IOMMU DMA
translation and forgot the x86 interrupt remapping stuff which
corresponds to what is done here.
> 
>>>  * 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.
> 
> Probably something like address_space_translate can be used?
Yes achieved with address_space_translate + memory_region_find

Thanks

Eric

> 
> Paolo
>