[PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[Philippe Mathieu-Daudé]

Missing review: patch #4

Since v2:
- Added R-b tags
- Addressed Richard's review comment: page_mask = -page_size

Philippe Mathieu-Daudé (6):
  hw/virtio: Propagate page_mask to
    vhost_vdpa_listener_skipped_section()
  hw/virtio: Propagate page_mask to vhost_vdpa_section_end()
  hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro
  hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()
  hw/virtio: Build vhost-vdpa.o once
  hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]

 hw/virtio/vhost-vdpa.c | 36 +++++++++++++++++++++---------------
 hw/virtio/meson.build  | 25 +++++++++++++------------
 2 files changed, 34 insertions(+), 27 deletions(-)

-- 
2.38.1

[PATCH v3 1/6] hw/virtio: Propagate page_mask to vhost_vdpa_listener_skipped_section()

[Philippe Mathieu-Daudé]

In order to make vhost-vdpa.c a target-agnostic source unit,
we need to remove the TARGET_PAGE_SIZE / TARGET_PAGE_MASK /
TARGET_PAGE_ALIGN uses. TARGET_PAGE_SIZE will be replaced by
the runtime qemu_target_page_size(). The other ones will be
deduced from TARGET_PAGE_SIZE.

Since the 3 macros are used in 3 related functions (sharing
the same call tree), we'll refactor them to only depend on
TARGET_PAGE_MASK.

Having the following call tree:

  vhost_vdpa_listener_region_del()
    -> vhost_vdpa_listener_skipped_section()
       -> vhost_vdpa_section_end()

The first step is to propagate TARGET_PAGE_MASK to
vhost_vdpa_listener_skipped_section().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/virtio/vhost-vdpa.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e..87653bf841 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -42,7 +42,8 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section)
 
 static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
                                                 uint64_t iova_min,
-                                                uint64_t iova_max)
+                                                uint64_t iova_max,
+                                                int page_mask)
 {
     Int128 llend;
 
@@ -313,7 +314,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
     int ret;
 
     if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-                                            v->iova_range.last)) {
+                                            v->iova_range.last, TARGET_PAGE_MASK)) {
         return;
     }
     if (memory_region_is_iommu(section->mr)) {
@@ -396,7 +397,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
     int ret;
 
     if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-                                            v->iova_range.last)) {
+                                            v->iova_range.last, TARGET_PAGE_MASK)) {
         return;
     }
     if (memory_region_is_iommu(section->mr)) {
-- 
2.38.1

[PATCH v3 2/6] hw/virtio: Propagate page_mask to vhost_vdpa_section_end()

[Philippe Mathieu-Daudé]

Propagate TARGET_PAGE_MASK (see the previous commit for
rationale).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/virtio/vhost-vdpa.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 87653bf841..3040bd8ee8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -31,11 +31,12 @@
  * Return one past the end of the end of section. Be careful with uint64_t
  * conversions!
  */
-static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section)
+static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section,
+                                     int page_mask)
 {
     Int128 llend = int128_make64(section->offset_within_address_space);
     llend = int128_add(llend, section->size);
-    llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
+    llend = int128_and(llend, int128_exts64(page_mask));
 
     return llend;
 }
@@ -69,7 +70,7 @@ static bool vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
      */
 
     if (!memory_region_is_iommu(section->mr)) {
-        llend = vhost_vdpa_section_end(section);
+        llend = vhost_vdpa_section_end(section, page_mask);
         if (int128_gt(llend, int128_make64(iova_max))) {
             error_report("RAM section out of device range (max=0x%" PRIx64
                          ", end addr=0x%" PRIx64 ")",
@@ -329,7 +330,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
     }
 
     iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
-    llend = vhost_vdpa_section_end(section);
+    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
     if (int128_ge(int128_make64(iova), llend)) {
         return;
     }
@@ -411,7 +412,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
     }
 
     iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
-    llend = vhost_vdpa_section_end(section);
+    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
 
     trace_vhost_vdpa_listener_region_del(v, iova,
         int128_get64(int128_sub(llend, int128_one())));
-- 
2.38.1

[PATCH v3 3/6] hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro

[Philippe Mathieu-Daudé]

Use TARGET_PAGE_SIZE to calculate TARGET_PAGE_ALIGN
(see the rationale in previous commits).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/virtio/vhost-vdpa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3040bd8ee8..a3dd7c712a 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -329,7 +329,7 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
         return;
     }
 
