[PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase

Zhenzhong Duan posted 20 patches 3 months, 2 weeks ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, Yi Liu <yi.l.liu@intel.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>
There is a newer version of this series
[PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase
Posted by Zhenzhong Duan 3 months, 2 weeks ago
When bypass_ro is true, read only memory section is bypassed from
mapping in the container.

This is a preparing patch to workaround Intel ERRATA_772415.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/listener.c                    | 13 +++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index bded6e993f..31fd784d76 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -51,6 +51,7 @@ typedef struct VFIOContainerBase {
     QLIST_HEAD(, VFIODevice) device_list;
     GList *iova_ranges;
     NotifierWithReturn cpr_reboot_notifier;
+    bool bypass_ro;
 } VFIOContainerBase;
 
 typedef struct VFIOGuestIOMMU {
diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
index f498e23a93..c64aa4539e 100644
--- a/hw/vfio/listener.c
+++ b/hw/vfio/listener.c
@@ -364,7 +364,8 @@ static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
     return true;
 }
 
-static bool vfio_listener_valid_section(MemoryRegionSection *section,
+static bool vfio_listener_valid_section(VFIOContainerBase *bcontainer,
+                                        MemoryRegionSection *section,
                                         const char *name)
 {
     if (vfio_listener_skipped_section(section)) {
@@ -375,6 +376,10 @@ static bool vfio_listener_valid_section(MemoryRegionSection *section,
         return false;
     }
 
+    if (bcontainer && bcontainer->bypass_ro && section->readonly) {
+        return false;
+    }
+
     if (unlikely((section->offset_within_address_space &
                   ~qemu_real_host_page_mask()) !=
                  (section->offset_within_region & ~qemu_real_host_page_mask()))) {
@@ -494,7 +499,7 @@ void vfio_container_region_add(VFIOContainerBase *bcontainer,
     int ret;
     Error *err = NULL;
 
-    if (!vfio_listener_valid_section(section, "region_add")) {
+    if (!vfio_listener_valid_section(bcontainer, section, "region_add")) {
         return;
     }
 
@@ -655,7 +660,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
     int ret;
     bool try_unmap = true;
 
-    if (!vfio_listener_valid_section(section, "region_del")) {
+    if (!vfio_listener_valid_section(bcontainer, section, "region_del")) {
         return;
     }
 
@@ -812,7 +817,7 @@ static void vfio_dirty_tracking_update(MemoryListener *listener,
         container_of(listener, VFIODirtyRangesListener, listener);
     hwaddr iova, end;
 
-    if (!vfio_listener_valid_section(section, "tracking_update") ||
+    if (!vfio_listener_valid_section(NULL, section, "tracking_update") ||
         !vfio_get_section_iova_range(dirty->bcontainer, section,
                                      &iova, &end, NULL)) {
         return;
-- 
2.47.1
Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase
Posted by Cédric Le Goater 3 months, 2 weeks ago
On 7/29/25 11:20, Zhenzhong Duan wrote:
> When bypass_ro is true, read only memory section is bypassed from
> mapping in the container.
> 
> This is a preparing patch to workaround Intel ERRATA_772415.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-container-base.h |  1 +
>   hw/vfio/listener.c                    | 13 +++++++++----
>   2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
> index bded6e993f..31fd784d76 100644
> --- a/include/hw/vfio/vfio-container-base.h
> +++ b/include/hw/vfio/vfio-container-base.h
> @@ -51,6 +51,7 @@ typedef struct VFIOContainerBase {
>       QLIST_HEAD(, VFIODevice) device_list;
>       GList *iova_ranges;
>       NotifierWithReturn cpr_reboot_notifier;
> +    bool bypass_ro;
>   } VFIOContainerBase;
>   
>   typedef struct VFIOGuestIOMMU {
> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
> index f498e23a93..c64aa4539e 100644
> --- a/hw/vfio/listener.c
> +++ b/hw/vfio/listener.c
> @@ -364,7 +364,8 @@ static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
>       return true;
>   }
>   
> -static bool vfio_listener_valid_section(MemoryRegionSection *section,
> +static bool vfio_listener_valid_section(VFIOContainerBase *bcontainer,
> +                                        MemoryRegionSection *section,
>                                           const char *name)

Instead of adding a 'VFIOContainerBase *' argument, I would add an
extra 'bool bypass_ro' argument.

Thanks,

C.


>   {
>       if (vfio_listener_skipped_section(section)) {
> @@ -375,6 +376,10 @@ static bool vfio_listener_valid_section(MemoryRegionSection *section,
>           return false;
>       }
>   
> +    if (bcontainer && bcontainer->bypass_ro && section->readonly) {
> +        return false;
> +    }
> +
>       if (unlikely((section->offset_within_address_space &
>                     ~qemu_real_host_page_mask()) !=
>                    (section->offset_within_region & ~qemu_real_host_page_mask()))) {
> @@ -494,7 +499,7 @@ void vfio_container_region_add(VFIOContainerBase *bcontainer,
>       int ret;
>       Error *err = NULL;
>   
> -    if (!vfio_listener_valid_section(section, "region_add")) {
> +    if (!vfio_listener_valid_section(bcontainer, section, "region_add")) {
>           return;
>       }
>   
> @@ -655,7 +660,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
>       int ret;
>       bool try_unmap = true;
>   
> -    if (!vfio_listener_valid_section(section, "region_del")) {
> +    if (!vfio_listener_valid_section(bcontainer, section, "region_del")) {
>           return;
>       }
>   
> @@ -812,7 +817,7 @@ static void vfio_dirty_tracking_update(MemoryListener *listener,
>           container_of(listener, VFIODirtyRangesListener, listener);
>       hwaddr iova, end;
>   
> -    if (!vfio_listener_valid_section(section, "tracking_update") ||
> +    if (!vfio_listener_valid_section(NULL, section, "tracking_update") ||
>           !vfio_get_section_iova_range(dirty->bcontainer, section,
>                                        &iova, &end, NULL)) {
>           return;
RE: [PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase
Posted by Duan, Zhenzhong 3 months, 2 weeks ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in
>VFIOContainerBase
>
>On 7/29/25 11:20, Zhenzhong Duan wrote:
>> When bypass_ro is true, read only memory section is bypassed from
>> mapping in the container.
>>
>> This is a preparing patch to workaround Intel ERRATA_772415.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/hw/vfio/vfio-container-base.h |  1 +
>>   hw/vfio/listener.c                    | 13 +++++++++----
>>   2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-container-base.h
>b/include/hw/vfio/vfio-container-base.h
>> index bded6e993f..31fd784d76 100644
>> --- a/include/hw/vfio/vfio-container-base.h
>> +++ b/include/hw/vfio/vfio-container-base.h
>> @@ -51,6 +51,7 @@ typedef struct VFIOContainerBase {
>>       QLIST_HEAD(, VFIODevice) device_list;
>>       GList *iova_ranges;
>>       NotifierWithReturn cpr_reboot_notifier;
>> +    bool bypass_ro;
>>   } VFIOContainerBase;
>>
>>   typedef struct VFIOGuestIOMMU {
>> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
>> index f498e23a93..c64aa4539e 100644
>> --- a/hw/vfio/listener.c
>> +++ b/hw/vfio/listener.c
>> @@ -364,7 +364,8 @@ static bool
>vfio_known_safe_misalignment(MemoryRegionSection *section)
>>       return true;
>>   }
>>
>> -static bool vfio_listener_valid_section(MemoryRegionSection *section,
>> +static bool vfio_listener_valid_section(VFIOContainerBase *bcontainer,
>> +                                        MemoryRegionSection
>*section,
>>                                           const char *name)
>
>Instead of adding a 'VFIOContainerBase *' argument, I would add an
>extra 'bool bypass_ro' argument.

Done, see https://github.com/yiliu1765/qemu/commit/b0be8bfc9a899334819f3f4f0704e47116944a53

Opportunistically, I also introduced another patch to bypass dirty tracking for readonly region, see https://github.com/yiliu1765/qemu/commit/a21ce0afd8aec5d5a9e6de46cf46757530cb7d9f

Thanks
Zhenzhong
Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase
Posted by Cédric Le Goater 3 months, 2 weeks ago
On 7/30/25 12:58, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in
>> VFIOContainerBase
>>
>> On 7/29/25 11:20, Zhenzhong Duan wrote:
>>> When bypass_ro is true, read only memory section is bypassed from
>>> mapping in the container.
>>>
>>> This is a preparing patch to workaround Intel ERRATA_772415.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>    include/hw/vfio/vfio-container-base.h |  1 +
>>>    hw/vfio/listener.c                    | 13 +++++++++----
>>>    2 files changed, 10 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-container-base.h
>> b/include/hw/vfio/vfio-container-base.h
>>> index bded6e993f..31fd784d76 100644
>>> --- a/include/hw/vfio/vfio-container-base.h
>>> +++ b/include/hw/vfio/vfio-container-base.h
>>> @@ -51,6 +51,7 @@ typedef struct VFIOContainerBase {
>>>        QLIST_HEAD(, VFIODevice) device_list;
>>>        GList *iova_ranges;
>>>        NotifierWithReturn cpr_reboot_notifier;
>>> +    bool bypass_ro;
>>>    } VFIOContainerBase;
>>>
>>>    typedef struct VFIOGuestIOMMU {
>>> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
>>> index f498e23a93..c64aa4539e 100644
>>> --- a/hw/vfio/listener.c
>>> +++ b/hw/vfio/listener.c
>>> @@ -364,7 +364,8 @@ static bool
>> vfio_known_safe_misalignment(MemoryRegionSection *section)
>>>        return true;
>>>    }
>>>
>>> -static bool vfio_listener_valid_section(MemoryRegionSection *section,
>>> +static bool vfio_listener_valid_section(VFIOContainerBase *bcontainer,
>>> +                                        MemoryRegionSection
>> *section,
>>>                                            const char *name)
>>
>> Instead of adding a 'VFIOContainerBase *' argument, I would add an
>> extra 'bool bypass_ro' argument.
> 
> Done, see https://github.com/yiliu1765/qemu/commit/b0be8bfc9a899334819f3f4f0704e47116944a53

I don't think the extra 'bool bypass_ro' are useful. A part from that,
looks good.


Thanks,

C.


> Opportunistically, I also introduced another patch to bypass dirty tracking for readonly region, see https://github.com/yiliu1765/qemu/commit/a21ce0afd8aec5d5a9e6de46cf46757530cb7d9f



RE: [PATCH v4 18/20] vfio: Add a new element bypass_ro in VFIOContainerBase
Posted by Duan, Zhenzhong 3 months, 2 weeks ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in
>VFIOContainerBase
>
>On 7/30/25 12:58, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Cédric Le Goater <clg@redhat.com>
>>> Subject: Re: [PATCH v4 18/20] vfio: Add a new element bypass_ro in
>>> VFIOContainerBase
>>>
>>> On 7/29/25 11:20, Zhenzhong Duan wrote:
>>>> When bypass_ro is true, read only memory section is bypassed from
>>>> mapping in the container.
>>>>
>>>> This is a preparing patch to workaround Intel ERRATA_772415.
>>>>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>> ---
>>>>    include/hw/vfio/vfio-container-base.h |  1 +
>>>>    hw/vfio/listener.c                    | 13 +++++++++----
>>>>    2 files changed, 10 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/include/hw/vfio/vfio-container-base.h
>>> b/include/hw/vfio/vfio-container-base.h
>>>> index bded6e993f..31fd784d76 100644
>>>> --- a/include/hw/vfio/vfio-container-base.h
>>>> +++ b/include/hw/vfio/vfio-container-base.h
>>>> @@ -51,6 +51,7 @@ typedef struct VFIOContainerBase {
>>>>        QLIST_HEAD(, VFIODevice) device_list;
>>>>        GList *iova_ranges;
>>>>        NotifierWithReturn cpr_reboot_notifier;
>>>> +    bool bypass_ro;
>>>>    } VFIOContainerBase;
>>>>
>>>>    typedef struct VFIOGuestIOMMU {
>>>> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
>>>> index f498e23a93..c64aa4539e 100644
>>>> --- a/hw/vfio/listener.c
>>>> +++ b/hw/vfio/listener.c
>>>> @@ -364,7 +364,8 @@ static bool
>>> vfio_known_safe_misalignment(MemoryRegionSection *section)
>>>>        return true;
>>>>    }
>>>>
>>>> -static bool vfio_listener_valid_section(MemoryRegionSection *section,
>>>> +static bool vfio_listener_valid_section(VFIOContainerBase *bcontainer,
>>>> +                                        MemoryRegionSection
>>> *section,
>>>>                                            const char *name)
>>>
>>> Instead of adding a 'VFIOContainerBase *' argument, I would add an
>>> extra 'bool bypass_ro' argument.
>>
>> Done, see
>https://github.com/yiliu1765/qemu/commit/b0be8bfc9a899334819f3f4f0704
>e47116944a53
>
>I don't think the extra 'bool bypass_ro' are useful. A part from that,
>looks good.

Guess you mean " bypass_ro = bcontainer->bypass_ro", I'll use bcontainer->bypass_ro directly.

Thanks
Zhenzhong

>
>
>Thanks,
>
>C.
>
>
>> Opportunistically, I also introduced another patch to bypass dirty tracking
>for readonly region, see
>https://github.com/yiliu1765/qemu/commit/a21ce0afd8aec5d5a9e6de46cf4
>6757530cb7d9f
>