[PATCH 00/11] hw/virtio: Build various target-agnostic objects just once

[PATCH 00/11] hw/virtio: Build various target-agnostic objects just once

[Philippe Mathieu-Daudé]

Less controvertial than my first approach [*] which caches
the access_is_big_endian value in VirtIODevice state, this
series just remove a unnecessary / pointless dependency on
"virtio-access.h", allowing to build various virtio objects
once for all targets.

First we introduce the qemu_target_page_mask() and _align()
helpers, similar to the _size() and _bits() equivalents,
then we mostly perform meson.build massages. Mostly trivial.

Last patch is RFC in case there is a performance issue.

https://lore.kernel.org/qemu-devel/20221212230517.28872-11-philmd@linaro.org/

Philippe Mathieu-Daudé (11):
  softmmu: Introduce qemu_target_page_mask/qemu_target_page_align
    helpers
  hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig
  hw/scsi: Rearrange meson.build
  hw/scsi: Rename target-specific source set as
    'specific_virtio_scsi_ss'
  hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig
  hw/virtio/virtio-mem: Use qemu_ram_get_fd() helper
  hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header
  hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()
  hw/virtio: Remove unnecessary 'virtio-access.h' header
  hw/virtio: Build various target-agnostic objects just once
  hw/virtio: Make vhost-vdpa.c target-agnostic to build it once

 include/exec/target_page.h      |  2 ++
 hw/block/dataplane/virtio-blk.c |  1 -
 hw/s390x/virtio-ccw.c           |  1 -
 hw/scsi/vhost-scsi.c            |  1 -
 hw/scsi/vhost-user-scsi.c       |  1 -
 hw/scsi/virtio-scsi-dataplane.c |  1 -
 hw/virtio/vdpa-dev.c            |  1 -
 hw/virtio/vhost-vdpa.c          | 17 ++++++++---------
 hw/virtio/vhost-vsock-common.c  |  2 +-
 hw/virtio/vhost.c               |  1 -
 hw/virtio/virtio-crypto.c       |  1 -
 hw/virtio/virtio-iommu.c        |  4 ++--
 hw/virtio/virtio-mem.c          |  3 +--
 softmmu/physmem.c               | 10 ++++++++++
 hw/block/dataplane/meson.build  |  2 +-
 hw/scsi/Kconfig                 |  6 ++++++
 hw/scsi/meson.build             | 19 ++++++++++++-------
 hw/virtio/Kconfig               |  6 ++++++
 hw/virtio/meson.build           | 17 ++++++++++-------
 19 files changed, 59 insertions(+), 37 deletions(-)

-- 
2.38.1

[PATCH 01/11] softmmu: Introduce qemu_target_page_mask/qemu_target_page_align helpers

[Philippe Mathieu-Daudé]

Since TARGET_PAGE_MASK and TARGET_PAGE_ALIGN are poisoned in
target-agnostic code, introduce the qemu_target_page_mask()
and qemu_target_page_align() helpers to get these values from
target-agnostic code at runtime.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/target_page.h |  2 ++
 softmmu/physmem.c          | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/exec/target_page.h b/include/exec/target_page.h
index bbf37aea17..660416920b 100644
--- a/include/exec/target_page.h
+++ b/include/exec/target_page.h
@@ -15,6 +15,8 @@
 #define EXEC_TARGET_PAGE_H
 
 size_t qemu_target_page_size(void);
+unsigned qemu_target_page_mask(void);
+uint64_t qemu_target_page_align(uint64_t value);
 int qemu_target_page_bits(void);
 int qemu_target_page_bits_min(void);
 
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index efaed36773..14fcba4fb2 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -3347,6 +3347,16 @@ size_t qemu_target_page_size(void)
     return TARGET_PAGE_SIZE;
 }
 
+unsigned qemu_target_page_mask(void)
+{
+    return TARGET_PAGE_MASK;
+}
+
+uint64_t qemu_target_page_align(uint64_t value)
+{
+    return TARGET_PAGE_ALIGN(value);
+}
+
 int qemu_target_page_bits(void)
 {
     return TARGET_PAGE_BITS;
-- 
2.38.1

Re: [PATCH 01/11] softmmu: Introduce qemu_target_page_mask/qemu_target_page_align helpers

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> Since TARGET_PAGE_MASK and TARGET_PAGE_ALIGN are poisoned in
> target-agnostic code, introduce the qemu_target_page_mask()
> and qemu_target_page_align() helpers to get these values from
> target-agnostic code at runtime.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/target_page.h |  2 ++
>   softmmu/physmem.c          | 10 ++++++++++
>   2 files changed, 12 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 01/11] softmmu: Introduce qemu_target_page_mask/qemu_target_page_align helpers

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> +unsigned qemu_target_page_mask(void);

Should be signed int, so that it sign-extends to whatever needed width.

r~

[PATCH 02/11] hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig

[Philippe Mathieu-Daudé]

Instead of adding 'vhost-scsi-common.c' twice (for VHOST_SCSI and
VHOST_USER_SCSI), have it depend on VHOST_SCSI_COMMON, selected by
both symbols.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/Kconfig     | 6 ++++++
 hw/scsi/meson.build | 6 ++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/Kconfig b/hw/scsi/Kconfig