-    iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
     llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
     if (int128_ge(int128_make64(iova), llend)) {
         return;
@@ -411,7 +411,7 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
         return;
     }
 
-    iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
     llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
 
     trace_vhost_vdpa_listener_region_del(v, iova,
-- 
2.38.1

[PATCH v3 4/6] hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()

[Philippe Mathieu-Daudé]

Similarly to commit e414ed2c47 ("virtio-iommu: Use
target-agnostic qemu_target_page_mask"), Replace the
target-specific TARGET_PAGE_SIZE and TARGET_PAGE_MASK
definitions by a call to the runtime qemu_target_page_size()
helper which is target agnostic.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/vhost-vdpa.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index a3dd7c712a..2717edf51d 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -14,6 +14,7 @@
 #include <linux/vfio.h>
 #include <sys/eventfd.h>
 #include <sys/ioctl.h>
+#include "exec/target_page.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-backend.h"
 #include "hw/virtio/virtio-net.h"
@@ -23,7 +24,6 @@
 #include "migration/blocker.h"
 #include "qemu/cutils.h"
 #include "qemu/main-loop.h"
-#include "cpu.h"
 #include "trace.h"
 #include "qapi/error.h"
 
@@ -313,9 +313,11 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
     Int128 llend, llsize;
     void *vaddr;
     int ret;
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
 
     if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-                                            v->iova_range.last, TARGET_PAGE_MASK)) {
+                                            v->iova_range.last, page_mask)) {
         return;
     }
     if (memory_region_is_iommu(section->mr)) {
@@ -323,14 +325,14 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
         return;
     }
 
-    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
-                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
+    if (unlikely((section->offset_within_address_space & ~page_mask) !=
+                 (section->offset_within_region & ~page_mask))) {
         error_report("%s received unaligned region", __func__);
         return;
     }
 
-    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
-    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
+    iova = ROUND_UP(section->offset_within_address_space, page_size);
+    llend = vhost_vdpa_section_end(section, page_mask);
     if (int128_ge(int128_make64(iova), llend)) {
         return;
     }
@@ -396,23 +398,25 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
     hwaddr iova;
     Int128 llend, llsize;
     int ret;
+    int page_size = qemu_target_page_size();
+    int page_mask = -page_size;
 
     if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
-                                            v->iova_range.last, TARGET_PAGE_MASK)) {
+                                            v->iova_range.last, page_mask)) {
         return;
     }
     if (memory_region_is_iommu(section->mr)) {
         vhost_vdpa_iommu_region_del(listener, section);
     }
 
-    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
-                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
+    if (unlikely((section->offset_within_address_space & ~page_mask) !=
+                 (section->offset_within_region & ~page_mask))) {
         error_report("%s received unaligned region", __func__);
         return;
     }
 
-    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
-    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
+    iova = ROUND_UP(section->offset_within_address_space, page_size);
+    llend = vhost_vdpa_section_end(section, page_mask);
 
     trace_vhost_vdpa_listener_region_del(v, iova,
         int128_get64(int128_sub(llend, int128_one())));
-- 
2.38.1

[PATCH v3 5/6] hw/virtio: Build vhost-vdpa.o once

[Philippe Mathieu-Daudé]

The previous commit removed the dependencies on the
target-specific TARGET_PAGE_FOO macros. We can now
move vhost-vdpa.c to the 'softmmu_virtio_ss' source
set to build it once for all our targets.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/virtio/meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index f32b22f61b..900864c1be 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -18,7 +18,8 @@ if have_vhost
     specific_virtio_ss.add(files('vhost-user.c'))
   endif
   if have_vhost_vdpa
-    specific_virtio_ss.add(files('vhost-vdpa.c', 'vhost-shadow-virtqueue.c'))
+    softmmu_virtio_ss.add(files('vhost-vdpa.c'))
+    specific_virtio_ss.add(files('vhost-shadow-virtqueue.c'))
   endif
 else
   softmmu_virtio_ss.add(files('vhost-stub.c'))
-- 
2.38.1

[PATCH v3 6/6] hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]

[Philippe Mathieu-Daudé]

