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 accomodate the lack of
VF dirty page tracking allowing us to minimize the VF requirements for
migration and thus enabling migration by default for those too.
While at it change the error messages to mention IOMMU dirty tracking as
well.
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
include/hw/vfio/vfio-common.h | 1 +
hw/vfio/iommufd.c | 2 +-
hw/vfio/migration.c | 11 ++++++-----
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 7e530c7869dc..00b9e933449e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -299,6 +299,7 @@ int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
uint64_t size, ram_addr_t ram_addr, Error **errp);
+bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
/* Returns 0 on success, or a negative errno. */
bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 7dd5d43ce06a..a998e8578552 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -111,7 +111,7 @@ static void iommufd_cdev_unbind_and_disconnect(VFIODevice *vbasedev)
iommufd_backend_disconnect(vbasedev->iommufd);
}
-static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
+bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
{
return hwpt && hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
}
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 34d4be2ce1b1..63ffa46c9652 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1036,16 +1036,17 @@ 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 &&
+ !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)) {
if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
error_setg(&err,
- "%s: VFIO device doesn't support device dirty tracking",
- vbasedev->name);
+ "%s: VFIO device doesn't support device and "
+ "IOMMU dirty tracking", vbasedev->name);
goto add_blocker;
}
- warn_report("%s: VFIO device doesn't support device dirty tracking",
- vbasedev->name);
+ warn_report("%s: VFIO device doesn't support device and "
+ "IOMMU dirty tracking", vbasedev->name);
}
ret = vfio_block_multiple_devices_migration(vbasedev, errp);
--
2.17.2On 7/19/24 14:05, Joao Martins wrote:
> 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 accomodate the lack of
> VF dirty page tracking allowing us to minimize the VF requirements for
> migration and thus enabling migration by default for those too.
>
> While at it change the error messages to mention IOMMU dirty tracking as
> well.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
> include/hw/vfio/vfio-common.h | 1 +
> hw/vfio/iommufd.c | 2 +-
> hw/vfio/migration.c | 11 ++++++-----
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 7e530c7869dc..00b9e933449e 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -299,6 +299,7 @@ int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
> VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
> int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
> uint64_t size, ram_addr_t ram_addr, Error **errp);
> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
>
> /* Returns 0 on success, or a negative errno. */
> bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 7dd5d43ce06a..a998e8578552 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -111,7 +111,7 @@ static void iommufd_cdev_unbind_and_disconnect(VFIODevice *vbasedev)
> iommufd_backend_disconnect(vbasedev->iommufd);
> }
>
> -static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
> {
> return hwpt && hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> }
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 34d4be2ce1b1..63ffa46c9652 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -1036,16 +1036,17 @@ 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 &&
> + !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)) {
Some platforms do not have IOMMUFD support and this call will need
some kind of abstract wrapper to reflect dirty tracking support in
the IOMMU backend.
Thanks,
C.
> if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
> error_setg(&err,
> - "%s: VFIO device doesn't support device dirty tracking",
> - vbasedev->name);
> + "%s: VFIO device doesn't support device and "
> + "IOMMU dirty tracking", vbasedev->name);
> goto add_blocker;
> }
>
> - warn_report("%s: VFIO device doesn't support device dirty tracking",
> - vbasedev->name);
> + warn_report("%s: VFIO device doesn't support device and "
> + "IOMMU dirty tracking", vbasedev->name);
> }
>
> ret = vfio_block_multiple_devices_migration(vbasedev, errp);
On 19/07/2024 15:17, Cédric Le Goater wrote:
> On 7/19/24 14:05, Joao Martins wrote:
>> 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 accomodate the lack of
>> VF dirty page tracking allowing us to minimize the VF requirements for
>> migration and thus enabling migration by default for those too.
>>
>> While at it change the error messages to mention IOMMU dirty tracking as
>> well.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> hw/vfio/iommufd.c | 2 +-
>> hw/vfio/migration.c | 11 ++++++-----
>> 3 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 7e530c7869dc..00b9e933449e 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -299,6 +299,7 @@ int vfio_devices_query_dirty_bitmap(const
>> VFIOContainerBase *bcontainer,
>> VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>> int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
>> uint64_t size, ram_addr_t ram_addr, Error **errp);
>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
>> /* Returns 0 on success, or a negative errno. */
>> bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 7dd5d43ce06a..a998e8578552 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -111,7 +111,7 @@ static void iommufd_cdev_unbind_and_disconnect(VFIODevice
>> *vbasedev)
>> iommufd_backend_disconnect(vbasedev->iommufd);
>> }
>> -static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>> {
>> return hwpt && hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>> }
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 34d4be2ce1b1..63ffa46c9652 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1036,16 +1036,17 @@ 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 &&
>> + !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)) {
>
>
> Some platforms do not have IOMMUFD support and this call will need
> some kind of abstract wrapper to reflect dirty tracking support in
> the IOMMU backend.
>
This was actually on purpose because only IOMMUFD presents a view of hardware
whereas type1 supporting dirty page tracking is not used as means to 'migration
is supported'.
The hwpt is nil in type1 and the helper checks that, so it should return false.
> Thanks,
>
> C.
>
>
>
>> if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>> error_setg(&err,
>> - "%s: VFIO device doesn't support device dirty tracking",
>> - vbasedev->name);
>> + "%s: VFIO device doesn't support device and "
>> + "IOMMU dirty tracking", vbasedev->name);
>> goto add_blocker;
>> }
>> - warn_report("%s: VFIO device doesn't support device dirty tracking",
>> - vbasedev->name);
>> + warn_report("%s: VFIO device doesn't support device and "
>> + "IOMMU dirty tracking", vbasedev->name);
>> }
>> ret = vfio_block_multiple_devices_migration(vbasedev, errp);
>
On 19/07/2024 15:24, Joao Martins wrote:
> On 19/07/2024 15:17, Cédric Le Goater wrote:
>> On 7/19/24 14:05, Joao Martins wrote:
>>> 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 accomodate the lack of
>>> VF dirty page tracking allowing us to minimize the VF requirements for
>>> migration and thus enabling migration by default for those too.
>>>
>>> While at it change the error messages to mention IOMMU dirty tracking as
>>> well.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> include/hw/vfio/vfio-common.h | 1 +
>>> hw/vfio/iommufd.c | 2 +-
>>> hw/vfio/migration.c | 11 ++++++-----
>>> 3 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 7e530c7869dc..00b9e933449e 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -299,6 +299,7 @@ int vfio_devices_query_dirty_bitmap(const
>>> VFIOContainerBase *bcontainer,
>>> VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>>> int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
>>> uint64_t size, ram_addr_t ram_addr, Error **errp);
>>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
>>> /* Returns 0 on success, or a negative errno. */
>>> bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index 7dd5d43ce06a..a998e8578552 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -111,7 +111,7 @@ static void iommufd_cdev_unbind_and_disconnect(VFIODevice
>>> *vbasedev)
>>> iommufd_backend_disconnect(vbasedev->iommufd);
>>> }
>>> -static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>>> {
>>> return hwpt && hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>>> }
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index 34d4be2ce1b1..63ffa46c9652 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -1036,16 +1036,17 @@ 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 &&
>>> + !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)) {
>>
>>
>> Some platforms do not have IOMMUFD support and this call will need
>> some kind of abstract wrapper to reflect dirty tracking support in
>> the IOMMU backend.
>>
>
> This was actually on purpose because only IOMMUFD presents a view of hardware
> whereas type1 supporting dirty page tracking is not used as means to 'migration
> is supported'.
>
> The hwpt is nil in type1 and the helper checks that, so it should return false.
>
Oh wait, maybe you're talking about CONFIG_IOMMUFD=n which I totally didn't
consider. Maybe this would be a elegant way to address it? Looks to pass my
build with CONFIG_IOMMUFD=n
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 61dd48e79b71..422ad4a5bdd1 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -300,7 +300,14 @@ int vfio_devices_query_dirty_bitmap(const VFIOContainerBase
*bcontainer,
VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
uint64_t size, ram_addr_t ram_addr, Error **errp);
+#ifdef CONFIG_IOMMUFD
bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
+#else
+static inline bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
+{
+ return false;
+}
+#endif
/* Returns 0 on success, or a negative errno. */
bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
On 19/07/2024 15:24, Joao Martins wrote:
> On 19/07/2024 15:17, Cédric Le Goater wrote:
>> On 7/19/24 14:05, Joao Martins wrote:
>>> 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 accomodate the lack of
>>> VF dirty page tracking allowing us to minimize the VF requirements for
>>> migration and thus enabling migration by default for those too.
>>>
>>> While at it change the error messages to mention IOMMU dirty tracking as
>>> well.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> include/hw/vfio/vfio-common.h | 1 +
>>> hw/vfio/iommufd.c | 2 +-
>>> hw/vfio/migration.c | 11 ++++++-----
>>> 3 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 7e530c7869dc..00b9e933449e 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -299,6 +299,7 @@ int vfio_devices_query_dirty_bitmap(const
>>> VFIOContainerBase *bcontainer,
>>> VFIOBitmap *vbmap, hwaddr iova, hwaddr size, Error **errp);
>>> int vfio_get_dirty_bitmap(const VFIOContainerBase *bcontainer, uint64_t iova,
>>> uint64_t size, ram_addr_t ram_addr, Error **errp);
>>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt);
>>> /* Returns 0 on success, or a negative errno. */
>>> bool vfio_device_get_name(VFIODevice *vbasedev, Error **errp);
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index 7dd5d43ce06a..a998e8578552 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -111,7 +111,7 @@ static void iommufd_cdev_unbind_and_disconnect(VFIODevice
>>> *vbasedev)
>>> iommufd_backend_disconnect(vbasedev->iommufd);
>>> }
>>> -static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>>> +bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
>>> {
>>> return hwpt && hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>>> }
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index 34d4be2ce1b1..63ffa46c9652 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -1036,16 +1036,17 @@ 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 &&
>>> + !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)) {
>>
>>
>> Some platforms do not have IOMMUFD support and this call will need
>> some kind of abstract wrapper to reflect dirty tracking support in
>> the IOMMU backend.
>>
>
> This was actually on purpose because only IOMMUFD presents a view of hardware
> whereas type1 supporting dirty page tracking is not used as means to 'migration
> is supported'.
>
> The hwpt is nil in type1 and the helper checks that, so it should return false.
>
Unless of course I misunderstood you.
This check is IOMMUFD specific, because it's one the mirroring hw support and
can be used to unblock live migration. Since initial VFIO live migration support
that type1 dirty tracking wasn't included in the checks that allow live
migration to occur. Another way of saying this is that: with type1 even if
dirty_pages_supported is true, we always add a live migration blocker in case
device doesn't have dirty page tracking. The change above is just meant to use
IOMMUFD dirty tracking which is hardware dependent and not block live migration.
>> Thanks,
>>
>> C.
>>
>>
>>
>>> if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>> error_setg(&err,
>>> - "%s: VFIO device doesn't support device dirty tracking",
>>> - vbasedev->name);
>>> + "%s: VFIO device doesn't support device and "
>>> + "IOMMU dirty tracking", vbasedev->name);
>>> goto add_blocker;
>>> }
>>> - warn_report("%s: VFIO device doesn't support device dirty tracking",
>>> - vbasedev->name);
>>> + warn_report("%s: VFIO device doesn't support device and "
>>> + "IOMMU dirty tracking", vbasedev->name);
>>> }
>>> ret = vfio_block_multiple_devices_migration(vbasedev, errp);
>>
>
© 2016 - 2024 Red Hat, Inc.