index e7b34dc8e2..1feab84c4c 100644
--- a/hw/scsi/Kconfig
+++ b/hw/scsi/Kconfig
@@ -48,13 +48,19 @@ config VIRTIO_SCSI
     depends on VIRTIO
     select SCSI
 
+config VHOST_SCSI_COMMON
+    bool
+    depends on VIRTIO
+
 config VHOST_SCSI
     bool
     default y
+    select VHOST_SCSI_COMMON
     depends on VIRTIO && VHOST_KERNEL
 
 config VHOST_USER_SCSI
     bool
     # Only PCI devices are provided for now
     default y if VIRTIO_PCI
+    select VHOST_SCSI_COMMON
     depends on VIRTIO && VHOST_USER && LINUX
diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
index 923a34f344..fa9198e69f 100644
--- a/hw/scsi/meson.build
+++ b/hw/scsi/meson.build
@@ -17,8 +17,10 @@ specific_scsi_ss = ss.source_set()
 
 virtio_scsi_ss = ss.source_set()
 virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
-virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi-common.c', 'vhost-scsi.c'))
-virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-scsi-common.c', 'vhost-user-scsi.c'))
+
+virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
+virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
+virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
 specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)
 
 specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
-- 
2.38.1

Re: [PATCH 02/11] hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> Instead of adding 'vhost-scsi-common.c' twice (for VHOST_SCSI and
> VHOST_USER_SCSI), have it depend on VHOST_SCSI_COMMON, selected by
> both symbols.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/scsi/Kconfig     | 6 ++++++
>   hw/scsi/meson.build | 6 ++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 02/11] hw/scsi: Introduce VHOST_SCSI_COMMON symbol in Kconfig

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> Instead of adding 'vhost-scsi-common.c' twice (for VHOST_SCSI and
> VHOST_USER_SCSI), have it depend on VHOST_SCSI_COMMON, selected by
> both symbols.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/scsi/Kconfig     | 6 ++++++
>   hw/scsi/meson.build | 6 ++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)

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

r~

[PATCH 03/11] hw/scsi: Rearrange meson.build

[Philippe Mathieu-Daudé]

We will modify this file shortly. Re-arrange it slightly first,
declaring source sets first.

No logical change.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/meson.build | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
index fa9198e69f..f3206a4756 100644
--- a/hw/scsi/meson.build
+++ b/hw/scsi/meson.build
@@ -1,4 +1,7 @@
 scsi_ss = ss.source_set()
+specific_scsi_ss = ss.source_set()
+virtio_scsi_ss = ss.source_set()
+
 scsi_ss.add(files(
   'emulation.c',
   'scsi-bus.c',
@@ -11,18 +14,13 @@ scsi_ss.add(when: 'CONFIG_LSI_SCSI_PCI', if_true: files('lsi53c895a.c'))
 scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: files('megasas.c'))
 scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: files('mptsas.c', 'mptconfig.c', 'mptendian.c'))
 scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: files('vmw_pvscsi.c'))
-softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
 
-specific_scsi_ss = ss.source_set()
-
-virtio_scsi_ss = ss.source_set()
 virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
-
 virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
 virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
 virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
+specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
 specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)
 
-specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
-
+softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
 specific_ss.add_all(when: 'CONFIG_SCSI', if_true: specific_scsi_ss)
-- 
2.38.1

Re: [PATCH 03/11] hw/scsi: Rearrange meson.build

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> We will modify this file shortly. Re-arrange it slightly first,
> declaring source sets first.
> 
> No logical change.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/scsi/meson.build | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)

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

r~

Re: [PATCH 03/11] hw/scsi: Rearrange meson.build

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> We will modify this file shortly. Re-arrange it slightly first,
> declaring source sets first.
> 
> No logical change.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/scsi/meson.build | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
> index fa9198e69f..f3206a4756 100644
> --- a/hw/scsi/meson.build
> +++ b/hw/scsi/meson.build
> @@ -1,4 +1,7 @@
>   scsi_ss = ss.source_set()
> +specific_scsi_ss = ss.source_set()
> +virtio_scsi_ss = ss.source_set()
> +
>   scsi_ss.add(files(
>     'emulation.c',
>     'scsi-bus.c',
> @@ -11,18 +14,13 @@ scsi_ss.add(when: 'CONFIG_LSI_SCSI_PCI', if_true: files('lsi53c895a.c'))
>   scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: files('megasas.c'))
>   scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: files('mptsas.c', 'mptconfig.c', 'mptendian.c'))
>   scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: files('vmw_pvscsi.c'))
> -softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
>   
> -specific_scsi_ss = ss.source_set()
> -
> -virtio_scsi_ss = ss.source_set()
>   virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
> -
>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
> +specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
>   specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)

I think it might still make sense to keep the virtio stuff together, i.e. 
add the SPAPR line after the VIRTIO_SCSI line instead of adding it in front 
of it?

  Thomas


> -specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
> -
> +softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
>   specific_ss.add_all(when: 'CONFIG_SCSI', if_true: specific_scsi_ss)

Re: [PATCH 03/11] hw/scsi: Rearrange meson.build

[Philippe Mathieu-Daudé]

