* [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).