Re: [PATCH] virtio-iommu: Handle non power of 2 range invalidations

[Auger Eric <eric.auger@redhat.com>]

Hi Peter,

On 2/18/21 5:42 PM, Peter Xu wrote:
> Eric,
> 
> On Thu, Feb 18, 2021 at 03:16:50PM +0100, Eric Auger wrote:
>> @@ -164,12 +166,27 @@ static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr virt_start,
>>  
>>      event.type = IOMMU_NOTIFIER_UNMAP;
>>      event.entry.target_as = &address_space_memory;
>> -    event.entry.addr_mask = virt_end - virt_start;
>> -    event.entry.iova = virt_start;
>>      event.entry.perm = IOMMU_NONE;
>>      event.entry.translated_addr = 0;
>> +    event.entry.addr_mask = mask;
>> +    event.entry.iova = virt_start;
>>  
>> -    memory_region_notify_iommu(mr, 0, event);
>> +    if (mask == UINT64_MAX) {
>> +        memory_region_notify_iommu(mr, 0, event);
>> +    }
>> +
>> +    size = mask + 1;
>> +
>> +    while (size) {
>> +        uint8_t highest_bit = 64 - clz64(size) - 1;
> 
> I'm not sure fetching highest bit would work right. E.g., with start=0x11000
> and size=0x11000 (then we need to unmap 0x11000-0x22000), current code will
> first try to invalidate range (0x11000, 0x10000), that seems still invalid
> since 0x11000 is not aligned to 0x10000 page mask.

Hum I thought aligning the size was sufficient. Where is it checked exactly?
> 
> I think the same trick in vtd_address_space_unmap() would work.  If you agree,
> maybe we can generalize that get_naturally_aligned_size() out, but maybe with a
> better name as a helper?

Yep I need to read the code again ;-)

Thank you!

Eric
> 
> Thanks,
>