On 24/5/23 09:14, Thomas Huth wrote:
> On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
>> We will modify this file shortly. Re-arrange it slightly first,
>> declaring source sets first.
>>
>> No logical change.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/scsi/meson.build | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
>> index fa9198e69f..f3206a4756 100644
>> --- a/hw/scsi/meson.build
>> +++ b/hw/scsi/meson.build
>> @@ -1,4 +1,7 @@
>>   scsi_ss = ss.source_set()
>> +specific_scsi_ss = ss.source_set()
>> +virtio_scsi_ss = ss.source_set()
>> +
>>   scsi_ss.add(files(
>>     'emulation.c',
>>     'scsi-bus.c',
>> @@ -11,18 +14,13 @@ scsi_ss.add(when: 'CONFIG_LSI_SCSI_PCI', if_true: 
>> files('lsi53c895a.c'))
>>   scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: 
>> files('megasas.c'))
>>   scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: 
>> files('mptsas.c', 'mptconfig.c', 'mptendian.c'))
>>   scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: 
>> files('vmw_pvscsi.c'))
>> -softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
>> -specific_scsi_ss = ss.source_set()
>> -
>> -virtio_scsi_ss = ss.source_set()
>>   virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
>> -
>>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: 
>> files('vhost-scsi-common.c'))
>>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: 
>> files('vhost-scsi.c'))
>>   virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: 
>> files('vhost-user-scsi.c'))
>> +specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: 
>> files('spapr_vscsi.c'))
>>   specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: 
>> virtio_scsi_ss)
> 
> I think it might still make sense to keep the virtio stuff together, 
> i.e. add the SPAPR line after the VIRTIO_SCSI line instead of adding it 
> in front of it?

OK will do.

[PATCH 04/11] hw/scsi: Rename target-specific source set as 'specific_virtio_scsi_ss'

[Philippe Mathieu-Daudé]

Following the SCSI variable named '[specific_]scsi_ss', rename the
target-specific VirtIO/SCSI set prefixed with 'specific_'. This will
help when adding target-agnostic VirtIO/SCSI set in few commits.

No logical change.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/scsi/meson.build | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
index f3206a4756..d88f7450e8 100644
--- a/hw/scsi/meson.build
+++ b/hw/scsi/meson.build
@@ -1,6 +1,6 @@
 scsi_ss = ss.source_set()
 specific_scsi_ss = ss.source_set()
-virtio_scsi_ss = ss.source_set()
+specific_virtio_scsi_ss = ss.source_set()
 
 scsi_ss.add(files(
   'emulation.c',
@@ -15,12 +15,13 @@ scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: files('megasas.c'))
 scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: files('mptsas.c', 'mptconfig.c', 'mptendian.c'))
 scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: files('vmw_pvscsi.c'))
 
-virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
-virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
-virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
-virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
+specific_virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
+specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
+specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
+specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
+
 specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
-specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)
+specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: specific_virtio_scsi_ss)
 
 softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
 specific_ss.add_all(when: 'CONFIG_SCSI', if_true: specific_scsi_ss)
-- 
2.38.1

Re: [PATCH 04/11] hw/scsi: Rename target-specific source set as 'specific_virtio_scsi_ss'

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> Following the SCSI variable named '[specific_]scsi_ss', rename the
> target-specific VirtIO/SCSI set prefixed with 'specific_'. This will
> help when adding target-agnostic VirtIO/SCSI set in few commits.
> 
> No logical change.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/scsi/meson.build | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)

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

r~

[PATCH 05/11] hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig

[Philippe Mathieu-Daudé]

Instead of adding 'vhost-vsock-common.c' twice (for VHOST_VSOCK
and VHOST_USER_VSOCK), have it depend on VHOST_VSOCK_COMMON,
selected by both symbols.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/Kconfig     | 6 ++++++
 hw/virtio/meson.build | 5 +++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index 89e9e426d8..de7a35429a 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -56,14 +56,20 @@ config VIRTIO_MEM
     depends on VIRTIO_MEM_SUPPORTED
     select MEM_DEVICE
 
+config VHOST_VSOCK_COMMON
+    bool
+    depends on VIRTIO
+
 config VHOST_VSOCK
     bool
     default y
+    select VHOST_VSOCK_COMMON
     depends on VIRTIO && VHOST_KERNEL
 
 config VHOST_USER_VSOCK
     bool
     default y
+    select VHOST_VSOCK_COMMON
     depends on VIRTIO && VHOST_USER
 
 config VHOST_USER_I2C
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index bdec78bfc6..54c90c24fb 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -23,8 +23,9 @@ specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-bal
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c', 'vhost-vsock-common.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c', 'vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK_COMMON', if_true: files('vhost-vsock-common.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
+specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
-- 
2.38.1

Re: [PATCH 05/11] hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> Instead of adding 'vhost-vsock-common.c' twice (for VHOST_VSOCK
> and VHOST_USER_VSOCK), have it depend on VHOST_VSOCK_COMMON,
> selected by both symbols.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/virtio/Kconfig     | 6 ++++++
>   hw/virtio/meson.build | 5 +++--
>   2 files changed, 9 insertions(+), 2 deletions(-)

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

