Re: [PATCH v2 01/20] memory: Adjust event ranges to fit within notifier boundaries

[Alejandro Jimenez <alejandro.j.jimenez@oracle.com>]

On 6/12/25 2:54 AM, Vasant Hegde wrote:
> Alejandro,
> 
> 
> On 5/2/2025 7:45 AM, Alejandro Jimenez wrote:
>> Invalidating the entire address space (i.e. range of [0, ~0ULL]) is a
>> valid and required operation by vIOMMU implementations. However, such
>> invalidations currently trigger an assertion unless they originate from
>> device IOTLB invalidations.
>>
>> Although in recent Linux guests this case is not exercised by the VTD
>> implementation due to various optimizations, the assertion will be hit
>> by upcoming AMD vIOMMU changes to support DMA address translation. More
>> specifically, when running a Linux guest with VFIO passthrough device,
>> and a kernel that does not contain commmit 3f2571fed2fa ("iommu/amd:
>> Remove redundant domain flush from attach_device()").
> 
> FYI. Its easy to send invalidate all without above commit (as it does it in
> every attach), there are other paths where kernel will still send invalidate
> all.. Like detaching/attaching device, etc.
> 

ACK, thank you for the comment/confirmation. I just mentioned an easily 
reproducible case where it causes a crash during guest boot with just 
passing a VF to the guest in the QEMU cmdline, not hoptplug or other 
more complex scenarios required. Your argument above is part of why I 
chose to remove the assertion completely instead of special casing and 
breaking down this range on the amd_iommu.c code handling the invalidation.

Thank you,
Alejandro

> 
> -Vasant
> 
> 
> 
>>
>> Remove the assertion altogether and adjust the range to ensure it does
>> not cross notifier boundaries.
>>
>> Signed-off-by: Alejandro Jimenez <alejandro.j.jimenez@oracle.com>
>> ---
>>   system/memory.c | 10 +++-------
>>   1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/system/memory.c b/system/memory.c
>> index 71434e7ad02c..7ad2fc098341 100644
>> --- a/system/memory.c
>> +++ b/system/memory.c
>> @@ -2021,13 +2021,9 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
>>           return;
>>       }
>>   
>> -    if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
>> -        /* Crop (iova, addr_mask) to range */
>> -        tmp.iova = MAX(tmp.iova, notifier->start);
>> -        tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova;
>> -    } else {
>> -        assert(entry->iova >= notifier->start && entry_end <= notifier->end);
>> -    }
>> +    /* Crop (iova, addr_mask) to range */
>> +    tmp.iova = MAX(tmp.iova, notifier->start);
>> +    tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova;
>>   
>>       if (event->type & notifier->notifier_flags) {
>>           notifier->notify(notifier, &tmp);
>