On 17/07/2024 17:02, Joao Martins wrote: > On 17/07/2024 16:35, Joao Martins wrote: >> On 17/07/2024 10:20, Joao Martins wrote: >>> On 17/07/2024 03:38, Duan, Zhenzhong wrote: >>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >>>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644 >>>>> --- a/hw/vfio/migration.c >>>>> +++ b/hw/vfio/migration.c >>>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice >>>>> *vbasedev, Error **errp) >>>>> return !vfio_block_migration(vbasedev, err, errp); >>>>> } >>>>> >>>>> - if (!vbasedev->dirty_pages_supported) { >>>>> + if (!vbasedev->dirty_pages_supported && >>>>> + !vbasedev->bcontainer->dirty_pages_supported) { >>>>> if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) { >>>>> error_setg(&err, >>>>> "%s: VFIO device doesn't support device dirty >>>>> tracking", >>>> >>>> I'm not sure if this message needs to be updated, " VFIO device doesn't >>>> support device and IOMMU dirty tracking" >>>> >>>> Same for the below: >>>> >>>> warn_report("%s: VFIO device doesn't support device dirty tracking" >>> >>> >>> Ah yes, good catch. Additionally I think I should check device hwpt rather >>> than >>> container::dirty_pages_supported i.e. >>> >>> if (!vbasedev->dirty_pages_supported && >>> (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt))) >>> >>> This makes sure that migration is blocked with more accuracy >> >> I retract this comment as I think it can all be easily detected by not OR-ing >> the setting of vbasedev->bcontainer->dirty_pages_supported. I should put a >> warn_report_once() there. > > Something like this below. > > To be clear: this is mostly a safe guard against a theoretic case that we > don't > know it exists. For example on x86, this is homogeneous and I suspect server > ARM > to be the case too. embedded ARM might be different as there's so many > incantations of it. >
Except that it won't work with hotplug :( so the previous snip was actually a bit better. > @@ -267,6 +282,13 @@ static bool iommufd_cdev_autodomains_get(VFIODevice > *vbasedev, > vbasedev->hwpt = hwpt; > QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); > QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); > + > + if (container->bcontainer.dirty_pages_supported && > + !iommufd_hwpt_dirty_tracking(hwpt)) { > + warn_report("%s: IOMMU dirty tracking not supported\n", > vbasedev->name); > + } > + container->bcontainer.dirty_pages_supported = > + iommufd_hwpt_dirty_tracking(hwpt); > return true; > } > >