Re: [PATCH v6 05/12] vfio: Support for RamDiscardMgr in the !vIOMMU case

[David Hildenbrand <david@redhat.com>]

On 22.02.21 14:20, Paolo Bonzini wrote:
> On 22/02/21 12:57, David Hildenbrand wrote:
>>
>> +static int vfio_sync_ram_discard_listener_dirty_bitmap(VFIOContainer *container,
>> +                                                   MemoryRegionSection *section)
>> +{
>> +    RamDiscardMgr *rdm = memory_region_get_ram_discard_mgr(section->mr);
>> +    RamDiscardMgrClass *rdmc = RAM_DISCARD_MGR_GET_CLASS(rdm);
>> +    VFIORamDiscardListener tmp_vrdl, *vrdl = NULL;
>> +
>> +    QLIST_FOREACH(vrdl, &container->vrdl_list, next) {
>> +        if (vrdl->mr == section->mr &&
>> +            vrdl->offset_within_region == section->offset_within_region) {
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (!vrdl) {
>> +        hw_error("vfio: Trying to sync missing RAM discard listener");
>> +    }
>> +
>> +    tmp_vrdl = *vrdl;
>> +    ram_discard_listener_init(&tmp_vrdl.listener,
>> +                              vfio_ram_discard_notify_dirty_bitmap, NULL, NULL);
>> +    return rdmc->replay_populated(rdm, section->mr, &tmp_vrdl.listener);
>> +}
>> +
> 
> Can you explain why this is related to the sync_dirty_bitmap call?  This
> needs a comment in vfio_sync_dirty_bitmap.

We can only synchronize the parts that actually got mapped via VFIO. So 
I have to walk all parts that are populated (and thus, were mapped via 
VFIO). This is similar to the IOMMU notifier handling.

> 
> Also, why can't you just pass vrdl to the call?

I have to modify the callbacks. Similarly done for 
memory_region_iommu_replay().

-- 
Thanks,

David / dhildenb