qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] hw/virtio: Build virtio-mem.c once
@ 2025-03-07 15:15 Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 1/7] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Replace compile-time checks by runtime ones:
- CONFIG_DEVICES::CONFIG_ACPI -> acpi_builtin()
- TARGET_FOO || TARGET_BAR -> qemu_arch_available(FOO|BAR)

Philippe Mathieu-Daudé (7):
  system: Replace arch_type global by qemu_arch_available() helper
  hw/acpi: Introduce acpi_builtin() helper
  hw/i386/fw_cfg: Check ACPI availability with acpi_builtin()
  hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS to runtime
  hw/virtio: Compile virtio-mem.c once

 include/hw/acpi/acpi.h     |  3 ++
 include/system/arch_init.h |  2 +-
 hw/acpi/acpi-stub.c        |  5 +++
 hw/acpi/core.c             |  5 +++
 hw/i386/fw_cfg.c           |  8 ++--
 hw/scsi/scsi-disk.c        |  2 +-
 hw/virtio/virtio-mem.c     | 88 ++++++++++++++++++++------------------
 system/arch_init.c         |  5 ++-
 system/qdev-monitor.c      |  4 +-
 system/vl.c                |  6 +--
 hw/virtio/meson.build      |  2 +-
 11 files changed, 75 insertions(+), 55 deletions(-)

-- 
2.47.1



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 1/7] system: Replace arch_type global by qemu_arch_available() helper
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

qemu_arch_available() is a bit simpler to understand while
reviewing than the undocumented arch_type variable.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/system/arch_init.h | 2 +-
 hw/scsi/scsi-disk.c        | 2 +-
 system/arch_init.c         | 5 ++++-
 system/qdev-monitor.c      | 4 ++--
 system/vl.c                | 6 +++---
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/system/arch_init.h b/include/system/arch_init.h
index d8b77440487..51e24c3091e 100644
--- a/include/system/arch_init.h
+++ b/include/system/arch_init.h
@@ -25,6 +25,6 @@ enum {
     QEMU_ARCH_LOONGARCH = (1 << 23),
 };
 
-extern const uint32_t arch_type;
+bool qemu_arch_available(unsigned qemu_arch_mask);
 
 #endif
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e7f738b4841..7c87b20e694 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -3165,7 +3165,7 @@ static void scsi_property_add_specifics(DeviceClass *dc)
     ObjectClass *oc = OBJECT_CLASS(dc);
 
     /* The loadparm property is only supported on s390x */