r~

Re: [PATCH 05/11] hw/virtio: Introduce VHOST_VSOCK_COMMON symbol in Kconfig

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> Instead of adding 'vhost-vsock-common.c' twice (for VHOST_VSOCK
> and VHOST_USER_VSOCK), have it depend on VHOST_VSOCK_COMMON,
> selected by both symbols.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/Kconfig     | 6 ++++++
>   hw/virtio/meson.build | 5 +++--
>   2 files changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 06/11] hw/virtio/virtio-mem: Use qemu_ram_get_fd() helper

[Philippe Mathieu-Daudé]

Avoid accessing RAMBlock internals, use the provided
qemu_ram_get_fd() getter to get the file descriptor.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 538b695c29..74e63bd47a 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -135,7 +135,7 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
      * anonymous RAM. In any other case, reading unplugged *can* populate a
      * fresh page, consuming actual memory.
      */
-    return !qemu_ram_is_shared(rb) && rb->fd < 0 &&
+    return !qemu_ram_is_shared(rb) && qemu_ram_get_fd(rb) < 0 &&
            qemu_ram_pagesize(rb) == qemu_real_host_page_size();
 }
 #endif /* VIRTIO_MEM_HAS_LEGACY_GUESTS */
-- 
2.38.1

Re: [PATCH 06/11] hw/virtio/virtio-mem: Use qemu_ram_get_fd() helper

[David Hildenbrand]

On 23.05.23 18:35, Philippe Mathieu-Daudé wrote:
> Avoid accessing RAMBlock internals, use the provided
> qemu_ram_get_fd() getter to get the file descriptor.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/virtio-mem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index 538b695c29..74e63bd47a 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -135,7 +135,7 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>        * anonymous RAM. In any other case, reading unplugged *can* populate a
>        * fresh page, consuming actual memory.
>        */
> -    return !qemu_ram_is_shared(rb) && rb->fd < 0 &&
> +    return !qemu_ram_is_shared(rb) && qemu_ram_get_fd(rb) < 0 &&
>              qemu_ram_pagesize(rb) == qemu_real_host_page_size();
>   }
>   #endif /* VIRTIO_MEM_HAS_LEGACY_GUESTS */

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH 06/11] hw/virtio/virtio-mem: Use qemu_ram_get_fd() helper

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> Avoid accessing RAMBlock internals, use the provided
> qemu_ram_get_fd() getter to get the file descriptor.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/virtio/virtio-mem.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

[PATCH 07/11] hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header

[Philippe Mathieu-Daudé]

Instead of having "virtio/virtio-bus.h" implicitly included,
explicit it, to avoid when rearranging headers:

  hw/virtio/vhost-vsock-common.c: In function ‘vhost_vsock_common_start’:
  hw/virtio/vhost-vsock-common.c:51:5: error: unknown type name ‘VirtioBusClass’; did you mean ‘VirtioDeviceClass’?
     51 |     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
        |     ^~~~~~~~~~~~~~
        |     VirtioDeviceClass
  hw/virtio/vhost-vsock-common.c:51:25: error: implicit declaration of function ‘VIRTIO_BUS_GET_CLASS’; did you mean ‘VIRTIO_DEVICE_CLASS’? [-Werror=implicit-function-declaration]
     51 |     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
        |                         ^~~~~~~~~~~~~~~~~~~~
        |                         VIRTIO_DEVICE_CLASS

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/vhost-vsock-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index d2b5519d5a..e89af9b329 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -11,6 +11,7 @@
 #include "qemu/osdep.h"
 #include "standard-headers/linux/virtio_vsock.h"
 #include "qapi/error.h"
+#include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
 #include "qemu/error-report.h"
 #include "hw/qdev-properties.h"
-- 
2.38.1

Re: [PATCH 07/11] hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> Instead of having "virtio/virtio-bus.h" implicitly included,
> explicit it, to avoid when rearranging headers:
> 
>    hw/virtio/vhost-vsock-common.c: In function ‘vhost_vsock_common_start’:
>    hw/virtio/vhost-vsock-common.c:51:5: error: unknown type name ‘VirtioBusClass’; did you mean ‘VirtioDeviceClass’?
>       51 |     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>          |     ^~~~~~~~~~~~~~
>          |     VirtioDeviceClass
>    hw/virtio/vhost-vsock-common.c:51:25: error: implicit declaration of function ‘VIRTIO_BUS_GET_CLASS’; did you mean ‘VIRTIO_DEVICE_CLASS’? [-Werror=implicit-function-declaration]
>       51 |     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>          |                         ^~~~~~~~~~~~~~~~~~~~
>          |                         VIRTIO_DEVICE_CLASS
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/virtio/vhost-vsock-common.c | 1 +
>   1 file changed, 1 insertion(+)

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

r~

Re: [PATCH 07/11] hw/virtio/vhost-vsock: Include missing 'virtio/virtio-bus.h' header

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> Instead of having "virtio/virtio-bus.h" implicitly included,
> explicit it, to avoid when rearranging headers:

s/explicit it/explicitly include it/ ?

With that change:
Reviewed-by: Thomas Huth <thuth@redhat.com>

[PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Philippe Mathieu-Daudé]

