Re: [PATCH v7 18/23] vfio: Add a new element bypass_ro in VFIOContainer

[Eric Auger <eric.auger@redhat.com>]

On 10/24/25 10:43 AM, Zhenzhong Duan wrote:
> When bypass_ro is true, readonly memory section is bypassed from mapping
> in the container.
>
> This is a preparing patch to workaround Intel ERRATA_772415, see changelog
> in next patch for details about the errata.
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> ---
>  include/hw/vfio/vfio-container.h |  1 +
>  hw/vfio/listener.c               | 21 ++++++++++++++-------
>  2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/vfio/vfio-container.h b/include/hw/vfio/vfio-container.h
> index 9f6e8cedfc..a7d5c5ed67 100644
> --- a/include/hw/vfio/vfio-container.h
> +++ b/include/hw/vfio/vfio-container.h
> @@ -52,6 +52,7 @@ struct VFIOContainer {
>      QLIST_HEAD(, VFIODevice) device_list;
>      GList *iova_ranges;
>      NotifierWithReturn cpr_reboot_notifier;
> +    bool bypass_ro;
>  };
>  
>  #define TYPE_VFIO_IOMMU "vfio-iommu"
> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
> index 2109101158..0862b2b834 100644
> --- a/hw/vfio/listener.c
> +++ b/hw/vfio/listener.c
> @@ -76,8 +76,13 @@ static bool vfio_log_sync_needed(const VFIOContainer *bcontainer)
>      return true;
>  }
>  
> -static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> +static bool vfio_listener_skipped_section(MemoryRegionSection *section,
> +                                          bool bypass_ro)
>  {
> +    if (bypass_ro && section->readonly) {
> +        return true;
> +    }
> +
>      return (!memory_region_is_ram(section->mr) &&
>              !memory_region_is_iommu(section->mr)) ||
>             memory_region_is_protected(section->mr) ||
> @@ -368,9 +373,9 @@ static bool vfio_known_safe_misalignment(MemoryRegionSection *section)
>  }
>  
>  static bool vfio_listener_valid_section(MemoryRegionSection *section,
> -                                        const char *name)
> +                                        bool bypass_ro, const char *name)
>  {
> -    if (vfio_listener_skipped_section(section)) {
> +    if (vfio_listener_skipped_section(section, bypass_ro)) {
>          trace_vfio_listener_region_skip(name,
>                  section->offset_within_address_space,
>                  section->offset_within_address_space +
> @@ -497,7 +502,8 @@ void vfio_container_region_add(VFIOContainer *bcontainer,
>      int ret;
>      Error *err = NULL;
>  
> -    if (!vfio_listener_valid_section(section, "region_add")) {
> +    if (!vfio_listener_valid_section(section, bcontainer->bypass_ro,
> +                                     "region_add")) {
>          return;
>      }
>  
> @@ -663,7 +669,8 @@ 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(section, bcontainer->bypass_ro,
> +                                     "region_del")) {
>          return;
>      }
>  
> @@ -821,7 +828,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(section, false, "tracking_update") ||
I think you shall document in the commit msg and maybe here why we set
bypass_ro to false in that case

Eric
>          !vfio_get_section_iova_range(dirty->bcontainer, section,
>                                       &iova, &end, NULL)) {
>          return;
> @@ -1215,7 +1222,7 @@ static void vfio_listener_log_sync(MemoryListener *listener,
>      int ret;
>      Error *local_err = NULL;
>  
> -    if (vfio_listener_skipped_section(section)) {
> +    if (vfio_listener_skipped_section(section, false)) {
>          return;
>      }
>