| Message ID | 20240212135643.5858-8-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/iommufd: IOMMUFD Dirty Tracking | expand |
Hi Joao, On 12/02/2024 15:56, Joao Martins wrote: > External email: Use caution opening links or attachments > > > By default VFIO migration is set to auto, which will support live > migration if the migration capability is set *and* also dirty page > tracking is supported. > > For testing purposes one can force enable without dirty page tracking > via enable-migration=on, but that option is generally left for testing > purposes. > > So starting with IOMMU dirty tracking it can use to acomodate the lack of > VF dirty page tracking allowing us to minimize the VF requirements for > migration and thus enabling migration by default for those. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > hw/vfio/iommufd.c | 3 +-- > hw/vfio/migration.c | 4 +++- > include/sysemu/iommufd.h | 1 + > 3 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index 697d40841d7f..78d8f4391b68 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -275,8 +275,7 @@ static int iommufd_cdev_detach_ioas_hwpt(VFIODevice *vbasedev, Error **errp) > return ret; > } > > -static bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, > - Error **errp) > +bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, Error **errp) > { > uint64_t caps; > int r; > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index 70e6b1a709f9..674e76b3f3df 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -938,7 +938,9 @@ 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->iommufd_dev.iommufd && Shouldn't we check the type of base_hdev instead? > + !iommufd_dirty_pages_supported(&vbasedev->iommufd_dev, &err))) { Maybe we can store IOMMUFD DPT support in iommufd_dev and use it instead of querying it here? Thanks. > if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) { > error_setg(&err, > "%s: VFIO device doesn't support device dirty tracking", > diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h > index bc6607e3d444..d6be49f2ac78 100644 > --- a/include/sysemu/iommufd.h > +++ b/include/sysemu/iommufd.h > @@ -53,6 +53,7 @@ typedef struct IOMMUFDDevice { > void iommufd_device_init(IOMMUFDDevice *idev); > int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t *caps, > Error **errp); > +bool iommufd_dirty_pages_supported(IOMMUFDDevice *idev, Error **errp); > int iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, > uint32_t pt_id, uint32_t flags, > uint32_t data_type, uint32_t data_len, > -- > 2.39.3 >
On 19/02/2024 10:12, Avihai Horon wrote: > Hi Joao, > > On 12/02/2024 15:56, Joao Martins wrote: >> External email: Use caution opening links or attachments >> >> >> By default VFIO migration is set to auto, which will support live >> migration if the migration capability is set *and* also dirty page >> tracking is supported. >> >> For testing purposes one can force enable without dirty page tracking >> via enable-migration=on, but that option is generally left for testing >> purposes. >> >> So starting with IOMMU dirty tracking it can use to acomodate the lack of >> VF dirty page tracking allowing us to minimize the VF requirements for >> migration and thus enabling migration by default for those. >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> hw/vfio/iommufd.c | 3 +-- >> hw/vfio/migration.c | 4 +++- >> include/sysemu/iommufd.h | 1 + >> 3 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >> index 697d40841d7f..78d8f4391b68 100644 >> --- a/hw/vfio/iommufd.c >> +++ b/hw/vfio/iommufd.c >> @@ -275,8 +275,7 @@ static int iommufd_cdev_detach_ioas_hwpt(VFIODevice >> *vbasedev, Error **errp) >> return ret; >> } >> >> -static bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, >> - Error **errp) >> +bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, Error **errp) >> { >> uint64_t caps; >> int r; >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >> index 70e6b1a709f9..674e76b3f3df 100644 >> --- a/hw/vfio/migration.c >> +++ b/hw/vfio/migration.c >> @@ -938,7 +938,9 @@ 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->iommufd_dev.iommufd && > > Shouldn't we check the type of base_hdev instead? > This was just me trying to be less tied to Zhenzhong series, but yes >> + !iommufd_dirty_pages_supported(&vbasedev->iommufd_dev, &err))) { > > Maybe we can store IOMMUFD DPT support in iommufd_dev and use it instead of > querying it here? It's a good idea, and originally I had the ::capabilities stored in the iommufd_dev and I was mainly checking this there. I could fetch the capabilities after we get an idev and then this just tests against HWCAP and this avoids having to call GET_HWINFO all the time
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index 697d40841d7f..78d8f4391b68 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -275,8 +275,7 @@ static int iommufd_cdev_detach_ioas_hwpt(VFIODevice *vbasedev, Error **errp) return ret; } -static bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, - Error **errp) +bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, Error **errp) { uint64_t caps; int r; diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 70e6b1a709f9..674e76b3f3df 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -938,7 +938,9 @@ 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->iommufd_dev.iommufd && + !iommufd_dirty_pages_supported(&vbasedev->iommufd_dev, &err))) { if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) { error_setg(&err, "%s: VFIO device doesn't support device dirty tracking", diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h index bc6607e3d444..d6be49f2ac78 100644 --- a/include/sysemu/iommufd.h +++ b/include/sysemu/iommufd.h @@ -53,6 +53,7 @@ typedef struct IOMMUFDDevice { void iommufd_device_init(IOMMUFDDevice *idev); int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t *caps, Error **errp); +bool iommufd_dirty_pages_supported(IOMMUFDDevice *idev, Error **errp); int iommufd_backend_alloc_hwpt(int iommufd, uint32_t dev_id, uint32_t pt_id, uint32_t flags, uint32_t data_type, uint32_t data_len,
By default VFIO migration is set to auto, which will support live migration if the migration capability is set *and* also dirty page tracking is supported. For testing purposes one can force enable without dirty page tracking via enable-migration=on, but that option is generally left for testing purposes. So starting with IOMMU dirty tracking it can use to acomodate the lack of VF dirty page tracking allowing us to minimize the VF requirements for migration and thus enabling migration by default for those. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- hw/vfio/iommufd.c | 3 +-- hw/vfio/migration.c | 4 +++- include/sysemu/iommufd.h | 1 + 3 files changed, 5 insertions(+), 3 deletions(-)