On 23/09/2015 12:40, Thomas Huth wrote:
> On 17/09/15 15:09, David Gibson wrote:
>> When we have guest visible IOMMUs, we allow notifiers to be registered
>> which will be informed of all changes to IOMMU mappings. This is used by
>> vfio to keep the host IOMMU mappings in sync with guest IOMMU mappings.
>>
>> However, unlike with a memory region listener, an iommu notifier won't be
>> told about any mappings which already exist in the (guest) IOMMU at the
>> time it is registered. This can cause problems if hotplugging a VFIO
>> device onto a guest bus which had existing guest IOMMU mappings, but didn't
>> previously have an VFIO devices (and hence no host IOMMU mappings).
>>
>> This adds a memory_region_register_iommu_notifier_replay() function to
>> handle this case. As well as registering the new notifier it replays
>> existing mappings. Because the IOMMU memory region doesn't internally
>> remember the granularity of the guest IOMMU it has a small hack where the
>> caller must specify a granularity at which to replay mappings.
>>
>> If there are finer mappings in the guest IOMMU these will be reported in
>> the iotlb structures passed to the notifier which it must handle (probably
>> causing it to flag an error). This isn't new - the VFIO iommu notifier
>> must already handle notifications about guest IOMMU mappings too short
>> for it to represent in the host IOMMU.
>>
>> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
>> ---
>> include/exec/memory.h | 16 ++++++++++++++++
>> memory.c | 18 ++++++++++++++++++
>> 2 files changed, 34 insertions(+)
>
>> diff --git a/memory.c b/memory.c
>> index 0d8b2d9..6b5a2f1 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -1403,6 +1403,24 @@ void
>> memory_region_register_iommu_notifier(MemoryRegion *mr, Notifier *n)
>> notifier_list_add(&mr->iommu_notify, n);
>> }
>>
>> +void memory_region_register_iommu_notifier_replay(MemoryRegion *mr,
>> Notifier *n,
>> + hwaddr granularity, bool
>> is_write)
>
> granularity itself is not an address, but a size, isn't it? So using
> "hwaddr" sounds wrong here.
I disagree: in memory.c, all size are defined with hwaddr.
>> +{
>> + hwaddr addr;
>
> dma_addr_t ?
>
>> + IOMMUTLBEntry iotlb;
>> +
>> + memory_region_register_iommu_notifier(mr, n);
>> +
>> + for (addr = 0;
>> + int128_lt(int128_make64(addr), mr->size);
>> + addr += granularity) {
>> +
>> + iotlb = mr->iommu_ops->translate(mr, addr, is_write);
>> + if (iotlb.perm != IOMMU_NONE)
>> + n->notify(n, &iotlb);
>
> Missing curly braces.
>
>> + }
>> +}
>> +
>
> Thomas
>
