diff mbox series

[1/9] vfio/container: Add dirty tracking started flag

Message ID 20241216094638.26406-2-avihaih@nvidia.com (mailing list archive)
State New
Headers show
Series migration: Drop/unexport migration_is_device() and migration_is_active() | expand

Commit Message

Avihai Horon Dec. 16, 2024, 9:46 a.m. UTC 
Add a flag to VFIOContainerBase that indicates whether dirty tracking
has been started for the container or not.

This will be used in the following patches to allow dirty page syncs
only if dirty tracking has been started.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/hw/vfio/vfio-container-base.h | 1 +
 hw/vfio/container-base.c              | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Joao Martins Dec. 16, 2024, 12:15 p.m. UTC | #1 
On 16/12/2024 09:46, Avihai Horon wrote:
> Add a flag to VFIOContainerBase that indicates whether dirty tracking
> has been started for the container or not.
> 
> This will be used in the following patches to allow dirty page syncs
> only if dirty tracking has been started.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

Just a nit below, but it's optional.

> ---
>  include/hw/vfio/vfio-container-base.h | 1 +
>  hw/vfio/container-base.c              | 8 +++++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index 62a8b60d87..4cff9943ab 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -44,6 +44,7 @@ typedef struct VFIOContainerBase {
>      unsigned long pgsizes;
>      unsigned int dma_max_mappings;
>      bool dirty_pages_supported;
> +    bool dirty_pages_started; /* Protected by BQL */
>      QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>      QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>      QLIST_ENTRY(VFIOContainerBase) next;
> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
> index 6f86c37d97..48fc75cd62 100644
> --- a/hw/vfio/container-base.c
> +++ b/hw/vfio/container-base.c
> @@ -64,13 +64,19 @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
>                                             bool start, Error **errp)
>  {
>      VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
> +    int ret;
>  
>      if (!bcontainer->dirty_pages_supported) {
>          return 0;
>      }
>  

It's a nop when no state is changed; don't know if you wanna capture that here.
Something like this below now that you track container dirty tracking status:

if (!(bcontainer->dirty_pages_started == start)) {
	return 0;
}

>      g_assert(vioc->set_dirty_page_tracking);
> -    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
> +    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
> +    if (!ret) {
> +        bcontainer->dirty_pages_started = start;
> +    }
> +
> +    return ret;
>  }


>  
>  int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
Avihai Horon Dec. 16, 2024, 2:46 p.m. UTC | #2 
On 16/12/2024 14:15, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> On 16/12/2024 09:46, Avihai Horon wrote:
>> Add a flag to VFIOContainerBase that indicates whether dirty tracking
>> has been started for the container or not.
>>
>> This will be used in the following patches to allow dirty page syncs
>> only if dirty tracking has been started.
>>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>
> Just a nit below, but it's optional.
>
>> ---
>>   include/hw/vfio/vfio-container-base.h | 1 +
>>   hw/vfio/container-base.c              | 8 +++++++-
>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
>> index 62a8b60d87..4cff9943ab 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -44,6 +44,7 @@ typedef struct VFIOContainerBase {
>>       unsigned long pgsizes;
>>       unsigned int dma_max_mappings;
>>       bool dirty_pages_supported;
>> +    bool dirty_pages_started; /* Protected by BQL */
>>       QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
>>       QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
>>       QLIST_ENTRY(VFIOContainerBase) next;
>> diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
>> index 6f86c37d97..48fc75cd62 100644
>> --- a/hw/vfio/container-base.c
>> +++ b/hw/vfio/container-base.c
>> @@ -64,13 +64,19 @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
>>                                              bool start, Error **errp)
>>   {
>>       VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
>> +    int ret;
>>
>>       if (!bcontainer->dirty_pages_supported) {
>>           return 0;
>>       }
>>
> It's a nop when no state is changed; don't know if you wanna capture that here.
> Something like this below now that you track container dirty tracking status:
>
> if (!(bcontainer->dirty_pages_started == start)) {
>          return 0;
> }

I guess you mean:

if (bcontainer->dirty_pages_started == start) {
     return 0;
}

Yes, I can add it.

Thanks!

>
>>       g_assert(vioc->set_dirty_page_tracking);
>> -    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
>> +    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
>> +    if (!ret) {
>> +        bcontainer->dirty_pages_started = start;
>> +    }
>> +
>> +    return ret;
>>   }
>
>>   int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index 62a8b60d87..4cff9943ab 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -44,6 +44,7 @@  typedef struct VFIOContainerBase {
     unsigned long pgsizes;
     unsigned int dma_max_mappings;
     bool dirty_pages_supported;
+    bool dirty_pages_started; /* Protected by BQL */
     QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
     QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
     QLIST_ENTRY(VFIOContainerBase) next;
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index 6f86c37d97..48fc75cd62 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -64,13 +64,19 @@  int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
                                            bool start, Error **errp)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    int ret;
 
     if (!bcontainer->dirty_pages_supported) {
         return 0;
     }
 
     g_assert(vioc->set_dirty_page_tracking);
-    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    if (!ret) {
+        bcontainer->dirty_pages_started = start;
+    }
+
+    return ret;
 }
 
 int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,