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

2025-07-30 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>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 
>>> 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 
>>>> ---
>>>>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
>



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

2025-07-30 Thread Cédric Le Goater

On 7/30/25 12:58, Duan, Zhenzhong wrote:




-Original Message-
From: Cédric Le Goater 
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 
---
   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

2025-07-30 Thread Duan, Zhenzhong


>-Original Message-
>From: Cédric Le Goater 
>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 
>> ---
>>   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

2025-07-29 Thread Cédric Le Goater

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 
---
  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;