In order to have virtio-iommu.c become target-agnostic,
we need to avoid using TARGET_PAGE_MASK. Get it with the
qemu_target_page_mask() helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 1cd258135d..85905a9e3d 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "qemu/log.h"
 #include "qemu/iov.h"
+#include "exec/target_page.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/virtio.h"
 #include "sysemu/kvm.h"
@@ -1164,7 +1165,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
      * in vfio realize
      */
     s->config.bypass = s->boot_bypass;
-    s->config.page_size_mask = TARGET_PAGE_MASK;
+    s->config.page_size_mask = qemu_target_page_mask();
     s->config.input_range.end = UINT64_MAX;
     s->config.domain_range.end = UINT32_MAX;
     s->config.probe_size = VIOMMU_PROBE_SIZE;
-- 
2.38.1

Re: [PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> In order to have virtio-iommu.c become target-agnostic,
> we need to avoid using TARGET_PAGE_MASK. Get it with the
> qemu_target_page_mask() helper.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/virtio-iommu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1cd258135d..85905a9e3d 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
>   #include "qemu/iov.h"
> +#include "exec/target_page.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/virtio/virtio.h"
>   #include "sysemu/kvm.h"
> @@ -1164,7 +1165,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>        * in vfio realize
>        */
>       s->config.bypass = s->boot_bypass;
> -    s->config.page_size_mask = TARGET_PAGE_MASK;
> +    s->config.page_size_mask = qemu_target_page_mask();

This could be

   = -(uint64_t)qemu_target_page_size()

without adding the new function.  But either way,

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


r~

Re: [PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Philippe Mathieu-Daudé]

On 24/5/23 01:28, Richard Henderson wrote:
> On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
>> In order to have virtio-iommu.c become target-agnostic,
>> we need to avoid using TARGET_PAGE_MASK. Get it with the
>> qemu_target_page_mask() helper.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/virtio/virtio-iommu.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)


>> @@ -1164,7 +1165,7 @@ static void 
>> virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>        * in vfio realize
>>        */
>>       s->config.bypass = s->boot_bypass;
>> -    s->config.page_size_mask = TARGET_PAGE_MASK;
>> +    s->config.page_size_mask = qemu_target_page_mask();
> 
> This could be
> 
>    = -(uint64_t)qemu_target_page_size()
> 
> without adding the new function.

Alex recommended on IRC to add a helper "by all means" :)

> But either way,
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!

Re: [PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> In order to have virtio-iommu.c become target-agnostic,
> we need to avoid using TARGET_PAGE_MASK. Get it with the
> qemu_target_page_mask() helper.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/virtio-iommu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1cd258135d..85905a9e3d 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/log.h"
>   #include "qemu/iov.h"
> +#include "exec/target_page.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/virtio/virtio.h"
>   #include "sysemu/kvm.h"
> @@ -1164,7 +1165,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>        * in vfio realize
>        */
>       s->config.bypass = s->boot_bypass;
> -    s->config.page_size_mask = TARGET_PAGE_MASK;
> +    s->config.page_size_mask = qemu_target_page_mask();
>       s->config.input_range.end = UINT64_MAX;
>       s->config.domain_range.end = UINT32_MAX;
>       s->config.probe_size = VIOMMU_PROBE_SIZE;

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 08/11] hw/virtio/virtio-iommu: Use target-agnostic qemu_target_page_mask()

[Eric Auger]

Hi Philippe,

On 5/23/23 18:35, Philippe Mathieu-Daudé wrote:
> In order to have virtio-iommu.c become target-agnostic,
> we need to avoid using TARGET_PAGE_MASK. Get it with the
> qemu_target_page_mask() helper.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  hw/virtio/virtio-iommu.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 1cd258135d..85905a9e3d 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -20,6 +20,7 @@
>  #include "qemu/osdep.h"
>  #include "qemu/log.h"
>  #include "qemu/iov.h"
> +#include "exec/target_page.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/virtio/virtio.h"
>  #include "sysemu/kvm.h"
> @@ -1164,7 +1165,7 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>       * in vfio realize
>       */
>      s->config.bypass = s->boot_bypass;
> -    s->config.page_size_mask = TARGET_PAGE_MASK;
> +    s->config.page_size_mask = qemu_target_page_mask();
>      s->config.input_range.end = UINT64_MAX;
>      s->config.domain_range.end = UINT32_MAX;
>      s->config.probe_size = VIOMMU_PROBE_SIZE;

[PATCH 09/11] hw/virtio: Remove unnecessary 'virtio-access.h' header

[Philippe Mathieu-Daudé]

None of these files use the VirtIO Load/Store API declared
by "hw/virtio/virtio-access.h". This header probably crept
in via copy/pasting, remove it.

