* [PULL 01/24] rust: Kconfig: Factor out whether PL011 is Rust or C
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 02/24] rust: Kconfig: Factor out whether HPET " Paolo Bonzini
` (23 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Tanish Desai, Philippe Mathieu-Daudé
From: Peter Maydell <peter.maydell@linaro.org>
Currently every board that uses the PL011 duplicates the logic that
selects the Rust implementation if Rust was enabled and the C
implementation if it does not. Factor this out into a separate
Kconfig stanza, so that boards can go back to simply doing "select
PL011" and get whichever implementation is correct for the build.
This fixes a compilation failure if CONFIG_VMAPPLE is enabled
in a Rust build, because hw/vmapple/Kconfig didn't have the
"pick the Rust PL011 if Rust is enabled" logic in it.
Fixes: 59f4d65584bd33 ("hw/vmapple/vmapple: Add vmapple machine type")
Reported-by: Tanish Desai <tanishdesai37@gmail.com>
Analyzed-by: Tanish Desai <tanishdesai37@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250319193110.1565578-2-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/arm/Kconfig | 30 ++++++++++--------------------
hw/char/Kconfig | 6 ++++++
hw/char/meson.build | 2 +-
3 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 15200a2d7e7..a55b44d7bde 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -21,8 +21,7 @@ config ARM_VIRT
select PCI_EXPRESS
select PCI_EXPRESS_GENERIC_BRIDGE
select PFLASH_CFI01
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL031 # RTC
select PL061 # GPIO
select GPIO_PWR
@@ -75,8 +74,7 @@ config HIGHBANK
select AHCI_SYSBUS
select ARM_TIMER # sp804
select ARM_V7M
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL022 # SPI
select PL031 # RTC
select PL061 # GPIO
@@ -89,8 +87,7 @@ config INTEGRATOR
depends on TCG && ARM
select ARM_TIMER
select INTEGRATOR_DEBUG
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL031 # RTC
select PL041 # audio
select PL050 # keyboard/mouse
@@ -108,8 +105,7 @@ config MUSCA
default y
depends on TCG && ARM
select ARMSSE
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL031
select SPLIT_IRQ
select UNIMP
@@ -173,8 +169,7 @@ config REALVIEW
select WM8750 # audio codec
select LSI_SCSI_PCI
select PCI
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL031 # RTC
select PL041 # audio codec
select PL050 # keyboard/mouse
@@ -199,8 +194,7 @@ config SBSA_REF
select PCI_EXPRESS
select PCI_EXPRESS_GENERIC_BRIDGE
select PFLASH_CFI01
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL031 # RTC
select PL061 # GPIO
select USB_XHCI_SYSBUS
@@ -224,8 +218,7 @@ config STELLARIS
select ARM_V7M
select CMSDK_APB_WATCHDOG
select I2C
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL022 # SPI
select PL061 # GPIO
select SSD0303 # OLED display
@@ -285,8 +278,7 @@ config VEXPRESS
select ARM_TIMER # sp804
select LAN9118
select PFLASH_CFI01
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select PL041 # audio codec
select PL181 # display
select REALVIEW
@@ -371,8 +363,7 @@ config RASPI
default y
depends on TCG && ARM
select FRAMEBUFFER
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select SDHCI
select USB_DWC2
select BCM2835_SPI
@@ -448,8 +439,7 @@ config XLNX_VERSAL
select ARM_GIC
select CPU_CLUSTER
select DEVICE_TREE
- select PL011 if !HAVE_RUST # UART
- select X_PL011_RUST if HAVE_RUST # UART
+ select PL011 # UART
select CADENCE
select VIRTIO_MMIO
select UNIMP
diff --git a/hw/char/Kconfig b/hw/char/Kconfig
index 3f702565e67..9d517f3e287 100644
--- a/hw/char/Kconfig
+++ b/hw/char/Kconfig
@@ -11,6 +11,12 @@ config PARALLEL
config PL011
bool
+ # The PL011 has both a Rust and a C implementation
+ select PL011_C if !HAVE_RUST
+ select X_PL011_RUST if HAVE_RUST
+
+config PL011_C
+ bool
config SERIAL
bool
diff --git a/hw/char/meson.build b/hw/char/meson.build
index 86ee808cae7..4e439da8b9e 100644
--- a/hw/char/meson.build
+++ b/hw/char/meson.build
@@ -9,7 +9,7 @@ system_ss.add(when: 'CONFIG_ISA_BUS', if_true: files('parallel-isa.c'))
system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugcon.c'))
system_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_uart.c'))
system_ss.add(when: 'CONFIG_PARALLEL', if_true: files('parallel.c'))
-system_ss.add(when: 'CONFIG_PL011', if_true: files('pl011.c'))
+system_ss.add(when: 'CONFIG_PL011_C', if_true: files('pl011.c'))
system_ss.add(when: 'CONFIG_SCLPCONSOLE', if_true: files('sclpconsole.c', 'sclpconsole-lm.c'))
system_ss.add(when: 'CONFIG_SERIAL', if_true: files('serial.c'))
system_ss.add(when: 'CONFIG_SERIAL_ISA', if_true: files('serial-isa.c'))
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 02/24] rust: Kconfig: Factor out whether HPET is Rust or C
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
2025-03-21 18:35 ` [PULL 01/24] rust: Kconfig: Factor out whether PL011 is Rust or C Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 03/24] qdev, rust/hpet: fix type of HPET "timers" property Paolo Bonzini
` (22 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell
From: Peter Maydell <peter.maydell@linaro.org>
Currently we require everywhere that wants to know if there
is an HPET device to check for "CONFIG_HPET || CONFIG_X_HPET_RUST".
Factor out whether the HPET device is Rust or C into a separate
Kconfig stanza, so that CONFIG_HPET means "there is an HPET",
and whether this has pulled in CONFIG_X_HPET_RUST or CONFIG_HPET_C
is something the rest of QEMU can ignore.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20250319193110.1565578-3-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configs/devices/i386-softmmu/default.mak | 1 -
hw/i386/fw_cfg.c | 2 +-
hw/i386/pc.c | 2 +-
hw/timer/Kconfig | 8 +++++++-
hw/timer/meson.build | 2 +-
rust/hw/timer/Kconfig | 1 -
tests/qtest/meson.build | 3 +--
7 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/configs/devices/i386-softmmu/default.mak b/configs/devices/i386-softmmu/default.mak
index 9ef343cace0..4faf2f0315e 100644
--- a/configs/devices/i386-softmmu/default.mak
+++ b/configs/devices/i386-softmmu/default.mak
@@ -6,7 +6,6 @@
#CONFIG_APPLESMC=n
#CONFIG_FDC=n
#CONFIG_HPET=n
-#CONFIG_X_HPET_RUST=n
#CONFIG_HYPERV=n
#CONFIG_ISA_DEBUG=n
#CONFIG_ISA_IPMI_BT=n
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index a7f1b60b98c..5c0bcd5f8a9 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -26,7 +26,7 @@
#include CONFIG_DEVICES
#include "target/i386/cpu.h"
-#if !defined(CONFIG_HPET) && !defined(CONFIG_X_HPET_RUST)
+#if !defined(CONFIG_HPET)
struct hpet_fw_config hpet_fw_cfg = {.count = UINT8_MAX};
#endif
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 63a96cd23f8..01d0581f62a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1704,7 +1704,7 @@ static void pc_machine_initfn(Object *obj)
pcms->sata_enabled = true;
pcms->i8042_enabled = true;
pcms->max_fw_size = 8 * MiB;
-#if defined(CONFIG_HPET) || defined(CONFIG_X_HPET_RUST)
+#if defined(CONFIG_HPET)
pcms->hpet_enabled = true;
#endif
pcms->fd_bootchk = true;
diff --git a/hw/timer/Kconfig b/hw/timer/Kconfig
index 9ac00845340..b3d823ce2c3 100644
--- a/hw/timer/Kconfig
+++ b/hw/timer/Kconfig
@@ -11,7 +11,13 @@ config A9_GTIMER
config HPET
bool
- default y if PC && !HAVE_RUST
+ default y if PC
+ # The HPET has both a Rust and a C implementation
+ select HPET_C if !HAVE_RUST
+ select X_HPET_RUST if HAVE_RUST
+
+config HPET_C
+ bool
config I8254
bool
diff --git a/hw/timer/meson.build b/hw/timer/meson.build
index f5f9eed2d0a..178321c029c 100644
--- a/hw/timer/meson.build
+++ b/hw/timer/meson.build
@@ -13,7 +13,7 @@ system_ss.add(when: 'CONFIG_DIGIC', if_true: files('digic-timer.c'))
system_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4210_mct.c'))
system_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4210_pwm.c'))
system_ss.add(when: 'CONFIG_GRLIB', if_true: files('grlib_gptimer.c'))
-system_ss.add(when: 'CONFIG_HPET', if_true: files('hpet.c'))
+system_ss.add(when: 'CONFIG_HPET_C', if_true: files('hpet.c'))
system_ss.add(when: 'CONFIG_I8254', if_true: files('i8254_common.c', 'i8254.c'))
system_ss.add(when: 'CONFIG_IMX', if_true: files('imx_epit.c'))
system_ss.add(when: 'CONFIG_IMX', if_true: files('imx_gpt.c'))
diff --git a/rust/hw/timer/Kconfig b/rust/hw/timer/Kconfig
index 42e421317a5..afd98033503 100644
--- a/rust/hw/timer/Kconfig
+++ b/rust/hw/timer/Kconfig
@@ -1,3 +1,2 @@
config X_HPET_RUST
bool
- default y if PC && HAVE_RUST
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 5a8c1f102c2..3136d15e0f8 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -103,8 +103,7 @@ qtests_i386 = \
config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \
slirp.found() ? ['virtio-net-failover'] : []) + \
(unpack_edk2_blobs and \
- (config_all_devices.has_key('CONFIG_HPET') or \
- config_all_devices.has_key('CONFIG_X_HPET_RUST')) and \
+ config_all_devices.has_key('CONFIG_HPET') and \
config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \
qtests_pci + \
qtests_cxl + \
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 03/24] qdev, rust/hpet: fix type of HPET "timers" property
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
2025-03-21 18:35 ` [PULL 01/24] rust: Kconfig: Factor out whether PL011 is Rust or C Paolo Bonzini
2025-03-21 18:35 ` [PULL 02/24] rust: Kconfig: Factor out whether HPET " Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 04/24] rust: assertions: add static_assert Paolo Bonzini
` (21 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/qdev-properties.h | 1 +
hw/core/qdev-properties.c | 37 ++++++++++++++++++++++++++++++++++
rust/hw/timer/hpet/src/hpet.rs | 6 +++---
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 15fcec5260c..2c99856caac 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -52,6 +52,7 @@ extern const PropertyInfo qdev_prop_bool;
extern const PropertyInfo qdev_prop_uint8;
extern const PropertyInfo qdev_prop_uint16;
extern const PropertyInfo qdev_prop_uint32;
+extern const PropertyInfo qdev_prop_usize;
extern const PropertyInfo qdev_prop_int32;
extern const PropertyInfo qdev_prop_uint64;
extern const PropertyInfo qdev_prop_uint64_checkmask;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index c04df3b337a..147b3ffd16e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -442,6 +442,43 @@ const PropertyInfo qdev_prop_uint64_checkmask = {
.set = set_uint64_checkmask,
};
+/* --- pointer-size integer --- */
+
+static void get_usize(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ const Property *prop = opaque;
+
+#if HOST_LONG_BITS == 32
+ uint32_t *ptr = object_field_prop_ptr(obj, prop);
+ visit_type_uint32(v, name, ptr, errp);
+#else
+ uint64_t *ptr = object_field_prop_ptr(obj, prop);
+ visit_type_uint64(v, name, ptr, errp);
+#endif
+}
+
+static void set_usize(Object *obj, Visitor *v, const char *name, void *opaque,
+ Error **errp)
+{
+ const Property *prop = opaque;
+
+#if HOST_LONG_BITS == 32
+ uint32_t *ptr = object_field_prop_ptr(obj, prop);
+ visit_type_uint32(v, name, ptr, errp);
+#else
+ uint64_t *ptr = object_field_prop_ptr(obj, prop);
+ visit_type_uint64(v, name, ptr, errp);
+#endif
+}
+
+const PropertyInfo qdev_prop_usize = {
+ .type = "usize",
+ .get = get_usize,
+ .set = set_usize,
+ .set_default_value = qdev_propinfo_set_default_value_uint,
+};
+
/* --- string --- */
static void release_string(Object *obj, const char *name, void *opaque)
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 20e0afdfca8..63c1971f0b5 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -12,7 +12,7 @@
use qemu_api::{
bindings::{
address_space_memory, address_space_stl_le, qdev_prop_bit, qdev_prop_bool,
- qdev_prop_uint32, qdev_prop_uint8,
+ qdev_prop_uint32, qdev_prop_usize,
},
c_str,
cell::{BqlCell, BqlRefCell},
@@ -859,8 +859,8 @@ impl ObjectImpl for HPETState {
c_str!("timers"),
HPETState,
num_timers,
- unsafe { &qdev_prop_uint8 },
- u8,
+ unsafe { &qdev_prop_usize },
+ usize,
default = HPET_MIN_TIMERS
),
qemu_api::define_property!(
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 04/24] rust: assertions: add static_assert
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (2 preceding siblings ...)
2025-03-21 18:35 ` [PULL 03/24] qdev, rust/hpet: fix type of HPET "timers" property Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 05/24] hw/char/pl011: Pad PL011State struct to same size as Rust impl Paolo Bonzini
` (20 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu
Add a new assertion that is similar to "const { assert!(...) }" but can be used
outside functions and with older versions of Rust. A similar macro is found in
Linux, whereas the "static_assertions" crate has a const_assert macro that
produces worse error messages.
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20250321112523.1774131-2-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/assertions.rs | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index 104dec39774..bba38cfda11 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -120,3 +120,25 @@ macro_rules! assert_match {
);
};
}
+
+/// Assert at compile time that an expression is true. This is similar
+/// to `const { assert!(...); }` but it works outside functions, as well as
+/// on versions of Rust before 1.79.
+///
+/// # Examples
+///
+/// ```
+/// # use qemu_api::static_assert;
+/// static_assert!("abc".len() == 3);
+/// ```
+///
+/// ```compile_fail
+/// # use qemu_api::static_assert;
+/// static_assert!("abc".len() == 2); // does not compile
+/// ```
+#[macro_export]
+macro_rules! static_assert {
+ ($x:expr) => {
+ const _: () = assert!($x);
+ };
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 05/24] hw/char/pl011: Pad PL011State struct to same size as Rust impl
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (3 preceding siblings ...)
2025-03-21 18:35 ` [PULL 04/24] rust: assertions: add static_assert Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 06/24] rust: pl011: Check size of state struct at compile time Paolo Bonzini
` (19 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu
From: Peter Maydell <peter.maydell@linaro.org>
We have some users of the PL011 struct which embed it directly into
their own state structs. This means that the Rust version of the
device must have a state struct that is the same size or smaller
than the C struct.
In commit 9b642097d6b7 ("rust: pl011: switch to safe chardev operation")
the Rust PL011 state struct changed from having a bindings::CharBackend
to a chardev::CharBackend, which made it grow larger than the C
version. This results in an assertion at startup when QEMU was
built with Rust enabled:
$ qemu-system-arm -M raspi2b -display none
ERROR:../../qom/object.c:562:object_initialize_with_type: assertion
failed: (size >= type->instance_size)
The long-term better approach to this problem would be to move
our C device code patterns away from "embed a struct" and (back)
to "have a pointer to the device", so we can make the C PL011State
struct a private implementation detail rather than exposed to
its users.
For the short term, add a padding field at the end of the C struct
so it's big enough that the Rust state struct can fit.
Fixes: 9b642097d6b7 ("rust: pl011: switch to safe chardev operation")
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20250321112523.1774131-3-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/char/pl011.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index 4fcaf3d7d30..299ca9b18bb 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -52,6 +52,11 @@ struct PL011State {
Clock *clk;
bool migrate_clk;
const unsigned char *id;
+ /*
+ * Since some users embed this struct directly, we must
+ * ensure that the C struct is at least as big as the Rust one.
+ */
+ uint8_t padding_for_rust[16];
};
DeviceState *pl011_create(hwaddr addr, qemu_irq irq, Chardev *chr);
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 06/24] rust: pl011: Check size of state struct at compile time
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (4 preceding siblings ...)
2025-03-21 18:35 ` [PULL 05/24] hw/char/pl011: Pad PL011State struct to same size as Rust impl Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 07/24] load_aout: replace bswap_needed with big_endian Paolo Bonzini
` (18 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu
From: Peter Maydell <peter.maydell@linaro.org>
The PL011 device's C implementation exposes its PL011State struct to
users of the device, and one common usage pattern is to embed that
struct into the user's own state struct. (The internals of the
struct are technically visible to the C user of the device, but in
practice are treated as implementation details.)
This means that the Rust version of the state struct must not be
larger than the C version's struct; otherwise it will trip a runtime
assertion in object_initialize_type() when the C user attempts to
in-place initialize the type.
Add a compile-time assertion on the Rust side, so that if we
accidentally make the Rust device state larger we know immediately
that we need to expand the padding in the C version of the struct.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Link: https://lore.kernel.org/r/20250321112523.1774131-4-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/wrapper.h | 1 +
rust/hw/char/pl011/src/device.rs | 9 ++++++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/rust/wrapper.h b/rust/wrapper.h
index d927ad6799d..d4fec546571 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -65,3 +65,4 @@ typedef enum memory_order {
#include "exec/memattrs.h"
#include "qemu/timer.h"
#include "exec/address-spaces.h"
+#include "hw/char/pl011.h"
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index f137b49feaf..bf88e0b00a0 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -2,7 +2,7 @@
// Author(s): Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{ffi::CStr, ptr::addr_of_mut};
+use std::{ffi::CStr, mem::size_of, ptr::addr_of_mut};
use qemu_api::{
chardev::{CharBackend, Chardev, Event},
@@ -12,6 +12,7 @@
prelude::*,
qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
qom::{ObjectImpl, Owned, ParentField},
+ static_assert,
sysbus::{SysBusDevice, SysBusDeviceImpl},
vmstate::VMStateDescription,
};
@@ -124,6 +125,12 @@ pub struct PL011State {
pub migrate_clock: bool,
}
+// Some C users of this device embed its state struct into their own
+// structs, so the size of the Rust version must not be any larger
+// than the size of the C one. If this assert triggers you need to
+// expand the padding_for_rust[] array in the C PL011State struct.
+static_assert!(size_of::<PL011State>() <= size_of::<qemu_api::bindings::PL011State>());
+
qom_isa!(PL011State : SysBusDevice, DeviceState, Object);
#[repr(C)]
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 07/24] load_aout: replace bswap_needed with big_endian
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (5 preceding siblings ...)
2025-03-21 18:35 ` [PULL 06/24] rust: pl011: Check size of state struct at compile time Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 08/24] exec/cpu-all: remove BSWAP_NEEDED Paolo Bonzini
` (17 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel
Targets know whether they are big-endian more than they know if
the endianness is different from the host: the former is mostly
a constant, at least in machine creation code, while the latter
has to be computed with TARGET_BIG_ENDIAN != HOST_BIG_ENDIAN or
something like that.
load_aout, however, takes a "bswap_needed" argument. Replace
it with a "big_endian" argument; even though all users are
big-endian, it is cheap enough to keep the optional swapping
functionality even for little-endian boards.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/hw/loader.h | 2 +-
hw/core/loader.c | 4 ++--
hw/ppc/mac_newworld.c | 7 +------
hw/ppc/mac_oldworld.c | 7 +------
hw/sparc/sun4m.c | 9 +--------
hw/sparc64/sun4u.c | 9 +--------
6 files changed, 7 insertions(+), 31 deletions(-)
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 784a93d6c17..d280dc33e96 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -190,7 +190,7 @@ ssize_t load_elf(const char *filename,
void load_elf_hdr(const char *filename, void *hdr, bool *is64, Error **errp);
ssize_t load_aout(const char *filename, hwaddr addr, int max_sz,
- int bswap_needed, hwaddr target_page_size);
+ bool big_endian, hwaddr target_page_size);
#define LOAD_UIMAGE_LOADADDR_INVALID (-1)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index ce6ff1b52e3..2e35f0aa905 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -226,7 +226,7 @@ static void bswap_ahdr(struct exec *e)
ssize_t load_aout(const char *filename, hwaddr addr, int max_sz,
- int bswap_needed, hwaddr target_page_size)
+ bool big_endian, hwaddr target_page_size)
{
int fd;
ssize_t size, ret;
@@ -241,7 +241,7 @@ ssize_t load_aout(const char *filename, hwaddr addr, int max_sz,
if (size < 0)
goto fail;
- if (bswap_needed) {
+ if (big_endian != HOST_BIG_ENDIAN) {
bswap_ahdr(&e);
}
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index cb3dc3ab482..2d5309d6f55 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -197,11 +197,6 @@ static void ppc_core99_init(MachineState *machine)
}
if (machine->kernel_filename) {
- int bswap_needed = 0;
-
-#ifdef BSWAP_NEEDED
- bswap_needed = 1;
-#endif
kernel_base = KERNEL_LOAD_ADDR;
kernel_size = load_elf(machine->kernel_filename, NULL,
translate_kernel_address, NULL, NULL, NULL,
@@ -209,7 +204,7 @@ static void ppc_core99_init(MachineState *machine)
if (kernel_size < 0) {
kernel_size = load_aout(machine->kernel_filename, kernel_base,
machine->ram_size - kernel_base,
- bswap_needed, TARGET_PAGE_SIZE);
+ true, TARGET_PAGE_SIZE);
}
if (kernel_size < 0) {
kernel_size = load_image_targphys(machine->kernel_filename,
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 0dbcea035c3..b5814690f5a 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -153,11 +153,6 @@ static void ppc_heathrow_init(MachineState *machine)
}
if (machine->kernel_filename) {
- int bswap_needed = 0;
-
-#ifdef BSWAP_NEEDED
- bswap_needed = 1;
-#endif
kernel_base = KERNEL_LOAD_ADDR;
kernel_size = load_elf(machine->kernel_filename, NULL,
translate_kernel_address, NULL, NULL, NULL,
@@ -165,7 +160,7 @@ static void ppc_heathrow_init(MachineState *machine)
if (kernel_size < 0) {
kernel_size = load_aout(machine->kernel_filename, kernel_base,
machine->ram_size - kernel_base,
- bswap_needed, TARGET_PAGE_SIZE);
+ true, TARGET_PAGE_SIZE);
}
if (kernel_size < 0) {
kernel_size = load_image_targphys(machine->kernel_filename,
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index a48d3622c5a..5aaafb40dac 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -233,20 +233,13 @@ static unsigned long sun4m_load_kernel(const char *kernel_filename,
kernel_size = 0;
if (linux_boot) {
- int bswap_needed;
-
-#ifdef BSWAP_NEEDED
- bswap_needed = 1;
-#else
- bswap_needed = 0;
-#endif
kernel_size = load_elf(kernel_filename, NULL,
translate_kernel_address, NULL,
NULL, NULL, NULL, NULL,
ELFDATA2MSB, EM_SPARC, 0, 0);
if (kernel_size < 0)
kernel_size = load_aout(kernel_filename, KERNEL_LOAD_ADDR,
- RAM_size - KERNEL_LOAD_ADDR, bswap_needed,
+ RAM_size - KERNEL_LOAD_ADDR, true,
TARGET_PAGE_SIZE);
if (kernel_size < 0)
kernel_size = load_image_targphys(kernel_filename,
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 8ab5cf0461f..d3cb7270ff5 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -168,13 +168,6 @@ static uint64_t sun4u_load_kernel(const char *kernel_filename,
kernel_size = 0;
if (linux_boot) {
- int bswap_needed;
-
-#ifdef BSWAP_NEEDED
- bswap_needed = 1;
-#else
- bswap_needed = 0;
-#endif
kernel_size = load_elf(kernel_filename, NULL, NULL, NULL, kernel_entry,
kernel_addr, &kernel_top, NULL,
ELFDATA2MSB, EM_SPARCV9, 0, 0);
@@ -182,7 +175,7 @@ static uint64_t sun4u_load_kernel(const char *kernel_filename,
*kernel_addr = KERNEL_LOAD_ADDR;
*kernel_entry = KERNEL_LOAD_ADDR;
kernel_size = load_aout(kernel_filename, KERNEL_LOAD_ADDR,
- RAM_size - KERNEL_LOAD_ADDR, bswap_needed,
+ RAM_size - KERNEL_LOAD_ADDR, true,
TARGET_PAGE_SIZE);
}
if (kernel_size < 0) {
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 08/24] exec/cpu-all: remove BSWAP_NEEDED
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (6 preceding siblings ...)
2025-03-21 18:35 ` [PULL 07/24] load_aout: replace bswap_needed with big_endian Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 09/24] rust/vmstate: Remove unnecessary unsafe Paolo Bonzini
` (16 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Pierrick Bouvier
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
This identifier is poisoned, so it can't be used from common code
anyway. We replace all occurrences with its definition directly.
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Link: https://lore.kernel.org/r/20250320223002.2915728-2-pierrick.bouvier@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/exec/cpu-all.h | 12 ------------
include/exec/poison.h | 1 -
linux-user/syscall_defs.h | 2 +-
bsd-user/elfload.c | 6 +++---
linux-user/elfload.c | 8 ++++----
5 files changed, 8 insertions(+), 21 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 8cd6c00cf89..47b14446b8f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -26,18 +26,6 @@
#include "exec/tswap.h"
#include "hw/core/cpu.h"
-/* some important defines:
- *
- * HOST_BIG_ENDIAN : whether the host cpu is big endian and
- * otherwise little endian.
- *
- * TARGET_BIG_ENDIAN : same for the target cpu
- */
-
-#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
-#define BSWAP_NEEDED
-#endif
-
/* Target-endianness CPU memory access functions. These fit into the
* {ld,st}{type}{sign}{size}{endian}_p naming scheme described in bswap.h.
*/
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 8ed04b31083..2c151fd1e0c 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -37,7 +37,6 @@
#pragma GCC poison TARGET_NAME
#pragma GCC poison TARGET_SUPPORTS_MTTCG
#pragma GCC poison TARGET_BIG_ENDIAN
-#pragma GCC poison BSWAP_NEEDED
#pragma GCC poison TARGET_LONG_BITS
#pragma GCC poison TARGET_FMT_lx
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 86d773add75..5d227599924 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -462,7 +462,7 @@ typedef struct {
abi_ulong sig[TARGET_NSIG_WORDS];
} target_sigset_t;
-#ifdef BSWAP_NEEDED
+#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
static inline void tswap_sigset(target_sigset_t *d, const target_sigset_t *s)
{
int i;
diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 833fa3bd057..3bca0cc9ede 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -44,7 +44,7 @@ static inline void memcpy_fromfs(void *to, const void *from, unsigned long n)
memcpy(to, from, n);
}
-#ifdef BSWAP_NEEDED
+#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
static void bswap_ehdr(struct elfhdr *ehdr)
{
bswap16s(&ehdr->e_type); /* Object file type */
@@ -111,7 +111,7 @@ static void bswap_note(struct elf_note *en)
bswap32s(&en->n_type);
}
-#else /* ! BSWAP_NEEDED */
+#else
static void bswap_ehdr(struct elfhdr *ehdr) { }
static void bswap_phdr(struct elf_phdr *phdr, int phnum) { }
@@ -119,7 +119,7 @@ static void bswap_shdr(struct elf_shdr *shdr, int shnum) { }
static void bswap_sym(struct elf_sym *sym) { }
static void bswap_note(struct elf_note *en) { }
-#endif /* ! BSWAP_NEEDED */
+#endif /* HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN */
#include "elfcore.c"
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 8799e4ea27d..fa83d78667c 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2121,7 +2121,7 @@ static inline void memcpy_fromfs(void * to, const void * from, unsigned long n)
memcpy(to, from, n);
}
-#ifdef BSWAP_NEEDED
+#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
static void bswap_ehdr(struct elfhdr *ehdr)
{
bswap16s(&ehdr->e_type); /* Object file type */
@@ -3143,7 +3143,7 @@ static bool parse_elf_properties(const ImageSource *src,
* The contents of a valid PT_GNU_PROPERTY is a sequence of uint32_t.
* Swap most of them now, beyond the header and namesz.
*/
-#ifdef BSWAP_NEEDED
+#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
for (int i = 4; i < n / 4; i++) {
bswap32s(note.data + i);
}
@@ -3999,7 +3999,7 @@ struct target_elf_prpsinfo {
char pr_psargs[ELF_PRARGSZ]; /* initial part of arg list */
};
-#ifdef BSWAP_NEEDED
+#if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
static void bswap_prstatus(struct target_elf_prstatus *prstatus)
{
prstatus->pr_info.si_signo = tswap32(prstatus->pr_info.si_signo);
@@ -4038,7 +4038,7 @@ static void bswap_note(struct elf_note *en)
static inline void bswap_prstatus(struct target_elf_prstatus *p) { }
static inline void bswap_psinfo(struct target_elf_prpsinfo *p) {}
static inline void bswap_note(struct elf_note *en) { }
-#endif /* BSWAP_NEEDED */
+#endif /* HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN */
/*
* Calculate file (dump) size of given memory region.
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 09/24] rust/vmstate: Remove unnecessary unsafe
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (7 preceding siblings ...)
2025-03-21 18:35 ` [PULL 08/24] exec/cpu-all: remove BSWAP_NEEDED Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 10/24] rust/vmstate: Fix num_offset in vmstate macros Paolo Bonzini
` (15 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Remove the `unsafe` block of vmsd, because vmsd (passed to
vmstate_struct) is defined in Rust side now, and it doesn't need
`unsafe`.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-2-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index f0510ae769d..6698dfe7aeb 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
},
size: ::core::mem::size_of::<$type>(),
flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
- vmsd: unsafe { $vmsd },
+ vmsd: $vmsd,
..$crate::zeroable::Zeroable::ZERO $(
.with_varray_flag($crate::call_func_with_field!(
$crate::vmstate::vmstate_varray_flag,
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 10/24] rust/vmstate: Fix num_offset in vmstate macros
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (8 preceding siblings ...)
2025-03-21 18:35 ` [PULL 09/24] rust/vmstate: Remove unnecessary unsafe Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 11/24] rust/vmstate: Fix num field when varray flags are set Paolo Bonzini
` (14 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
`num_offset` is a member of `VMStateField`, and there's no need to use
"." to access this field in a `VMStateField` instance.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-3-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 6698dfe7aeb..9533b1250fa 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -208,7 +208,7 @@ macro_rules! vmstate_of {
.as_bytes()
.as_ptr() as *const ::std::os::raw::c_char,
offset: $crate::offset_of!($struct_name, $field_name),
- $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+ $(num_offset: $crate::offset_of!($struct_name, $num),)?
// The calls to `call_func_with_field!` are the magic that
// computes most of the VMStateField from the type of the field.
info: $crate::info_enum_to_ref!($crate::call_func_with_field!(
@@ -440,7 +440,7 @@ macro_rules! vmstate_struct {
name: ::core::concat!(::core::stringify!($field_name), "\0")
.as_bytes()
.as_ptr() as *const ::std::os::raw::c_char,
- $(.num_offset: $crate::offset_of!($struct_name, $num),)?
+ $(num_offset: $crate::offset_of!($struct_name, $num),)?
offset: {
$crate::assert_field_type!($struct_name, $field_name, $type);
$crate::offset_of!($struct_name, $field_name)
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 11/24] rust/vmstate: Fix num field when varray flags are set
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (9 preceding siblings ...)
2025-03-21 18:35 ` [PULL 10/24] rust/vmstate: Fix num_offset in vmstate macros Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 12/24] rust/vmstate: Fix size field of VMStateField with VMS_ARRAY_OF_POINTER flag Paolo Bonzini
` (13 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Array type vmstate has the VMStateField with `num` equals its length.
When the varray vmstate is built based a array type, the `num` field
should be cleaned to 0, because varray uses `num_offset` instead of
`num` to store elements number information.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-4-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9533b1250fa..e3233303f20 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,6 +275,7 @@ pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMSta
assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
self.flags = VMStateFlags(self.flags.0 | flag.0);
+ self.num = 0; // varray uses num_offset instead of num.
self
}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 12/24] rust/vmstate: Fix size field of VMStateField with VMS_ARRAY_OF_POINTER flag
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (10 preceding siblings ...)
2025-03-21 18:35 ` [PULL 11/24] rust/vmstate: Fix num field when varray flags are set Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 13/24] rust/vmstate: Fix type check for varray in vmstate_struct Paolo Bonzini
` (12 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The `size` field of the VMStateField with VMS_ARRAY_OF_POINTER flag
should stores the size of pointer, which depends on platform.
Currently, `*const`, `*mut`, `NonNull`, `Box<>` and their wrapper are
supported, and they have the same size as `usize`.
Store the size (of `usize`) when VMS_ARRAY_OF_POINTER flag is set.
The size may be changed when more smart pointers are supported, but now
the size of "usize" is enough.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-5-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index e3233303f20..e2a1f7a97aa 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -256,6 +256,10 @@ pub const fn with_array_flag(mut self, num: usize) -> Self {
if (self.flags.0 & VMStateFlags::VMS_POINTER.0) != 0 {
self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_POINTER.0);
self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0);
+ // VMS_ARRAY_OF_POINTER flag stores the size of pointer.
+ // FIXME: *const, *mut, NonNull and Box<> have the same size as usize.
+ // Resize if more smart pointers are supported.
+ self.size = std::mem::size_of::<usize>();
}
self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_SINGLE.0);
self.flags = VMStateFlags(self.flags.0 | VMStateFlags::VMS_ARRAY.0);
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 13/24] rust/vmstate: Fix type check for varray in vmstate_struct
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (11 preceding siblings ...)
2025-03-21 18:35 ` [PULL 12/24] rust/vmstate: Fix size field of VMStateField with VMS_ARRAY_OF_POINTER flag Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 14/24] rust/vmstate: Fix "cannot infer type" error " Paolo Bonzini
` (11 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
When pass a varray to vmstate_struct, the `type` parameter should be the
type of the element in the varray, for example:
vmstate_struct!(HPETState, timers, [0 .. num_timers], VMSTATE_HPET_TIMER,
BqlRefCell<HPETTimer>).with_version_id(0)
But this breaks current type check, because it checks the type of
`field`, which is an array type (for the above example, type of timers
is [BqlRefCell<HPETTimer>; 32], not BqlRefCell<HPETTimer>).
But the current assert_field_type() can no longer be extended to include
new arguments, so a variant of it (a second macro containing the
`num = $num:ident` parameter) had to be added to handle array cases.
In this new macro, it not only checks the type of element, but also
checks whether the `num` (number of elements in varray) is out of range.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-6-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/assertions.rs | 15 +++++++++++++++
rust/qemu-api/src/vmstate.rs | 2 +-
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/rust/qemu-api/src/assertions.rs b/rust/qemu-api/src/assertions.rs
index bba38cfda11..eb12e9499a7 100644
--- a/rust/qemu-api/src/assertions.rs
+++ b/rust/qemu-api/src/assertions.rs
@@ -91,6 +91,21 @@ fn types_must_be_equal<T, U>(_: T)
}
};
};
+
+ ($t:ty, $i:tt, $ti:ty, num = $num:ident) => {
+ const _: () = {
+ #[allow(unused)]
+ fn assert_field_type(v: $t) {
+ fn types_must_be_equal<T, U>(_: T)
+ where
+ T: $crate::assertions::EqType<Itself = U>,
+ {
+ }
+ let index: usize = v.$num.try_into().unwrap();
+ types_must_be_equal::<_, &$ti>(&v.$i[index]);
+ }
+ };
+ };
}
/// Assert that an expression matches a pattern. This can also be
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index e2a1f7a97aa..9d9cdda993c 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -447,7 +447,7 @@ macro_rules! vmstate_struct {
.as_ptr() as *const ::std::os::raw::c_char,
$(num_offset: $crate::offset_of!($struct_name, $num),)?
offset: {
- $crate::assert_field_type!($struct_name, $field_name, $type);
+ $crate::assert_field_type!($struct_name, $field_name, $type $(, num = $num)?);
$crate::offset_of!($struct_name, $field_name)
},
size: ::core::mem::size_of::<$type>(),
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 14/24] rust/vmstate: Fix "cannot infer type" error in vmstate_struct
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (12 preceding siblings ...)
2025-03-21 18:35 ` [PULL 13/24] rust/vmstate: Fix type check for varray in vmstate_struct Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 15/24] rust/vmstate: Fix unnecessary VMState bound of with_varray_flag() Paolo Bonzini
` (10 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Rust cannot infer the type (it should be VMStateField) after
Zeroable::ZERO, which cause the compiling error.
To fix this error, call with_varray_flag() after VMStateField's
initialization.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-7-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9d9cdda993c..3be3a7260fe 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -453,13 +453,15 @@ macro_rules! vmstate_struct {
size: ::core::mem::size_of::<$type>(),
flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
vmsd: $vmsd,
- ..$crate::zeroable::Zeroable::ZERO $(
- .with_varray_flag($crate::call_func_with_field!(
- $crate::vmstate::vmstate_varray_flag,
- $struct_name,
- $num))
- $(.with_varray_multiply($factor))?)?
- }
+ ..$crate::zeroable::Zeroable::ZERO
+ } $(.with_varray_flag(
+ $crate::call_func_with_field!(
+ $crate::vmstate::vmstate_varray_flag,
+ $struct_name,
+ $num
+ )
+ )
+ $(.with_varray_multiply($factor))?)?
};
}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 15/24] rust/vmstate: Fix unnecessary VMState bound of with_varray_flag()
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (13 preceding siblings ...)
2025-03-21 18:35 ` [PULL 14/24] rust/vmstate: Fix "cannot infer type" error " Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 16/24] rust/vmstate: Relax array check when build varray in vmstate_struct Paolo Bonzini
` (9 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The VMState type bound is not used in with_varray_flag().
And for vmstate_struct, Rust cannot infer the type of `num` from the
call_func_with_field(), so this causes the compiling error because it
complains "cannot satisfy `_: VMState`" in with_varray_flag().
Note Rust can infer the type in vmstate_of macro so that
with_varray_flag() can work at there. It is possible that the different
initialization ways in the two macros cause differences in Rust's
type inference.
But in fact, the VMState type bound is not used in with_varray_flag()
and vmstate_varray_flag() has already checked the VMState type, it's
safe to drop VMState bound of with_varray_flag(), which can fix the
above compiling error.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-8-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 3be3a7260fe..792a74fdfc0 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,7 +275,7 @@ pub const fn with_pointer_flag(mut self) -> Self {
}
#[must_use]
- pub const fn with_varray_flag<T: VMState>(mut self, flag: VMStateFlags) -> VMStateField {
+ pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
self.flags = VMStateFlags(self.flags.0 | flag.0);
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 16/24] rust/vmstate: Relax array check when build varray in vmstate_struct
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (14 preceding siblings ...)
2025-03-21 18:35 ` [PULL 15/24] rust/vmstate: Fix unnecessary VMState bound of with_varray_flag() Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 17/24] rust/vmstate: Re-implement VMState trait for timer binding Paolo Bonzini
` (8 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The varry of structure created by vmstate_struct is different with
vmstate_of. This is because vmstate_struct uses the `vmsd` to traverse
the vmstates of structure's fields, rather than treating the structure
directly as a well-defined vmstate.
Therefore, there's no need to check array flag when building varray by
vmstate_struct.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-9-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 792a74fdfc0..0b5af1c90b1 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -275,14 +275,20 @@ pub const fn with_pointer_flag(mut self) -> Self {
}
#[must_use]
- pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
- assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
+ pub const fn with_varray_flag_unchecked(mut self, flag: VMStateFlags) -> VMStateField {
self.flags = VMStateFlags(self.flags.0 & !VMStateFlags::VMS_ARRAY.0);
self.flags = VMStateFlags(self.flags.0 | flag.0);
self.num = 0; // varray uses num_offset instead of num.
self
}
+ #[must_use]
+ #[allow(unused_mut)]
+ pub const fn with_varray_flag(mut self, flag: VMStateFlags) -> VMStateField {
+ assert!((self.flags.0 & VMStateFlags::VMS_ARRAY.0) != 0);
+ self.with_varray_flag_unchecked(flag)
+ }
+
#[must_use]
pub const fn with_varray_multiply(mut self, num: u32) -> VMStateField {
assert!(num <= 0x7FFF_FFFFu32);
@@ -454,7 +460,7 @@ macro_rules! vmstate_struct {
flags: $crate::bindings::VMStateFlags::VMS_STRUCT,
vmsd: $vmsd,
..$crate::zeroable::Zeroable::ZERO
- } $(.with_varray_flag(
+ } $(.with_varray_flag_unchecked(
$crate::call_func_with_field!(
$crate::vmstate::vmstate_varray_flag,
$struct_name,
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 17/24] rust/vmstate: Re-implement VMState trait for timer binding
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (15 preceding siblings ...)
2025-03-21 18:35 ` [PULL 16/24] rust/vmstate: Relax array check when build varray in vmstate_struct Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 18/24] rust/vmstate: Support vmstate_validate Paolo Bonzini
` (7 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
At present, Rust side has a timer binding "timer::Timer", so the vmstate
for timer should base on that binding instead of the raw
"binding::QEMUTimer".
It's possible to apply impl_vmstate_transparent for cell::Opaque and
then impl_vmstate_forward for timer::Timer. But binding::QEMUTimer
shouldn't be used directly, so that vmstate for such raw timer type is
useless.
Thus, apply impl_vmstate_scalar for timer::Timer. And since Opaque<> is
useful, apply impl_vmstate_transparent for cell::Opaque as well.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-10-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 0b5af1c90b1..01f06ed7379 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -27,12 +27,7 @@
use core::{marker::PhantomData, mem, ptr::NonNull};
pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{
- bindings::{self, VMStateFlags},
- prelude::*,
- qom::Owned,
- zeroable::Zeroable,
-};
+use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, zeroable::Zeroable};
/// This macro is used to call a function with a generic argument bound
/// to the type of a field. The function must take a
@@ -344,6 +339,7 @@ unsafe impl<$base> VMState for $type where $base: VMState $($where)* {
impl_vmstate_transparent!(std::pin::Pin<T> where T: VMState);
impl_vmstate_transparent!(crate::cell::BqlCell<T> where T: VMState);
impl_vmstate_transparent!(crate::cell::BqlRefCell<T> where T: VMState);
+impl_vmstate_transparent!(crate::cell::Opaque<T> where T: VMState);
#[macro_export]
macro_rules! impl_vmstate_bitsized {
@@ -390,7 +386,7 @@ unsafe impl VMState for $type {
impl_vmstate_scalar!(vmstate_info_uint16, u16, VMS_VARRAY_UINT16);
impl_vmstate_scalar!(vmstate_info_uint32, u32, VMS_VARRAY_UINT32);
impl_vmstate_scalar!(vmstate_info_uint64, u64);
-impl_vmstate_scalar!(vmstate_info_timer, bindings::QEMUTimer);
+impl_vmstate_scalar!(vmstate_info_timer, crate::timer::Timer);
// Pointer types using the underlying type's VMState plus VMS_POINTER
// Note that references are not supported, though references to cells
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 18/24] rust/vmstate: Support vmstate_validate
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (16 preceding siblings ...)
2025-03-21 18:35 ` [PULL 17/24] rust/vmstate: Re-implement VMState trait for timer binding Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 19/24] rust/vmstate: Add unit test for vmstate_of macro Paolo Bonzini
` (6 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
In C version, VMSTATE_VALIDATE accepts the function pointer, which is
used to check if some conditions of structure could meet, although the
C version macro doesn't accept any structure as the opaque type.
But it's hard to integrate VMSTATE_VALIDAE into vmstate_struct, a new
macro has to be introduced to specifically handle the case corresponding
to VMSTATE_VALIDATE.
One of the difficulties is inferring the type of a callback by its name
`test_fn`. We can't directly use `test_fn` as a parameter of
test_cb_builder__() to get its type "F", because in this way, Rust
compiler will be too conservative on drop check and complain "the
destructor for this type cannot be evaluated in constant functions".
Fortunately, PhantomData<T> could help in this case, because it is
considered to never have a destructor, no matter its field type [*].
The `phantom__()` in the `call_func_with_field` macro provides a good
example of using PhantomData to infer type. So copy this idea and apply
it to the `vmstate_validate` macro.
[*]: https://doc.rust-lang.org/std/ops/trait.Drop.html#drop-check
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-11-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/vmstate.rs | 52 +++++++++++++++++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 01f06ed7379..9740931fb16 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -25,9 +25,12 @@
//! functionality that is missing from `vmstate_of!`.
use core::{marker::PhantomData, mem, ptr::NonNull};
+use std::os::raw::{c_int, c_void};
pub use crate::bindings::{VMStateDescription, VMStateField};
-use crate::{bindings::VMStateFlags, prelude::*, qom::Owned, zeroable::Zeroable};
+use crate::{
+ bindings::VMStateFlags, callbacks::FnCall, prelude::*, qom::Owned, zeroable::Zeroable,
+};
/// This macro is used to call a function with a generic argument bound
/// to the type of a field. The function must take a
@@ -508,6 +511,53 @@ macro_rules! vmstate_fields {
}}
}
+pub extern "C" fn rust_vms_test_field_exists<T, F: for<'a> FnCall<(&'a T, u8), bool>>(
+ opaque: *mut c_void,
+ version_id: c_int,
+) -> bool {
+ let owner: &T = unsafe { &*(opaque.cast::<T>()) };
+ let version: u8 = version_id.try_into().unwrap();
+ // SAFETY: the opaque was passed as a reference to `T`.
+ F::call((owner, version))
+}
+
+pub type VMSFieldExistCb = unsafe extern "C" fn(
+ opaque: *mut std::os::raw::c_void,
+ version_id: std::os::raw::c_int,
+) -> bool;
+
+#[doc(alias = "VMSTATE_VALIDATE")]
+#[macro_export]
+macro_rules! vmstate_validate {
+ ($struct_name:ty, $test_name:expr, $test_fn:expr $(,)?) => {
+ $crate::bindings::VMStateField {
+ name: ::std::ffi::CStr::as_ptr($test_name),
+ field_exists: {
+ const fn test_cb_builder__<
+ T,
+ F: for<'a> $crate::callbacks::FnCall<(&'a T, u8), bool>,
+ >(
+ _phantom: ::core::marker::PhantomData<F>,
+ ) -> $crate::vmstate::VMSFieldExistCb {
+ let _: () = F::ASSERT_IS_SOME;
+ $crate::vmstate::rust_vms_test_field_exists::<T, F>
+ }
+
+ const fn phantom__<T>(_: &T) -> ::core::marker::PhantomData<T> {
+ ::core::marker::PhantomData
+ }
+ Some(test_cb_builder__::<$struct_name, _>(phantom__(&$test_fn)))
+ },
+ flags: $crate::bindings::VMStateFlags(
+ $crate::bindings::VMStateFlags::VMS_MUST_EXIST.0
+ | $crate::bindings::VMStateFlags::VMS_ARRAY.0,
+ ),
+ num: 0, // 0 elements: no data, only run test_fn callback
+ ..$crate::zeroable::Zeroable::ZERO
+ }
+ };
+}
+
/// A transparent wrapper type for the `subsections` field of
/// [`VMStateDescription`].
///
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 19/24] rust/vmstate: Add unit test for vmstate_of macro
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (17 preceding siblings ...)
2025-03-21 18:35 ` [PULL 18/24] rust/vmstate: Support vmstate_validate Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 20/24] rust/vmstate: Add unit test for vmstate_{of|struct} macro Paolo Bonzini
` (5 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The vmstate has too many combinations of VMStateFlags and VMStateField.
Currently, the best way to test is to ensure that the Rust vmstate
definition is consistent with the (possibly corresponding) C version.
Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
* VMSTATE_U16
* VMSTATE_UNUSED
* VMSTATE_VARRAY_UINT16_UNSAFE
* VMSTATE_VARRAY_MULTIPLY
Note: Because vmstate_info_* are defined in vmstate-types.c, it's
necessary to link libmigration to rust unit tests. In the future,
maybe it's possible to spilt libmigration from rust_qemu_api_objs.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-12-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/meson.build | 5 +-
rust/qemu-api/tests/tests.rs | 2 +
rust/qemu-api/tests/vmstate_tests.rs | 134 +++++++++++++++++++++++++++
3 files changed, 139 insertions(+), 2 deletions(-)
create mode 100644 rust/qemu-api/tests/vmstate_tests.rs
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index a3f226ccc2a..858685ddd4a 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -58,7 +58,8 @@ rust_qemu_api_objs = static_library(
libchardev.extract_all_objects(recursive: false),
libcrypto.extract_all_objects(recursive: false),
libauthz.extract_all_objects(recursive: false),
- libio.extract_all_objects(recursive: false)])
+ libio.extract_all_objects(recursive: false),
+ libmigration.extract_all_objects(recursive: false)])
rust_qemu_api_deps = declare_dependency(
dependencies: [
qom_ss.dependencies(),
@@ -71,7 +72,7 @@ rust_qemu_api_deps = declare_dependency(
test('rust-qemu-api-integration',
executable(
'rust-qemu-api-integration',
- 'tests/tests.rs',
+ files('tests/tests.rs', 'tests/vmstate_tests.rs'),
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_args: ['--test'],
install: false,
diff --git a/rust/qemu-api/tests/tests.rs b/rust/qemu-api/tests/tests.rs
index 269122e7ec1..99a7aab6fed 100644
--- a/rust/qemu-api/tests/tests.rs
+++ b/rust/qemu-api/tests/tests.rs
@@ -17,6 +17,8 @@
zeroable::Zeroable,
};
+mod vmstate_tests;
+
// Test that macros can compile.
pub static VMSTATE: VMStateDescription = VMStateDescription {
name: c_str!("name").as_ptr(),
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
new file mode 100644
index 00000000000..eb1bb2f8a04
--- /dev/null
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -0,0 +1,134 @@
+// Copyright (C) 2025 Intel Corporation.
+// Author(s): Zhao Liu <zhai1.liu@intel.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+use std::{ffi::CStr, mem::size_of, slice};
+
+use qemu_api::{
+ bindings::{vmstate_info_int8, vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags},
+ c_str,
+ vmstate::{VMStateDescription, VMStateField},
+ vmstate_fields, vmstate_of, vmstate_unused,
+ zeroable::Zeroable,
+};
+
+const FOO_ARRAY_MAX: usize = 3;
+
+// =========================== Test VMSTATE_FOOA ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+// * VMSTATE_FOOA:
+// - VMSTATE_U16
+// - VMSTATE_UNUSED
+// - VMSTATE_VARRAY_UINT16_UNSAFE
+// - VMSTATE_VARRAY_MULTIPLY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooA {
+ arr: [u8; FOO_ARRAY_MAX],
+ num: u16,
+ arr_mul: [i8; FOO_ARRAY_MAX],
+ num_mul: u32,
+ elem: i8,
+}
+
+static VMSTATE_FOOA: VMStateDescription = VMStateDescription {
+ name: c_str!("foo_a").as_ptr(),
+ version_id: 1,
+ minimum_version_id: 1,
+ fields: vmstate_fields! {
+ vmstate_of!(FooA, elem),
+ vmstate_unused!(size_of::<i64>()),
+ vmstate_of!(FooA, arr[0 .. num]).with_version_id(0),
+ vmstate_of!(FooA, arr_mul[0 .. num_mul * 16]),
+ },
+ ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_uint16() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+ // 1st VMStateField ("elem") in VMSTATE_FOOA (corresponding to VMSTATE_UINT16)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+ b"elem\0"
+ );
+ assert_eq!(foo_fields[0].offset, 16);
+ assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_int8 });
+ assert_eq!(foo_fields[0].version_id, 0);
+ assert_eq!(foo_fields[0].size, 1);
+ assert_eq!(foo_fields[0].num, 0);
+ assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+ assert!(foo_fields[0].vmsd.is_null());
+ assert!(foo_fields[0].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_unused() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+ // 2nd VMStateField ("unused") in VMSTATE_FOOA (corresponding to VMSTATE_UNUSED)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+ b"unused\0"
+ );
+ assert_eq!(foo_fields[1].offset, 0);
+ assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].info, unsafe { &vmstate_info_unused_buffer });
+ assert_eq!(foo_fields[1].version_id, 0);
+ assert_eq!(foo_fields[1].size, 8);
+ assert_eq!(foo_fields[1].num, 0);
+ assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_BUFFER);
+ assert!(foo_fields[1].vmsd.is_null());
+ assert!(foo_fields[1].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_varray_uint16_unsafe() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+ // 3rd VMStateField ("arr") in VMSTATE_FOOA (corresponding to
+ // VMSTATE_VARRAY_UINT16_UNSAFE)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+ b"arr\0"
+ );
+ assert_eq!(foo_fields[2].offset, 0);
+ assert_eq!(foo_fields[2].num_offset, 4);
+ assert_eq!(foo_fields[2].info, unsafe { &vmstate_info_uint8 });
+ assert_eq!(foo_fields[2].version_id, 0);
+ assert_eq!(foo_fields[2].size, 1);
+ assert_eq!(foo_fields[2].num, 0);
+ assert_eq!(foo_fields[2].flags, VMStateFlags::VMS_VARRAY_UINT16);
+ assert!(foo_fields[2].vmsd.is_null());
+ assert!(foo_fields[2].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_varray_multiply() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOA.fields, 5) };
+
+ // 4th VMStateField ("arr_mul") in VMSTATE_FOOA (corresponding to
+ // VMSTATE_VARRAY_MULTIPLY)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+ b"arr_mul\0"
+ );
+ assert_eq!(foo_fields[3].offset, 6);
+ assert_eq!(foo_fields[3].num_offset, 12);
+ assert_eq!(foo_fields[3].info, unsafe { &vmstate_info_int8 });
+ assert_eq!(foo_fields[3].version_id, 0);
+ assert_eq!(foo_fields[3].size, 1);
+ assert_eq!(foo_fields[3].num, 16);
+ assert_eq!(
+ foo_fields[3].flags.0,
+ VMStateFlags::VMS_VARRAY_UINT32.0 | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
+ );
+ assert!(foo_fields[3].vmsd.is_null());
+ assert!(foo_fields[3].field_exists.is_none());
+
+ // The last VMStateField in VMSTATE_FOOA.
+ assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 20/24] rust/vmstate: Add unit test for vmstate_{of|struct} macro
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (18 preceding siblings ...)
2025-03-21 18:35 ` [PULL 19/24] rust/vmstate: Add unit test for vmstate_of macro Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 21/24] rust/vmstate: Add unit test for pointer case Paolo Bonzini
` (4 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add a unit test to cover some patterns accepted by vmstate_of and
vmstate_struct macros, which correspond to the following C version
macros:
* VMSTATE_BOOL_V
* VMSTATE_U64
* VMSTATE_STRUCT_VARRAY_UINT8
* (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
* VMSTATE_ARRAY
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-13-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/tests/vmstate_tests.rs | 158 ++++++++++++++++++++++++++-
1 file changed, 156 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index eb1bb2f8a04..2c3301e02af 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -5,10 +5,14 @@
use std::{ffi::CStr, mem::size_of, slice};
use qemu_api::{
- bindings::{vmstate_info_int8, vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags},
+ bindings::{
+ vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, vmstate_info_uint64,
+ vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags,
+ },
c_str,
+ cell::BqlCell,
vmstate::{VMStateDescription, VMStateField},
- vmstate_fields, vmstate_of, vmstate_unused,
+ vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
zeroable::Zeroable,
};
@@ -132,3 +136,153 @@ fn test_vmstate_varray_multiply() {
// The last VMStateField in VMSTATE_FOOA.
assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
}
+
+// =========================== Test VMSTATE_FOOB ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+// * VMSTATE_FOOB:
+// - VMSTATE_BOOL_V
+// - VMSTATE_U64
+// - VMSTATE_STRUCT_VARRAY_UINT8
+// - (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32
+// - VMSTATE_ARRAY
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooB {
+ arr_a: [FooA; FOO_ARRAY_MAX],
+ num_a: u8,
+ arr_a_mul: [FooA; FOO_ARRAY_MAX],
+ num_a_mul: u32,
+ wrap: BqlCell<u64>,
+ val: bool,
+ // FIXME: Use Timer array. Now we can't since it's hard to link savevm.c to test.
+ arr_i64: [i64; FOO_ARRAY_MAX],
+}
+
+static VMSTATE_FOOB: VMStateDescription = VMStateDescription {
+ name: c_str!("foo_b").as_ptr(),
+ version_id: 2,
+ minimum_version_id: 1,
+ fields: vmstate_fields! {
+ vmstate_of!(FooB, val).with_version_id(2),
+ vmstate_of!(FooB, wrap),
+ vmstate_struct!(FooB, arr_a[0 .. num_a], &VMSTATE_FOOA, FooA).with_version_id(1),
+ vmstate_struct!(FooB, arr_a_mul[0 .. num_a_mul * 32], &VMSTATE_FOOA, FooA).with_version_id(2),
+ vmstate_of!(FooB, arr_i64),
+ },
+ ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_bool_v() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+ // 1st VMStateField ("val") in VMSTATE_FOOB (corresponding to VMSTATE_BOOL_V)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+ b"val\0"
+ );
+ assert_eq!(foo_fields[0].offset, 136);
+ assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_bool });
+ assert_eq!(foo_fields[0].version_id, 2);
+ assert_eq!(foo_fields[0].size, 1);
+ assert_eq!(foo_fields[0].num, 0);
+ assert_eq!(foo_fields[0].flags, VMStateFlags::VMS_SINGLE);
+ assert!(foo_fields[0].vmsd.is_null());
+ assert!(foo_fields[0].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_uint64() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+ // 2nd VMStateField ("wrap") in VMSTATE_FOOB (corresponding to VMSTATE_U64)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+ b"wrap\0"
+ );
+ assert_eq!(foo_fields[1].offset, 128);
+ assert_eq!(foo_fields[1].num_offset, 0);
+ assert_eq!(foo_fields[1].info, unsafe { &vmstate_info_uint64 });
+ assert_eq!(foo_fields[1].version_id, 0);
+ assert_eq!(foo_fields[1].size, 8);
+ assert_eq!(foo_fields[1].num, 0);
+ assert_eq!(foo_fields[1].flags, VMStateFlags::VMS_SINGLE);
+ assert!(foo_fields[1].vmsd.is_null());
+ assert!(foo_fields[1].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_struct_varray_uint8() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+ // 3rd VMStateField ("arr_a") in VMSTATE_FOOB (corresponding to
+ // VMSTATE_STRUCT_VARRAY_UINT8)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+ b"arr_a\0"
+ );
+ assert_eq!(foo_fields[2].offset, 0);
+ assert_eq!(foo_fields[2].num_offset, 60);
+ assert!(foo_fields[2].info.is_null()); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
+ assert_eq!(foo_fields[2].version_id, 1);
+ assert_eq!(foo_fields[2].size, 20);
+ assert_eq!(foo_fields[2].num, 0);
+ assert_eq!(
+ foo_fields[2].flags.0,
+ VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_VARRAY_UINT8.0
+ );
+ assert_eq!(foo_fields[2].vmsd, &VMSTATE_FOOA);
+ assert!(foo_fields[2].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_struct_varray_uint32_multiply() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+ // 4th VMStateField ("arr_a_mul") in VMSTATE_FOOB (corresponding to
+ // (no C version) MULTIPLY variant of VMSTATE_STRUCT_VARRAY_UINT32)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+ b"arr_a_mul\0"
+ );
+ assert_eq!(foo_fields[3].offset, 64);
+ assert_eq!(foo_fields[3].num_offset, 124);
+ assert!(foo_fields[3].info.is_null()); // VMSTATE_STRUCT_VARRAY_UINT8 doesn't set info field.
+ assert_eq!(foo_fields[3].version_id, 2);
+ assert_eq!(foo_fields[3].size, 20);
+ assert_eq!(foo_fields[3].num, 32);
+ assert_eq!(
+ foo_fields[3].flags.0,
+ VMStateFlags::VMS_STRUCT.0
+ | VMStateFlags::VMS_VARRAY_UINT32.0
+ | VMStateFlags::VMS_MULTIPLY_ELEMENTS.0
+ );
+ assert_eq!(foo_fields[3].vmsd, &VMSTATE_FOOA);
+ assert!(foo_fields[3].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_macro_array() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOB.fields, 6) };
+
+ // 5th VMStateField ("arr_i64") in VMSTATE_FOOB (corresponding to
+ // VMSTATE_ARRAY)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[4].name) }.to_bytes_with_nul(),
+ b"arr_i64\0"
+ );
+ assert_eq!(foo_fields[4].offset, 144);
+ assert_eq!(foo_fields[4].num_offset, 0);
+ assert_eq!(foo_fields[4].info, unsafe { &vmstate_info_int64 });
+ assert_eq!(foo_fields[4].version_id, 0);
+ assert_eq!(foo_fields[4].size, 8);
+ assert_eq!(foo_fields[4].num, FOO_ARRAY_MAX as i32);
+ assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_ARRAY);
+ assert!(foo_fields[4].vmsd.is_null());
+ assert!(foo_fields[4].field_exists.is_none());
+
+ // The last VMStateField in VMSTATE_FOOB.
+ assert_eq!(foo_fields[5].flags, VMStateFlags::VMS_END);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 21/24] rust/vmstate: Add unit test for pointer case
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (19 preceding siblings ...)
2025-03-21 18:35 ` [PULL 20/24] rust/vmstate: Add unit test for vmstate_{of|struct} macro Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 22/24] rust/vmstate: Add unit test for vmstate_validate Paolo Bonzini
` (3 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add a unit test to cover some patterns accepted by vmstate_of macro,
which correspond to the following C version macros:
* VMSTATE_POINTER
* VMSTATE_ARRAY_OF_POINTER
Note: Currently, vmstate_struct can't handle the pointer to structure
case. Leave this case as a FIXME and use vmstate_unused as a place
holder.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-14-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/tests/vmstate_tests.rs | 119 ++++++++++++++++++++++++++-
1 file changed, 115 insertions(+), 4 deletions(-)
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index 2c3301e02af..027b2266753 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,15 +2,16 @@
// Author(s): Zhao Liu <zhai1.liu@intel.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{ffi::CStr, mem::size_of, slice};
+use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
use qemu_api::{
bindings::{
- vmstate_info_bool, vmstate_info_int64, vmstate_info_int8, vmstate_info_uint64,
- vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags,
+ vmstate_info_bool, vmstate_info_int32, vmstate_info_int64, vmstate_info_int8,
+ vmstate_info_uint64, vmstate_info_uint8, vmstate_info_unused_buffer, VMStateFlags,
},
c_str,
- cell::BqlCell,
+ cell::{BqlCell, Opaque},
+ impl_vmstate_forward,
vmstate::{VMStateDescription, VMStateField},
vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
zeroable::Zeroable,
@@ -286,3 +287,113 @@ fn test_vmstate_macro_array() {
// The last VMStateField in VMSTATE_FOOB.
assert_eq!(foo_fields[5].flags, VMStateFlags::VMS_END);
}
+
+// =========================== Test VMSTATE_FOOC ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+// * VMSTATE_FOOC:
+// - VMSTATE_POINTER
+// - VMSTATE_ARRAY_OF_POINTER
+struct FooCWrapper([Opaque<*mut u8>; FOO_ARRAY_MAX]); // Though Opaque<> array is almost impossible.
+
+impl_vmstate_forward!(FooCWrapper);
+
+#[repr(C)]
+#[derive(qemu_api_macros::offsets)]
+struct FooC {
+ ptr: *const i32,
+ ptr_a: NonNull<FooA>,
+ arr_ptr: [Box<u8>; FOO_ARRAY_MAX],
+ arr_ptr_wrap: FooCWrapper,
+}
+
+static VMSTATE_FOOC: VMStateDescription = VMStateDescription {
+ name: c_str!("foo_c").as_ptr(),
+ version_id: 3,
+ minimum_version_id: 1,
+ fields: vmstate_fields! {
+ vmstate_of!(FooC, ptr).with_version_id(2),
+ // FIXME: Currently vmstate_struct doesn't support the pointer to structure.
+ // VMSTATE_STRUCT_POINTER: vmstate_struct!(FooC, ptr_a, VMSTATE_FOOA, NonNull<FooA>)
+ vmstate_unused!(size_of::<NonNull<FooA>>()),
+ vmstate_of!(FooC, arr_ptr),
+ vmstate_of!(FooC, arr_ptr_wrap),
+ },
+ ..Zeroable::ZERO
+};
+
+const PTR_SIZE: usize = size_of::<*mut ()>();
+
+#[test]
+fn test_vmstate_pointer() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+
+ // 1st VMStateField ("ptr") in VMSTATE_FOOC (corresponding to VMSTATE_POINTER)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+ b"ptr\0"
+ );
+ assert_eq!(foo_fields[0].offset, 0);
+ assert_eq!(foo_fields[0].num_offset, 0);
+ assert_eq!(foo_fields[0].info, unsafe { &vmstate_info_int32 });
+ assert_eq!(foo_fields[0].version_id, 2);
+ assert_eq!(foo_fields[0].size, 4);
+ assert_eq!(foo_fields[0].num, 0);
+ assert_eq!(
+ foo_fields[0].flags.0,
+ VMStateFlags::VMS_SINGLE.0 | VMStateFlags::VMS_POINTER.0
+ );
+ assert!(foo_fields[0].vmsd.is_null());
+ assert!(foo_fields[0].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_macro_array_of_pointer() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+
+ // 3rd VMStateField ("arr_ptr") in VMSTATE_FOOC (corresponding to
+ // VMSTATE_ARRAY_OF_POINTER)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+ b"arr_ptr\0"
+ );
+ assert_eq!(foo_fields[2].offset, 2 * PTR_SIZE);
+ assert_eq!(foo_fields[2].num_offset, 0);
+ assert_eq!(foo_fields[2].info, unsafe { &vmstate_info_uint8 });
+ assert_eq!(foo_fields[2].version_id, 0);
+ assert_eq!(foo_fields[2].size, PTR_SIZE);
+ assert_eq!(foo_fields[2].num, FOO_ARRAY_MAX as i32);
+ assert_eq!(
+ foo_fields[2].flags.0,
+ VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0
+ );
+ assert!(foo_fields[2].vmsd.is_null());
+ assert!(foo_fields[2].field_exists.is_none());
+}
+
+#[test]
+fn test_vmstate_macro_array_of_pointer_wrapped() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOC.fields, 6) };
+
+ // 4th VMStateField ("arr_ptr_wrap") in VMSTATE_FOOC (corresponding to
+ // VMSTATE_ARRAY_OF_POINTER)
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[3].name) }.to_bytes_with_nul(),
+ b"arr_ptr_wrap\0"
+ );
+ assert_eq!(foo_fields[3].offset, (FOO_ARRAY_MAX + 2) * PTR_SIZE);
+ assert_eq!(foo_fields[3].num_offset, 0);
+ assert_eq!(foo_fields[2].info, unsafe { &vmstate_info_uint8 });
+ assert_eq!(foo_fields[3].version_id, 0);
+ assert_eq!(foo_fields[3].size, PTR_SIZE);
+ assert_eq!(foo_fields[3].num, FOO_ARRAY_MAX as i32);
+ assert_eq!(
+ foo_fields[2].flags.0,
+ VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_ARRAY_OF_POINTER.0
+ );
+ assert!(foo_fields[3].vmsd.is_null());
+ assert!(foo_fields[3].field_exists.is_none());
+
+ // The last VMStateField in VMSTATE_FOOC.
+ assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 22/24] rust/vmstate: Add unit test for vmstate_validate
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (20 preceding siblings ...)
2025-03-21 18:35 ` [PULL 21/24] rust/vmstate: Add unit test for pointer case Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 23/24] rust/vmstate: Include complete crate path of VMStateFlags in vmstate_clock Paolo Bonzini
` (2 subsequent siblings)
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Add a unit test for vmstate_validate, which corresponds to the C version
macro: VMSTATE_VALIDATE.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-15-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/tests/vmstate_tests.rs | 82 +++++++++++++++++++++++++++-
1 file changed, 80 insertions(+), 2 deletions(-)
diff --git a/rust/qemu-api/tests/vmstate_tests.rs b/rust/qemu-api/tests/vmstate_tests.rs
index 027b2266753..b8d8b45b19d 100644
--- a/rust/qemu-api/tests/vmstate_tests.rs
+++ b/rust/qemu-api/tests/vmstate_tests.rs
@@ -2,7 +2,7 @@
// Author(s): Zhao Liu <zhai1.liu@intel.com>
// SPDX-License-Identifier: GPL-2.0-or-later
-use std::{ffi::CStr, mem::size_of, ptr::NonNull, slice};
+use std::{ffi::CStr, mem::size_of, os::raw::c_void, ptr::NonNull, slice};
use qemu_api::{
bindings::{
@@ -13,7 +13,7 @@
cell::{BqlCell, Opaque},
impl_vmstate_forward,
vmstate::{VMStateDescription, VMStateField},
- vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused,
+ vmstate_fields, vmstate_of, vmstate_struct, vmstate_unused, vmstate_validate,
zeroable::Zeroable,
};
@@ -397,3 +397,81 @@ fn test_vmstate_macro_array_of_pointer_wrapped() {
// The last VMStateField in VMSTATE_FOOC.
assert_eq!(foo_fields[4].flags, VMStateFlags::VMS_END);
}
+
+// =========================== Test VMSTATE_FOOD ===========================
+// Test the use cases of the vmstate macro, corresponding to the following C
+// macro variants:
+// * VMSTATE_FOOD:
+// - VMSTATE_VALIDATE
+
+// Add more member fields when vmstate_of/vmstate_struct support "test"
+// parameter.
+struct FooD;
+
+impl FooD {
+ fn validate_food_0(&self, _version_id: u8) -> bool {
+ true
+ }
+
+ fn validate_food_1(_state: &FooD, _version_id: u8) -> bool {
+ false
+ }
+}
+
+fn validate_food_2(_state: &FooD, _version_id: u8) -> bool {
+ true
+}
+
+static VMSTATE_FOOD: VMStateDescription = VMStateDescription {
+ name: c_str!("foo_d").as_ptr(),
+ version_id: 3,
+ minimum_version_id: 1,
+ fields: vmstate_fields! {
+ vmstate_validate!(FooD, c_str!("foo_d_0"), FooD::validate_food_0),
+ vmstate_validate!(FooD, c_str!("foo_d_1"), FooD::validate_food_1),
+ vmstate_validate!(FooD, c_str!("foo_d_2"), validate_food_2),
+ },
+ ..Zeroable::ZERO
+};
+
+#[test]
+fn test_vmstate_validate() {
+ let foo_fields: &[VMStateField] = unsafe { slice::from_raw_parts(VMSTATE_FOOD.fields, 4) };
+ let mut foo_d = FooD;
+ let foo_d_p = std::ptr::addr_of_mut!(foo_d).cast::<c_void>();
+
+ // 1st VMStateField in VMSTATE_FOOD
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[0].name) }.to_bytes_with_nul(),
+ b"foo_d_0\0"
+ );
+ assert_eq!(foo_fields[0].offset, 0);
+ assert_eq!(foo_fields[0].num_offset, 0);
+ assert!(foo_fields[0].info.is_null());
+ assert_eq!(foo_fields[0].version_id, 0);
+ assert_eq!(foo_fields[0].size, 0);
+ assert_eq!(foo_fields[0].num, 0);
+ assert_eq!(
+ foo_fields[0].flags.0,
+ VMStateFlags::VMS_ARRAY.0 | VMStateFlags::VMS_MUST_EXIST.0
+ );
+ assert!(foo_fields[0].vmsd.is_null());
+ assert!(unsafe { foo_fields[0].field_exists.unwrap()(foo_d_p, 0) });
+
+ // 2nd VMStateField in VMSTATE_FOOD
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[1].name) }.to_bytes_with_nul(),
+ b"foo_d_1\0"
+ );
+ assert!(!unsafe { foo_fields[1].field_exists.unwrap()(foo_d_p, 1) });
+
+ // 3rd VMStateField in VMSTATE_FOOD
+ assert_eq!(
+ unsafe { CStr::from_ptr(foo_fields[2].name) }.to_bytes_with_nul(),
+ b"foo_d_2\0"
+ );
+ assert!(unsafe { foo_fields[2].field_exists.unwrap()(foo_d_p, 2) });
+
+ // The last VMStateField in VMSTATE_FOOD.
+ assert_eq!(foo_fields[3].flags, VMStateFlags::VMS_END);
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 23/24] rust/vmstate: Include complete crate path of VMStateFlags in vmstate_clock
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (21 preceding siblings ...)
2025-03-21 18:35 ` [PULL 22/24] rust/vmstate: Add unit test for vmstate_validate Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-21 18:35 ` [PULL 24/24] rust: hpet: fix decoding of timer registers Paolo Bonzini
2025-03-23 22:30 ` [PULL 00/24] Mostly Rust changes for QEMU 10.0 Stefan Hajnoczi
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
The use of "bindings::*" masks incomplete path of VMStateFlags.
Include complete crate path of VMStateFlags in vmstate_clock, and clean
up "bindings::*" in device_class.rs of pl011.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250318130219.1799170-16-zhao1.liu@intel.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device_class.rs | 8 ++++++--
rust/qemu-api/src/vmstate.rs | 5 ++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/rust/hw/char/pl011/src/device_class.rs b/rust/hw/char/pl011/src/device_class.rs
index 0b2076ddaa0..b4d4a7eb432 100644
--- a/rust/hw/char/pl011/src/device_class.rs
+++ b/rust/hw/char/pl011/src/device_class.rs
@@ -8,8 +8,12 @@
};
use qemu_api::{
- bindings::*, c_str, prelude::*, vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct,
- vmstate_subsections, vmstate_unused, zeroable::Zeroable,
+ bindings::{qdev_prop_bool, qdev_prop_chr},
+ c_str,
+ prelude::*,
+ vmstate::VMStateDescription,
+ vmstate_clock, vmstate_fields, vmstate_of, vmstate_struct, vmstate_subsections, vmstate_unused,
+ zeroable::Zeroable,
};
use crate::device::{PL011Registers, PL011State};
diff --git a/rust/qemu-api/src/vmstate.rs b/rust/qemu-api/src/vmstate.rs
index 9740931fb16..1b2b12eadd6 100644
--- a/rust/qemu-api/src/vmstate.rs
+++ b/rust/qemu-api/src/vmstate.rs
@@ -487,7 +487,10 @@ macro_rules! vmstate_clock {
$crate::offset_of!($struct_name, $field_name)
},
size: ::core::mem::size_of::<*const $crate::qdev::Clock>(),
- flags: VMStateFlags(VMStateFlags::VMS_STRUCT.0 | VMStateFlags::VMS_POINTER.0),
+ flags: $crate::bindings::VMStateFlags(
+ $crate::bindings::VMStateFlags::VMS_STRUCT.0
+ | $crate::bindings::VMStateFlags::VMS_POINTER.0,
+ ),
vmsd: unsafe { ::core::ptr::addr_of!($crate::bindings::vmstate_clock) },
..$crate::zeroable::Zeroable::ZERO
}
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PULL 24/24] rust: hpet: fix decoding of timer registers
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (22 preceding siblings ...)
2025-03-21 18:35 ` [PULL 23/24] rust/vmstate: Include complete crate path of VMStateFlags in vmstate_clock Paolo Bonzini
@ 2025-03-21 18:35 ` Paolo Bonzini
2025-03-23 22:30 ` [PULL 00/24] Mostly Rust changes for QEMU 10.0 Stefan Hajnoczi
24 siblings, 0 replies; 26+ messages in thread
From: Paolo Bonzini @ 2025-03-21 18:35 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell
Due to a missing "& 0x18", timer registers are not decoded correctly.
This breaks the tests/functional/test_x86_64_tuxrun.py functional
test.
Fixes: 519088b7cf6 ("rust: hpet: decode HPET registers into enums", 2025-03-06)
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/hpet.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rust/hw/timer/hpet/src/hpet.rs b/rust/hw/timer/hpet/src/hpet.rs
index 63c1971f0b5..3ae3ec25f17 100644
--- a/rust/hw/timer/hpet/src/hpet.rs
+++ b/rust/hw/timer/hpet/src/hpet.rs
@@ -776,7 +776,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
if timer_id <= self.num_timers.get() {
// TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
- TimerRegister::try_from(addr)
+ TimerRegister::try_from(addr & 0x18)
.map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg))
} else {
// TODO: Add trace point - trace_hpet_timer_id_out_of_range(timer_id)
--
2.49.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PULL 00/24] Mostly Rust changes for QEMU 10.0
2025-03-21 18:35 [PULL 00/24] Mostly Rust changes for QEMU 10.0 Paolo Bonzini
` (23 preceding siblings ...)
2025-03-21 18:35 ` [PULL 24/24] rust: hpet: fix decoding of timer registers Paolo Bonzini
@ 2025-03-23 22:30 ` Stefan Hajnoczi
24 siblings, 0 replies; 26+ messages in thread
From: Stefan Hajnoczi @ 2025-03-23 22:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 116 bytes --]
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/10.0 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 26+ messages in thread