Similarly to commit de6cd7599b ("meson: Replace softmmu_ss
-> system_ss"), rename the virtio source set common to all
system emulation as 'system_virtio_ss[]'. This is clearer
because softmmu can be used for user emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/virtio/meson.build | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 900864c1be..7a36eabb39 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -1,28 +1,28 @@
-softmmu_virtio_ss = ss.source_set()
-softmmu_virtio_ss.add(files('virtio-bus.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('virtio-pci.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK_COMMON', if_true: files('vhost-vsock-common.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
-softmmu_virtio_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev.c'))
+system_virtio_ss = ss.source_set()
+system_virtio_ss.add(files('virtio-bus.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_PCI', if_true: files('virtio-pci.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_MMIO', if_true: files('virtio-mmio.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK_COMMON', if_true: files('vhost-vsock-common.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
+system_virtio_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev.c'))
 
 specific_virtio_ss = ss.source_set()
 specific_virtio_ss.add(files('virtio.c'))
 specific_virtio_ss.add(files('virtio-config-io.c', 'virtio-qmp.c'))
 
 if have_vhost
-  softmmu_virtio_ss.add(files('vhost.c'))
+  system_virtio_ss.add(files('vhost.c'))
   specific_virtio_ss.add(files('vhost-backend.c', 'vhost-iova-tree.c'))
   if have_vhost_user
     specific_virtio_ss.add(files('vhost-user.c'))
   endif
   if have_vhost_vdpa
-    softmmu_virtio_ss.add(files('vhost-vdpa.c'))
+    system_virtio_ss.add(files('vhost-vdpa.c'))
     specific_virtio_ss.add(files('vhost-shadow-virtqueue.c'))
   endif
 else
-  softmmu_virtio_ss.add(files('vhost-stub.c'))
+  system_virtio_ss.add(files('vhost-stub.c'))
 endif
 
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
@@ -65,7 +65,7 @@ virtio_pci_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev-pci.c'
 
 specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
-system_ss.add_all(when: 'CONFIG_VIRTIO', if_true: softmmu_virtio_ss)
+system_ss.add_all(when: 'CONFIG_VIRTIO', if_true: system_virtio_ss)
 system_ss.add(when: 'CONFIG_VIRTIO', if_false: files('vhost-stub.c'))
 system_ss.add(when: 'CONFIG_VIRTIO', if_false: files('virtio-stub.c'))
 system_ss.add(when: 'CONFIG_ALL', if_true: files('vhost-stub.c'))
-- 
2.38.1

Re: [PATCH v3 4/6] hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()

[Philippe Mathieu-Daudé]

ping?

On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
> Similarly to commit e414ed2c47 ("virtio-iommu: Use
> target-agnostic qemu_target_page_mask"), Replace the
> target-specific TARGET_PAGE_SIZE and TARGET_PAGE_MASK
> definitions by a call to the runtime qemu_target_page_size()
> helper which is target agnostic.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/vhost-vdpa.c | 26 +++++++++++++++-----------
>   1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index a3dd7c712a..2717edf51d 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -14,6 +14,7 @@
>   #include <linux/vfio.h>
>   #include <sys/eventfd.h>
>   #include <sys/ioctl.h>
> +#include "exec/target_page.h"
>   #include "hw/virtio/vhost.h"
>   #include "hw/virtio/vhost-backend.h"
>   #include "hw/virtio/virtio-net.h"
> @@ -23,7 +24,6 @@
>   #include "migration/blocker.h"
>   #include "qemu/cutils.h"
>   #include "qemu/main-loop.h"
> -#include "cpu.h"
>   #include "trace.h"
>   #include "qapi/error.h"
>   
> @@ -313,9 +313,11 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
>       Int128 llend, llsize;
>       void *vaddr;
>       int ret;
> +    int page_size = qemu_target_page_size();
> +    int page_mask = -page_size;
>   
>       if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
> -                                            v->iova_range.last, TARGET_PAGE_MASK)) {
> +                                            v->iova_range.last, page_mask)) {
>           return;
>       }
>       if (memory_region_is_iommu(section->mr)) {
> @@ -323,14 +325,14 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
>           return;
>       }
>   
> -    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
> +                 (section->offset_within_region & ~page_mask))) {
>           error_report("%s received unaligned region", __func__);
>           return;
>       }
>   
> -    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
> +    llend = vhost_vdpa_section_end(section, page_mask);
>       if (int128_ge(int128_make64(iova), llend)) {
>           return;
>       }
> @@ -396,23 +398,25 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
>       hwaddr iova;
>       Int128 llend, llsize;
>       int ret;
> +    int page_size = qemu_target_page_size();
> +    int page_mask = -page_size;
>   
>       if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
> -                                            v->iova_range.last, TARGET_PAGE_MASK)) {
> +                                            v->iova_range.last, page_mask)) {
>           return;
>       }
>       if (memory_region_is_iommu(section->mr)) {
>           vhost_vdpa_iommu_region_del(listener, section);
>       }
>   
> -    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
> +                 (section->offset_within_region & ~page_mask))) {
>           error_report("%s received unaligned region", __func__);
>           return;
>       }
>   
> -    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
> +    llend = vhost_vdpa_section_end(section, page_mask);
>   
>       trace_vhost_vdpa_listener_region_del(v, iova,
>           int128_get64(int128_sub(llend, int128_one())));

