Re: [PATCH v6 9/9] vfio/common: Allow disabling device dirty page tracking

[Eric Auger]

On 7/23/24 10:42, Joao Martins wrote:
> On 23/07/2024 09:31, Eric Auger wrote:
>>
>> On 7/22/24 23:13, Joao Martins wrote:
>>> The property 'x-pre-copy-dirty-page-tracking' allows disabling the whole
>>> tracking of VF pre-copy phase of dirty page tracking, though it means
>>> that it will only be used at the start of the switchover phase.
>>>
>>> Add an option that disables the VF dirty page tracking, and fall
>>> back into container-based dirty page tracking. This also allows to
>>> use IOMMU dirty tracking even on VFs with their own dirty
>>> tracker scheme.
>> Forced ON value isn't really meaningful though. I mean only valid values
>> are AUTO and OFF, correct?
>>
> It's more like 'on' if supported.
>
> You could argue that AUTO_AUTO is the right value considering that device may 
> or
> may not support. But really both AUTO/ON are the same as the property that
> outlines device dirty tracking is ::dirty_tracking for the VF which is
> eventually seeded from VFIO pci feature. So this is mostly to override the
> default and hence AUTO_OFF is the only thing that it's tested against in the
> various places.
>
> Me picking AUTO_ON just mirrored how it's already done for
> x-pre-copy-dirty-page-tracking (and I see used e.g. kernel_irqchip and others)

OK

Thanks

Eric
>
>> Eric
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>>  include/hw/vfio/vfio-common.h | 1 +
>>>  hw/vfio/common.c              | 3 +++
>>>  hw/vfio/migration.c           | 4 +++-
>>>  hw/vfio/pci.c                 | 3 +++
>>>  4 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 1e02c98b09ba..fed499b199f0 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -138,6 +138,7 @@ typedef struct VFIODevice {
>>>      VFIOMigration *migration;
>>>      Error *migration_blocker;
>>>      OnOffAuto pre_copy_dirty_page_tracking;
>>> +    OnOffAuto device_dirty_page_tracking;
>>>      bool dirty_pages_supported;
>>>      bool dirty_tracking;
>>>      bool iommu_dirty_tracking;
>>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>>> index da12cbd56408..36d0cf6585b2 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -199,6 +199,9 @@ bool vfio_devices_all_device_dirty_tracking(const 
>>> VFIOContainerBase *bcontainer)
>>>      VFIODevice *vbasedev;
>>>  
>>>      QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>> +        if (vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF) {
>>> +            return false;
>>> +        }
>>>          if (!vbasedev->dirty_pages_supported) {
>>>              return false;
>>>          }
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index cbfaef7afffe..262d42a46e58 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -1036,7 +1036,9 @@ bool vfio_migration_realize(VFIODevice *vbasedev, 
>>> Error **errp)
>>>          return !vfio_block_migration(vbasedev, err, errp);
>>>      }
>>>  
>>> -    if (!vbasedev->dirty_pages_supported && 
>>> !vbasedev->iommu_dirty_tracking) {
>>> +    if ((!vbasedev->dirty_pages_supported ||
>>> +         vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF) &&
>>> +        !vbasedev->iommu_dirty_tracking) {
>>>          if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>              error_setg(&err,
>>>                         "%s: VFIO device doesn't support device and "
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 8c0f212a163e..a0767de54b8d 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -3364,6 +3364,9 @@ static Property vfio_pci_dev_properties[] = {
>>>      DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", 
>>> VFIOPCIDevice,
>>>                              vbasedev.pre_copy_dirty_page_tracking,
>>>                              ON_OFF_AUTO_ON),
>>> +    DEFINE_PROP_ON_OFF_AUTO("x-device-dirty-page-tracking", VFIOPCIDevice,
>>> +                            vbasedev.device_dirty_page_tracking,
>>> +                            ON_OFF_AUTO_ON),
>>>      DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
>>>                              display, ON_OFF_AUTO_OFF),
>>>      DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),