-    if (arch_type & QEMU_ARCH_S390X) {
+    if (qemu_arch_available(QEMU_ARCH_S390X)) {
         object_class_property_add_str(oc, "loadparm",
                                       scsi_property_get_loadparm,
                                       scsi_property_set_loadparm);
diff --git a/system/arch_init.c b/system/arch_init.c
index b1baed18a30..61c6f680c94 100644
--- a/system/arch_init.c
+++ b/system/arch_init.c
@@ -38,4 +38,7 @@ int graphic_height = 600;
 int graphic_depth = 32;
 #endif
 
-const uint32_t arch_type = QEMU_ARCH;
+bool qemu_arch_available(unsigned qemu_arch_mask)
+{
+    return qemu_arch_mask & QEMU_ARCH;
+}
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 856c9e8c32e..5588ed2047d 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -132,7 +132,7 @@ static const char *qdev_class_get_alias(DeviceClass *dc)
 
     for (i = 0; qdev_alias_table[i].typename; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
@@ -218,7 +218,7 @@ static const char *find_typename_by_alias(const char *alias)
 
     for (i = 0; qdev_alias_table[i].alias; i++) {
         if (qdev_alias_table[i].arch_mask &&
-            !(qdev_alias_table[i].arch_mask & arch_type)) {
+            !qemu_arch_available(qdev_alias_table[i].arch_mask)) {
             continue;
         }
 
diff --git a/system/vl.c b/system/vl.c
index 04f78466c41..ec93988a03a 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -878,11 +878,11 @@ static void help(int exitcode)
             g_get_prgname());
 
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
-    if ((arch_mask) & arch_type)                               \
+    if (qemu_arch_available(arch_mask)) \
         fputs(opt_help, stdout);
 
 #define ARCHHEADING(text, arch_mask) \
-    if ((arch_mask) & arch_type)    \
+    if (qemu_arch_available(arch_mask)) \
         puts(stringify(text));
 
 #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
@@ -2929,7 +2929,7 @@ void qemu_init(int argc, char **argv)
             const QEMUOption *popt;
 
             popt = lookup_opt(argc, argv, &optarg, &optind);
-            if (!(popt->arch_mask & arch_type)) {
+            if (!qemu_arch_available(popt->arch_mask)) {
                 error_report("Option not supported for this target");
                 exit(1);
             }
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 1/7] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 15:42   ` Ani Sinha
  2025-03-07 15:15 ` [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

acpi_builtin() can be used to check at runtime whether
the ACPI subsystem is built in a qemu-system binary.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/acpi/acpi.h | 3 +++
 hw/acpi/acpi-stub.c    | 5 +++++
 hw/acpi/core.c         | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index e0e51e85b41..d1a4fa2af84 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -150,6 +150,9 @@ struct ACPIREGS {
     Notifier wakeup;
 };
 
+/* Return whether ACPI subsystem is built in */
+bool acpi_builtin(void);
+
 /* PM_TMR */
 void acpi_pm_tmr_update(ACPIREGS *ar, bool enable);
 void acpi_pm_tmr_calc_overflow_time(ACPIREGS *ar);
diff --git a/hw/acpi/acpi-stub.c b/hw/acpi/acpi-stub.c
index e268ce9b1a9..790bf509e5d 100644
--- a/hw/acpi/acpi-stub.c
+++ b/hw/acpi/acpi-stub.c
@@ -25,3 +25,8 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
 {
     g_assert_not_reached();
 }
+
+bool acpi_builtin(void)
+{
+    return false;
+}
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 870391ed7c8..58f8964e130 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -78,6 +78,11 @@ static void acpi_register_config(void)
 
 opts_init(acpi_register_config);
 
+bool acpi_builtin(void)
+{
+    return true;
+}
+
 static int acpi_checksum(const uint8_t *data, int len)
 {
     int sum, i;
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin()
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 1/7] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 15:44   ` Ani Sinha
  2025-03-07 15:15 ` [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Replace the compile-time CONFIG_ACPI check by a runtime one.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/fw_cfg.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index d08aefa0291..a7f1b60b98c 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -145,10 +145,10 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
      */
     fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
     fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, ms->ram_size);
-#ifdef CONFIG_ACPI
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
-                     acpi_tables, acpi_tables_len);
-#endif
+    if (acpi_builtin()) {
+        fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
+                         acpi_tables, acpi_tables_len);
+    }
     fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
 
     fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_fw_cfg, sizeof(hpet_fw_cfg));
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2025-03-07 15:15 ` [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 17:48   ` David Hildenbrand
  2025-03-07 19:21   ` Pierrick Bouvier
  2025-03-07 15:15 ` [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Rather than checking ACPI availability at compile time by
checking the CONFIG_ACPI definition from CONFIG_DEVICES,
check at runtime via acpi_builtin().

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

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 7b140add765..5f57eccbb66 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -28,7 +28,7 @@
 #include "migration/misc.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
-#include CONFIG_DEVICES
+#include "hw/acpi/acpi.h"
 #include "trace.h"
 
 static const VMStateDescription vmstate_virtio_mem_device_early;
@@ -883,10 +883,8 @@ static uint64_t virtio_mem_get_features(VirtIODevice *vdev, uint64_t features,
     MachineState *ms = MACHINE(qdev_get_machine());
     VirtIOMEM *vmem = VIRTIO_MEM(vdev);
 
-    if (ms->numa_state) {
-#if defined(CONFIG_ACPI)
+    if (ms->numa_state && acpi_builtin()) {
         virtio_add_feature(&features, VIRTIO_MEM_F_ACPI_PXM);
-#endif
     }
     assert(vmem->unplugged_inaccessible != ON_OFF_AUTO_AUTO);
     if (vmem->unplugged_inaccessible == ON_OFF_AUTO_ON) {
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2025-03-07 15:15 ` [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 16:38   ` Alex Bennée
  2025-03-07 15:15 ` [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS " Philippe Mathieu-Daudé
  2025-03-07 15:15 ` [PATCH 7/7] hw/virtio: Compile virtio-mem.c once Philippe Mathieu-Daudé
  6 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Use qemu_arch_available() to check at runtime if a target
architecture is built in.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-mem.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 5f57eccbb66..8c40042108c 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -15,6 +15,7 @@
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/units.h"
+#include "system/arch_init.h"
 #include "system/numa.h"
 #include "system/system.h"
 #include "system/reset.h"
@@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
  * necessary (as the section size can change). But it's more likely that the
  * section size will rather get smaller and not bigger over time.
  */
-#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
-#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
-#elif defined(TARGET_ARM)
-#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
-#else
-#error VIRTIO_MEM_USABLE_EXTENT not defined
-#endif
+static uint64_t virtio_mem_usable_extent_size(void)
+{
+    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
+        return 2 * 128 * MiB;
+    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
+        return 2 * 512 * MiB;
+    } else {
+        g_assert_not_reached();
+    }
+}
 
 static bool virtio_mem_is_busy(void)
 {
@@ -721,7 +725,7 @@ static void virtio_mem_resize_usable_region(VirtIOMEM *vmem,
                                             bool can_shrink)
 {
     uint64_t newsize = MIN(memory_region_size(&vmem->memdev->mr),
-                           requested_size + VIRTIO_MEM_USABLE_EXTENT);
+                           requested_size + virtio_mem_usable_extent_size());
 
     /* The usable region size always has to be multiples of the block size. */
     newsize = QEMU_ALIGN_UP(newsize, vmem->block_size);
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS to runtime
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2025-03-07 15:15 ` [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 17:54   ` David Hildenbrand
  2025-03-07 15:15 ` [PATCH 7/7] hw/virtio: Compile virtio-mem.c once Philippe Mathieu-Daudé
  6 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Use qemu_arch_available() to check at runtime if a target
architecture is built in.
Register virtio_mem_legacy_guests_properties[] at runtime.
Code churn in virtio_mem_device_realize() is due to re-indentation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/virtio/virtio-mem.c | 61 ++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 8c40042108c..ea7229ce28c 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -38,9 +38,10 @@ static const VMStateDescription vmstate_virtio_mem_device_early;
  * We only had legacy x86 guests that did not support
  * VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE. Other targets don't have legacy guests.
  */
-#if defined(TARGET_X86_64) || defined(TARGET_I386)
-#define VIRTIO_MEM_HAS_LEGACY_GUESTS
-#endif
+static bool virtio_mem_has_legacy_guests(void)
+{
+    return qemu_arch_available(QEMU_ARCH_I386);
+}
 
 /*
  * Let's not allow blocks smaller than 1 MiB, for example, to keep the tracking
@@ -144,7 +145,6 @@ static uint64_t virtio_mem_default_block_size(RAMBlock *rb)
     return MAX(page_size, VIRTIO_MEM_MIN_BLOCK_SIZE);
 }
 
-#if defined(VIRTIO_MEM_HAS_LEGACY_GUESTS)
 static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
 {
     /*
@@ -155,7 +155,6 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
     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 */
 
 /*
  * Size the usable region bigger than the requested size if possible. Esp.
@@ -1001,28 +1000,28 @@ static void virtio_mem_device_realize(DeviceState *dev, Error **errp)
     rb = vmem->memdev->mr.ram_block;
     page_size = qemu_ram_pagesize(rb);
 
-#if defined(VIRTIO_MEM_HAS_LEGACY_GUESTS)
-    switch (vmem->unplugged_inaccessible) {
-    case ON_OFF_AUTO_AUTO:
-        if (virtio_mem_has_shared_zeropage(rb)) {
-            vmem->unplugged_inaccessible = ON_OFF_AUTO_OFF;
-        } else {
-            vmem->unplugged_inaccessible = ON_OFF_AUTO_ON;
+    if (virtio_mem_has_legacy_guests()) {
+        switch (vmem->unplugged_inaccessible) {
+        case ON_OFF_AUTO_AUTO:
+            if (virtio_mem_has_shared_zeropage(rb)) {
+                vmem->unplugged_inaccessible = ON_OFF_AUTO_OFF;
+            } else {
+                vmem->unplugged_inaccessible = ON_OFF_AUTO_ON;
+            }
+            break;
+        case ON_OFF_AUTO_OFF:
+            if (!virtio_mem_has_shared_zeropage(rb)) {
+                warn_report("'%s' property set to 'off' with a memdev that does"
+                            " not support the shared zeropage.",
+                            VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP);
+            }
+            break;
+        default:
+            break;
         }
-        break;
-    case ON_OFF_AUTO_OFF:
-        if (!virtio_mem_has_shared_zeropage(rb)) {
-            warn_report("'%s' property set to 'off' with a memdev that does"
-                        " not support the shared zeropage.",
-                        VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP);
-        }
-        break;
-    default:
-        break;
+    } else {
+        vmem->unplugged_inaccessible = ON_OFF_AUTO_ON;
     }
-#else /* VIRTIO_MEM_HAS_LEGACY_GUESTS */
-    vmem->unplugged_inaccessible = ON_OFF_AUTO_ON;
-#endif /* VIRTIO_MEM_HAS_LEGACY_GUESTS */
 
     if (vmem->dynamic_memslots &&
         vmem->unplugged_inaccessible != ON_OFF_AUTO_ON) {
@@ -1715,16 +1714,17 @@ static const Property virtio_mem_properties[] = {
     DEFINE_PROP_BOOL(VIRTIO_MEM_PREALLOC_PROP, VirtIOMEM, prealloc, false),
     DEFINE_PROP_LINK(VIRTIO_MEM_MEMDEV_PROP, VirtIOMEM, memdev,
                      TYPE_MEMORY_BACKEND, HostMemoryBackend *),
-#if defined(VIRTIO_MEM_HAS_LEGACY_GUESTS)
-    DEFINE_PROP_ON_OFF_AUTO(VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP, VirtIOMEM,
-                            unplugged_inaccessible, ON_OFF_AUTO_ON),
-#endif
     DEFINE_PROP_BOOL(VIRTIO_MEM_EARLY_MIGRATION_PROP, VirtIOMEM,
                      early_migration, true),
     DEFINE_PROP_BOOL(VIRTIO_MEM_DYNAMIC_MEMSLOTS_PROP, VirtIOMEM,
                      dynamic_memslots, false),
 };
 
+static const Property virtio_mem_legacy_guests_properties[] = {
+    DEFINE_PROP_ON_OFF_AUTO(VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP, VirtIOMEM,
+                            unplugged_inaccessible, ON_OFF_AUTO_ON),
+};
+
 static uint64_t virtio_mem_rdm_get_min_granularity(const RamDiscardManager *rdm,
                                                    const MemoryRegion *mr)
 {
@@ -1877,6 +1877,9 @@ static void virtio_mem_class_init(ObjectClass *klass, void *data)
     RamDiscardManagerClass *rdmc = RAM_DISCARD_MANAGER_CLASS(klass);
 
     device_class_set_props(dc, virtio_mem_properties);
+    if (virtio_mem_has_legacy_guests()) {
+        device_class_set_props(dc, virtio_mem_legacy_guests_properties);
+    }
     dc->vmsd = &vmstate_virtio_mem;
 
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 7/7] hw/virtio: Compile virtio-mem.c once
  2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
                   ` (5 preceding siblings ...)
  2025-03-07 15:15 ` [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS " Philippe Mathieu-Daudé
@ 2025-03-07 15:15 ` Philippe Mathieu-Daudé
  2025-03-07 17:55   ` David Hildenbrand
  6 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth, Philippe Mathieu-Daudé

Remove unused "exec/ram_addr.h" header. This file doesn't
use any target specific definitions anymore, compile it
once by moving it to system_virtio_ss[].

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

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ea7229ce28c..a5d732ac6d9 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -25,7 +25,6 @@
 #include "hw/virtio/virtio-mem.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
-#include "exec/ram_addr.h"
 #include "migration/misc.h"
 #include "hw/boards.h"
 #include "hw/qdev-properties.h"
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index a5f9f7999dd..7c3513315cb 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -55,7 +55,7 @@ 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_NSM', if_true: [files('virtio-nsm.c', 'cbor-helpers.c'), libcbor])
-specific_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
+system_virtio_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: files('virtio-mem.c'))
 specific_virtio_ss.add(when: 'CONFIG_VHOST_USER_SCMI', if_true: files('vhost-user-scmi.c'))
 specific_virtio_ss.add(when: ['CONFIG_VIRTIO_PCI', 'CONFIG_VHOST_USER_SCMI'], if_true: files('vhost-user-scmi-pci.c'))
 
-- 
2.47.1



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper
  2025-03-07 15:15 ` [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
@ 2025-03-07 15:42   ` Ani Sinha
  0 siblings, 0 replies; 20+ messages in thread
From: Ani Sinha @ 2025-03-07 15:42 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Alex Bennée, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth

On Fri, Mar 7, 2025 at 8:46 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> acpi_builtin() can be used to check at runtime whether
> the ACPI subsystem is built in a qemu-system binary.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Ah, very similar to something I did recently.
Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
>  include/hw/acpi/acpi.h | 3 +++
>  hw/acpi/acpi-stub.c    | 5 +++++
>  hw/acpi/core.c         | 5 +++++
>  3 files changed, 13 insertions(+)
>
> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> index e0e51e85b41..d1a4fa2af84 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -150,6 +150,9 @@ struct ACPIREGS {
>      Notifier wakeup;
>  };
>
> +/* Return whether ACPI subsystem is built in */
> +bool acpi_builtin(void);
> +
>  /* PM_TMR */
>  void acpi_pm_tmr_update(ACPIREGS *ar, bool enable);
>  void acpi_pm_tmr_calc_overflow_time(ACPIREGS *ar);
> diff --git a/hw/acpi/acpi-stub.c b/hw/acpi/acpi-stub.c
> index e268ce9b1a9..790bf509e5d 100644
> --- a/hw/acpi/acpi-stub.c
> +++ b/hw/acpi/acpi-stub.c
> @@ -25,3 +25,8 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
>  {
>      g_assert_not_reached();
>  }
> +
> +bool acpi_builtin(void)
> +{
> +    return false;
> +}
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 870391ed7c8..58f8964e130 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -78,6 +78,11 @@ static void acpi_register_config(void)
>
>  opts_init(acpi_register_config);
>
> +bool acpi_builtin(void)
> +{
> +    return true;
> +}
> +
>  static int acpi_checksum(const uint8_t *data, int len)
>  {
>      int sum, i;
> --
> 2.47.1
>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin()
  2025-03-07 15:15 ` [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
@ 2025-03-07 15:44   ` Ani Sinha
  0 siblings, 0 replies; 20+ messages in thread
From: Ani Sinha @ 2025-03-07 15:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Alex Bennée, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, David Hildenbrand,
	Igor Mammedov, Thomas Huth

On Fri, Mar 7, 2025 at 8:46 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Replace the compile-time CONFIG_ACPI check by a runtime one.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
>  hw/i386/fw_cfg.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
> index d08aefa0291..a7f1b60b98c 100644
> --- a/hw/i386/fw_cfg.c
> +++ b/hw/i386/fw_cfg.c
> @@ -145,10 +145,10 @@ FWCfgState *fw_cfg_arch_create(MachineState *ms,
>       */
>      fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, apic_id_limit);
>      fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, ms->ram_size);
> -#ifdef CONFIG_ACPI
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
> -                     acpi_tables, acpi_tables_len);
> -#endif
> +    if (acpi_builtin()) {
> +        fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
> +                         acpi_tables, acpi_tables_len);
> +    }
>      fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, 1);
>
>      fw_cfg_add_bytes(fw_cfg, FW_CFG_HPET, &hpet_fw_cfg, sizeof(hpet_fw_cfg));
> --
> 2.47.1
>



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 15:15 ` [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime Philippe Mathieu-Daudé
@ 2025-03-07 16:38   ` Alex Bennée
  2025-03-07 16:49     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 20+ messages in thread
From: Alex Bennée @ 2025-03-07 16:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini, Michael S. Tsirkin,
	Richard Henderson, David Hildenbrand, Igor Mammedov, Thomas Huth

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Use qemu_arch_available() to check at runtime if a target
> architecture is built in.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/virtio/virtio-mem.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index 5f57eccbb66..8c40042108c 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -15,6 +15,7 @@
>  #include "qemu/cutils.h"
>  #include "qemu/error-report.h"
>  #include "qemu/units.h"
> +#include "system/arch_init.h"
>  #include "system/numa.h"
>  #include "system/system.h"
>  #include "system/reset.h"
> @@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>   * necessary (as the section size can change). But it's more likely that the
>   * section size will rather get smaller and not bigger over time.
>   */
> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
> -#elif defined(TARGET_ARM)
> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
> -#else
> -#error VIRTIO_MEM_USABLE_EXTENT not defined
> -#endif
> +static uint64_t virtio_mem_usable_extent_size(void)
> +{
> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
> +        return 2 * 128 * MiB;
> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
> +        return 2 * 512 * MiB;
> +    } else {
> +        g_assert_not_reached();
> +    }
> +}

What happens if/when we have multiple arches available? Won't we want to
know which CPU the virtio-mem device is attached to or do we take the
minimal value over the whole system?

>  
>  static bool virtio_mem_is_busy(void)
>  {
> @@ -721,7 +725,7 @@ static void virtio_mem_resize_usable_region(VirtIOMEM *vmem,
>                                              bool can_shrink)
>  {
>      uint64_t newsize = MIN(memory_region_size(&vmem->memdev->mr),
> -                           requested_size + VIRTIO_MEM_USABLE_EXTENT);
> +                           requested_size + virtio_mem_usable_extent_size());
>  
>      /* The usable region size always has to be multiples of the block size. */
>      newsize = QEMU_ALIGN_UP(newsize, vmem->block_size);

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 16:38   ` Alex Bennée
@ 2025-03-07 16:49     ` Philippe Mathieu-Daudé
  2025-03-07 17:52       ` David Hildenbrand
                         ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 16:49 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini, Michael S. Tsirkin,
	Richard Henderson, David Hildenbrand, Igor Mammedov, Thomas Huth

On 7/3/25 17:38, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Use qemu_arch_available() to check at runtime if a target
>> architecture is built in.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/virtio/virtio-mem.c | 20 ++++++++++++--------
>>   1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>> index 5f57eccbb66..8c40042108c 100644
>> --- a/hw/virtio/virtio-mem.c
>> +++ b/hw/virtio/virtio-mem.c
>> @@ -15,6 +15,7 @@
>>   #include "qemu/cutils.h"
>>   #include "qemu/error-report.h"
>>   #include "qemu/units.h"
>> +#include "system/arch_init.h"
>>   #include "system/numa.h"
>>   #include "system/system.h"
>>   #include "system/reset.h"
>> @@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>    * necessary (as the section size can change). But it's more likely that the
>>    * section size will rather get smaller and not bigger over time.
>>    */
>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>> -#elif defined(TARGET_ARM)
>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>> -#else
>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>> -#endif
>> +static uint64_t virtio_mem_usable_extent_size(void)
>> +{
>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>> +        return 2 * 128 * MiB;
>> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
>> +        return 2 * 512 * MiB;
>> +    } else {
>> +        g_assert_not_reached();
>> +    }
>> +}
> 
> What happens if/when we have multiple arches available? Won't we want to
> know which CPU the virtio-mem device is attached to or do we take the
> minimal value over the whole system?

"per attached vcpu" is how I was previously considering this problem,
but IIUC from the discussions with Pierrick, we should consider single
binary as a first step before heterogeneous emulation.

If you think the minimal value is good enough, then that'd be my
preferred choice, as the simplest to implement.

> 
>>   
>>   static bool virtio_mem_is_busy(void)
>>   {
>> @@ -721,7 +725,7 @@ static void virtio_mem_resize_usable_region(VirtIOMEM *vmem,
>>                                               bool can_shrink)
>>   {
>>       uint64_t newsize = MIN(memory_region_size(&vmem->memdev->mr),
>> -                           requested_size + VIRTIO_MEM_USABLE_EXTENT);
>> +                           requested_size + virtio_mem_usable_extent_size());
>>   
>>       /* The usable region size always has to be multiples of the block size. */
>>       newsize = QEMU_ALIGN_UP(newsize, vmem->block_size);
> 



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  2025-03-07 15:15 ` [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
@ 2025-03-07 17:48   ` David Hildenbrand
  2025-03-07 19:21   ` Pierrick Bouvier
  1 sibling, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2025-03-07 17:48 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, Igor Mammedov, Thomas Huth

On 07.03.25 16:15, Philippe Mathieu-Daudé wrote:
> Rather than checking ACPI availability at compile time by
> checking the CONFIG_ACPI definition from CONFIG_DEVICES,
> check at runtime via acpi_builtin().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 16:49     ` Philippe Mathieu-Daudé
@ 2025-03-07 17:52       ` David Hildenbrand
  2025-03-07 19:08       ` Richard Henderson
  2025-03-07 19:28       ` Pierrick Bouvier
  2 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2025-03-07 17:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Alex Bennée
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini, Michael S. Tsirkin,
	Richard Henderson, Igor Mammedov, Thomas Huth

On 07.03.25 17:49, Philippe Mathieu-Daudé wrote:
> On 7/3/25 17:38, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> Use qemu_arch_available() to check at runtime if a target
>>> architecture is built in.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/virtio/virtio-mem.c | 20 ++++++++++++--------
>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>>> index 5f57eccbb66..8c40042108c 100644
>>> --- a/hw/virtio/virtio-mem.c
>>> +++ b/hw/virtio/virtio-mem.c
>>> @@ -15,6 +15,7 @@
>>>    #include "qemu/cutils.h"
>>>    #include "qemu/error-report.h"
>>>    #include "qemu/units.h"
>>> +#include "system/arch_init.h"
>>>    #include "system/numa.h"
>>>    #include "system/system.h"
>>>    #include "system/reset.h"
>>> @@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>>     * necessary (as the section size can change). But it's more likely that the
>>>     * section size will rather get smaller and not bigger over time.
>>>     */
>>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>>> -#elif defined(TARGET_ARM)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>>> -#else
>>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>>> -#endif
>>> +static uint64_t virtio_mem_usable_extent_size(void)
>>> +{
>>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>>> +        return 2 * 128 * MiB;
>>> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
>>> +        return 2 * 512 * MiB;
>>> +    } else {
>>> +        g_assert_not_reached();
>>> +    }
>>> +}
>>
>> What happens if/when we have multiple arches available? Won't we want to
>> know which CPU the virtio-mem device is attached to or do we take the
>> minimal value over the whole system?

We should take the maximum value here, not the minimum.

> 
> "per attached vcpu" is how I was previously considering this problem,
> but IIUC from the discussions with Pierrick, we should consider single
> binary as a first step before heterogeneous emulation.

It would be related to the machine/vcpu, yes.

Taking the maximum over all available arches is easier; it's a pure 
optimization.

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS to runtime
  2025-03-07 15:15 ` [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS " Philippe Mathieu-Daudé
@ 2025-03-07 17:54   ` David Hildenbrand
  0 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2025-03-07 17:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, Igor Mammedov, Thomas Huth

On 07.03.25 16:15, Philippe Mathieu-Daudé wrote:
> Use qemu_arch_available() to check at runtime if a target
> architecture is built in.
> Register virtio_mem_legacy_guests_properties[] at runtime.
> Code churn in virtio_mem_device_realize() is due to re-indentation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

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

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 7/7] hw/virtio: Compile virtio-mem.c once
  2025-03-07 15:15 ` [PATCH 7/7] hw/virtio: Compile virtio-mem.c once Philippe Mathieu-Daudé
@ 2025-03-07 17:55   ` David Hildenbrand
  0 siblings, 0 replies; 20+ messages in thread
From: David Hildenbrand @ 2025-03-07 17:55 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Pierrick Bouvier, Paolo Bonzini,
	Michael S. Tsirkin, Richard Henderson, Igor Mammedov, Thomas Huth

On 07.03.25 16:15, Philippe Mathieu-Daudé wrote:
> Remove unused "exec/ram_addr.h" header. This file doesn't
> use any target specific definitions anymore, compile it
> once by moving it to system_virtio_ss[].
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

-- 
Cheers,

David / dhildenb



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 16:49     ` Philippe Mathieu-Daudé
  2025-03-07 17:52       ` David Hildenbrand
@ 2025-03-07 19:08       ` Richard Henderson
  2025-03-07 19:28       ` Pierrick Bouvier
  2 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2025-03-07 19:08 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Alex Bennée
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini, Michael S. Tsirkin,
	David Hildenbrand, Igor Mammedov, Thomas Huth

On 3/7/25 08:49, Philippe Mathieu-Daudé wrote:
>>> @@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>>    * necessary (as the section size can change). But it's more likely that the
>>>    * section size will rather get smaller and not bigger over time.
>>>    */
>>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>>> -#elif defined(TARGET_ARM)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>>> -#else
>>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>>> -#endif
>>> +static uint64_t virtio_mem_usable_extent_size(void)
>>> +{
>>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>>> +        return 2 * 128 * MiB;
>>> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
>>> +        return 2 * 512 * MiB;
>>> +    } else {
>>> +        g_assert_not_reached();
>>> +    }
>>> +}
>>
>> What happens if/when we have multiple arches available? Won't we want to
>> know which CPU the virtio-mem device is attached to or do we take the
>> minimal value over the whole system?
> 
> "per attached vcpu" is how I was previously considering this problem,
> but IIUC from the discussions with Pierrick, we should consider single
> binary as a first step before heterogeneous emulation.
> 
> If you think the minimal value is good enough, then that'd be my
> preferred choice, as the simplest to implement.

Indeed, you have already done so; see above.

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


r~


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include
  2025-03-07 15:15 ` [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
  2025-03-07 17:48   ` David Hildenbrand
@ 2025-03-07 19:21   ` Pierrick Bouvier
  1 sibling, 0 replies; 20+ messages in thread
From: Pierrick Bouvier @ 2025-03-07 19:21 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Alex Bennée, Ani Sinha, Daniel P. Berrangé,
	Marcel Apfelbaum, Paolo Bonzini, Michael S. Tsirkin,
	Richard Henderson, David Hildenbrand, Igor Mammedov, Thomas Huth

On 3/7/25 07:15, Philippe Mathieu-Daudé wrote:
> Rather than checking ACPI availability at compile time by
> checking the CONFIG_ACPI definition from CONFIG_DEVICES,
> check at runtime via acpi_builtin().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/virtio/virtio-mem.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
> index 7b140add765..5f57eccbb66 100644
> --- a/hw/virtio/virtio-mem.c
> +++ b/hw/virtio/virtio-mem.c
> @@ -28,7 +28,7 @@
>   #include "migration/misc.h"
>   #include "hw/boards.h"
>   #include "hw/qdev-properties.h"
> -#include CONFIG_DEVICES
> +#include "hw/acpi/acpi.h"
>   #include "trace.h"
>   
>   static const VMStateDescription vmstate_virtio_mem_device_early;
> @@ -883,10 +883,8 @@ static uint64_t virtio_mem_get_features(VirtIODevice *vdev, uint64_t features,
>       MachineState *ms = MACHINE(qdev_get_machine());
>       VirtIOMEM *vmem = VIRTIO_MEM(vdev);
>   
> -    if (ms->numa_state) {
> -#if defined(CONFIG_ACPI)
> +    if (ms->numa_state && acpi_builtin()) {
>           virtio_add_feature(&features, VIRTIO_MEM_F_ACPI_PXM);
> -#endif
>       }
>       assert(vmem->unplugged_inaccessible != ON_OFF_AUTO_AUTO);
>       if (vmem->unplugged_inaccessible == ON_OFF_AUTO_ON) {

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 16:49     ` Philippe Mathieu-Daudé
  2025-03-07 17:52       ` David Hildenbrand
  2025-03-07 19:08       ` Richard Henderson
@ 2025-03-07 19:28       ` Pierrick Bouvier
  2025-03-07 21:00         ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 20+ messages in thread
From: Pierrick Bouvier @ 2025-03-07 19:28 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Alex Bennée
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Paolo Bonzini, Michael S. Tsirkin, Richard Henderson,
	David Hildenbrand, Igor Mammedov, Thomas Huth

On 3/7/25 08:49, Philippe Mathieu-Daudé wrote:
> On 7/3/25 17:38, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> Use qemu_arch_available() to check at runtime if a target
>>> architecture is built in.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>    hw/virtio/virtio-mem.c | 20 ++++++++++++--------
>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>>> index 5f57eccbb66..8c40042108c 100644
>>> --- a/hw/virtio/virtio-mem.c
>>> +++ b/hw/virtio/virtio-mem.c
>>> @@ -15,6 +15,7 @@
>>>    #include "qemu/cutils.h"
>>>    #include "qemu/error-report.h"
>>>    #include "qemu/units.h"
>>> +#include "system/arch_init.h"
>>>    #include "system/numa.h"
>>>    #include "system/system.h"
>>>    #include "system/reset.h"
>>> @@ -170,13 +171,16 @@ static bool virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>>     * necessary (as the section size can change). But it's more likely that the
>>>     * section size will rather get smaller and not bigger over time.
>>>     */
>>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || defined(TARGET_S390X)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>>> -#elif defined(TARGET_ARM)
>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>>> -#else
>>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>>> -#endif
>>> +static uint64_t virtio_mem_usable_extent_size(void)
>>> +{
>>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>>> +        return 2 * 128 * MiB;
>>> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
>>> +        return 2 * 512 * MiB;
>>> +    } else {
>>> +        g_assert_not_reached();
>>> +    }
>>> +}
>>
>> What happens if/when we have multiple arches available? Won't we want to
>> know which CPU the virtio-mem device is attached to or do we take the
>> minimal value over the whole system?
> 
> "per attached vcpu" is how I was previously considering this problem,
> but IIUC from the discussions with Pierrick, we should consider single
> binary as a first step before heterogeneous emulation.
> 

I think it's safe to assume only a single arch is enable for now, in the 
context of the single binary.
A thing we could do is introduce qemu_arch_heterogenenous_emulation(), 
that returns false for now. And assert this in places that will need to 
be changed. So spots that will need refactoring will already be flagged 
in the codebase.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

> If you think the minimal value is good enough, then that'd be my
> preferred choice, as the simplest to implement.
> 
>>
>>>    
>>>    static bool virtio_mem_is_busy(void)
>>>    {
>>> @@ -721,7 +725,7 @@ static void virtio_mem_resize_usable_region(VirtIOMEM *vmem,
>>>                                                bool can_shrink)
>>>    {
>>>        uint64_t newsize = MIN(memory_region_size(&vmem->memdev->mr),
>>> -                           requested_size + VIRTIO_MEM_USABLE_EXTENT);
>>> +                           requested_size + virtio_mem_usable_extent_size());
>>>    
>>>        /* The usable region size always has to be multiples of the block size. */
>>>        newsize = QEMU_ALIGN_UP(newsize, vmem->block_size);
>>
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime
  2025-03-07 19:28       ` Pierrick Bouvier
@ 2025-03-07 21:00         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 21:00 UTC (permalink / raw)
  To: Pierrick Bouvier, Alex Bennée
  Cc: qemu-devel, Ani Sinha, Daniel P. Berrangé, Marcel Apfelbaum,
	Paolo Bonzini, Michael S. Tsirkin, Richard Henderson,
	David Hildenbrand, Igor Mammedov, Thomas Huth

On 7/3/25 20:28, Pierrick Bouvier wrote:
> On 3/7/25 08:49, Philippe Mathieu-Daudé wrote:
>> On 7/3/25 17:38, Alex Bennée wrote:
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> Use qemu_arch_available() to check at runtime if a target
>>>> architecture is built in.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    hw/virtio/virtio-mem.c | 20 ++++++++++++--------
>>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
>>>> index 5f57eccbb66..8c40042108c 100644
>>>> --- a/hw/virtio/virtio-mem.c
>>>> +++ b/hw/virtio/virtio-mem.c
>>>> @@ -15,6 +15,7 @@
>>>>    #include "qemu/cutils.h"
>>>>    #include "qemu/error-report.h"
>>>>    #include "qemu/units.h"
>>>> +#include "system/arch_init.h"
>>>>    #include "system/numa.h"
>>>>    #include "system/system.h"
>>>>    #include "system/reset.h"
>>>> @@ -170,13 +171,16 @@ static bool 
>>>> virtio_mem_has_shared_zeropage(RAMBlock *rb)
>>>>     * necessary (as the section size can change). But it's more 
>>>> likely that the
>>>>     * section size will rather get smaller and not bigger over time.
>>>>     */
>>>> -#if defined(TARGET_X86_64) || defined(TARGET_I386) || 
>>>> defined(TARGET_S390X)
>>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (128 * MiB))
>>>> -#elif defined(TARGET_ARM)
>>>> -#define VIRTIO_MEM_USABLE_EXTENT (2 * (512 * MiB))
>>>> -#else
>>>> -#error VIRTIO_MEM_USABLE_EXTENT not defined
>>>> -#endif
>>>> +static uint64_t virtio_mem_usable_extent_size(void)
>>>> +{
>>>> +    if (qemu_arch_available(QEMU_ARCH_I386 | QEMU_ARCH_S390X)) {
>>>> +        return 2 * 128 * MiB;
>>>> +    } else if (qemu_arch_available(QEMU_ARCH_ARM)) {
>>>> +        return 2 * 512 * MiB;
>>>> +    } else {
>>>> +        g_assert_not_reached();
>>>> +    }
>>>> +}
>>>
>>> What happens if/when we have multiple arches available? Won't we want to
>>> know which CPU the virtio-mem device is attached to or do we take the
>>> minimal value over the whole system?
>>
>> "per attached vcpu" is how I was previously considering this problem,
>> but IIUC from the discussions with Pierrick, we should consider single
>> binary as a first step before heterogeneous emulation.
>>
> 
> I think it's safe to assume only a single arch is enable for now, in the 
> context of the single binary.
> A thing we could do is introduce qemu_arch_heterogenenous_emulation(), 
> that returns false for now. And assert this in places that will need to 
> be changed. So spots that will need refactoring will already be flagged 
> in the codebase.

Yes, after some discussion with Markus I started a branch adding such
macro. I didn't posted it so far waiting to show more realistic changes
w.r.t. heterogeneous emulation, to not add code that could bitrot.
Since we might be at a pivot position, I'll consider rebase and post it.

> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Thanks!


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2025-03-07 21:01 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07 15:15 [PATCH 0/7] hw/virtio: Build virtio-mem.c once Philippe Mathieu-Daudé
2025-03-07 15:15 ` [PATCH 1/7] system: Replace arch_type global by qemu_arch_available() helper Philippe Mathieu-Daudé
2025-03-07 15:15 ` [PATCH 2/7] hw/acpi: Introduce acpi_builtin() helper Philippe Mathieu-Daudé
2025-03-07 15:42   ` Ani Sinha
2025-03-07 15:15 ` [PATCH 3/7] hw/i386/fw_cfg: Check ACPI availability with acpi_builtin() Philippe Mathieu-Daudé
2025-03-07 15:44   ` Ani Sinha
2025-03-07 15:15 ` [PATCH 4/7] hw/virtio/virtio-mem: Remove CONFIG_DEVICES include Philippe Mathieu-Daudé
2025-03-07 17:48   ` David Hildenbrand
2025-03-07 19:21   ` Pierrick Bouvier
2025-03-07 15:15 ` [PATCH 5/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_USABLE_EXTENT to runtime Philippe Mathieu-Daudé
2025-03-07 16:38   ` Alex Bennée
2025-03-07 16:49     ` Philippe Mathieu-Daudé
2025-03-07 17:52       ` David Hildenbrand
2025-03-07 19:08       ` Richard Henderson
2025-03-07 19:28       ` Pierrick Bouvier
2025-03-07 21:00         ` Philippe Mathieu-Daudé
2025-03-07 15:15 ` [PATCH 6/7] hw/virtio/virtio-mem: Convert VIRTIO_MEM_HAS_LEGACY_GUESTS " Philippe Mathieu-Daudé
2025-03-07 17:54   ` David Hildenbrand
2025-03-07 15:15 ` [PATCH 7/7] hw/virtio: Compile virtio-mem.c once Philippe Mathieu-Daudé
2025-03-07 17:55   ` David Hildenbrand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).