Re: [PATCH v3 4/6] hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()

[Philippe Mathieu-Daudé]

On 18/8/23 13:00, Philippe Mathieu-Daudé wrote:
> ping?
> 
> On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
>> Similarly to commit e414ed2c47 ("virtio-iommu: Use
>> target-agnostic qemu_target_page_mask"), Replace the
>> target-specific TARGET_PAGE_SIZE and TARGET_PAGE_MASK
>> definitions by a call to the runtime qemu_target_page_size()
>> helper which is target agnostic.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/virtio/vhost-vdpa.c | 26 +++++++++++++++-----------
>>   1 file changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index a3dd7c712a..2717edf51d 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/vfio.h>
>>   #include <sys/eventfd.h>
>>   #include <sys/ioctl.h>
>> +#include "exec/target_page.h"
>>   #include "hw/virtio/vhost.h"
>>   #include "hw/virtio/vhost-backend.h"
>>   #include "hw/virtio/virtio-net.h"
>> @@ -23,7 +24,6 @@
>>   #include "migration/blocker.h"
>>   #include "qemu/cutils.h"
>>   #include "qemu/main-loop.h"
>> -#include "cpu.h"
>>   #include "trace.h"
>>   #include "qapi/error.h"
>> @@ -313,9 +313,11 @@ static void 
>> vhost_vdpa_listener_region_add(MemoryListener *listener,
>>       Int128 llend, llsize;
>>       void *vaddr;
>>       int ret;
>> +    int page_size = qemu_target_page_size();
>> +    int page_mask = -page_size;

Richard, this is the change you asked here:
https://lore.kernel.org/qemu-devel/f877dda3-a3d9-5081-c2b3-c10eeb7b6814@linaro.org/

Did I miss something else?

>>       if (vhost_vdpa_listener_skipped_section(section, 
>> v->iova_range.first,
>> -                                            v->iova_range.last, 
>> TARGET_PAGE_MASK)) {
>> +                                            v->iova_range.last, 
>> page_mask)) {
>>           return;
>>       }
>>       if (memory_region_is_iommu(section->mr)) {
>> @@ -323,14 +325,14 @@ static void 
>> vhost_vdpa_listener_region_add(MemoryListener *listener,
>>           return;
>>       }
>> -    if (unlikely((section->offset_within_address_space & 
>> ~TARGET_PAGE_MASK) !=
>> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
>> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
>> +                 (section->offset_within_region & ~page_mask))) {
>>           error_report("%s received unaligned region", __func__);
>>           return;
>>       }
>> -    iova = ROUND_UP(section->offset_within_address_space, 
>> TARGET_PAGE_SIZE);
>> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
>> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
>> +    llend = vhost_vdpa_section_end(section, page_mask);
>>       if (int128_ge(int128_make64(iova), llend)) {
>>           return;
>>       }
>> @@ -396,23 +398,25 @@ static void 
>> vhost_vdpa_listener_region_del(MemoryListener *listener,
>>       hwaddr iova;
>>       Int128 llend, llsize;
>>       int ret;
>> +    int page_size = qemu_target_page_size();
>> +    int page_mask = -page_size;
>>       if (vhost_vdpa_listener_skipped_section(section, 
>> v->iova_range.first,
>> -                                            v->iova_range.last, 
>> TARGET_PAGE_MASK)) {
>> +                                            v->iova_range.last, 
>> page_mask)) {
>>           return;
>>       }
>>       if (memory_region_is_iommu(section->mr)) {
>>           vhost_vdpa_iommu_region_del(listener, section);
>>       }
>> -    if (unlikely((section->offset_within_address_space & 
>> ~TARGET_PAGE_MASK) !=
>> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
>> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
>> +                 (section->offset_within_region & ~page_mask))) {
>>           error_report("%s received unaligned region", __func__);
>>           return;
>>       }
>> -    iova = ROUND_UP(section->offset_within_address_space, 
>> TARGET_PAGE_SIZE);
>> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
>> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
>> +    llend = vhost_vdpa_section_end(section, page_mask);
>>       trace_vhost_vdpa_listener_region_del(v, iova,
>>           int128_get64(int128_sub(llend, int128_one())));
> 

Re: [PATCH v3 4/6] hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()

[Richard Henderson]

On 8/18/23 04:00, Philippe Mathieu-Daudé wrote:
> ping?
> 
> On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
>> Similarly to commit e414ed2c47 ("virtio-iommu: Use
>> target-agnostic qemu_target_page_mask"), Replace the
>> target-specific TARGET_PAGE_SIZE and TARGET_PAGE_MASK
>> definitions by a call to the runtime qemu_target_page_size()
>> helper which is target agnostic.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


>> ---
>>   hw/virtio/vhost-vdpa.c | 26 +++++++++++++++-----------
>>   1 file changed, 15 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
>> index a3dd7c712a..2717edf51d 100644
>> --- a/hw/virtio/vhost-vdpa.c
>> +++ b/hw/virtio/vhost-vdpa.c
>> @@ -14,6 +14,7 @@
>>   #include <linux/vfio.h>
>>   #include <sys/eventfd.h>
>>   #include <sys/ioctl.h>
>> +#include "exec/target_page.h"
>>   #include "hw/virtio/vhost.h"
>>   #include "hw/virtio/vhost-backend.h"
>>   #include "hw/virtio/virtio-net.h"
>> @@ -23,7 +24,6 @@
>>   #include "migration/blocker.h"
>>   #include "qemu/cutils.h"
>>   #include "qemu/main-loop.h"
>> -#include "cpu.h"
>>   #include "trace.h"
>>   #include "qapi/error.h"
>> @@ -313,9 +313,11 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
>>       Int128 llend, llsize;
>>       void *vaddr;
>>       int ret;
>> +    int page_size = qemu_target_page_size();
>> +    int page_mask = -page_size;
>>       if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
>> -                                            v->iova_range.last, TARGET_PAGE_MASK)) {
>> +                                            v->iova_range.last, page_mask)) {
>>           return;
>>       }
>>       if (memory_region_is_iommu(section->mr)) {
>> @@ -323,14 +325,14 @@ static void vhost_vdpa_listener_region_add(MemoryListener *listener,
>>           return;
>>       }
>> -    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
>> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
>> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
>> +                 (section->offset_within_region & ~page_mask))) {
>>           error_report("%s received unaligned region", __func__);
>>           return;
>>       }
>> -    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
>> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
>> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
>> +    llend = vhost_vdpa_section_end(section, page_mask);
>>       if (int128_ge(int128_make64(iova), llend)) {
>>           return;
>>       }
>> @@ -396,23 +398,25 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
>>       hwaddr iova;
>>       Int128 llend, llsize;
>>       int ret;
>> +    int page_size = qemu_target_page_size();
>> +    int page_mask = -page_size;
>>       if (vhost_vdpa_listener_skipped_section(section, v->iova_range.first,
>> -                                            v->iova_range.last, TARGET_PAGE_MASK)) {
>> +                                            v->iova_range.last, page_mask)) {
>>           return;
>>       }
>>       if (memory_region_is_iommu(section->mr)) {
>>           vhost_vdpa_iommu_region_del(listener, section);
>>       }
>> -    if (unlikely((section->offset_within_address_space & ~TARGET_PAGE_MASK) !=
>> -                 (section->offset_within_region & ~TARGET_PAGE_MASK))) {
>> +    if (unlikely((section->offset_within_address_space & ~page_mask) !=
>> +                 (section->offset_within_region & ~page_mask))) {
>>           error_report("%s received unaligned region", __func__);
>>           return;
>>       }
>> -    iova = ROUND_UP(section->offset_within_address_space, TARGET_PAGE_SIZE);
>> -    llend = vhost_vdpa_section_end(section, TARGET_PAGE_MASK);
>> +    iova = ROUND_UP(section->offset_within_address_space, page_size);
>> +    llend = vhost_vdpa_section_end(section, page_mask);
>>       trace_vhost_vdpa_listener_region_del(v, iova,
>>           int128_get64(int128_sub(llend, int128_one())));
> 

Re: [PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[Philippe Mathieu-Daudé]

Hi Michael,

This series is now fully reviewed.

On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
> Missing review: patch #4
> 
> Since v2:
> - Added R-b tags
> - Addressed Richard's review comment: page_mask = -page_size
> 
> Philippe Mathieu-Daudé (6):
>    hw/virtio: Propagate page_mask to
>      vhost_vdpa_listener_skipped_section()
>    hw/virtio: Propagate page_mask to vhost_vdpa_section_end()
>    hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro
>    hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()
>    hw/virtio: Build vhost-vdpa.o once
>    hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]
> 
>   hw/virtio/vhost-vdpa.c | 36 +++++++++++++++++++++---------------
>   hw/virtio/meson.build  | 25 +++++++++++++------------
>   2 files changed, 34 insertions(+), 27 deletions(-)
> 

Re: [PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[Philippe Mathieu-Daudé]

On 30/8/23 15:35, Philippe Mathieu-Daudé wrote:
> Hi Michael,
> 
> This series is now fully reviewed.
> 
> On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
>> Missing review: patch #4
>>
>> Since v2:
>> - Added R-b tags
>> - Addressed Richard's review comment: page_mask = -page_size
>>
>> Philippe Mathieu-Daudé (6):
>>    hw/virtio: Propagate page_mask to
>>      vhost_vdpa_listener_skipped_section()
>>    hw/virtio: Propagate page_mask to vhost_vdpa_section_end()
>>    hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro
>>    hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()
>>    hw/virtio: Build vhost-vdpa.o once
>>    hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]

Michael, I have another series unifying virtio endianness blocked
by this one. I can merge it if you provide your Ack-by.

Thanks,

Phil.

Re: [PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[Philippe Mathieu-Daudé]

Hi Michael,

On 6/9/23 08:31, Philippe Mathieu-Daudé wrote:
> On 30/8/23 15:35, Philippe Mathieu-Daudé wrote:
>>
>> This series is now fully reviewed.
>>
>> On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
>>> Missing review: patch #4
>>>
>>> Since v2:
>>> - Added R-b tags
>>> - Addressed Richard's review comment: page_mask = -page_size
>>>
>>> Philippe Mathieu-Daudé (6):
>>>    hw/virtio: Propagate page_mask to
>>>      vhost_vdpa_listener_skipped_section()
>>>    hw/virtio: Propagate page_mask to vhost_vdpa_section_end()
>>>    hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro
>>>    hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()
>>>    hw/virtio: Build vhost-vdpa.o once
>>>    hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]
> 
> Michael, I have another series unifying virtio endianness blocked
> by this one. I can merge it if you provide your Ack-by.

Unless you object, I'll merge this series. Since we are early enough
in the development window, if something breaks, we'll catch it in
time and fix or revert.

Regards,

Phil.

Re: [PATCH v3 0/6] hw/virtio: Build vhost-vdpa.o once for all targets

[Michael S. Tsirkin]

On Mon, Sep 18, 2023 at 12:42:02PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Michael,
> 
> On 6/9/23 08:31, Philippe Mathieu-Daudé wrote:
> > On 30/8/23 15:35, Philippe Mathieu-Daudé wrote:
> > > 
> > > This series is now fully reviewed.
> > > 
> > > On 10/7/23 11:49, Philippe Mathieu-Daudé wrote:
> > > > Missing review: patch #4
> > > > 
> > > > Since v2:
> > > > - Added R-b tags
> > > > - Addressed Richard's review comment: page_mask = -page_size
> > > > 
> > > > Philippe Mathieu-Daudé (6):
> > > >    hw/virtio: Propagate page_mask to
> > > >      vhost_vdpa_listener_skipped_section()
> > > >    hw/virtio: Propagate page_mask to vhost_vdpa_section_end()
> > > >    hw/virtio/vhost-vdpa: Inline TARGET_PAGE_ALIGN() macro
> > > >    hw/virtio/vhost-vdpa: Use target-agnostic qemu_target_page_mask()
> > > >    hw/virtio: Build vhost-vdpa.o once
> > > >    hw/virtio/meson: Rename softmmu_virtio_ss[] -> system_virtio_ss[]
> > 
> > Michael, I have another series unifying virtio endianness blocked
> > by this one. I can merge it if you provide your Ack-by.
> 
> Unless you object, I'll merge this series. Since we are early enough
> in the development window, if something breaks, we'll catch it in
> time and fix or revert.
> 
> Regards,
> 
> Phil.

It's actually in my tree, and should go out in a couple of days finally.

-- 
MST