Note, "virtio-access.h" is target-specific, so any file
including it also become tainted as target-specific.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/block/dataplane/virtio-blk.c | 1 -
 hw/s390x/virtio-ccw.c           | 1 -
 hw/scsi/vhost-scsi.c            | 1 -
 hw/scsi/vhost-user-scsi.c       | 1 -
 hw/scsi/virtio-scsi-dataplane.c | 1 -
 hw/virtio/vdpa-dev.c            | 1 -
 hw/virtio/vhost-vdpa.c          | 1 -
 hw/virtio/vhost-vsock-common.c  | 1 -
 hw/virtio/vhost.c               | 1 -
 hw/virtio/virtio-crypto.c       | 1 -
 hw/virtio/virtio-iommu.c        | 1 -
 hw/virtio/virtio-mem.c          | 1 -
 12 files changed, 12 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index af1c24c40c..03ecb51664 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -19,7 +19,6 @@
 #include "qemu/main-loop.h"
 #include "qemu/thread.h"
 #include "qemu/error-report.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-blk.h"
 #include "virtio-blk.h"
 #include "block/aio.h"
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index f44de1a8c1..17c548b84f 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -22,7 +22,6 @@
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/s390x/adapter.h"
 #include "hw/s390x/s390_flic.h"
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 6a0fd0dfb1..443f67daa4 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -26,7 +26,6 @@
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/virtio-scsi.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/fw-path-provider.h"
 #include "hw/qdev-properties.h"
 #include "qemu/cutils.h"
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index b7a71a802c..ee99b19e7a 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -26,7 +26,6 @@
 #include "hw/virtio/vhost-backend.h"
 #include "hw/virtio/vhost-user-scsi.h"
 #include "hw/virtio/virtio.h"
-#include "hw/virtio/virtio-access.h"
 #include "chardev/char-fe.h"
 #include "sysemu/sysemu.h"
 
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index f3214e1c57..21344c7cfe 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -19,7 +19,6 @@
 #include "hw/scsi/scsi.h"
 #include "scsi/constants.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 
 /* Context: QEMU global mutex held */
 void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp)
diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
index 01b41eb0f1..e08e830006 100644
--- a/hw/virtio/vdpa-dev.c
+++ b/hw/virtio/vdpa-dev.c
@@ -25,7 +25,6 @@
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/virtio/vdpa-dev.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index b3094e8a8b..3c575a9a6e 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -26,7 +26,6 @@
 #include "cpu.h"
 #include "trace.h"
 #include "qapi/error.h"
