Re: [PATCH v4 12/15] vfio/common: Support device dirty page tracking with vIOMMU

[Joao Martins]

On 08/09/2023 12:52, Duan, Zhenzhong wrote:
> On 9/8/2023 6:11 PM, Joao Martins wrote:
>> On 08/09/2023 07:11, Duan, Zhenzhong wrote:
>>> Hi Joao,
>>>
>>> On 6/23/2023 5:48 AM, Joao Martins wrote:
>>>> Currently, device dirty page tracking with vIOMMU is not supported,
>>>> and a blocker is added and the migration is prevented.
>>>>
>>>> When vIOMMU is used, IOVA ranges are DMA mapped/unmapped on the fly as
>>>> requesting by the vIOMMU. These IOVA ranges can potentially be mapped
>>>> anywhere in the vIOMMU IOVA space as advertised by the VMM.
>>>>
>>>> To support device dirty tracking when vIOMMU enabled instead create the
>>>> dirty ranges based on the vIOMMU provided limits, which leads to the
>>>> tracking of the whole IOVA space regardless of what devices use.
>>>>
>>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>>> ---
>>>>    include/hw/vfio/vfio-common.h |  1 +
>>>>    hw/vfio/common.c              | 58 +++++++++++++++++++++++++++++------
>>>>    hw/vfio/pci.c                 |  7 +++++
>>>>    3 files changed, 56 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>>> index f41860988d6b..c4bafad084b4 100644
>>>> --- a/include/hw/vfio/vfio-common.h
>>>> +++ b/include/hw/vfio/vfio-common.h
>>>> @@ -71,6 +71,7 @@ typedef struct VFIOMigration {
>>>>    typedef struct VFIOAddressSpace {
>>>>        AddressSpace *as;
>>>>        bool no_dma_translation;
>>>> +    hwaddr max_iova;
>>>>        QLIST_HEAD(, VFIOContainer) containers;
>>>>        QLIST_ENTRY(VFIOAddressSpace) list;
>>>>    } VFIOAddressSpace;
>>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>>> index ecfb9afb3fb6..85fddef24026 100644
>>>> --- a/hw/vfio/common.c
>>>> +++ b/hw/vfio/common.c
>>>> @@ -428,6 +428,25 @@ static bool vfio_viommu_preset(void)
>>>>        return false;
>>>>    }
>>>>    +static int vfio_viommu_get_max_iova(hwaddr *max_iova)
>>>> +{
>>>> +    VFIOAddressSpace *space;
>>>> +
>>>> +    *max_iova = 0;
>>>> +
>>>> +    QLIST_FOREACH(space, &vfio_address_spaces, list) {
>>>> +        if (space->as == &address_space_memory) {
>>>> +            continue;
>>>> +        }
>>> Just curious why address_space_memory is bypassed?
>>>
>> But address_space_memory part is done by memory listeners
> 
> Only this part. Still think about the case with two vfio devices, one bypass
> iommu, the other not.
> 
> The device bypassing iommu will get address_space_memory, the other get iommu
> 
> address space. vfio_viommu_preset() return true for any device, so we never 
> run
> into
> 
> memory listener even for device bypassing iommu?

Yeap, that's correct. When I said earlier 'reworking this' I meant this part
exactly to do both.