-#include "hw/virtio/virtio-access.h"
 
 /*
  * Return one past the end of the end of section. Be careful with uint64_t
diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
index e89af9b329..321262f6b3 100644
--- a/hw/virtio/vhost-vsock-common.c
+++ b/hw/virtio/vhost-vsock-common.c
@@ -12,7 +12,6 @@
 #include "standard-headers/linux/virtio_vsock.h"
 #include "qapi/error.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "qemu/error-report.h"
 #include "hw/qdev-properties.h"
 #include "hw/virtio/vhost.h"
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 23da579ce2..7f3c727777 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -23,7 +23,6 @@
 #include "qemu/log.h"
 #include "standard-headers/linux/vhost_types.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "migration/blocker.h"
 #include "migration/qemu-file-types.h"
 #include "sysemu/dma.h"
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index c729a1f79e..a6d7e1e8ec 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -21,7 +21,6 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-crypto.h"
 #include "hw/qdev-properties.h"
-#include "hw/virtio/virtio-access.h"
 #include "standard-headers/linux/virtio_ids.h"
 #include "sysemu/cryptodev-vhost.h"
 
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 85905a9e3d..1bbad23f4a 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -32,7 +32,6 @@
 #include "standard-headers/linux/virtio_ids.h"
 
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci/pci.h"
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 74e63bd47a..12ea58d5ad 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -20,7 +20,6 @@
 #include "sysemu/reset.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 #include "hw/virtio/virtio-mem.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
-- 
2.38.1

Re: [PATCH 09/11] hw/virtio: Remove unnecessary 'virtio-access.h' header

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> None of these files use the VirtIO Load/Store API declared
> by "hw/virtio/virtio-access.h". This header probably crept
> in via copy/pasting, remove it.
> 
> Note, "virtio-access.h" is target-specific, so any file
> including it also become tainted as target-specific.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---

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

r~

Re: [PATCH 09/11] hw/virtio: Remove unnecessary 'virtio-access.h' header

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> None of these files use the VirtIO Load/Store API declared
> by "hw/virtio/virtio-access.h". This header probably crept
> in via copy/pasting, remove it.
> 
> Note, "virtio-access.h" is target-specific, so any file
> including it also become tainted as target-specific.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/block/dataplane/virtio-blk.c | 1 -
>   hw/s390x/virtio-ccw.c           | 1 -
>   hw/scsi/vhost-scsi.c            | 1 -
>   hw/scsi/vhost-user-scsi.c       | 1 -
>   hw/scsi/virtio-scsi-dataplane.c | 1 -
>   hw/virtio/vdpa-dev.c            | 1 -
>   hw/virtio/vhost-vdpa.c          | 1 -
>   hw/virtio/vhost-vsock-common.c  | 1 -
>   hw/virtio/vhost.c               | 1 -
>   hw/virtio/virtio-crypto.c       | 1 -
>   hw/virtio/virtio-iommu.c        | 1 -
>   hw/virtio/virtio-mem.c          | 1 -
>   12 files changed, 12 deletions(-)

Very good catch! I checked that it compiles and links fine with this change, so:

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 09/11] hw/virtio: Remove unnecessary 'virtio-access.h' header

[Philippe Mathieu-Daudé]

On 24/5/23 09:29, Thomas Huth wrote:
> On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
>> None of these files use the VirtIO Load/Store API declared
>> by "hw/virtio/virtio-access.h". This header probably crept
>> in via copy/pasting, remove it.
>>
>> Note, "virtio-access.h" is target-specific, so any file
>> including it also become tainted as target-specific.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/block/dataplane/virtio-blk.c | 1 -
>>   hw/s390x/virtio-ccw.c           | 1 -
>>   hw/scsi/vhost-scsi.c            | 1 -
>>   hw/scsi/vhost-user-scsi.c       | 1 -
>>   hw/scsi/virtio-scsi-dataplane.c | 1 -
>>   hw/virtio/vdpa-dev.c            | 1 -
>>   hw/virtio/vhost-vdpa.c          | 1 -
>>   hw/virtio/vhost-vsock-common.c  | 1 -
>>   hw/virtio/vhost.c               | 1 -
>>   hw/virtio/virtio-crypto.c       | 1 -
>>   hw/virtio/virtio-iommu.c        | 1 -
>>   hw/virtio/virtio-mem.c          | 1 -
>>   12 files changed, 12 deletions(-)
> 
> Very good catch! I checked that it compiles and links fine with this 
> change, so:
> 
> Tested-by: Thomas Huth <thuth@redhat.com>

Thanks, FYI this list was built doing:

   $ git grep -l virtio-access.h \
     | xargs git grep -LE 'virtio_(st|ld)' \
     | xargs git grep virtio-access.h

(I don't think the command is relevant enough to include in the
  commit description, but can amend it)

[PATCH 10/11] hw/virtio: Build various target-agnostic objects just once

[Philippe Mathieu-Daudé]

The previous commit remove the unnecessary "virtio-access.h"
header. These files no longer have target-specific dependency.
Move them to the generic 'softmmu_ss' source set.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/block/dataplane/meson.build |  2 +-
 hw/scsi/meson.build            | 10 +++++++---
 hw/virtio/meson.build          | 11 ++++++-----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/block/dataplane/meson.build b/hw/block/dataplane/meson.build
index 78d7ac1a11..dec73e7486 100644
--- a/hw/block/dataplane/meson.build
+++ b/hw/block/dataplane/meson.build
@@ -1,2 +1,2 @@
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
+softmmu_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-block.c'))
diff --git a/hw/scsi/meson.build b/hw/scsi/meson.build
index d88f7450e8..12a1ca644f 100644
--- a/hw/scsi/meson.build
+++ b/hw/scsi/meson.build
@@ -1,5 +1,6 @@
 scsi_ss = ss.source_set()
 specific_scsi_ss = ss.source_set()
+virtio_scsi_ss = ss.source_set()
 specific_virtio_scsi_ss = ss.source_set()
 
 scsi_ss.add(files(
@@ -15,13 +16,16 @@ scsi_ss.add(when: 'CONFIG_MEGASAS_SCSI_PCI', if_true: files('megasas.c'))
 scsi_ss.add(when: 'CONFIG_MPTSAS_SCSI_PCI', if_true: files('mptsas.c', 'mptconfig.c', 'mptendian.c'))
 scsi_ss.add(when: 'CONFIG_VMW_PVSCSI_SCSI_PCI', if_true: files('vmw_pvscsi.c'))
 
-specific_virtio_scsi_ss.add(files('virtio-scsi.c', 'virtio-scsi-dataplane.c'))
+virtio_scsi_ss.add(files('virtio-scsi-dataplane.c'))
+virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
+virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
+
+specific_virtio_scsi_ss.add(files('virtio-scsi.c'))
 specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI_COMMON', if_true: files('vhost-scsi-common.c'))
-specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_SCSI', if_true: files('vhost-scsi.c'))
-specific_virtio_scsi_ss.add(when: 'CONFIG_VHOST_USER_SCSI', if_true: files('vhost-user-scsi.c'))
 
 specific_scsi_ss.add(when: 'CONFIG_SPAPR_VSCSI', if_true: files('spapr_vscsi.c'))
 specific_scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: specific_virtio_scsi_ss)
+scsi_ss.add_all(when: 'CONFIG_VIRTIO_SCSI', if_true: virtio_scsi_ss)
 
 softmmu_ss.add_all(when: 'CONFIG_SCSI', if_true: scsi_ss)
 specific_ss.add_all(when: 'CONFIG_SCSI', if_true: specific_scsi_ss)
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 54c90c24fb..16e64e1cf1 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -2,13 +2,18 @@ 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'))
 
 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
-  specific_virtio_ss.add(files('vhost.c', 'vhost-backend.c', 'vhost-iova-tree.c'))
+  softmmu_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
@@ -20,20 +25,16 @@ else
 endif
 
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_BALLOON', if_true: files('virtio-balloon.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('virtio-crypto.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_FS', if_true: files('vhost-user-fs.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: files('virtio-pmem.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK_COMMON', if_true: files('vhost-vsock-common.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_VSOCK', if_true: files('vhost-user-vsock.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_RNG', if_true: files('virtio-rng.c'))
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: files('virtio-iommu.c'))
 specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_I2C', if_true: files('vhost-user-i2c.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_RNG', if_true: files('vhost-user-rng.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_GPIO', if_true: files('vhost-user-gpio.c'))
 specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_GPIO'], if_true: files('vhost-user-gpio-pci.c'))
-specific_virtio_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: files('vdpa-dev.c'))
 
 virtio_pci_ss = ss.source_set()
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VSOCK', if_true: files('vhost-vsock-pci.c'))
-- 
2.38.1

Re: [PATCH 10/11] hw/virtio: Build various target-agnostic objects just once

[Richard Henderson]

On 5/23/23 09:35, Philippe Mathieu-Daudé wrote:
> The previous commit remove the unnecessary "virtio-access.h"
> header. These files no longer have target-specific dependency.
> Move them to the generic 'softmmu_ss' source set.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/block/dataplane/meson.build |  2 +-
>   hw/scsi/meson.build            | 10 +++++++---
>   hw/virtio/meson.build          | 11 ++++++-----
>   3 files changed, 14 insertions(+), 9 deletions(-)

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

r~

Re: [PATCH 10/11] hw/virtio: Build various target-agnostic objects just once

[Thomas Huth]

On 23/05/2023 18.35, Philippe Mathieu-Daudé wrote:
> The previous commit remove the unnecessary "virtio-access.h"
> header. These files no longer have target-specific dependency.
> Move them to the generic 'softmmu_ss' source set.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/block/dataplane/meson.build |  2 +-
>   hw/scsi/meson.build            | 10 +++++++---
>   hw/virtio/meson.build          | 11 ++++++-----
>   3 files changed, 14 insertions(+), 9 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

[RFC PATCH 11/11] hw/virtio: Make vhost-vdpa.c target-agnostic to build it once

[Philippe Mathieu-Daudé]

Replace TARGET_PAGE_MASK -> qemu_target_page_mask() and
TARGET_PAGE_ALIGN() -> qemu_target_page_align() so we don't
need the target-specific "cpu.h" header.

These macros are used in the MemoryListener add/del handlers
(vhost_vdpa_listener_skipped_section is only called by
vhost_vdpa_listener_region_add) which are not hot-path.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/vhost-vdpa.c | 16 ++++++++--------
 hw/virtio/meson.build  |  3 ++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3c575a9a6e..a51497aaf1 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"
 
@@ -35,7 +35,7 @@ static Int128 vhost_vdpa_section_end(const MemoryRegionSection *section)
 {
     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(qemu_target_page_mask()));
 
     return llend;
 }
@@ -321,13 +321,13 @@ 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 & ~qemu_target_page_mask()) !=
+                 (section->offset_within_region & ~qemu_target_page_mask()))) {
         error_report("%s received unaligned region", __func__);
         return;
     }
 
-    iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+    iova = qemu_target_page_align(section->offset_within_address_space);
     llend = vhost_vdpa_section_end(section);
     if (int128_ge(int128_make64(iova), llend)) {
         return;
@@ -403,13 +403,13 @@ static void vhost_vdpa_listener_region_del(MemoryListener *listener,
         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 & ~qemu_target_page_mask()) !=
+                 (section->offset_within_region & ~qemu_target_page_mask()))) {
         error_report("%s received unaligned region", __func__);
         return;
     }
 
-    iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
+    iova = qemu_target_page_align(section->offset_within_address_space);
     llend = vhost_vdpa_section_end(section);
 
     trace_vhost_vdpa_listener_region_del(v, iova,
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index 16e64e1cf1..c29be98ab0 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

Re: [RFC PATCH 11/11] hw/virtio: Make vhost-vdpa.c target-agnostic to build it once

[Richard Henderson]

On 5/23/23 09:36, Philippe Mathieu-Daudé wrote:
> @@ -321,13 +321,13 @@ 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 & ~qemu_target_page_mask()) !=
> +                 (section->offset_within_region & ~qemu_target_page_mask()))) {
>           error_report("%s received unaligned region", __func__);
>           return;
>       }
>   
> -    iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
> +    iova = qemu_target_page_align(section->offset_within_address_space);
>       llend = vhost_vdpa_section_end(section);
>       if (int128_ge(int128_make64(iova), llend)) {
>           return;

I'm not keen on using 3 function calls to get one constant.
This could be

     int page_size = qemu_target_page_size();
     int page_mask = page_size - 1;

     if (section->foo & page_mask) { ...

     iova = ROUND_UP(section->bar, page_size);

Also in vhost_vdpa_listener_region_del.

This then removes the only uses of qemu_target_page_align, so you don't need to add that 
either.


r~

Re: [RFC PATCH 11/11] hw/virtio: Make vhost-vdpa.c target-agnostic to build it once

[Thomas Huth]

On 23/05/2023 18.36, Philippe Mathieu-Daudé wrote:
> Replace TARGET_PAGE_MASK -> qemu_target_page_mask() and
> TARGET_PAGE_ALIGN() -> qemu_target_page_align() so we don't
> need the target-specific "cpu.h" header.
> 
> These macros are used in the MemoryListener add/del handlers
> (vhost_vdpa_listener_skipped_section is only called by
> vhost_vdpa_listener_region_add) which are not hot-path.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/vhost-vdpa.c | 16 ++++++++--------
>   hw/virtio/meson.build  |  3 ++-
>   2 files changed, 10 insertions(+), 9 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>