* [PULL 01/31] subprojects: add the anyhow crate
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 02/31] subprojects: add the foreign crate Paolo Bonzini
` (30 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
This is a standard replacement for Box<dyn Error> which is more efficient (it only
occcupies one word) and provides a backtrace of the error. This could be plumbed
into &error_abort in the future.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 2 ++
rust/qemu-api/meson.build | 2 +-
scripts/archive-source.sh | 2 +-
scripts/make-release | 2 +-
subprojects/.gitignore | 1 +
subprojects/anyhow-1-rs.wrap | 7 ++++
.../packagefiles/anyhow-1-rs/meson.build | 33 +++++++++++++++++++
7 files changed, 46 insertions(+), 3 deletions(-)
create mode 100644 subprojects/anyhow-1-rs.wrap
create mode 100644 subprojects/packagefiles/anyhow-1-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index b1b3315be97..f752a064649 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,7 +1,9 @@
+subproject('anyhow-1-rs', required: true)
subproject('bilge-0.2-rs', required: true)
subproject('bilge-impl-0.2-rs', required: true)
subproject('libc-0.2-rs', required: true)
+anyhow_rs = dependency('anyhow-1-rs')
bilge_rs = dependency('bilge-0.2-rs')
bilge_impl_rs = dependency('bilge-impl-0.2-rs')
libc_rs = dependency('libc-0.2-rs')
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index b532281e8c0..da70720e4c1 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -35,7 +35,7 @@ _qemu_api_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _qemu_api_cfg,
- dependencies: [libc_rs, qemu_api_macros, qemuutil_rs,
+ dependencies: [anyhow_rs, libc_rs, qemu_api_macros, qemuutil_rs,
qom, hwcore, chardev, migration],
)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index e461c1531ed..816062fee94 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -27,7 +27,7 @@ sub_file="${sub_tdir}/submodule.tar"
# in their checkout, because the build environment is completely
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/scripts/make-release b/scripts/make-release
index 8c3594a1a47..ea65bdcc0cf 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -40,7 +40,7 @@ fi
# Only include wraps that are invoked with subproject()
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
- berkeley-testfloat-3 arbitrary-int-1-rs bilge-0.2-rs
+ berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index d12d34618cc..b9ae507b85a 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -6,6 +6,7 @@
/keycodemapdb
/libvfio-user
/slirp
+/anyhow-1.0.98
/arbitrary-int-1.2.7
/bilge-0.2.0
/bilge-impl-0.2.0
diff --git a/subprojects/anyhow-1-rs.wrap b/subprojects/anyhow-1-rs.wrap
new file mode 100644
index 00000000000..a69a3645b49
--- /dev/null
+++ b/subprojects/anyhow-1-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = anyhow-1.0.98
+source_url = https://crates.io/api/v1/crates/anyhow/1.0.98/download
+source_filename = anyhow-1.0.98.tar.gz
+source_hash = e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487
+#method = cargo
+patch_directory = anyhow-1-rs
diff --git a/subprojects/packagefiles/anyhow-1-rs/meson.build b/subprojects/packagefiles/anyhow-1-rs/meson.build
new file mode 100644
index 00000000000..348bab98b9f
--- /dev/null
+++ b/subprojects/packagefiles/anyhow-1-rs/meson.build
@@ -0,0 +1,33 @@
+project('anyhow-1-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '1.0.98',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+rustc = meson.get_compiler('rust')
+
+rust_args = ['--cap-lints', 'allow']
+rust_args += ['--cfg', 'feature="std"']
+if rustc.version().version_compare('<1.65.0')
+ error('rustc version ' + rustc.version() + ' is unsupported. Please upgrade to at least 1.65.0')
+endif
+rust_args += [ '--cfg', 'std_backtrace' ] # >= 1.65.0
+if rustc.version().version_compare('<1.81.0')
+ rust_args += [ '--cfg', 'anyhow_no_core_error' ]
+endif
+
+_anyhow_rs = static_library(
+ 'anyhow',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2018', 'build.rust_std=2018'],
+ rust_abi: 'rust',
+ rust_args: rust_args,
+ dependencies: [],
+)
+
+anyhow_dep = declare_dependency(
+ link_with: _anyhow_rs,
+)
+
+meson.override_dependency('anyhow-1-rs', anyhow_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 02/31] subprojects: add the foreign crate
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
2025-06-06 12:34 ` [PULL 01/31] subprojects: add the anyhow crate Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 03/31] util/error: expose Error definition to Rust code Paolo Bonzini
` (29 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
This is a cleaned up and separated version of the patches at
https://lore.kernel.org/all/20240701145853.1394967-4-pbonzini@redhat.com/
https://lore.kernel.org/all/20240701145853.1394967-5-pbonzini@redhat.com/
Its first user will be the Error bindings; for example a QEMU Error ** can be
converted to a Rust Option using
unsafe { Option::<Error>::from_foreign(c_error) }
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/meson.build | 2 ++
rust/qemu-api/meson.build | 2 +-
scripts/archive-source.sh | 3 ++-
scripts/make-release | 3 ++-
subprojects/.gitignore | 1 +
subprojects/foreign-0.3-rs.wrap | 7 +++++
.../packagefiles/foreign-0.3-rs/meson.build | 26 +++++++++++++++++++
7 files changed, 41 insertions(+), 3 deletions(-)
create mode 100644 subprojects/foreign-0.3-rs.wrap
create mode 100644 subprojects/packagefiles/foreign-0.3-rs/meson.build
diff --git a/rust/meson.build b/rust/meson.build
index f752a064649..99ae7956cd0 100644
--- a/rust/meson.build
+++ b/rust/meson.build
@@ -1,11 +1,13 @@
subproject('anyhow-1-rs', required: true)
subproject('bilge-0.2-rs', required: true)
subproject('bilge-impl-0.2-rs', required: true)
+subproject('foreign-0.3-rs', required: true)
subproject('libc-0.2-rs', required: true)
anyhow_rs = dependency('anyhow-1-rs')
bilge_rs = dependency('bilge-0.2-rs')
bilge_impl_rs = dependency('bilge-impl-0.2-rs')
+foreign_rs = dependency('foreign-0.3-rs')
libc_rs = dependency('libc-0.2-rs')
subproject('proc-macro2-1-rs', required: true)
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index da70720e4c1..2f0f3b2aae1 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -35,7 +35,7 @@ _qemu_api_rs = static_library(
override_options: ['rust_std=2021', 'build.rust_std=2021'],
rust_abi: 'rust',
rust_args: _qemu_api_cfg,
- dependencies: [anyhow_rs, libc_rs, qemu_api_macros, qemuutil_rs,
+ dependencies: [anyhow_rs, foreign_rs, libc_rs, qemu_api_macros, qemuutil_rs,
qom, hwcore, chardev, migration],
)
diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh
index 816062fee94..035828c532e 100755
--- a/scripts/archive-source.sh
+++ b/scripts/archive-source.sh
@@ -28,7 +28,8 @@ sub_file="${sub_tdir}/submodule.tar"
# different to the host OS.
subprojects="keycodemapdb libvfio-user berkeley-softfloat-3
berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
+ bilge-impl-0.2-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
+ libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
sub_deinit=""
diff --git a/scripts/make-release b/scripts/make-release
index ea65bdcc0cf..4509a9fabf5 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -41,7 +41,8 @@ fi
# Only include wraps that are invoked with subproject()
SUBPROJECTS="libvfio-user keycodemapdb berkeley-softfloat-3
berkeley-testfloat-3 anyhow-1-rs arbitrary-int-1-rs bilge-0.2-rs
- bilge-impl-0.2-rs either-1-rs itertools-0.11-rs libc-0.2-rs proc-macro2-1-rs
+ bilge-impl-0.2-rs either-1-rs foreign-0.3-rs itertools-0.11-rs
+ libc-0.2-rs proc-macro2-1-rs
proc-macro-error-1-rs proc-macro-error-attr-1-rs quote-1-rs
syn-2-rs unicode-ident-1-rs"
diff --git a/subprojects/.gitignore b/subprojects/.gitignore
index b9ae507b85a..f4281934ce1 100644
--- a/subprojects/.gitignore
+++ b/subprojects/.gitignore
@@ -11,6 +11,7 @@
/bilge-0.2.0
/bilge-impl-0.2.0
/either-1.12.0
+/foreign-0.3.1
/itertools-0.11.0
/libc-0.2.162
/proc-macro-error-1.0.4
diff --git a/subprojects/foreign-0.3-rs.wrap b/subprojects/foreign-0.3-rs.wrap
new file mode 100644
index 00000000000..0d218ec2c25
--- /dev/null
+++ b/subprojects/foreign-0.3-rs.wrap
@@ -0,0 +1,7 @@
+[wrap-file]
+directory = foreign-0.3.1
+source_url = https://crates.io/api/v1/crates/foreign/0.3.1/download
+source_filename = foreign-0.3.1.tar.gz
+source_hash = 17ca1b5be8c9d320daf386f1809c7acc0cb09accbae795c2001953fa50585846
+#method = cargo
+patch_directory = foreign-0.3-rs
diff --git a/subprojects/packagefiles/foreign-0.3-rs/meson.build b/subprojects/packagefiles/foreign-0.3-rs/meson.build
new file mode 100644
index 00000000000..0901c02c527
--- /dev/null
+++ b/subprojects/packagefiles/foreign-0.3-rs/meson.build
@@ -0,0 +1,26 @@
+project('foreign-0.3-rs', 'rust',
+ meson_version: '>=1.5.0',
+ version: '0.2.0',
+ license: 'MIT OR Apache-2.0',
+ default_options: [])
+
+subproject('libc-0.2-rs', required: true)
+libc_rs = dependency('libc-0.2-rs')
+
+_foreign_rs = static_library(
+ 'foreign',
+ files('src/lib.rs'),
+ gnu_symbol_visibility: 'hidden',
+ override_options: ['rust_std=2021', 'build.rust_std=2021'],
+ rust_abi: 'rust',
+ rust_args: [
+ '--cap-lints', 'allow',
+ ],
+ dependencies: [libc_rs],
+)
+
+foreign_dep = declare_dependency(
+ link_with: _foreign_rs,
+)
+
+meson.override_dependency('foreign-0.3-rs', foreign_dep)
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 03/31] util/error: expose Error definition to Rust code
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
2025-06-06 12:34 ` [PULL 01/31] subprojects: add the anyhow crate Paolo Bonzini
2025-06-06 12:34 ` [PULL 02/31] subprojects: add the foreign crate Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 04/31] util/error: allow non-NUL-terminated err->src Paolo Bonzini
` (28 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel
This is used to preserve the file and line in a roundtrip from
C Error to Rust and back to C.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 26 ++++++++++++++++++++++++++
rust/wrapper.h | 1 +
util/error.c | 10 +---------
3 files changed, 28 insertions(+), 9 deletions(-)
create mode 100644 include/qapi/error-internal.h
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
new file mode 100644
index 00000000000..d5c3904adec
--- /dev/null
+++ b/include/qapi/error-internal.h
@@ -0,0 +1,26 @@
+/*
+ * QEMU Error Objects - struct definition
+ *
+ * Copyright IBM, Corp. 2011
+ * Copyright (C) 2011-2015 Red Hat, Inc.
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ * Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2. See
+ * the COPYING.LIB file in the top-level directory.
+ */
+
+#ifndef QAPI_ERROR_INTERNAL_H
+
+struct Error
+{
+ char *msg;
+ ErrorClass err_class;
+ const char *src, *func;
+ int line;
+ GString *hint;
+};
+
+#endif
diff --git a/rust/wrapper.h b/rust/wrapper.h
index beddd9aab2f..6060d3ba1ab 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -60,6 +60,7 @@ typedef enum memory_order {
#include "hw/qdev-properties-system.h"
#include "hw/irq.h"
#include "qapi/error.h"
+#include "qapi/error-internal.h"
#include "migration/vmstate.h"
#include "chardev/char-serial.h"
#include "exec/memattrs.h"
diff --git a/util/error.c b/util/error.c
index 673011b89e9..e5bcb7c0225 100644
--- a/util/error.c
+++ b/util/error.c
@@ -15,15 +15,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
-
-struct Error
-{
- char *msg;
- ErrorClass err_class;
- const char *src, *func;
- int line;
- GString *hint;
-};
+#include "qapi/error-internal.h"
Error *error_abort;
Error *error_fatal;
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 04/31] util/error: allow non-NUL-terminated err->src
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (2 preceding siblings ...)
2025-06-06 12:34 ` [PULL 03/31] util/error: expose Error definition to Rust code Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 05/31] util/error: make func optional Paolo Bonzini
` (27 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Rust makes the current file available as a statically-allocated string,
but without a NUL terminator. Allow this by storing an optional maximum
length in the Error.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 9 ++++++++-
util/error.c | 5 +++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
index d5c3904adec..1ec3ceb40f0 100644
--- a/include/qapi/error-internal.h
+++ b/include/qapi/error-internal.h
@@ -18,7 +18,14 @@ struct Error
{
char *msg;
ErrorClass err_class;
- const char *src, *func;
+ const char *func;
+
+ /*
+ * src might be NUL-terminated or not. If it is, src_len is negative.
+ * If it is not, src_len is the length.
+ */
+ const char *src;
+ int src_len;
int line;
GString *hint;
};
diff --git a/util/error.c b/util/error.c
index e5bcb7c0225..3449ecc0b92 100644
--- a/util/error.c
+++ b/util/error.c
@@ -24,8 +24,8 @@ Error *error_warn;
static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
- fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
- err->func, err->src, err->line);
+ fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
+ err->func, err->src_len, err->src, err->line);
error_report("%s", error_get_pretty(err));
if (err->hint) {
error_printf("%s", err->hint->str);
@@ -67,6 +67,7 @@ static void error_setv(Error **errp,
g_free(msg);
}
err->err_class = err_class;
+ err->src_len = -1;
err->src = src;
err->line = line;
err->func = func;
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 05/31] util/error: make func optional
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (3 preceding siblings ...)
2025-06-06 12:34 ` [PULL 04/31] util/error: allow non-NUL-terminated err->src Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 06/31] rust: qemu-api: add bindings to Error Paolo Bonzini
` (26 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
The function name is not available in Rust, so make it optional.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qapi/error-internal.h | 2 ++
util/error.c | 9 +++++++--
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/qapi/error-internal.h b/include/qapi/error-internal.h
index 1ec3ceb40f0..ff18a2086d2 100644
--- a/include/qapi/error-internal.h
+++ b/include/qapi/error-internal.h
@@ -18,6 +18,8 @@ struct Error
{
char *msg;
ErrorClass err_class;
+
+ /* Used for error_abort only, may be NULL. */
const char *func;
/*
diff --git a/util/error.c b/util/error.c
index 3449ecc0b92..daea2142f30 100644
--- a/util/error.c
+++ b/util/error.c
@@ -24,8 +24,13 @@ Error *error_warn;
static void error_handle(Error **errp, Error *err)
{
if (errp == &error_abort) {
- fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
- err->func, err->src_len, err->src, err->line);
+ if (err->func) {
+ fprintf(stderr, "Unexpected error in %s() at %.*s:%d:\n",
+ err->func, err->src_len, err->src, err->line);
+ } else {
+ fprintf(stderr, "Unexpected error at %.*s:%d:\n",
+ err->src_len, err->src, err->line);
+ }
error_report("%s", error_get_pretty(err));
if (err->hint) {
error_printf("%s", err->hint->str);
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 06/31] rust: qemu-api: add bindings to Error
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (4 preceding siblings ...)
2025-06-06 12:34 ` [PULL 05/31] util/error: make func optional Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 07/31] rust: qemu-api: add tests for Error bindings Paolo Bonzini
` (25 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Provide an implementation of std::error::Error that bridges the Rust
anyhow::Error and std::panic::Location types with QEMU's Error*.
It also has several utility methods, analogous to error_propagate(),
that convert a Result into a return value + Error** pair. One important
difference is that these propagation methods *panic* if *errp is NULL,
unlike error_propagate() which eats subsequent errors[1]. The reason
for this is that in C you have an error_set*() call at the site where
the error is created, and calls to error_propagate() are relatively rare.
In Rust instead, even though these functions do "propagate" a
qemu_api::Error into a C Error**, there is no error_setg() anywhere that
could check for non-NULL errp and call abort(). error_propagate()'s
behavior of ignoring subsequent errors is generally considered weird,
and there would be a bigger risk of triggering it from Rust code.
[1] This is actually a violation of the preconditions of error_propagate(),
so it should not happen. But you never know...
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 5 +
rust/Cargo.lock | 17 ++
rust/Cargo.toml | 1 +
rust/qemu-api/Cargo.toml | 2 +
rust/qemu-api/meson.build | 1 +
rust/qemu-api/src/error.rs | 312 +++++++++++++++++++++++++++++++++++++
rust/qemu-api/src/lib.rs | 3 +
7 files changed, 341 insertions(+)
create mode 100644 rust/qemu-api/src/error.rs
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index 34d9c7945b7..d60d56d0a69 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -96,6 +96,11 @@ are missing:
architecture (VMState). Right now, VMState lacks type safety because
it is hard to place the ``VMStateField`` definitions in traits.
+* NUL-terminated file names with ``#[track_caller]`` are scheduled for
+ inclusion as ``#![feature(location_file_nul)]``, but it will be a while
+ before QEMU can use them. For now, there is special code in
+ ``util/error.c`` to support non-NUL-terminated file names.
+
* associated const equality would be nice to have for some users of
``callbacks::FnCall``, but is still experimental. ``ASSERT_IS_SOME``
replaces it.
diff --git a/rust/Cargo.lock b/rust/Cargo.lock
index bccfe855a70..b785c718f31 100644
--- a/rust/Cargo.lock
+++ b/rust/Cargo.lock
@@ -2,6 +2,12 @@
# It is not intended for manual editing.
version = 3
+[[package]]
+name = "anyhow"
+version = "1.0.98"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e16d2d3311acee920a9eb8d33b8cbc1787ce4a264e85f964c2404b969bdcd487"
+
[[package]]
name = "arbitrary-int"
version = "1.2.7"
@@ -44,6 +50,15 @@ version = "1.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
+[[package]]
+name = "foreign"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "17ca1b5be8c9d320daf386f1809c7acc0cb09accbae795c2001953fa50585846"
+dependencies = [
+ "libc",
+]
+
[[package]]
name = "hpet"
version = "0.1.0"
@@ -114,6 +129,8 @@ dependencies = [
name = "qemu_api"
version = "0.1.0"
dependencies = [
+ "anyhow",
+ "foreign",
"libc",
"qemu_api_macros",
]
diff --git a/rust/Cargo.toml b/rust/Cargo.toml
index fd4c2fbf84b..0868e1b4268 100644
--- a/rust/Cargo.toml
+++ b/rust/Cargo.toml
@@ -67,6 +67,7 @@ missing_safety_doc = "deny"
mut_mut = "deny"
needless_bitwise_bool = "deny"
needless_pass_by_ref_mut = "deny"
+needless_update = "deny"
no_effect_underscore_binding = "deny"
option_option = "deny"
or_fun_call = "deny"
diff --git a/rust/qemu-api/Cargo.toml b/rust/qemu-api/Cargo.toml
index c96cf50e7a1..db7000dee44 100644
--- a/rust/qemu-api/Cargo.toml
+++ b/rust/qemu-api/Cargo.toml
@@ -15,7 +15,9 @@ rust-version.workspace = true
[dependencies]
qemu_api_macros = { path = "../qemu-api-macros" }
+anyhow = "~1.0"
libc = "0.2.162"
+foreign = "~0.3.1"
[features]
default = ["debug_cell"]
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index 2f0f3b2aae1..cac8595a148 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -19,6 +19,7 @@ _qemu_api_rs = static_library(
'src/cell.rs',
'src/chardev.rs',
'src/errno.rs',
+ 'src/error.rs',
'src/irq.rs',
'src/memory.rs',
'src/module.rs',
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
new file mode 100644
index 00000000000..80157f6ea1b
--- /dev/null
+++ b/rust/qemu-api/src/error.rs
@@ -0,0 +1,312 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Error propagation for QEMU Rust code
+//!
+//! This module contains [`Error`], the bridge between Rust errors and
+//! [`Result`](std::result::Result)s and QEMU's C [`Error`](bindings::Error)
+//! struct.
+//!
+//! For FFI code, [`Error`] provides functions to simplify conversion between
+//! the Rust ([`Result<>`](std::result::Result)) and C (`Error**`) conventions:
+//!
+//! * [`ok_or_propagate`](crate::Error::ok_or_propagate),
+//! [`bool_or_propagate`](crate::Error::bool_or_propagate),
+//! [`ptr_or_propagate`](crate::Error::ptr_or_propagate) can be used to build
+//! a C return value while also propagating an error condition
+//!
+//! * [`err_or_else`](crate::Error::err_or_else) and
+//! [`err_or_unit`](crate::Error::err_or_unit) can be used to build a `Result`
+//!
+//! This module is most commonly used at the boundary between C and Rust code;
+//! other code will usually access it through the
+//! [`qemu_api::Result`](crate::Result) type alias, and will use the
+//! [`std::error::Error`] interface to let C errors participate in Rust's error
+//! handling functionality.
+//!
+//! Rust code can also create use this module to create an error object that
+//! will be passed up to C code, though in most cases this will be done
+//! transparently through the `?` operator. Errors can be constructed from a
+//! simple error string, from an [`anyhow::Error`] to pass any other Rust error
+//! type up to C code, or from a combination of the two.
+//!
+//! The third case, corresponding to [`Error::with_error`], is the only one that
+//! requires mentioning [`qemu_api::Error`](crate::Error) explicitly. Similar
+//! to how QEMU's C code handles errno values, the string and the
+//! `anyhow::Error` object will be concatenated with `:` as the separator.
+
+use std::{
+ borrow::Cow,
+ ffi::{c_char, c_int, c_void, CStr},
+ fmt::{self, Display},
+ panic, ptr,
+};
+
+use foreign::{prelude::*, OwnedPointer};
+
+use crate::bindings;
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Debug)]
+pub struct Error {
+ msg: Option<Cow<'static, str>>,
+ /// Appends the print string of the error to the msg if not None
+ cause: Option<anyhow::Error>,
+ file: &'static str,
+ line: u32,
+}
+
+impl std::error::Error for Error {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ self.cause.as_ref().map(AsRef::as_ref)
+ }
+
+ #[allow(deprecated)]
+ fn description(&self) -> &str {
+ self.msg
+ .as_deref()
+ .or_else(|| self.cause.as_deref().map(std::error::Error::description))
+ .expect("no message nor cause?")
+ }
+}
+
+impl Display for Error {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ let mut prefix = "";
+ if let Some(ref msg) = self.msg {
+ write!(f, "{msg}")?;
+ prefix = ": ";
+ }
+ if let Some(ref cause) = self.cause {
+ write!(f, "{prefix}{cause}")?;
+ } else if prefix.is_empty() {
+ panic!("no message nor cause?");
+ }
+ Ok(())
+ }
+}
+
+impl From<String> for Error {
+ #[track_caller]
+ fn from(msg: String) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(Cow::Owned(msg)),
+ cause: None,
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl From<&'static str> for Error {
+ #[track_caller]
+ fn from(msg: &'static str) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(Cow::Borrowed(msg)),
+ cause: None,
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl From<anyhow::Error> for Error {
+ #[track_caller]
+ fn from(error: anyhow::Error) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: None,
+ cause: Some(error),
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+}
+
+impl Error {
+ /// Create a new error, prepending `msg` to the
+ /// description of `cause`
+ #[track_caller]
+ pub fn with_error(msg: impl Into<Cow<'static, str>>, cause: impl Into<anyhow::Error>) -> Self {
+ let location = panic::Location::caller();
+ Error {
+ msg: Some(msg.into()),
+ cause: Some(cause.into()),
+ file: location.file(),
+ line: location.line(),
+ }
+ }
+
+ /// Consume a result, returning `false` if it is an error and
+ /// `true` if it is successful. The error is propagated into
+ /// `errp` like the C API `error_propagate` would do.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ pub unsafe fn bool_or_propagate(result: Result<()>, errp: *mut *mut bindings::Error) -> bool {
+ // SAFETY: caller guarantees errp is valid
+ unsafe { Self::ok_or_propagate(result, errp) }.is_some()
+ }
+
+ /// Consume a result, returning a `NULL` pointer if it is an error and
+ /// a C representation of the contents if it is successful. This is
+ /// similar to the C API `error_propagate`, but it panics if `*errp`
+ /// is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ ///
+ /// See [`propagate`](Error::propagate) for more information.
+ #[must_use]
+ pub unsafe fn ptr_or_propagate<T: CloneToForeign>(
+ result: Result<T>,
+ errp: *mut *mut bindings::Error,
+ ) -> *mut T::Foreign {
+ // SAFETY: caller guarantees errp is valid
+ unsafe { Self::ok_or_propagate(result, errp) }.clone_to_foreign_ptr()
+ }
+
+ /// Consume a result in the same way as `self.ok()`, but also propagate
+ /// a possible error into `errp`. This is similar to the C API
+ /// `error_propagate`, but it panics if `*errp` is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`;
+ /// typically it is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ ///
+ /// See [`propagate`](Error::propagate) for more information.
+ pub unsafe fn ok_or_propagate<T>(
+ result: Result<T>,
+ errp: *mut *mut bindings::Error,
+ ) -> Option<T> {
+ result.map_err(|err| unsafe { err.propagate(errp) }).ok()
+ }
+
+ /// Equivalent of the C function `error_propagate`. Fill `*errp`
+ /// with the information container in `self` if `errp` is not NULL;
+ /// then consume it.
+ ///
+ /// This is similar to the C API `error_propagate`, but it panics if
+ /// `*errp` is not `NULL`.
+ ///
+ /// # Safety
+ ///
+ /// `errp` must be a valid argument to `error_propagate`; it can be
+ /// `NULL` or it can point to any of:
+ /// * `error_abort`
+ /// * `error_fatal`
+ /// * a local variable of (C) type `Error *`
+ ///
+ /// Typically `errp` is received from C code and need not be
+ /// checked further at the Rust↔C boundary.
+ pub unsafe fn propagate(self, errp: *mut *mut bindings::Error) {
+ if errp.is_null() {
+ return;
+ }
+
+ // SAFETY: caller guarantees that errp and *errp are valid
+ unsafe {
+ assert_eq!(*errp, ptr::null_mut());
+ bindings::error_propagate(errp, self.clone_to_foreign_ptr());
+ }
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, using
+ /// `Ok(())` if `c_error` is NULL. Free the `Error*`.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be `NULL` or valid; typically it was initialized
+ /// with `ptr::null_mut()` and passed by reference to a C function.
+ pub unsafe fn err_or_unit(c_error: *mut bindings::Error) -> Result<()> {
+ // SAFETY: caller guarantees c_error is valid
+ unsafe { Self::err_or_else(c_error, || ()) }
+ }
+
+ /// Convert a C `Error*` into a Rust `Result`, calling `f()` to
+ /// obtain an `Ok` value if `c_error` is NULL. Free the `Error*`.
+ ///
+ /// # Safety
+ ///
+ /// `c_error` must be `NULL` or point to a valid C [`struct
+ /// Error`](bindings::Error); typically it was initialized with
+ /// `ptr::null_mut()` and passed by reference to a C function.
+ pub unsafe fn err_or_else<T, F: FnOnce() -> T>(
+ c_error: *mut bindings::Error,
+ f: F,
+ ) -> Result<T> {
+ // SAFETY: caller guarantees c_error is valid
+ let err = unsafe { Option::<Self>::from_foreign(c_error) };
+ match err {
+ None => Ok(f()),
+ Some(err) => Err(err),
+ }
+ }
+}
+
+impl FreeForeign for Error {
+ type Foreign = bindings::Error;
+
+ unsafe fn free_foreign(p: *mut bindings::Error) {
+ // SAFETY: caller guarantees p is valid
+ unsafe {
+ bindings::error_free(p);
+ }
+ }
+}
+
+impl CloneToForeign for Error {
+ fn clone_to_foreign(&self) -> OwnedPointer<Self> {
+ // SAFETY: all arguments are controlled by this function
+ unsafe {
+ let err: *mut c_void = libc::malloc(std::mem::size_of::<bindings::Error>());
+ let err: &mut bindings::Error = &mut *err.cast();
+ *err = bindings::Error {
+ msg: format!("{self}").clone_to_foreign_ptr(),
+ err_class: bindings::ERROR_CLASS_GENERIC_ERROR,
+ src_len: self.file.len() as c_int,
+ src: self.file.as_ptr().cast::<c_char>(),
+ line: self.line as c_int,
+ func: ptr::null_mut(),
+ hint: ptr::null_mut(),
+ };
+ OwnedPointer::new(err)
+ }
+ }
+}
+
+impl FromForeign for Error {
+ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
+ // SAFETY: caller guarantees c_error is valid
+ unsafe {
+ let error = &*c_error;
+ let file = if error.src_len < 0 {
+ // NUL-terminated
+ CStr::from_ptr(error.src).to_str()
+ } else {
+ // Can become str::from_utf8 with Rust 1.87.0
+ std::str::from_utf8(std::slice::from_raw_parts(
+ &*error.src.cast::<u8>(),
+ error.src_len as usize,
+ ))
+ };
+
+ Error {
+ msg: FromForeign::cloned_from_foreign(error.msg),
+ cause: None,
+ file: file.unwrap(),
+ line: error.line as u32,
+ }
+ }
+ }
+}
diff --git a/rust/qemu-api/src/lib.rs b/rust/qemu-api/src/lib.rs
index 234a94e3c29..93902fc94bc 100644
--- a/rust/qemu-api/src/lib.rs
+++ b/rust/qemu-api/src/lib.rs
@@ -19,6 +19,7 @@
pub mod cell;
pub mod chardev;
pub mod errno;
+pub mod error;
pub mod irq;
pub mod memory;
pub mod module;
@@ -34,6 +35,8 @@
ffi::c_void,
};
+pub use error::{Error, Result};
+
#[cfg(HAVE_GLIB_WITH_ALIGNED_ALLOC)]
extern "C" {
fn g_aligned_alloc0(
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 07/31] rust: qemu-api: add tests for Error bindings
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (5 preceding siblings ...)
2025-06-06 12:34 ` [PULL 06/31] rust: qemu-api: add bindings to Error Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 08/31] rust: qdev: support returning errors from realize Paolo Bonzini
` (24 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api/src/error.rs | 104 +++++++++++++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/rust/qemu-api/src/error.rs b/rust/qemu-api/src/error.rs
index 80157f6ea1b..e114fc4178b 100644
--- a/rust/qemu-api/src/error.rs
+++ b/rust/qemu-api/src/error.rs
@@ -310,3 +310,107 @@ unsafe fn cloned_from_foreign(c_error: *const bindings::Error) -> Self {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ use std::ffi::CStr;
+
+ use anyhow::anyhow;
+ use foreign::OwnedPointer;
+
+ use super::*;
+ use crate::{assert_match, bindings};
+
+ #[track_caller]
+ fn error_for_test(msg: &CStr) -> OwnedPointer<Error> {
+ // SAFETY: all arguments are controlled by this function
+ let location = panic::Location::caller();
+ unsafe {
+ let err: *mut c_void = libc::malloc(std::mem::size_of::<bindings::Error>());
+ let err: &mut bindings::Error = &mut *err.cast();
+ *err = bindings::Error {
+ msg: msg.clone_to_foreign_ptr(),
+ err_class: bindings::ERROR_CLASS_GENERIC_ERROR,
+ src_len: location.file().len() as c_int,
+ src: location.file().as_ptr().cast::<c_char>(),
+ line: location.line() as c_int,
+ func: ptr::null_mut(),
+ hint: ptr::null_mut(),
+ };
+ OwnedPointer::new(err)
+ }
+ }
+
+ unsafe fn error_get_pretty<'a>(local_err: *mut bindings::Error) -> &'a CStr {
+ unsafe { CStr::from_ptr(bindings::error_get_pretty(local_err)) }
+ }
+
+ #[test]
+ #[allow(deprecated)]
+ fn test_description() {
+ use std::error::Error;
+
+ assert_eq!(super::Error::from("msg").description(), "msg");
+ assert_eq!(super::Error::from("msg".to_owned()).description(), "msg");
+ }
+
+ #[test]
+ fn test_display() {
+ assert_eq!(&*format!("{}", Error::from("msg")), "msg");
+ assert_eq!(&*format!("{}", Error::from("msg".to_owned())), "msg");
+ assert_eq!(&*format!("{}", Error::from(anyhow!("msg"))), "msg");
+
+ assert_eq!(
+ &*format!("{}", Error::with_error("msg", anyhow!("cause"))),
+ "msg: cause"
+ );
+ }
+
+ #[test]
+ fn test_bool_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ assert!(Error::bool_or_propagate(Ok(()), &mut local_err));
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert!(!Error::bool_or_propagate(Err(my_err), &mut local_err));
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_ptr_or_propagate() {
+ unsafe {
+ let mut local_err: *mut bindings::Error = ptr::null_mut();
+
+ let ret = Error::ptr_or_propagate(Ok("abc".to_owned()), &mut local_err);
+ assert_eq!(String::from_foreign(ret), "abc");
+ assert_eq!(local_err, ptr::null_mut());
+
+ let my_err = Error::from("msg");
+ assert_eq!(
+ Error::ptr_or_propagate(Err::<String, _>(my_err), &mut local_err),
+ ptr::null_mut()
+ );
+ assert_ne!(local_err, ptr::null_mut());
+ assert_eq!(error_get_pretty(local_err), c"msg");
+ bindings::error_free(local_err);
+ }
+ }
+
+ #[test]
+ fn test_err_or_unit() {
+ unsafe {
+ let result = Error::err_or_unit(ptr::null_mut());
+ assert_match!(result, Ok(()));
+
+ let err = error_for_test(c"msg");
+ let err = Error::err_or_unit(err.into_inner()).unwrap_err();
+ assert_eq!(&*format!("{err}"), "msg");
+ }
+ }
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 08/31] rust: qdev: support returning errors from realize
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (6 preceding siblings ...)
2025-06-06 12:34 ` [PULL 07/31] rust: qemu-api: add tests for Error bindings Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 09/31] rust/hpet: change type of num_timers to usize Paolo Bonzini
` (23 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/char/pl011/src/device.rs | 5 +++--
rust/hw/timer/hpet/src/device.rs | 5 +++--
rust/qemu-api/src/qdev.rs | 12 ++++++++----
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/rust/hw/char/pl011/src/device.rs b/rust/hw/char/pl011/src/device.rs
index 0501fa5be9c..be8387f6f2d 100644
--- a/rust/hw/char/pl011/src/device.rs
+++ b/rust/hw/char/pl011/src/device.rs
@@ -175,7 +175,7 @@ fn properties() -> &'static [Property] {
fn vmsd() -> Option<&'static VMStateDescription> {
Some(&device_class::VMSTATE_PL011)
}
- const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+ const REALIZE: Option<fn(&Self) -> qemu_api::Result<()>> = Some(Self::realize);
}
impl ResettablePhasesImpl for PL011State {
@@ -619,9 +619,10 @@ fn event(&self, event: Event) {
}
}
- fn realize(&self) {
+ fn realize(&self) -> qemu_api::Result<()> {
self.char_backend
.enable_handlers(self, Self::can_receive, Self::receive, Self::event);
+ Ok(())
}
fn reset_hold(&self, _type: ResetType) {
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index e3ba62b2875..68c82b09b60 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -724,7 +724,7 @@ fn post_init(&self) {
}
}
- fn realize(&self) {
+ fn realize(&self) -> qemu_api::Result<()> {
if self.int_route_cap == 0 {
// TODO: Add error binding: warn_report()
println!("Hpet's hpet-intcap property not initialized");
@@ -751,6 +751,7 @@ fn realize(&self) {
self.init_gpio_in(2, HPETState::handle_legacy_irq);
self.init_gpio_out(from_ref(&self.pit_enabled));
+ Ok(())
}
fn reset_hold(&self, _type: ResetType) {
@@ -1042,7 +1043,7 @@ fn vmsd() -> Option<&'static VMStateDescription> {
Some(&VMSTATE_HPET)
}
- const REALIZE: Option<fn(&Self)> = Some(Self::realize);
+ const REALIZE: Option<fn(&Self) -> qemu_api::Result<()>> = Some(Self::realize);
}
impl ResettablePhasesImpl for HPETState {
diff --git a/rust/qemu-api/src/qdev.rs b/rust/qemu-api/src/qdev.rs
index 1279d7a58d5..0610959f467 100644
--- a/rust/qemu-api/src/qdev.rs
+++ b/rust/qemu-api/src/qdev.rs
@@ -12,10 +12,11 @@
pub use bindings::{ClockEvent, DeviceClass, Property, ResetType};
use crate::{
- bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, Error, ResettableClass},
+ bindings::{self, qdev_init_gpio_in, qdev_init_gpio_out, ResettableClass},
callbacks::FnCall,
cell::{bql_locked, Opaque},
chardev::Chardev,
+ error::{Error, Result},
irq::InterruptSource,
prelude::*,
qom::{ObjectClass, ObjectImpl, Owned},
@@ -108,7 +109,7 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
///
/// If not `None`, the parent class's `realize` method is overridden
/// with the function pointed to by `REALIZE`.
- const REALIZE: Option<fn(&Self)> = None;
+ const REALIZE: Option<fn(&Self) -> Result<()>> = None;
/// An array providing the properties that the user can set on the
/// device. Not a `const` because referencing statics in constants
@@ -134,10 +135,13 @@ fn vmsd() -> Option<&'static VMStateDescription> {
/// readable/writeable from one thread at any time.
unsafe extern "C" fn rust_realize_fn<T: DeviceImpl>(
dev: *mut bindings::DeviceState,
- _errp: *mut *mut Error,
+ errp: *mut *mut bindings::Error,
) {
let state = NonNull::new(dev).unwrap().cast::<T>();
- T::REALIZE.unwrap()(unsafe { state.as_ref() });
+ let result = T::REALIZE.unwrap()(unsafe { state.as_ref() });
+ unsafe {
+ Error::ok_or_propagate(result, errp);
+ }
}
unsafe impl InterfaceType for ResettableClass {
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 09/31] rust/hpet: change type of num_timers to usize
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (7 preceding siblings ...)
2025-06-06 12:34 ` [PULL 08/31] rust: qdev: support returning errors from realize Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 10/31] hpet: adjust VMState for consistency with Rust version Paolo Bonzini
` (22 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Remove the need to convert after every read of the BqlCell. Because the
vmstate uses a u8 as the size of the VARRAY, this requires switching
the VARRAY to use num_timers_save; which in turn requires ensuring that
the num_timers_save is always there. For simplicity do this by
removing support for version 1, which QEMU has not been producing for
~15 years.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index 68c82b09b60..a957de1e767 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.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,
},
cell::{BqlCell, BqlRefCell},
irq::InterruptSource,
@@ -36,9 +36,9 @@
const HPET_REG_SPACE_LEN: u64 = 0x400; // 1024 bytes
/// Minimum recommended hardware implementation.
-const HPET_MIN_TIMERS: u8 = 3;
+const HPET_MIN_TIMERS: usize = 3;
/// Maximum timers in each timer block.
-const HPET_MAX_TIMERS: u8 = 32;
+const HPET_MAX_TIMERS: usize = 32;
/// Flags that HPETState.flags supports.
const HPET_FLAG_MSI_SUPPORT_SHIFT: usize = 0;
@@ -561,8 +561,8 @@ pub struct HPETState {
/// HPET timer array managed by this timer block.
#[doc(alias = "timer")]
- timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS as usize],
- num_timers: BqlCell<u8>,
+ timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
+ num_timers: BqlCell<usize>,
num_timers_save: BqlCell<u8>,
/// Instance id (HPET timer block ID).
@@ -572,7 +572,7 @@ pub struct HPETState {
impl HPETState {
// Get num_timers with `usize` type, which is useful to play with array index.
fn get_num_timers(&self) -> usize {
- self.num_timers.get().into()
+ self.num_timers.get()
}
const fn has_msi_flag(&self) -> bool {
@@ -854,7 +854,7 @@ fn pre_save(&self) -> i32 {
* also added to the migration stream. Check that it matches the value
* that was configured.
*/
- self.num_timers_save.set(self.num_timers.get());
+ self.num_timers_save.set(self.num_timers.get() as u8);
0
}
@@ -884,7 +884,7 @@ fn is_offset_needed(&self) -> bool {
}
fn validate_num_timers(&self, _version_id: u8) -> bool {
- self.num_timers.get() == self.num_timers_save.get()
+ self.num_timers.get() == self.num_timers_save.get().into()
}
}
@@ -911,7 +911,7 @@ impl ObjectImpl for HPETState {
c"timers",
HPETState,
num_timers,
- unsafe { &qdev_prop_uint8 },
+ unsafe { &qdev_prop_usize },
u8,
default = HPET_MIN_TIMERS
),
@@ -1016,16 +1016,16 @@ impl ObjectImpl for HPETState {
static VMSTATE_HPET: VMStateDescription = VMStateDescription {
name: c"hpet".as_ptr(),
version_id: 2,
- minimum_version_id: 1,
+ minimum_version_id: 2,
pre_save: Some(hpet_pre_save),
post_load: Some(hpet_post_load),
fields: vmstate_fields! {
vmstate_of!(HPETState, config),
vmstate_of!(HPETState, int_status),
vmstate_of!(HPETState, counter),
- vmstate_of!(HPETState, num_timers_save).with_version_id(2),
+ vmstate_of!(HPETState, num_timers_save),
vmstate_validate!(HPETState, VALIDATE_TIMERS_NAME, HPETState::validate_num_timers),
- vmstate_struct!(HPETState, timers[0 .. num_timers], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
+ vmstate_struct!(HPETState, timers[0 .. num_timers_save], &VMSTATE_HPET_TIMER, BqlRefCell<HPETTimer>, HPETState::validate_num_timers).with_version_id(0),
},
subsections: vmstate_subsections! {
VMSTATE_HPET_RTC_IRQ_LEVEL,
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 10/31] hpet: adjust VMState for consistency with Rust version
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (8 preceding siblings ...)
2025-06-06 12:34 ` [PULL 09/31] rust/hpet: change type of num_timers to usize Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 11/31] hpet: return errors from realize if properties are incorrect Paolo Bonzini
` (21 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
No functional change intended.
Suggested-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 0fd1337a156..9db027cf76f 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -328,16 +328,16 @@ static const VMStateDescription vmstate_hpet_timer = {
static const VMStateDescription vmstate_hpet = {
.name = "hpet",
.version_id = 2,
- .minimum_version_id = 1,
+ .minimum_version_id = 2,
.pre_save = hpet_pre_save,
.post_load = hpet_post_load,
.fields = (const VMStateField[]) {
VMSTATE_UINT64(config, HPETState),
VMSTATE_UINT64(isr, HPETState),
VMSTATE_UINT64(hpet_counter, HPETState),
- VMSTATE_UINT8_V(num_timers_save, HPETState, 2),
+ VMSTATE_UINT8(num_timers_save, HPETState),
VMSTATE_VALIDATE("num_timers must match", hpet_validate_num_timers),
- VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
+ VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers_save, 0,
vmstate_hpet_timer, HPETTimer),
VMSTATE_END_OF_LIST()
},
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 11/31] hpet: return errors from realize if properties are incorrect
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (9 preceding siblings ...)
2025-06-06 12:34 ` [PULL 10/31] hpet: adjust VMState for consistency with Rust version Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 12/31] rust/hpet: " Paolo Bonzini
` (20 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Zhao Liu
Do not silently adjust num_timers, and fail if intcap is 0.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/timer/hpet.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 9db027cf76f..cb48cc151f1 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -691,8 +691,14 @@ static void hpet_realize(DeviceState *dev, Error **errp)
int i;
HPETTimer *timer;
+ if (s->num_timers < HPET_MIN_TIMERS || s->num_timers > HPET_MAX_TIMERS) {
+ error_setg(errp, "hpet.num_timers must be between %d and %d",
+ HPET_MIN_TIMERS, HPET_MAX_TIMERS);
+ return;
+ }
if (!s->intcap) {
- warn_report("Hpet's intcap not initialized");
+ error_setg(errp, "hpet.hpet-intcap not initialized");
+ return;
}
if (hpet_fw_cfg.count == UINT8_MAX) {
/* first instance */
@@ -700,7 +706,7 @@ static void hpet_realize(DeviceState *dev, Error **errp)
}
if (hpet_fw_cfg.count == 8) {
- error_setg(errp, "Only 8 instances of HPET is allowed");
+ error_setg(errp, "Only 8 instances of HPET are allowed");
return;
}
@@ -710,11 +716,6 @@ static void hpet_realize(DeviceState *dev, Error **errp)
sysbus_init_irq(sbd, &s->irqs[i]);
}
- if (s->num_timers < HPET_MIN_TIMERS) {
- s->num_timers = HPET_MIN_TIMERS;
- } else if (s->num_timers > HPET_MAX_TIMERS) {
- s->num_timers = HPET_MAX_TIMERS;
- }
for (i = 0; i < HPET_MAX_TIMERS; i++) {
timer = &s->timer[i];
timer->qemu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, hpet_timer, timer);
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 12/31] rust/hpet: return errors from realize if properties are incorrect
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (10 preceding siblings ...)
2025-06-06 12:34 ` [PULL 11/31] hpet: return errors from realize if properties are incorrect Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 13/31] rust/hpet: Drop BqlCell wrapper for num_timers Paolo Bonzini
` (19 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
Match the code in hpet.c; this also allows removing the
BqlCell from the num_timers field.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 16 +++++++---------
rust/hw/timer/hpet/src/fw_cfg.rs | 7 +++----
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index a957de1e767..cd439f90b7e 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -725,18 +725,16 @@ fn post_init(&self) {
}
fn realize(&self) -> qemu_api::Result<()> {
+ if self.num_timers.get() < HPET_MIN_TIMERS || self.num_timers.get() > HPET_MAX_TIMERS {
+ Err(format!(
+ "hpet.num_timers must be between {HPET_MIN_TIMERS} and {HPET_MAX_TIMERS}"
+ ))?;
+ }
if self.int_route_cap == 0 {
- // TODO: Add error binding: warn_report()
- println!("Hpet's hpet-intcap property not initialized");
+ Err("hpet.hpet-intcap property not initialized")?;
}
- self.hpet_id.set(HPETFwConfig::assign_hpet_id());
-
- if self.num_timers.get() < HPET_MIN_TIMERS {
- self.num_timers.set(HPET_MIN_TIMERS);
- } else if self.num_timers.get() > HPET_MAX_TIMERS {
- self.num_timers.set(HPET_MAX_TIMERS);
- }
+ self.hpet_id.set(HPETFwConfig::assign_hpet_id()?);
self.init_timer();
// 64-bit General Capabilities and ID Register; LegacyReplacementRoute.
diff --git a/rust/hw/timer/hpet/src/fw_cfg.rs b/rust/hw/timer/hpet/src/fw_cfg.rs
index 6c10316104c..619d662ee1e 100644
--- a/rust/hw/timer/hpet/src/fw_cfg.rs
+++ b/rust/hw/timer/hpet/src/fw_cfg.rs
@@ -36,7 +36,7 @@ unsafe impl Zeroable for HPETFwConfig {}
};
impl HPETFwConfig {
- pub(crate) fn assign_hpet_id() -> usize {
+ pub(crate) fn assign_hpet_id() -> Result<usize, &'static str> {
assert!(bql_locked());
// SAFETY: all accesses go through these methods, which guarantee
// that the accesses are protected by the BQL.
@@ -48,13 +48,12 @@ pub(crate) fn assign_hpet_id() -> usize {
}
if fw_cfg.count == 8 {
- // TODO: Add error binding: error_setg()
- panic!("Only 8 instances of HPET is allowed");
+ Err("Only 8 instances of HPET are allowed")?;
}
let id: usize = fw_cfg.count.into();
fw_cfg.count += 1;
- id
+ Ok(id)
}
pub(crate) fn update_hpet_cfg(hpet_id: usize, timer_block_id: u32, address: u64) {
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 13/31] rust/hpet: Drop BqlCell wrapper for num_timers
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (11 preceding siblings ...)
2025-06-06 12:34 ` [PULL 12/31] rust/hpet: " Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 14/31] docs: update Rust module status Paolo Bonzini
` (18 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
From: Zhao Liu <zhao1.liu@intel.com>
Now that the num_timers field is initialized as a property, someone may
change its default value using qdev_prop_set_uint8(), but the value is
fixed after the Rust code sees it first. Since there is no need to modify
it after realize(), it is not to be necessary to have a BqlCell wrapper.
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250520152750.2542612-4-zhao1.liu@intel.com
[Remove .into() as well. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/hw/timer/hpet/src/device.rs | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/rust/hw/timer/hpet/src/device.rs b/rust/hw/timer/hpet/src/device.rs
index cd439f90b7e..735b2fbef79 100644
--- a/rust/hw/timer/hpet/src/device.rs
+++ b/rust/hw/timer/hpet/src/device.rs
@@ -562,7 +562,7 @@ pub struct HPETState {
/// HPET timer array managed by this timer block.
#[doc(alias = "timer")]
timers: [BqlRefCell<HPETTimer>; HPET_MAX_TIMERS],
- num_timers: BqlCell<usize>,
+ num_timers: usize,
num_timers_save: BqlCell<u8>,
/// Instance id (HPET timer block ID).
@@ -570,11 +570,6 @@ pub struct HPETState {
}
impl HPETState {
- // Get num_timers with `usize` type, which is useful to play with array index.
- fn get_num_timers(&self) -> usize {
- self.num_timers.get()
- }
-
const fn has_msi_flag(&self) -> bool {
self.flags & (1 << HPET_FLAG_MSI_SUPPORT_SHIFT) != 0
}
@@ -636,7 +631,7 @@ fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
self.hpet_offset
.set(ticks_to_ns(self.counter.get()) - CLOCK_VIRTUAL.get_ns());
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
let mut t = timer.borrow_mut();
if t.is_int_enabled() && t.is_int_active() {
@@ -648,7 +643,7 @@ fn set_cfg_reg(&self, shift: u32, len: u32, val: u64) {
// Halt main counter and disable interrupt generation.
self.counter.set(self.get_ticks());
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
timer.borrow_mut().del_timer();
}
}
@@ -671,7 +666,7 @@ fn set_int_status_reg(&self, shift: u32, _len: u32, val: u64) {
let new_val = val << shift;
let cleared = new_val & self.int_status.get();
- for (index, timer) in self.timers.iter().take(self.get_num_timers()).enumerate() {
+ for (index, timer) in self.timers.iter().take(self.num_timers).enumerate() {
if cleared & (1 << index) != 0 {
timer.borrow_mut().update_irq(false);
}
@@ -725,7 +720,7 @@ fn post_init(&self) {
}
fn realize(&self) -> qemu_api::Result<()> {
- if self.num_timers.get() < HPET_MIN_TIMERS || self.num_timers.get() > HPET_MAX_TIMERS {
+ if self.num_timers < HPET_MIN_TIMERS || self.num_timers > HPET_MAX_TIMERS {
Err(format!(
"hpet.num_timers must be between {HPET_MIN_TIMERS} and {HPET_MAX_TIMERS}"
))?;
@@ -743,7 +738,7 @@ fn realize(&self) -> qemu_api::Result<()> {
1 << HPET_CAP_COUNT_SIZE_CAP_SHIFT |
1 << HPET_CAP_LEG_RT_CAP_SHIFT |
HPET_CAP_VENDER_ID_VALUE << HPET_CAP_VENDER_ID_SHIFT |
- ((self.get_num_timers() - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
+ ((self.num_timers - 1) as u64) << HPET_CAP_NUM_TIM_SHIFT | // indicate the last timer
(HPET_CLK_PERIOD * FS_PER_NS) << HPET_CAP_CNT_CLK_PERIOD_SHIFT, // 10 ns
);
@@ -753,7 +748,7 @@ fn realize(&self) -> qemu_api::Result<()> {
}
fn reset_hold(&self, _type: ResetType) {
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
timer.borrow_mut().reset();
}
@@ -781,7 +776,7 @@ fn decode(&self, mut addr: hwaddr, size: u32) -> HPETAddrDecode {
GlobalRegister::try_from(addr).map(HPETRegister::Global)
} else {
let timer_id: usize = ((addr - 0x100) / 0x20) as usize;
- if timer_id <= self.get_num_timers() {
+ if timer_id < self.num_timers {
// TODO: Add trace point - trace_hpet_ram_[read|write]_timer_id(timer_id)
TimerRegister::try_from(addr & 0x18)
.map(|reg| HPETRegister::Timer(&self.timers[timer_id], reg))
@@ -852,12 +847,12 @@ fn pre_save(&self) -> i32 {
* also added to the migration stream. Check that it matches the value
* that was configured.
*/
- self.num_timers_save.set(self.num_timers.get() as u8);
+ self.num_timers_save.set(self.num_timers as u8);
0
}
fn post_load(&self, _version_id: u8) -> i32 {
- for timer in self.timers.iter().take(self.get_num_timers()) {
+ for timer in self.timers.iter().take(self.num_timers) {
let mut t = timer.borrow_mut();
t.cmp64 = t.calculate_cmp64(t.get_state().counter.get(), t.cmp);
@@ -882,7 +877,7 @@ fn is_offset_needed(&self) -> bool {
}
fn validate_num_timers(&self, _version_id: u8) -> bool {
- self.num_timers.get() == self.num_timers_save.get().into()
+ self.num_timers == self.num_timers_save.get().into()
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 14/31] docs: update Rust module status
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (12 preceding siblings ...)
2025-06-06 12:34 ` [PULL 13/31] rust/hpet: Drop BqlCell wrapper for num_timers Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 15/31] rust: make TryFrom macro more resilient Paolo Bonzini
` (17 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Zhao Liu
error is new; offset_of is gone.
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/rust.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/devel/rust.rst b/docs/devel/rust.rst
index d60d56d0a69..47e9677fcb0 100644
--- a/docs/devel/rust.rst
+++ b/docs/devel/rust.rst
@@ -160,10 +160,10 @@ module status
``callbacks`` complete
``cell`` stable
``errno`` complete
+``error`` stable
``irq`` complete
``memory`` stable
``module`` complete
-``offset_of`` stable
``qdev`` stable
``qom`` stable
``sysbus`` stable
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 15/31] rust: make TryFrom macro more resilient
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (13 preceding siblings ...)
2025-06-06 12:34 ` [PULL 14/31] docs: update Rust module status Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 16/31] i386/kvm: Prefault memory on page state change Paolo Bonzini
` (16 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel
If the enum includes values such as "Ok", "Err", or "Error", the TryInto
macro can cause errors. Be careful and qualify identifiers with the full
path, or in the case of TryFrom<>::Error do not use the associated type
at all.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
rust/qemu-api-macros/src/lib.rs | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/rust/qemu-api-macros/src/lib.rs b/rust/qemu-api-macros/src/lib.rs
index 103470785e3..c18bb4e036f 100644
--- a/rust/qemu-api-macros/src/lib.rs
+++ b/rust/qemu-api-macros/src/lib.rs
@@ -203,8 +203,8 @@ fn derive_tryinto_body(
Ok(quote! {
#(const #discriminants: #repr = #name::#discriminants as #repr;)*;
match value {
- #(#discriminants => Ok(#name::#discriminants),)*
- _ => Err(value),
+ #(#discriminants => core::result::Result::Ok(#name::#discriminants),)*
+ _ => core::result::Result::Err(value),
}
})
}
@@ -236,7 +236,8 @@ pub const fn from_bits(value: #repr) -> Self {
impl core::convert::TryFrom<#repr> for #name {
type Error = #repr;
- fn try_from(value: #repr) -> Result<Self, Self::Error> {
+ #[allow(ambiguous_associated_items)]
+ fn try_from(value: #repr) -> Result<Self, #repr> {
#body
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 16/31] i386/kvm: Prefault memory on page state change
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (14 preceding siblings ...)
2025-06-06 12:34 ` [PULL 15/31] rust: make TryFrom macro more resilient Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-11 2:55 ` Xiaoyao Li
2025-06-06 12:34 ` [PULL 17/31] futex: Check value after qemu_futex_wait() Paolo Bonzini
` (15 subsequent siblings)
31 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Tom Lendacky
From: Tom Lendacky <thomas.lendacky@amd.com>
A page state change is typically followed by an access of the page(s) and
results in another VMEXIT in order to map the page into the nested page
table. Depending on the size of page state change request, this can
generate a number of additional VMEXITs. For example, under SNP, when
Linux is utilizing lazy memory acceptance, memory is typically accepted in
4M chunks. A page state change request is submitted to mark the pages as
private, followed by validation of the memory. Since the guest_memfd
currently only supports 4K pages, each page validation will result in
VMEXIT to map the page, resulting in 1024 additional exits.
When performing a page state change, invoke KVM_PRE_FAULT_MEMORY for the
size of the page state change in order to pre-map the pages and avoid the
additional VMEXITs. This helps speed up boot times.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://lore.kernel.org/r/f5411c42340bd2f5c14972551edb4e959995e42b.1743193824.git.thomas.lendacky@amd.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/system/kvm.h | 1 +
accel/kvm/kvm-all.c | 2 ++
target/i386/kvm/kvm.c | 31 ++++++++++++++++++++++++++-----
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/include/system/kvm.h b/include/system/kvm.h
index 62ec131d4d8..7cc60d26f24 100644
--- a/include/system/kvm.h
+++ b/include/system/kvm.h
@@ -42,6 +42,7 @@ extern bool kvm_gsi_routing_allowed;
extern bool kvm_gsi_direct_mapping;
extern bool kvm_readonly_mem_allowed;
extern bool kvm_msi_use_devid;
+extern bool kvm_pre_fault_memory_supported;
#define kvm_enabled() (kvm_allowed)
/**
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 51526d301b9..a31778341c2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -99,6 +99,7 @@ bool kvm_allowed;
bool kvm_readonly_mem_allowed;
bool kvm_vm_attributes_allowed;
bool kvm_msi_use_devid;
+bool kvm_pre_fault_memory_supported;
static bool kvm_has_guest_debug;
static int kvm_sstep_flags;
static bool kvm_immediate_exit;
@@ -2745,6 +2746,7 @@ static int kvm_init(MachineState *ms)
kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
kvm_check_extension(s, KVM_CAP_USER_MEMORY2) &&
(kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE);
+ kvm_pre_fault_memory_supported = kvm_vm_check_extension(s, KVM_CAP_PRE_FAULT_MEMORY);
if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) {
s->kernel_irqchip_split = mc->default_kernel_irqchip_split ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index a6bc089d020..56a6b9b6381 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -6018,9 +6018,11 @@ static bool host_supports_vmx(void)
* because private/shared page tracking is already provided through other
* means, these 2 use-cases should be treated as being mutually-exclusive.
*/
-static int kvm_handle_hc_map_gpa_range(struct kvm_run *run)
+static int kvm_handle_hc_map_gpa_range(X86CPU *cpu, struct kvm_run *run)
{
+ struct kvm_pre_fault_memory mem;
uint64_t gpa, size, attributes;
+ int ret;
if (!machine_require_guest_memfd(current_machine))
return -EINVAL;
@@ -6031,13 +6033,32 @@ static int kvm_handle_hc_map_gpa_range(struct kvm_run *run)
trace_kvm_hc_map_gpa_range(gpa, size, attributes, run->hypercall.flags);
- return kvm_convert_memory(gpa, size, attributes & KVM_MAP_GPA_RANGE_ENCRYPTED);
+ ret = kvm_convert_memory(gpa, size, attributes & KVM_MAP_GPA_RANGE_ENCRYPTED);
+ if (ret || !kvm_pre_fault_memory_supported) {
+ return ret;
+ }
+
+ /*
+ * Opportunistically pre-fault memory in. Failures are ignored so that any
+ * errors in faulting in the memory will get captured in KVM page fault
+ * path when the guest first accesses the page.
+ */
+ memset(&mem, 0, sizeof(mem));
+ mem.gpa = gpa;
+ mem.size = size;
+ while (mem.size) {
+ if (kvm_vcpu_ioctl(CPU(cpu), KVM_PRE_FAULT_MEMORY, &mem)) {
+ break;
+ }
+ }
+
+ return 0;
}
-static int kvm_handle_hypercall(struct kvm_run *run)
+static int kvm_handle_hypercall(X86CPU *cpu, struct kvm_run *run)
{
if (run->hypercall.nr == KVM_HC_MAP_GPA_RANGE)
- return kvm_handle_hc_map_gpa_range(run);
+ return kvm_handle_hc_map_gpa_range(cpu, run);
return -EINVAL;
}
@@ -6137,7 +6158,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
break;
#endif
case KVM_EXIT_HYPERCALL:
- ret = kvm_handle_hypercall(run);
+ ret = kvm_handle_hypercall(cpu, run);
break;
case KVM_EXIT_SYSTEM_EVENT:
switch (run->system_event.type) {
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PULL 16/31] i386/kvm: Prefault memory on page state change
2025-06-06 12:34 ` [PULL 16/31] i386/kvm: Prefault memory on page state change Paolo Bonzini
@ 2025-06-11 2:55 ` Xiaoyao Li
2025-06-11 6:12 ` Paolo Bonzini
0 siblings, 1 reply; 36+ messages in thread
From: Xiaoyao Li @ 2025-06-11 2:55 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: Tom Lendacky, Edgecombe, Rick P
Paolo,
This one is not supposed to be pulled until we fix KVM as you said:
https://lore.kernel.org/qemu-devel/d0983ba3-383b-4c81-9cfd-b5b0d26a5d17@redhat.com/
BTW, I have sent the KVM fix:
https://lore.kernel.org/all/20250611001018.2179964-1-xiaoyao.li@intel.com/
On 6/6/2025 8:34 PM, Paolo Bonzini wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
>
> A page state change is typically followed by an access of the page(s) and
> results in another VMEXIT in order to map the page into the nested page
> table. Depending on the size of page state change request, this can
> generate a number of additional VMEXITs. For example, under SNP, when
> Linux is utilizing lazy memory acceptance, memory is typically accepted in
> 4M chunks. A page state change request is submitted to mark the pages as
> private, followed by validation of the memory. Since the guest_memfd
> currently only supports 4K pages, each page validation will result in
> VMEXIT to map the page, resulting in 1024 additional exits.
>
> When performing a page state change, invoke KVM_PRE_FAULT_MEMORY for the
> size of the page state change in order to pre-map the pages and avoid the
> additional VMEXITs. This helps speed up boot times.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> Link: https://lore.kernel.org/r/f5411c42340bd2f5c14972551edb4e959995e42b.1743193824.git.thomas.lendacky@amd.com
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/system/kvm.h | 1 +
> accel/kvm/kvm-all.c | 2 ++
> target/i386/kvm/kvm.c | 31 ++++++++++++++++++++++++++-----
> 3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/include/system/kvm.h b/include/system/kvm.h
> index 62ec131d4d8..7cc60d26f24 100644
> --- a/include/system/kvm.h
> +++ b/include/system/kvm.h
> @@ -42,6 +42,7 @@ extern bool kvm_gsi_routing_allowed;
> extern bool kvm_gsi_direct_mapping;
> extern bool kvm_readonly_mem_allowed;
> extern bool kvm_msi_use_devid;
> +extern bool kvm_pre_fault_memory_supported;
>
> #define kvm_enabled() (kvm_allowed)
> /**
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 51526d301b9..a31778341c2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -99,6 +99,7 @@ bool kvm_allowed;
> bool kvm_readonly_mem_allowed;
> bool kvm_vm_attributes_allowed;
> bool kvm_msi_use_devid;
> +bool kvm_pre_fault_memory_supported;
> static bool kvm_has_guest_debug;
> static int kvm_sstep_flags;
> static bool kvm_immediate_exit;
> @@ -2745,6 +2746,7 @@ static int kvm_init(MachineState *ms)
> kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
> kvm_check_extension(s, KVM_CAP_USER_MEMORY2) &&
> (kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE);
> + kvm_pre_fault_memory_supported = kvm_vm_check_extension(s, KVM_CAP_PRE_FAULT_MEMORY);
>
> if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) {
> s->kernel_irqchip_split = mc->default_kernel_irqchip_split ? ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index a6bc089d020..56a6b9b6381 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -6018,9 +6018,11 @@ static bool host_supports_vmx(void)
> * because private/shared page tracking is already provided through other
> * means, these 2 use-cases should be treated as being mutually-exclusive.
> */
> -static int kvm_handle_hc_map_gpa_range(struct kvm_run *run)
> +static int kvm_handle_hc_map_gpa_range(X86CPU *cpu, struct kvm_run *run)
> {
> + struct kvm_pre_fault_memory mem;
> uint64_t gpa, size, attributes;
> + int ret;
>
> if (!machine_require_guest_memfd(current_machine))
> return -EINVAL;
> @@ -6031,13 +6033,32 @@ static int kvm_handle_hc_map_gpa_range(struct kvm_run *run)
>
> trace_kvm_hc_map_gpa_range(gpa, size, attributes, run->hypercall.flags);
>
> - return kvm_convert_memory(gpa, size, attributes & KVM_MAP_GPA_RANGE_ENCRYPTED);
> + ret = kvm_convert_memory(gpa, size, attributes & KVM_MAP_GPA_RANGE_ENCRYPTED);
> + if (ret || !kvm_pre_fault_memory_supported) {
> + return ret;
> + }
> +
> + /*
> + * Opportunistically pre-fault memory in. Failures are ignored so that any
> + * errors in faulting in the memory will get captured in KVM page fault
> + * path when the guest first accesses the page.
> + */
> + memset(&mem, 0, sizeof(mem));
> + mem.gpa = gpa;
> + mem.size = size;
> + while (mem.size) {
> + if (kvm_vcpu_ioctl(CPU(cpu), KVM_PRE_FAULT_MEMORY, &mem)) {
> + break;
> + }
> + }
> +
> + return 0;
> }
>
> -static int kvm_handle_hypercall(struct kvm_run *run)
> +static int kvm_handle_hypercall(X86CPU *cpu, struct kvm_run *run)
> {
> if (run->hypercall.nr == KVM_HC_MAP_GPA_RANGE)
> - return kvm_handle_hc_map_gpa_range(run);
> + return kvm_handle_hc_map_gpa_range(cpu, run);
>
> return -EINVAL;
> }
> @@ -6137,7 +6158,7 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
> break;
> #endif
> case KVM_EXIT_HYPERCALL:
> - ret = kvm_handle_hypercall(run);
> + ret = kvm_handle_hypercall(cpu, run);
> break;
> case KVM_EXIT_SYSTEM_EVENT:
> switch (run->system_event.type) {
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PULL 16/31] i386/kvm: Prefault memory on page state change
2025-06-11 2:55 ` Xiaoyao Li
@ 2025-06-11 6:12 ` Paolo Bonzini
2025-06-11 6:44 ` Xiaoyao Li
0 siblings, 1 reply; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-11 6:12 UTC (permalink / raw)
To: Xiaoyao Li; +Cc: qemu-devel, Tom Lendacky, Edgecombe, Rick P
On Wed, Jun 11, 2025 at 4:56 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>
> Paolo,
>
> This one is not supposed to be pulled until we fix KVM as you said:
> https://lore.kernel.org/qemu-devel/d0983ba3-383b-4c81-9cfd-b5b0d26a5d17@redhat.com/
The bug was understood and fix just a few days away, so I included it.
I'll send the fix to Linus today too.
Paolo
> BTW, I have sent the KVM fix:
> https://lore.kernel.org/all/20250611001018.2179964-1-xiaoyao.li@intel.com/
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PULL 16/31] i386/kvm: Prefault memory on page state change
2025-06-11 6:12 ` Paolo Bonzini
@ 2025-06-11 6:44 ` Xiaoyao Li
0 siblings, 0 replies; 36+ messages in thread
From: Xiaoyao Li @ 2025-06-11 6:44 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Tom Lendacky, Edgecombe, Rick P
On 6/11/2025 2:12 PM, Paolo Bonzini wrote:
> On Wed, Jun 11, 2025 at 4:56 AM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>>
>> Paolo,
>>
>> This one is not supposed to be pulled until we fix KVM as you said:
>> https://lore.kernel.org/qemu-devel/d0983ba3-383b-4c81-9cfd-b5b0d26a5d17@redhat.com/
>
> The bug was understood and fix just a few days away, so I included it.
> I'll send the fix to Linus today too.
I thought kernel fix will go in before QEMU's.
Anyway, it sounds good that you will send KVM pull request today.
Thanks!
> Paolo
>
>> BTW, I have sent the KVM fix:
>> https://lore.kernel.org/all/20250611001018.2179964-1-xiaoyao.li@intel.com/
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PULL 17/31] futex: Check value after qemu_futex_wait()
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (15 preceding siblings ...)
2025-06-06 12:34 ` [PULL 16/31] i386/kvm: Prefault memory on page state change Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 18/31] futex: Support Windows Paolo Bonzini
` (14 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
futex(2) - Linux manual page
https://man7.org/linux/man-pages/man2/futex.2.html
> Note that a wake-up can also be caused by common futex usage patterns
> in unrelated code that happened to have previously used the futex
> word's memory location (e.g., typical futex-based implementations of
> Pthreads mutexes can cause this under some conditions). Therefore,
> callers should always conservatively assume that a return value of 0
> can mean a spurious wake-up, and use the futex word's value (i.e.,
> the user-space synchronization scheme) to decide whether to continue
> to block or not.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/r/20250529-event-v5-1-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/futex.h | 9 +++++++++
tests/unit/test-aio-multithread.c | 4 +++-
util/qemu-thread-posix.c | 24 ++++++++++++------------
3 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/include/qemu/futex.h b/include/qemu/futex.h
index 91ae88966e1..f5777400533 100644
--- a/include/qemu/futex.h
+++ b/include/qemu/futex.h
@@ -24,6 +24,15 @@ static inline void qemu_futex_wake(void *f, int n)
qemu_futex(f, FUTEX_WAKE, n, NULL, NULL, 0);
}
+/*
+ * Note that a wake-up can also be caused by common futex usage patterns in
+ * unrelated code that happened to have previously used the futex word's
+ * memory location (e.g., typical futex-based implementations of Pthreads
+ * mutexes can cause this under some conditions). Therefore, callers should
+ * always conservatively assume that it is a spurious wake-up, and use the futex
+ * word's value (i.e., the user-space synchronization scheme) to decide whether
+ * to continue to block or not.
+ */
static inline void qemu_futex_wait(void *f, unsigned val)
{
while (qemu_futex(f, FUTEX_WAIT, (int) val, NULL, NULL, 0)) {
diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c
index 08d4570ccb1..8c2e41545a2 100644
--- a/tests/unit/test-aio-multithread.c
+++ b/tests/unit/test-aio-multithread.c
@@ -305,7 +305,9 @@ static void mcs_mutex_lock(void)
prev = qatomic_xchg(&mutex_head, id);
if (prev != -1) {
qatomic_set(&nodes[prev].next, id);
- qemu_futex_wait(&nodes[id].locked, 1);
+ while (qatomic_read(&nodes[id].locked) == 1) {
+ qemu_futex_wait(&nodes[id].locked, 1);
+ }
}
}
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index b2e26e21205..f81fca5c116 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -428,17 +428,17 @@ void qemu_event_wait(QemuEvent *ev)
assert(ev->initialized);
- /*
- * qemu_event_wait must synchronize with qemu_event_set even if it does
- * not go down the slow path, so this load-acquire is needed that
- * synchronizes with the first memory barrier in qemu_event_set().
- *
- * If we do go down the slow path, there is no requirement at all: we
- * might miss a qemu_event_set() here but ultimately the memory barrier in
- * qemu_futex_wait() will ensure the check is done correctly.
- */
- value = qatomic_load_acquire(&ev->value);
- if (value != EV_SET) {
+ while (true) {
+ /*
+ * qemu_event_wait must synchronize with qemu_event_set even if it does
+ * not go down the slow path, so this load-acquire is needed that
+ * synchronizes with the first memory barrier in qemu_event_set().
+ */
+ value = qatomic_load_acquire(&ev->value);
+ if (value == EV_SET) {
+ break;
+ }
+
if (value == EV_FREE) {
/*
* Leave the event reset and tell qemu_event_set that there are
@@ -452,7 +452,7 @@ void qemu_event_wait(QemuEvent *ev)
* like the load above.
*/
if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
- return;
+ break;
}
}
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 18/31] futex: Support Windows
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (16 preceding siblings ...)
2025-06-06 12:34 ` [PULL 17/31] futex: Check value after qemu_futex_wait() Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 19/31] qemu-thread: Replace __linux__ with CONFIG_LINUX Paolo Bonzini
` (13 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Windows supports futex-like APIs since Windows 8 and Windows Server
2012.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/r/20250529-event-v5-2-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
meson.build | 7 ++++
include/qemu/futex.h | 53 ++++++++++++++++++++++++-------
tests/unit/test-aio-multithread.c | 2 +-
util/lockcnt.c | 2 +-
util/qemu-thread-posix.c | 4 +--
util/meson.build | 2 +-
6 files changed, 53 insertions(+), 17 deletions(-)
diff --git a/meson.build b/meson.build
index 967a10e80b8..34729c2a3dd 100644
--- a/meson.build
+++ b/meson.build
@@ -838,11 +838,18 @@ emulator_link_args = []
midl = not_found
widl = not_found
pathcch = not_found
+synchronization = not_found
host_dsosuf = '.so'
if host_os == 'windows'
midl = find_program('midl', required: false)
widl = find_program('widl', required: false)
pathcch = cc.find_library('pathcch')
+ synchronization = cc.find_library('Synchronization', required: false)
+ if not synchronization.found()
+ # The library name is lowercase on mingw
+ synchronization = cc.find_library('synchronization', required: true)
+ endif
+
socket = cc.find_library('ws2_32')
winmm = cc.find_library('winmm')
diff --git a/include/qemu/futex.h b/include/qemu/futex.h
index f5777400533..607613eec83 100644
--- a/include/qemu/futex.h
+++ b/include/qemu/futex.h
@@ -1,5 +1,5 @@
/*
- * Wrappers around Linux futex syscall
+ * Wrappers around Linux futex syscall and similar
*
* Copyright Red Hat, Inc. 2017
*
@@ -11,28 +11,37 @@
*
*/
+/*
+ * Note that a wake-up can also be caused by common futex usage patterns in
+ * unrelated code that happened to have previously used the futex word's
+ * memory location (e.g., typical futex-based implementations of Pthreads
+ * mutexes can cause this under some conditions). Therefore, qemu_futex_wait()
+ * callers should always conservatively assume that it is a spurious wake-up,
+ * and use the futex word's value (i.e., the user-space synchronization scheme)
+ * to decide whether to continue to block or not.
+ */
+
#ifndef QEMU_FUTEX_H
#define QEMU_FUTEX_H
+#define HAVE_FUTEX
+
+#ifdef CONFIG_LINUX
#include <sys/syscall.h>
#include <linux/futex.h>
#define qemu_futex(...) syscall(__NR_futex, __VA_ARGS__)
-static inline void qemu_futex_wake(void *f, int n)
+static inline void qemu_futex_wake_all(void *f)
{
- qemu_futex(f, FUTEX_WAKE, n, NULL, NULL, 0);
+ qemu_futex(f, FUTEX_WAKE, INT_MAX, NULL, NULL, 0);
+}
+
+static inline void qemu_futex_wake_single(void *f)
+{
+ qemu_futex(f, FUTEX_WAKE, 1, NULL, NULL, 0);
}
-/*
- * Note that a wake-up can also be caused by common futex usage patterns in
- * unrelated code that happened to have previously used the futex word's
- * memory location (e.g., typical futex-based implementations of Pthreads
- * mutexes can cause this under some conditions). Therefore, callers should
- * always conservatively assume that it is a spurious wake-up, and use the futex
- * word's value (i.e., the user-space synchronization scheme) to decide whether
- * to continue to block or not.
- */
static inline void qemu_futex_wait(void *f, unsigned val)
{
while (qemu_futex(f, FUTEX_WAIT, (int) val, NULL, NULL, 0)) {
@@ -46,5 +55,25 @@ static inline void qemu_futex_wait(void *f, unsigned val)
}
}
}
+#elif defined(CONFIG_WIN32)
+#include <synchapi.h>
+
+static inline void qemu_futex_wake_all(void *f)
+{
+ WakeByAddressAll(f);
+}
+
+static inline void qemu_futex_wake_single(void *f)
+{
+ WakeByAddressSingle(f);
+}
+
+static inline void qemu_futex_wait(void *f, unsigned val)
+{
+ WaitOnAddress(f, &val, sizeof(val), INFINITE);
+}
+#else
+#undef HAVE_FUTEX
+#endif
#endif /* QEMU_FUTEX_H */
diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c
index 8c2e41545a2..0ead6bf34ad 100644
--- a/tests/unit/test-aio-multithread.c
+++ b/tests/unit/test-aio-multithread.c
@@ -330,7 +330,7 @@ static void mcs_mutex_unlock(void)
/* Wake up the next in line. */
next = qatomic_read(&nodes[id].next);
nodes[next].locked = 0;
- qemu_futex_wake(&nodes[next].locked, 1);
+ qemu_futex_wake_single(&nodes[next].locked);
}
static void test_multi_fair_mutex_entry(void *opaque)
diff --git a/util/lockcnt.c b/util/lockcnt.c
index d07c6cc5cee..ca27d8e61a5 100644
--- a/util/lockcnt.c
+++ b/util/lockcnt.c
@@ -106,7 +106,7 @@ static bool qemu_lockcnt_cmpxchg_or_wait(QemuLockCnt *lockcnt, int *val,
static void lockcnt_wake(QemuLockCnt *lockcnt)
{
trace_lockcnt_futex_wake(lockcnt);
- qemu_futex_wake(&lockcnt->count, 1);
+ qemu_futex_wake_single(&lockcnt->count);
}
void qemu_lockcnt_inc(QemuLockCnt *lockcnt)
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index f81fca5c116..f80f564e762 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -345,7 +345,7 @@ static inline void qemu_futex_wait(QemuEvent *ev, unsigned val)
/* Valid transitions:
* - free->set, when setting the event
- * - busy->set, when setting the event, followed by qemu_futex_wake
+ * - busy->set, when setting the event, followed by qemu_futex_wake_all
* - set->free, when resetting the event
* - free->busy, when waiting
*
@@ -400,7 +400,7 @@ void qemu_event_set(QemuEvent *ev)
smp_mb__after_rmw();
if (old == EV_BUSY) {
/* There were waiters, wake them up. */
- qemu_futex_wake(ev, INT_MAX);
+ qemu_futex_wake_all(ev);
}
}
}
diff --git a/util/meson.build b/util/meson.build
index 1adff96ebd5..5735f65f199 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -27,7 +27,7 @@ else
util_ss.add(files('event_notifier-win32.c'))
util_ss.add(files('oslib-win32.c'))
util_ss.add(files('qemu-thread-win32.c'))
- util_ss.add(winmm, pathcch)
+ util_ss.add(winmm, pathcch, synchronization)
endif
util_ss.add(when: linux_io_uring, if_true: files('fdmon-io_uring.c'))
if glib_has_gslice
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 19/31] qemu-thread: Replace __linux__ with CONFIG_LINUX
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (17 preceding siblings ...)
2025-06-06 12:34 ` [PULL 18/31] futex: Support Windows Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 20/31] qemu-thread: Avoid futex abstraction for non-Linux Paolo Bonzini
` (12 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Philippe Mathieu-Daudé
From: Akihiko Odaki <akihiko.odaki@daynix.com>
scripts/checkpatch.pl warns for __linux__ saying "architecture specific
defines should be avoided".
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250526-event-v4-4-5b784cc8e1de@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/thread-posix.h | 2 +-
util/qemu-thread-posix.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index 5f2f3d1386b..c412623a914 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -33,7 +33,7 @@ struct QemuSemaphore {
};
struct QemuEvent {
-#ifndef __linux__
+#ifndef CONFIG_LINUX
pthread_mutex_t lock;
pthread_cond_t cond;
#endif
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index f80f564e762..3dc4d30052e 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -317,7 +317,7 @@ void qemu_sem_wait(QemuSemaphore *sem)
qemu_mutex_unlock(&sem->mutex);
}
-#ifdef __linux__
+#ifdef CONFIG_LINUX
#include "qemu/futex.h"
#else
static inline void qemu_futex_wake(QemuEvent *ev, int n)
@@ -363,7 +363,7 @@ static inline void qemu_futex_wait(QemuEvent *ev, unsigned val)
void qemu_event_init(QemuEvent *ev, bool init)
{
-#ifndef __linux__
+#ifndef CONFIG_LINUX
pthread_mutex_init(&ev->lock, NULL);
pthread_cond_init(&ev->cond, NULL);
#endif
@@ -376,7 +376,7 @@ void qemu_event_destroy(QemuEvent *ev)
{
assert(ev->initialized);
ev->initialized = false;
-#ifndef __linux__
+#ifndef CONFIG_LINUX
pthread_mutex_destroy(&ev->lock);
pthread_cond_destroy(&ev->cond);
#endif
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 20/31] qemu-thread: Avoid futex abstraction for non-Linux
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (18 preceding siblings ...)
2025-06-06 12:34 ` [PULL 19/31] qemu-thread: Replace __linux__ with CONFIG_LINUX Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 21/31] qemu-thread: Use futex for QemuEvent on Windows Paolo Bonzini
` (11 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Phil Dennis-Jordan
From: Akihiko Odaki <akihiko.odaki@daynix.com>
qemu-thread used to abstract pthread primitives into futex for the
QemuEvent implementation of POSIX systems other than Linux. However,
this abstraction has one key difference: unlike futex, pthread
primitives require an explicit destruction, and it must be ordered after
wait and wake operations.
It would be easier to perform destruction if a wait operation ensures
the corresponding wake operation finishes as POSIX semaphore does, but
that requires to protect state accesses in qemu_event_set() and
qemu_event_wait() with a mutex. On the other hand, real futex does not
need such a protection but needs complex barrier and atomic operations
to ensure ordering between the two functions.
Add special implementations of qemu_event_set() and qemu_event_wait()
using pthread primitives. qemu_event_wait() will ensure qemu_event_set()
finishes, and these functions will avoid complex barrier and atomic
operations to ensure ordering between them.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Tested-by: Phil Dennis-Jordan <phil@philjordan.eu>
Reviewed-by: Phil Dennis-Jordan <phil@philjordan.eu>
Link: https://lore.kernel.org/r/20250526-event-v4-5-5b784cc8e1de@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
util/qemu-thread-posix.c | 84 +++++++++++++++++++++++++---------------
1 file changed, 53 insertions(+), 31 deletions(-)
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 3dc4d30052e..7fafbedbc4f 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -319,38 +319,23 @@ void qemu_sem_wait(QemuSemaphore *sem)
#ifdef CONFIG_LINUX
#include "qemu/futex.h"
-#else
-static inline void qemu_futex_wake(QemuEvent *ev, int n)
-{
- assert(ev->initialized);
- pthread_mutex_lock(&ev->lock);
- if (n == 1) {
- pthread_cond_signal(&ev->cond);
- } else {
- pthread_cond_broadcast(&ev->cond);
- }
- pthread_mutex_unlock(&ev->lock);
-}
-
-static inline void qemu_futex_wait(QemuEvent *ev, unsigned val)
-{
- assert(ev->initialized);
- pthread_mutex_lock(&ev->lock);
- if (ev->value == val) {
- pthread_cond_wait(&ev->cond, &ev->lock);
- }
- pthread_mutex_unlock(&ev->lock);
-}
#endif
/* Valid transitions:
- * - free->set, when setting the event
- * - busy->set, when setting the event, followed by qemu_futex_wake_all
- * - set->free, when resetting the event
- * - free->busy, when waiting
+ * - FREE -> SET (qemu_event_set)
+ * - BUSY -> SET (qemu_event_set)
+ * - SET -> FREE (qemu_event_reset)
+ * - FREE -> BUSY (qemu_event_wait)
*
- * set->busy does not happen (it can be observed from the outside but
- * it really is set->free->busy).
+ * With futex, the waking and blocking operations follow
+ * BUSY -> SET and FREE -> BUSY, respectively.
+ *
+ * Without futex, BUSY -> SET and FREE -> BUSY never happen. Instead, the waking
+ * operation follows FREE -> SET and the blocking operation will happen in
+ * qemu_event_wait() if the event is not SET.
+ *
+ * SET->BUSY does not happen (it can be observed from the outside but
+ * it really is SET->FREE->BUSY).
*
* busy->free provably cannot happen; to enforce it, the set->free transition
* is done with an OR, which becomes a no-op if the event has concurrently
@@ -386,6 +371,7 @@ void qemu_event_set(QemuEvent *ev)
{
assert(ev->initialized);
+#ifdef CONFIG_LINUX
/*
* Pairs with both qemu_event_reset() and qemu_event_wait().
*
@@ -403,12 +389,20 @@ void qemu_event_set(QemuEvent *ev)
qemu_futex_wake_all(ev);
}
}
+#else
+ pthread_mutex_lock(&ev->lock);
+ /* Pairs with qemu_event_reset()'s load acquire. */
+ qatomic_store_release(&ev->value, EV_SET);
+ pthread_cond_broadcast(&ev->cond);
+ pthread_mutex_unlock(&ev->lock);
+#endif
}
void qemu_event_reset(QemuEvent *ev)
{
assert(ev->initialized);
+#ifdef CONFIG_LINUX
/*
* If there was a concurrent reset (or even reset+wait),
* do nothing. Otherwise change EV_SET->EV_FREE.
@@ -420,21 +414,42 @@ void qemu_event_reset(QemuEvent *ev)
* Pairs with the first memory barrier in qemu_event_set().
*/
smp_mb__after_rmw();
+#else
+ /*
+ * If futexes are not available, there are no EV_FREE->EV_BUSY
+ * transitions because wakeups are done entirely through the
+ * condition variable. Since qatomic_set() only writes EV_FREE,
+ * the load seems useless but in reality, the acquire synchronizes
+ * with qemu_event_set()'s store release: if qemu_event_reset()
+ * sees EV_SET here, then the caller will certainly see a
+ * successful condition and skip qemu_event_wait():
+ *
+ * done = 1; if (done == 0)
+ * qemu_event_set() { qemu_event_reset() {
+ * lock();
+ * ev->value = EV_SET -----> load ev->value
+ * ev->value = old value | EV_FREE
+ * cond_broadcast()
+ * unlock(); }
+ * } if (done == 0)
+ * // qemu_event_wait() not called
+ */
+ qatomic_set(&ev->value, qatomic_load_acquire(&ev->value) | EV_FREE);
+#endif
}
void qemu_event_wait(QemuEvent *ev)
{
- unsigned value;
-
assert(ev->initialized);
+#ifdef CONFIG_LINUX
while (true) {
/*
* qemu_event_wait must synchronize with qemu_event_set even if it does
* not go down the slow path, so this load-acquire is needed that
* synchronizes with the first memory barrier in qemu_event_set().
*/
- value = qatomic_load_acquire(&ev->value);
+ unsigned value = qatomic_load_acquire(&ev->value);
if (value == EV_SET) {
break;
}
@@ -463,6 +478,13 @@ void qemu_event_wait(QemuEvent *ev)
*/
qemu_futex_wait(ev, EV_BUSY);
}
+#else
+ pthread_mutex_lock(&ev->lock);
+ while (qatomic_read(&ev->value) != EV_SET) {
+ pthread_cond_wait(&ev->cond, &ev->lock);
+ }
+ pthread_mutex_unlock(&ev->lock);
+#endif
}
static __thread NotifierList thread_exit;
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 21/31] qemu-thread: Use futex for QemuEvent on Windows
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (19 preceding siblings ...)
2025-06-06 12:34 ` [PULL 20/31] qemu-thread: Avoid futex abstraction for non-Linux Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 22/31] qemu-thread: Use futex if available for QemuLockCnt Paolo Bonzini
` (10 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Philippe Mathieu-Daudé
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Use the futex-based implementation of QemuEvent on Windows to
remove code duplication and remove the overhead of event object
construction and destruction.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250526-event-v4-6-5b784cc8e1de@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/thread-posix.h | 9 --
include/qemu/thread-win32.h | 6 --
include/qemu/thread.h | 11 ++-
util/event.c | 171 ++++++++++++++++++++++++++++++++++++
util/qemu-thread-posix.c | 170 -----------------------------------
util/qemu-thread-win32.c | 129 ---------------------------
util/meson.build | 1 +
7 files changed, 182 insertions(+), 315 deletions(-)
create mode 100644 util/event.c
diff --git a/include/qemu/thread-posix.h b/include/qemu/thread-posix.h
index c412623a914..758808b705e 100644
--- a/include/qemu/thread-posix.h
+++ b/include/qemu/thread-posix.h
@@ -32,15 +32,6 @@ struct QemuSemaphore {
unsigned int count;
};
-struct QemuEvent {
-#ifndef CONFIG_LINUX
- pthread_mutex_t lock;
- pthread_cond_t cond;
-#endif
- unsigned value;
- bool initialized;
-};
-
struct QemuThread {
pthread_t thread;
};
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index d95af4498fc..da9e7322990 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -28,12 +28,6 @@ struct QemuSemaphore {
bool initialized;
};
-struct QemuEvent {
- int value;
- HANDLE event;
- bool initialized;
-};
-
typedef struct QemuThreadData QemuThreadData;
struct QemuThread {
QemuThreadData *data;
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 6f800aad31a..573f8c9ede2 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -3,13 +3,22 @@
#include "qemu/processor.h"
#include "qemu/atomic.h"
+#include "qemu/futex.h"
typedef struct QemuCond QemuCond;
typedef struct QemuSemaphore QemuSemaphore;
-typedef struct QemuEvent QemuEvent;
typedef struct QemuLockCnt QemuLockCnt;
typedef struct QemuThread QemuThread;
+typedef struct QemuEvent {
+#ifndef HAVE_FUTEX
+ pthread_mutex_t lock;
+ pthread_cond_t cond;
+#endif
+ unsigned value;
+ bool initialized;
+} QemuEvent;
+
#ifdef _WIN32
#include "qemu/thread-win32.h"
#else
diff --git a/util/event.c b/util/event.c
new file mode 100644
index 00000000000..5a8141cd0e4
--- /dev/null
+++ b/util/event.c
@@ -0,0 +1,171 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "qemu/thread.h"
+
+/*
+ * Valid transitions:
+ * - FREE -> SET (qemu_event_set)
+ * - BUSY -> SET (qemu_event_set)
+ * - SET -> FREE (qemu_event_reset)
+ * - FREE -> BUSY (qemu_event_wait)
+ *
+ * With futex, the waking and blocking operations follow
+ * BUSY -> SET and FREE -> BUSY, respectively.
+ *
+ * Without futex, BUSY -> SET and FREE -> BUSY never happen. Instead, the waking
+ * operation follows FREE -> SET and the blocking operation will happen in
+ * qemu_event_wait() if the event is not SET.
+ *
+ * SET->BUSY does not happen (it can be observed from the outside but
+ * it really is SET->FREE->BUSY).
+ *
+ * busy->free provably cannot happen; to enforce it, the set->free transition
+ * is done with an OR, which becomes a no-op if the event has concurrently
+ * transitioned to free or busy.
+ */
+
+#define EV_SET 0
+#define EV_FREE 1
+#define EV_BUSY -1
+
+void qemu_event_init(QemuEvent *ev, bool init)
+{
+#ifndef HAVE_FUTEX
+ pthread_mutex_init(&ev->lock, NULL);
+ pthread_cond_init(&ev->cond, NULL);
+#endif
+
+ ev->value = (init ? EV_SET : EV_FREE);
+ ev->initialized = true;
+}
+
+void qemu_event_destroy(QemuEvent *ev)
+{
+ assert(ev->initialized);
+ ev->initialized = false;
+#ifndef HAVE_FUTEX
+ pthread_mutex_destroy(&ev->lock);
+ pthread_cond_destroy(&ev->cond);
+#endif
+}
+
+void qemu_event_set(QemuEvent *ev)
+{
+ assert(ev->initialized);
+
+#ifdef HAVE_FUTEX
+ /*
+ * Pairs with both qemu_event_reset() and qemu_event_wait().
+ *
+ * qemu_event_set has release semantics, but because it *loads*
+ * ev->value we need a full memory barrier here.
+ */
+ smp_mb();
+ if (qatomic_read(&ev->value) != EV_SET) {
+ int old = qatomic_xchg(&ev->value, EV_SET);
+
+ /* Pairs with memory barrier in kernel futex_wait system call. */
+ smp_mb__after_rmw();
+ if (old == EV_BUSY) {
+ /* There were waiters, wake them up. */
+ qemu_futex_wake_all(ev);
+ }
+ }
+#else
+ pthread_mutex_lock(&ev->lock);
+ /* Pairs with qemu_event_reset()'s load acquire. */
+ qatomic_store_release(&ev->value, EV_SET);
+ pthread_cond_broadcast(&ev->cond);
+ pthread_mutex_unlock(&ev->lock);
+#endif
+}
+
+void qemu_event_reset(QemuEvent *ev)
+{
+ assert(ev->initialized);
+
+#ifdef HAVE_FUTEX
+ /*
+ * If there was a concurrent reset (or even reset+wait),
+ * do nothing. Otherwise change EV_SET->EV_FREE.
+ */
+ qatomic_or(&ev->value, EV_FREE);
+
+ /*
+ * Order reset before checking the condition in the caller.
+ * Pairs with the first memory barrier in qemu_event_set().
+ */
+ smp_mb__after_rmw();
+#else
+ /*
+ * If futexes are not available, there are no EV_FREE->EV_BUSY
+ * transitions because wakeups are done entirely through the
+ * condition variable. Since qatomic_set() only writes EV_FREE,
+ * the load seems useless but in reality, the acquire synchronizes
+ * with qemu_event_set()'s store release: if qemu_event_reset()
+ * sees EV_SET here, then the caller will certainly see a
+ * successful condition and skip qemu_event_wait():
+ *
+ * done = 1; if (done == 0)
+ * qemu_event_set() { qemu_event_reset() {
+ * lock();
+ * ev->value = EV_SET -----> load ev->value
+ * ev->value = old value | EV_FREE
+ * cond_broadcast()
+ * unlock(); }
+ * } if (done == 0)
+ * // qemu_event_wait() not called
+ */
+ qatomic_set(&ev->value, qatomic_load_acquire(&ev->value) | EV_FREE);
+#endif
+}
+
+void qemu_event_wait(QemuEvent *ev)
+{
+ assert(ev->initialized);
+
+#ifdef HAVE_FUTEX
+ while (true) {
+ /*
+ * qemu_event_wait must synchronize with qemu_event_set even if it does
+ * not go down the slow path, so this load-acquire is needed that
+ * synchronizes with the first memory barrier in qemu_event_set().
+ */
+ unsigned value = qatomic_load_acquire(&ev->value);
+ if (value == EV_SET) {
+ break;
+ }
+
+ if (value == EV_FREE) {
+ /*
+ * Leave the event reset and tell qemu_event_set that there are
+ * waiters. No need to retry, because there cannot be a concurrent
+ * busy->free transition. After the CAS, the event will be either
+ * set or busy.
+ *
+ * This cmpxchg doesn't have particular ordering requirements if it
+ * succeeds (moving the store earlier can only cause
+ * qemu_event_set() to issue _more_ wakeups), the failing case needs
+ * acquire semantics like the load above.
+ */
+ if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
+ break;
+ }
+ }
+
+ /*
+ * This is the final check for a concurrent set, so it does need
+ * a smp_mb() pairing with the second barrier of qemu_event_set().
+ * The barrier is inside the FUTEX_WAIT system call.
+ */
+ qemu_futex_wait(ev, EV_BUSY);
+ }
+#else
+ pthread_mutex_lock(&ev->lock);
+ while (qatomic_read(&ev->value) != EV_SET) {
+ pthread_cond_wait(&ev->cond, &ev->lock);
+ }
+ pthread_mutex_unlock(&ev->lock);
+#endif
+}
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 7fafbedbc4f..ba725444ba6 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -317,176 +317,6 @@ void qemu_sem_wait(QemuSemaphore *sem)
qemu_mutex_unlock(&sem->mutex);
}
-#ifdef CONFIG_LINUX
-#include "qemu/futex.h"
-#endif
-
-/* Valid transitions:
- * - FREE -> SET (qemu_event_set)
- * - BUSY -> SET (qemu_event_set)
- * - SET -> FREE (qemu_event_reset)
- * - FREE -> BUSY (qemu_event_wait)
- *
- * With futex, the waking and blocking operations follow
- * BUSY -> SET and FREE -> BUSY, respectively.
- *
- * Without futex, BUSY -> SET and FREE -> BUSY never happen. Instead, the waking
- * operation follows FREE -> SET and the blocking operation will happen in
- * qemu_event_wait() if the event is not SET.
- *
- * SET->BUSY does not happen (it can be observed from the outside but
- * it really is SET->FREE->BUSY).
- *
- * busy->free provably cannot happen; to enforce it, the set->free transition
- * is done with an OR, which becomes a no-op if the event has concurrently
- * transitioned to free or busy.
- */
-
-#define EV_SET 0
-#define EV_FREE 1
-#define EV_BUSY -1
-
-void qemu_event_init(QemuEvent *ev, bool init)
-{
-#ifndef CONFIG_LINUX
- pthread_mutex_init(&ev->lock, NULL);
- pthread_cond_init(&ev->cond, NULL);
-#endif
-
- ev->value = (init ? EV_SET : EV_FREE);
- ev->initialized = true;
-}
-
-void qemu_event_destroy(QemuEvent *ev)
-{
- assert(ev->initialized);
- ev->initialized = false;
-#ifndef CONFIG_LINUX
- pthread_mutex_destroy(&ev->lock);
- pthread_cond_destroy(&ev->cond);
-#endif
-}
-
-void qemu_event_set(QemuEvent *ev)
-{
- assert(ev->initialized);
-
-#ifdef CONFIG_LINUX
- /*
- * Pairs with both qemu_event_reset() and qemu_event_wait().
- *
- * qemu_event_set has release semantics, but because it *loads*
- * ev->value we need a full memory barrier here.
- */
- smp_mb();
- if (qatomic_read(&ev->value) != EV_SET) {
- int old = qatomic_xchg(&ev->value, EV_SET);
-
- /* Pairs with memory barrier in kernel futex_wait system call. */
- smp_mb__after_rmw();
- if (old == EV_BUSY) {
- /* There were waiters, wake them up. */
- qemu_futex_wake_all(ev);
- }
- }
-#else
- pthread_mutex_lock(&ev->lock);
- /* Pairs with qemu_event_reset()'s load acquire. */
- qatomic_store_release(&ev->value, EV_SET);
- pthread_cond_broadcast(&ev->cond);
- pthread_mutex_unlock(&ev->lock);
-#endif
-}
-
-void qemu_event_reset(QemuEvent *ev)
-{
- assert(ev->initialized);
-
-#ifdef CONFIG_LINUX
- /*
- * If there was a concurrent reset (or even reset+wait),
- * do nothing. Otherwise change EV_SET->EV_FREE.
- */
- qatomic_or(&ev->value, EV_FREE);
-
- /*
- * Order reset before checking the condition in the caller.
- * Pairs with the first memory barrier in qemu_event_set().
- */
- smp_mb__after_rmw();
-#else
- /*
- * If futexes are not available, there are no EV_FREE->EV_BUSY
- * transitions because wakeups are done entirely through the
- * condition variable. Since qatomic_set() only writes EV_FREE,
- * the load seems useless but in reality, the acquire synchronizes
- * with qemu_event_set()'s store release: if qemu_event_reset()
- * sees EV_SET here, then the caller will certainly see a
- * successful condition and skip qemu_event_wait():
- *
- * done = 1; if (done == 0)
- * qemu_event_set() { qemu_event_reset() {
- * lock();
- * ev->value = EV_SET -----> load ev->value
- * ev->value = old value | EV_FREE
- * cond_broadcast()
- * unlock(); }
- * } if (done == 0)
- * // qemu_event_wait() not called
- */
- qatomic_set(&ev->value, qatomic_load_acquire(&ev->value) | EV_FREE);
-#endif
-}
-
-void qemu_event_wait(QemuEvent *ev)
-{
- assert(ev->initialized);
-
-#ifdef CONFIG_LINUX
- while (true) {
- /*
- * qemu_event_wait must synchronize with qemu_event_set even if it does
- * not go down the slow path, so this load-acquire is needed that
- * synchronizes with the first memory barrier in qemu_event_set().
- */
- unsigned value = qatomic_load_acquire(&ev->value);
- if (value == EV_SET) {
- break;
- }
-
- if (value == EV_FREE) {
- /*
- * Leave the event reset and tell qemu_event_set that there are
- * waiters. No need to retry, because there cannot be a concurrent
- * busy->free transition. After the CAS, the event will be either
- * set or busy.
- *
- * This cmpxchg doesn't have particular ordering requirements if it
- * succeeds (moving the store earlier can only cause qemu_event_set()
- * to issue _more_ wakeups), the failing case needs acquire semantics
- * like the load above.
- */
- if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
- break;
- }
- }
-
- /*
- * This is the final check for a concurrent set, so it does need
- * a smp_mb() pairing with the second barrier of qemu_event_set().
- * The barrier is inside the FUTEX_WAIT system call.
- */
- qemu_futex_wait(ev, EV_BUSY);
- }
-#else
- pthread_mutex_lock(&ev->lock);
- while (qatomic_read(&ev->value) != EV_SET) {
- pthread_cond_wait(&ev->cond, &ev->lock);
- }
- pthread_mutex_unlock(&ev->lock);
-#endif
-}
-
static __thread NotifierList thread_exit;
/*
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index a7fe3cc345f..ca2e0b512e2 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -231,135 +231,6 @@ void qemu_sem_wait(QemuSemaphore *sem)
}
}
-/* Wrap a Win32 manual-reset event with a fast userspace path. The idea
- * is to reset the Win32 event lazily, as part of a test-reset-test-wait
- * sequence. Such a sequence is, indeed, how QemuEvents are used by
- * RCU and other subsystems!
- *
- * Valid transitions:
- * - free->set, when setting the event
- * - busy->set, when setting the event, followed by SetEvent
- * - set->free, when resetting the event
- * - free->busy, when waiting
- *
- * set->busy does not happen (it can be observed from the outside but
- * it really is set->free->busy).
- *
- * busy->free provably cannot happen; to enforce it, the set->free transition
- * is done with an OR, which becomes a no-op if the event has concurrently
- * transitioned to free or busy (and is faster than cmpxchg).
- */
-
-#define EV_SET 0
-#define EV_FREE 1
-#define EV_BUSY -1
-
-void qemu_event_init(QemuEvent *ev, bool init)
-{
- /* Manual reset. */
- ev->event = CreateEvent(NULL, TRUE, TRUE, NULL);
- ev->value = (init ? EV_SET : EV_FREE);
- ev->initialized = true;
-}
-
-void qemu_event_destroy(QemuEvent *ev)
-{
- assert(ev->initialized);
- ev->initialized = false;
- CloseHandle(ev->event);
-}
-
-void qemu_event_set(QemuEvent *ev)
-{
- assert(ev->initialized);
-
- /*
- * Pairs with both qemu_event_reset() and qemu_event_wait().
- *
- * qemu_event_set has release semantics, but because it *loads*
- * ev->value we need a full memory barrier here.
- */
- smp_mb();
- if (qatomic_read(&ev->value) != EV_SET) {
- int old = qatomic_xchg(&ev->value, EV_SET);
-
- /* Pairs with memory barrier after ResetEvent. */
- smp_mb__after_rmw();
- if (old == EV_BUSY) {
- /* There were waiters, wake them up. */
- SetEvent(ev->event);
- }
- }
-}
-
-void qemu_event_reset(QemuEvent *ev)
-{
- assert(ev->initialized);
-
- /*
- * If there was a concurrent reset (or even reset+wait),
- * do nothing. Otherwise change EV_SET->EV_FREE.
- */
- qatomic_or(&ev->value, EV_FREE);
-
- /*
- * Order reset before checking the condition in the caller.
- * Pairs with the first memory barrier in qemu_event_set().
- */
- smp_mb__after_rmw();
-}
-
-void qemu_event_wait(QemuEvent *ev)
-{
- unsigned value;
-
- assert(ev->initialized);
-
- /*
- * qemu_event_wait must synchronize with qemu_event_set even if it does
- * not go down the slow path, so this load-acquire is needed that
- * synchronizes with the first memory barrier in qemu_event_set().
- *
- * If we do go down the slow path, there is no requirement at all: we
- * might miss a qemu_event_set() here but ultimately the memory barrier in
- * qemu_futex_wait() will ensure the check is done correctly.
- */
- value = qatomic_load_acquire(&ev->value);
- if (value != EV_SET) {
- if (value == EV_FREE) {
- /*
- * Here the underlying kernel event is reset, but qemu_event_set is
- * not yet going to call SetEvent. However, there will be another
- * check for EV_SET below when setting EV_BUSY. At that point it
- * is safe to call WaitForSingleObject.
- */
- ResetEvent(ev->event);
-
- /*
- * It is not clear whether ResetEvent provides this barrier; kernel
- * APIs (KeResetEvent/KeClearEvent) do not. Better safe than sorry!
- */
- smp_mb();
-
- /*
- * Leave the event reset and tell qemu_event_set that there are
- * waiters. No need to retry, because there cannot be a concurrent
- * busy->free transition. After the CAS, the event will be either
- * set or busy.
- */
- if (qatomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
- return;
- }
- }
-
- /*
- * ev->value is now EV_BUSY. Since we didn't observe EV_SET,
- * qemu_event_set() must observe EV_BUSY and call SetEvent().
- */
- WaitForSingleObject(ev->event, INFINITE);
- }
-}
-
struct QemuThreadData {
/* Passed to win32_start_routine. */
void *(*start_routine)(void *);
diff --git a/util/meson.build b/util/meson.build
index 5735f65f199..35029380a37 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -35,6 +35,7 @@ if glib_has_gslice
endif
util_ss.add(files('defer-call.c'))
util_ss.add(files('envlist.c', 'path.c', 'module.c'))
+util_ss.add(files('event.c'))
util_ss.add(files('host-utils.c'))
util_ss.add(files('bitmap.c', 'bitops.c'))
util_ss.add(files('fifo8.c'))
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 22/31] qemu-thread: Use futex if available for QemuLockCnt
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (20 preceding siblings ...)
2025-06-06 12:34 ` [PULL 21/31] qemu-thread: Use futex for QemuEvent on Windows Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 23/31] qemu-thread: Document QemuEvent Paolo Bonzini
` (9 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
This unlocks the futex-based implementation of QemuLockCnt to Windows.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/r/20250529-event-v5-6-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/lockcnt.h | 2 +-
util/lockcnt.c | 7 ++++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/qemu/lockcnt.h b/include/qemu/lockcnt.h
index f4b62a3f701..5a2800e3f18 100644
--- a/include/qemu/lockcnt.h
+++ b/include/qemu/lockcnt.h
@@ -17,7 +17,7 @@
typedef struct QemuLockCnt QemuLockCnt;
struct QemuLockCnt {
-#ifndef CONFIG_LINUX
+#ifndef HAVE_FUTEX
QemuMutex mutex;
#endif
unsigned count;
diff --git a/util/lockcnt.c b/util/lockcnt.c
index ca27d8e61a5..92c9f8ceca8 100644
--- a/util/lockcnt.c
+++ b/util/lockcnt.c
@@ -12,10 +12,11 @@
#include "qemu/atomic.h"
#include "trace.h"
-#ifdef CONFIG_LINUX
-#include "qemu/futex.h"
+#ifdef HAVE_FUTEX
-/* On Linux, bits 0-1 are a futex-based lock, bits 2-31 are the counter.
+/*
+ * When futex is available, bits 0-1 are a futex-based lock, bits 2-31 are the
+ * counter.
* For the mutex algorithm see Ulrich Drepper's "Futexes Are Tricky" (ok,
* this is not the most relaxing citation I could make...). It is similar
* to mutex2 in the paper.
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 23/31] qemu-thread: Document QemuEvent
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (21 preceding siblings ...)
2025-06-06 12:34 ` [PULL 22/31] qemu-thread: Use futex if available for QemuLockCnt Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 24/31] migration: Replace QemuSemaphore with QemuEvent Paolo Bonzini
` (8 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki
From: Akihiko Odaki <akihiko.odaki@daynix.com>
Document QemuEvent to help choose an appropriate synchronization
primitive.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Link: https://lore.kernel.org/r/20250529-event-v5-12-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/thread.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index 573f8c9ede2..f0302ed01fd 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -10,6 +10,16 @@ typedef struct QemuSemaphore QemuSemaphore;
typedef struct QemuLockCnt QemuLockCnt;
typedef struct QemuThread QemuThread;
+/*
+ * QemuEvent
+ * =========
+ *
+ * QemuEvent is an implementation of Win32 manual-reset event object.
+ * For details, refer to:
+ * https://learn.microsoft.com/en-us/windows/win32/sync/using-event-objects
+ *
+ * QemuEvent is more lightweight than QemuSemaphore when HAVE_FUTEX is defined.
+ */
typedef struct QemuEvent {
#ifndef HAVE_FUTEX
pthread_mutex_t lock;
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 24/31] migration: Replace QemuSemaphore with QemuEvent
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (22 preceding siblings ...)
2025-06-06 12:34 ` [PULL 23/31] qemu-thread: Document QemuEvent Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 25/31] migration/colo: " Paolo Bonzini
` (7 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Philippe Mathieu-Daudé
From: Akihiko Odaki <akihiko.odaki@daynix.com>
pause_event can utilize qemu_event_reset() to discard events.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250529-event-v5-7-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
migration/migration.h | 2 +-
migration/migration.c | 21 +++++++++------------
2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index d53f7cad84d..21aa6a3c8fe 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -379,7 +379,7 @@ struct MigrationState {
QemuSemaphore wait_unplug_sem;
/* Migration is paused due to pause-before-switchover */
- QemuSemaphore pause_sem;
+ QemuEvent pause_event;
/* The semaphore is used to notify COLO thread that failover is finished */
QemuSemaphore colo_exit_sem;
diff --git a/migration/migration.c b/migration/migration.c
index 4697732bef9..4098870bce3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1630,7 +1630,7 @@ void migration_cancel(void)
}
/* If the migration is paused, kick it out of the pause */
if (old_state == MIGRATION_STATUS_PRE_SWITCHOVER) {
- qemu_sem_post(&s->pause_sem);
+ qemu_event_set(&s->pause_event);
}
migrate_set_state(&s->state, old_state, MIGRATION_STATUS_CANCELLING);
} while (s->state != MIGRATION_STATUS_CANCELLING);
@@ -2342,7 +2342,7 @@ void qmp_migrate_continue(MigrationStatus state, Error **errp)
MigrationStatus_str(s->state));
return;
}
- qemu_sem_post(&s->pause_sem);
+ qemu_event_set(&s->pause_event);
}
int migration_rp_wait(MigrationState *s)
@@ -2911,21 +2911,18 @@ static bool migration_switchover_prepare(MigrationState *s)
return true;
}
- /* Since leaving this state is not atomic with posting the semaphore
+ /*
+ * Since leaving this state is not atomic with setting the event
* it's possible that someone could have issued multiple migrate_continue
- * and the semaphore is incorrectly positive at this point;
- * the docs say it's undefined to reinit a semaphore that's already
- * init'd, so use timedwait to eat up any existing posts.
+ * and the event is incorrectly set at this point so reset it.
*/
- while (qemu_sem_timedwait(&s->pause_sem, 1) == 0) {
- /* This block intentionally left blank */
- }
+ qemu_event_reset(&s->pause_event);
/* Update [POSTCOPY_]ACTIVE to PRE_SWITCHOVER */
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_PRE_SWITCHOVER);
bql_unlock();
- qemu_sem_wait(&s->pause_sem);
+ qemu_event_wait(&s->pause_event);
bql_lock();
/*
@@ -4057,7 +4054,7 @@ static void migration_instance_finalize(Object *obj)
qemu_mutex_destroy(&ms->qemu_file_lock);
qemu_sem_destroy(&ms->wait_unplug_sem);
qemu_sem_destroy(&ms->rate_limit_sem);
- qemu_sem_destroy(&ms->pause_sem);
+ qemu_event_destroy(&ms->pause_event);
qemu_sem_destroy(&ms->postcopy_pause_sem);
qemu_sem_destroy(&ms->rp_state.rp_sem);
qemu_sem_destroy(&ms->rp_state.rp_pong_acks);
@@ -4072,7 +4069,7 @@ static void migration_instance_init(Object *obj)
ms->state = MIGRATION_STATUS_NONE;
ms->mbps = -1;
ms->pages_per_second = -1;
- qemu_sem_init(&ms->pause_sem, 0);
+ qemu_event_init(&ms->pause_event, false);
qemu_mutex_init(&ms->error_mutex);
migrate_params_init(&ms->parameters);
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 25/31] migration/colo: Replace QemuSemaphore with QemuEvent
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (23 preceding siblings ...)
2025-06-06 12:34 ` [PULL 24/31] migration: Replace QemuSemaphore with QemuEvent Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 26/31] migration/postcopy: " Paolo Bonzini
` (6 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Fabiano Rosas, Philippe Mathieu-Daudé
From: Akihiko Odaki <akihiko.odaki@daynix.com>
colo_exit_sem and colo_incoming_sem represent one-shot events so they
can be converted into QemuEvent, which is more lightweight.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250529-event-v5-8-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
migration/migration.h | 6 +++---
migration/colo.c | 20 ++++++++++----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 21aa6a3c8fe..aaec471c00f 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -186,7 +186,7 @@ struct MigrationIncomingState {
/* The coroutine we should enter (back) after failover */
Coroutine *colo_incoming_co;
- QemuSemaphore colo_incoming_sem;
+ QemuEvent colo_incoming_event;
/* Optional load threads pool and its thread exit request flag */
ThreadPool *load_threads;
@@ -381,8 +381,8 @@ struct MigrationState {
/* Migration is paused due to pause-before-switchover */
QemuEvent pause_event;
- /* The semaphore is used to notify COLO thread that failover is finished */
- QemuSemaphore colo_exit_sem;
+ /* The event is used to notify COLO thread that failover is finished */
+ QemuEvent colo_exit_event;
/* The event is used to notify COLO thread to do checkpoint */
QemuEvent colo_checkpoint_event;
diff --git a/migration/colo.c b/migration/colo.c
index c976b3ff344..e0f713c837f 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -146,7 +146,7 @@ static void secondary_vm_do_failover(void)
return;
}
/* Notify COLO incoming thread that failover work is finished */
- qemu_sem_post(&mis->colo_incoming_sem);
+ qemu_event_set(&mis->colo_incoming_event);
/* For Secondary VM, jump to incoming co */
if (mis->colo_incoming_co) {
@@ -195,7 +195,7 @@ static void primary_vm_do_failover(void)
}
/* Notify COLO thread that failover work is finished */
- qemu_sem_post(&s->colo_exit_sem);
+ qemu_event_set(&s->colo_exit_event);
}
COLOMode get_colo_mode(void)
@@ -620,8 +620,8 @@ out:
}
/* Hope this not to be too long to wait here */
- qemu_sem_wait(&s->colo_exit_sem);
- qemu_sem_destroy(&s->colo_exit_sem);
+ qemu_event_wait(&s->colo_exit_event);
+ qemu_event_destroy(&s->colo_exit_event);
/*
* It is safe to unregister notifier after failover finished.
@@ -651,7 +651,7 @@ void migrate_start_colo_process(MigrationState *s)
s->colo_delay_timer = timer_new_ms(QEMU_CLOCK_HOST,
colo_checkpoint_notify_timer, NULL);
- qemu_sem_init(&s->colo_exit_sem, 0);
+ qemu_event_init(&s->colo_exit_event, false);
colo_process_checkpoint(s);
bql_lock();
}
@@ -808,11 +808,11 @@ void colo_shutdown(void)
case COLO_MODE_PRIMARY:
s = migrate_get_current();
qemu_event_set(&s->colo_checkpoint_event);
- qemu_sem_post(&s->colo_exit_sem);
+ qemu_event_set(&s->colo_exit_event);
break;
case COLO_MODE_SECONDARY:
mis = migration_incoming_get_current();
- qemu_sem_post(&mis->colo_incoming_sem);
+ qemu_event_set(&mis->colo_incoming_event);
break;
default:
break;
@@ -827,7 +827,7 @@ static void *colo_process_incoming_thread(void *opaque)
Error *local_err = NULL;
rcu_register_thread();
- qemu_sem_init(&mis->colo_incoming_sem, 0);
+ qemu_event_init(&mis->colo_incoming_event, false);
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_COLO);
@@ -923,8 +923,8 @@ out:
}
/* Hope this not to be too long to loop here */
- qemu_sem_wait(&mis->colo_incoming_sem);
- qemu_sem_destroy(&mis->colo_incoming_sem);
+ qemu_event_wait(&mis->colo_incoming_event);
+ qemu_event_destroy(&mis->colo_incoming_event);
rcu_unregister_thread();
return NULL;
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 26/31] migration/postcopy: Replace QemuSemaphore with QemuEvent
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (24 preceding siblings ...)
2025-06-06 12:34 ` [PULL 25/31] migration/colo: " Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 27/31] hw/display/apple-gfx: " Paolo Bonzini
` (5 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Fabiano Rosas
From: Akihiko Odaki <akihiko.odaki@daynix.com>
thread_sync_sem is an one-shot event so it can be converted into
QemuEvent, which is more lightweight.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20250529-event-v5-9-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
migration/migration.h | 4 ++--
migration/postcopy-ram.c | 10 +++++-----
migration/savevm.c | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index aaec471c00f..739289de934 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -98,9 +98,9 @@ struct MigrationIncomingState {
void (*transport_cleanup)(void *data);
/*
* Used to sync thread creations. Note that we can't create threads in
- * parallel with this sem.
+ * parallel with this event.
*/
- QemuSemaphore thread_sync_sem;
+ QemuEvent thread_sync_event;
/*
* Free at the start of the main state load, set as the main thread finishes
* loading state.
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 995614b38c9..75fd310fb2b 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -90,10 +90,10 @@ void postcopy_thread_create(MigrationIncomingState *mis,
QemuThread *thread, const char *name,
void *(*fn)(void *), int joinable)
{
- qemu_sem_init(&mis->thread_sync_sem, 0);
+ qemu_event_init(&mis->thread_sync_event, false);
qemu_thread_create(thread, name, fn, mis, joinable);
- qemu_sem_wait(&mis->thread_sync_sem);
- qemu_sem_destroy(&mis->thread_sync_sem);
+ qemu_event_wait(&mis->thread_sync_event);
+ qemu_event_destroy(&mis->thread_sync_event);
}
/* Postcopy needs to detect accesses to pages that haven't yet been copied
@@ -964,7 +964,7 @@ static void *postcopy_ram_fault_thread(void *opaque)
trace_postcopy_ram_fault_thread_entry();
rcu_register_thread();
mis->last_rb = NULL; /* last RAMBlock we sent part of */
- qemu_sem_post(&mis->thread_sync_sem);
+ qemu_event_set(&mis->thread_sync_event);
struct pollfd *pfd;
size_t pfd_len = 2 + mis->postcopy_remote_fds->len;
@@ -1716,7 +1716,7 @@ void *postcopy_preempt_thread(void *opaque)
rcu_register_thread();
- qemu_sem_post(&mis->thread_sync_sem);
+ qemu_event_set(&mis->thread_sync_event);
/*
* The preempt channel is established in asynchronous way. Wait
diff --git a/migration/savevm.c b/migration/savevm.c
index 006514c3e30..52105dd2f10 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2078,7 +2078,7 @@ static void *postcopy_ram_listen_thread(void *opaque)
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_POSTCOPY_ACTIVE);
- qemu_sem_post(&mis->thread_sync_sem);
+ qemu_event_set(&mis->thread_sync_event);
trace_postcopy_ram_listen_thread_start();
rcu_register_thread();
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 27/31] hw/display/apple-gfx: Replace QemuSemaphore with QemuEvent
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (25 preceding siblings ...)
2025-06-06 12:34 ` [PULL 26/31] migration/postcopy: " Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 28/31] target/i386: Detect flush-to-zero after rounding Paolo Bonzini
` (4 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Akihiko Odaki, Philippe Mathieu-Daudé
From: Akihiko Odaki <akihiko.odaki@daynix.com>
sem in AppleGFXReadMemoryJob is an one-shot event so it can be converted
into QemuEvent, which is more specialized for such a use case.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Link: https://lore.kernel.org/r/20250529-event-v5-10-53b285203794@daynix.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/display/apple-gfx.m | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m
index 8dde1f138dc..174d56ae05b 100644
--- a/hw/display/apple-gfx.m
+++ b/hw/display/apple-gfx.m
@@ -454,7 +454,7 @@ static void set_cursor_glyph(void *opaque)
/* ------ DMA (device reading system memory) ------ */
typedef struct AppleGFXReadMemoryJob {
- QemuSemaphore sem;
+ QemuEvent event;
hwaddr physical_address;
uint64_t length;
void *dst;
@@ -470,7 +470,7 @@ static void apple_gfx_do_read_memory(void *opaque)
job->dst, job->length, MEMTXATTRS_UNSPECIFIED);
job->success = (r == MEMTX_OK);
- qemu_sem_post(&job->sem);
+ qemu_event_set(&job->event);
}
static bool apple_gfx_read_memory(AppleGFXState *s, hwaddr physical_address,
@@ -483,11 +483,11 @@ static bool apple_gfx_read_memory(AppleGFXState *s, hwaddr physical_address,
trace_apple_gfx_read_memory(physical_address, length, dst);
/* Performing DMA requires BQL, so do it in a BH. */
- qemu_sem_init(&job.sem, 0);
+ qemu_event_init(&job.event, 0);
aio_bh_schedule_oneshot(qemu_get_aio_context(),
apple_gfx_do_read_memory, &job);
- qemu_sem_wait(&job.sem);
- qemu_sem_destroy(&job.sem);
+ qemu_event_wait(&job.event);
+ qemu_event_destroy(&job.event);
return job.success;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 28/31] target/i386: Detect flush-to-zero after rounding
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (26 preceding siblings ...)
2025-06-06 12:34 ` [PULL 27/31] hw/display/apple-gfx: " Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 29/31] target/i386: Use correct type for get_float_exception_flags() values Paolo Bonzini
` (3 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Zhao Liu
From: Peter Maydell <peter.maydell@linaro.org>
The Intel SDM section 10.2.3.3 on the MXCSR.FTZ bit says that we
flush outputs to zero when we detect underflow, which is after
rounding. Set the detect_ftz flag accordingly.
This allows us to enable the test in fma.c which checks this
behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250519145114.2786534-2-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/fpu_helper.c | 8 ++++----
tests/tcg/x86_64/fma.c | 5 -----
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 1cbadb14533..9ea67ea76c8 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -189,13 +189,13 @@ void cpu_init_fp_statuses(CPUX86State *env)
set_float_default_nan_pattern(0b11000000, &env->mmx_status);
set_float_default_nan_pattern(0b11000000, &env->sse_status);
/*
- * TODO: x86 does flush-to-zero detection after rounding (the SDM
+ * x86 does flush-to-zero detection after rounding (the SDM
* section 10.2.3.3 on the FTZ bit of MXCSR says that we flush
* when we detect underflow, which x86 does after rounding).
*/
- set_float_ftz_detection(float_ftz_before_rounding, &env->fp_status);
- set_float_ftz_detection(float_ftz_before_rounding, &env->mmx_status);
- set_float_ftz_detection(float_ftz_before_rounding, &env->sse_status);
+ set_float_ftz_detection(float_ftz_after_rounding, &env->fp_status);
+ set_float_ftz_detection(float_ftz_after_rounding, &env->mmx_status);
+ set_float_ftz_detection(float_ftz_after_rounding, &env->sse_status);
}
static inline uint8_t save_exception_flags(CPUX86State *env)
diff --git a/tests/tcg/x86_64/fma.c b/tests/tcg/x86_64/fma.c
index 09c622ebc00..46f863005ed 100644
--- a/tests/tcg/x86_64/fma.c
+++ b/tests/tcg/x86_64/fma.c
@@ -79,14 +79,9 @@ static testdata tests[] = {
/*
* Flushing of denormal outputs to zero should also happen after
* rounding, so setting FTZ should not affect the result or the flags.
- * QEMU currently does not emulate this correctly because we do the
- * flush-to-zero check before rounding, so we incorrectly produce a
- * zero result and set Underflow as well as Precision.
*/
-#ifdef ENABLE_FAILING_TESTS
{ 0x3fdfffffffffffff, 0x001fffffffffffff, 0x801fffffffffffff, true,
0x8010000000000000, 0x20 }, /* Enabling FTZ shouldn't change flags */
-#endif
};
int main(void)
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 29/31] target/i386: Use correct type for get_float_exception_flags() values
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (27 preceding siblings ...)
2025-06-06 12:34 ` [PULL 28/31] target/i386: Detect flush-to-zero after rounding Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 30/31] target/i386: Wire up MXCSR.DE and FPUS.DE correctly Paolo Bonzini
` (2 subsequent siblings)
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Richard Henderson, Philippe Mathieu-Daudé,
Zhao Liu
From: Peter Maydell <peter.maydell@linaro.org>
The softfloat get_float_exception_flags() function returns 'int', but
in various places in target/i386 we incorrectly store the returned
value into a uint8_t. This currently has no ill effects because i386
doesn't care about any of the float_flag enum values above 0x40.
However, we want to start using float_flag_input_denormal_used, which
is 0x4000.
Switch to using 'int' so that we can handle all the possible valid
float_flag_* values. This includes changing the return type of
save_exception_flags() and the argument to merge_exception_flags().
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250519145114.2786534-3-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/ops_sse.h | 16 +++----
target/i386/tcg/fpu_helper.c | 82 ++++++++++++++++++------------------
2 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index f0aa1894aa2..a2e4d480399 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -842,7 +842,7 @@ int64_t helper_cvttsd2sq(CPUX86State *env, ZMMReg *s)
void glue(helper_rsqrtps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
int i;
for (i = 0; i < 2 << SHIFT; i++) {
d->ZMM_S(i) = float32_div(float32_one,
@@ -855,7 +855,7 @@ void glue(helper_rsqrtps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
#if SHIFT == 1
void helper_rsqrtss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
int i;
d->ZMM_S(0) = float32_div(float32_one,
float32_sqrt(s->ZMM_S(0), &env->sse_status),
@@ -869,7 +869,7 @@ void helper_rsqrtss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
void glue(helper_rcpps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
int i;
for (i = 0; i < 2 << SHIFT; i++) {
d->ZMM_S(i) = float32_div(float32_one, s->ZMM_S(i), &env->sse_status);
@@ -880,7 +880,7 @@ void glue(helper_rcpps, SUFFIX)(CPUX86State *env, ZMMReg *d, ZMMReg *s)
#if SHIFT == 1
void helper_rcpss(CPUX86State *env, ZMMReg *d, ZMMReg *v, ZMMReg *s)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
int i;
d->ZMM_S(0) = float32_div(float32_one, s->ZMM_S(0), &env->sse_status);
for (i = 1; i < 2 << SHIFT; i++) {
@@ -1714,7 +1714,7 @@ void glue(helper_phminposuw, SUFFIX)(CPUX86State *env, Reg *d, Reg *s)
void glue(helper_roundps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
uint32_t mode)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
signed char prev_rounding_mode;
int i;
@@ -1738,7 +1738,7 @@ void glue(helper_roundps, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
void glue(helper_roundpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
uint32_t mode)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
signed char prev_rounding_mode;
int i;
@@ -1763,7 +1763,7 @@ void glue(helper_roundpd, SUFFIX)(CPUX86State *env, Reg *d, Reg *s,
void glue(helper_roundss, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
uint32_t mode)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
signed char prev_rounding_mode;
int i;
@@ -1788,7 +1788,7 @@ void glue(helper_roundss, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
void glue(helper_roundsd, SUFFIX)(CPUX86State *env, Reg *d, Reg *v, Reg *s,
uint32_t mode)
{
- uint8_t old_flags = get_float_exception_flags(&env->sse_status);
+ int old_flags = get_float_exception_flags(&env->sse_status);
signed char prev_rounding_mode;
int i;
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 9ea67ea76c8..4732b718129 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -198,16 +198,16 @@ void cpu_init_fp_statuses(CPUX86State *env)
set_float_ftz_detection(float_ftz_after_rounding, &env->sse_status);
}
-static inline uint8_t save_exception_flags(CPUX86State *env)
+static inline int save_exception_flags(CPUX86State *env)
{
- uint8_t old_flags = get_float_exception_flags(&env->fp_status);
+ int old_flags = get_float_exception_flags(&env->fp_status);
set_float_exception_flags(0, &env->fp_status);
return old_flags;
}
-static void merge_exception_flags(CPUX86State *env, uint8_t old_flags)
+static void merge_exception_flags(CPUX86State *env, int old_flags)
{
- uint8_t new_flags = get_float_exception_flags(&env->fp_status);
+ int new_flags = get_float_exception_flags(&env->fp_status);
float_raise(old_flags, &env->fp_status);
fpu_set_exception(env,
((new_flags & float_flag_invalid ? FPUS_IE : 0) |
@@ -220,7 +220,7 @@ static void merge_exception_flags(CPUX86State *env, uint8_t old_flags)
static inline floatx80 helper_fdiv(CPUX86State *env, floatx80 a, floatx80 b)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
floatx80 ret = floatx80_div(a, b, &env->fp_status);
merge_exception_flags(env, old_flags);
return ret;
@@ -240,7 +240,7 @@ static void fpu_raise_exception(CPUX86State *env, uintptr_t retaddr)
void helper_flds_FT0(CPUX86State *env, uint32_t val)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
union {
float32 f;
uint32_t i;
@@ -253,7 +253,7 @@ void helper_flds_FT0(CPUX86State *env, uint32_t val)
void helper_fldl_FT0(CPUX86State *env, uint64_t val)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
union {
float64 f;
uint64_t i;
@@ -271,7 +271,7 @@ void helper_fildl_FT0(CPUX86State *env, int32_t val)
void helper_flds_ST0(CPUX86State *env, uint32_t val)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int new_fpstt;
union {
float32 f;
@@ -288,7 +288,7 @@ void helper_flds_ST0(CPUX86State *env, uint32_t val)
void helper_fldl_ST0(CPUX86State *env, uint64_t val)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int new_fpstt;
union {
float64 f;
@@ -338,7 +338,7 @@ void helper_fildll_ST0(CPUX86State *env, int64_t val)
uint32_t helper_fsts_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
union {
float32 f;
uint32_t i;
@@ -351,7 +351,7 @@ uint32_t helper_fsts_ST0(CPUX86State *env)
uint64_t helper_fstl_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
union {
float64 f;
uint64_t i;
@@ -364,7 +364,7 @@ uint64_t helper_fstl_ST0(CPUX86State *env)
int32_t helper_fist_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int32_t val;
val = floatx80_to_int32(ST0, &env->fp_status);
@@ -378,7 +378,7 @@ int32_t helper_fist_ST0(CPUX86State *env)
int32_t helper_fistl_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int32_t val;
val = floatx80_to_int32(ST0, &env->fp_status);
@@ -391,7 +391,7 @@ int32_t helper_fistl_ST0(CPUX86State *env)
int64_t helper_fistll_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int64_t val;
val = floatx80_to_int64(ST0, &env->fp_status);
@@ -404,7 +404,7 @@ int64_t helper_fistll_ST0(CPUX86State *env)
int32_t helper_fistt_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int32_t val;
val = floatx80_to_int32_round_to_zero(ST0, &env->fp_status);
@@ -418,7 +418,7 @@ int32_t helper_fistt_ST0(CPUX86State *env)
int32_t helper_fisttl_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int32_t val;
val = floatx80_to_int32_round_to_zero(ST0, &env->fp_status);
@@ -431,7 +431,7 @@ int32_t helper_fisttl_ST0(CPUX86State *env)
int64_t helper_fisttll_ST0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int64_t val;
val = floatx80_to_int64_round_to_zero(ST0, &env->fp_status);
@@ -527,7 +527,7 @@ static const int fcom_ccval[4] = {0x0100, 0x4000, 0x0000, 0x4500};
void helper_fcom_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
FloatRelation ret;
ret = floatx80_compare(ST0, FT0, &env->fp_status);
@@ -537,7 +537,7 @@ void helper_fcom_ST0_FT0(CPUX86State *env)
void helper_fucom_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
FloatRelation ret;
ret = floatx80_compare_quiet(ST0, FT0, &env->fp_status);
@@ -549,7 +549,7 @@ static const int fcomi_ccval[4] = {CC_C, CC_Z, 0, CC_Z | CC_P | CC_C};
void helper_fcomi_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int eflags;
FloatRelation ret;
@@ -562,7 +562,7 @@ void helper_fcomi_ST0_FT0(CPUX86State *env)
void helper_fucomi_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int eflags;
FloatRelation ret;
@@ -575,28 +575,28 @@ void helper_fucomi_ST0_FT0(CPUX86State *env)
void helper_fadd_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST0 = floatx80_add(ST0, FT0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fmul_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST0 = floatx80_mul(ST0, FT0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fsub_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST0 = floatx80_sub(ST0, FT0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fsubr_ST0_FT0(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST0 = floatx80_sub(FT0, ST0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
@@ -615,28 +615,28 @@ void helper_fdivr_ST0_FT0(CPUX86State *env)
void helper_fadd_STN_ST0(CPUX86State *env, int st_index)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST(st_index) = floatx80_add(ST(st_index), ST0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fmul_STN_ST0(CPUX86State *env, int st_index)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST(st_index) = floatx80_mul(ST(st_index), ST0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fsub_STN_ST0(CPUX86State *env, int st_index)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST(st_index) = floatx80_sub(ST(st_index), ST0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fsubr_STN_ST0(CPUX86State *env, int st_index)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST(st_index) = floatx80_sub(ST0, ST(st_index), &env->fp_status);
merge_exception_flags(env, old_flags);
}
@@ -861,7 +861,7 @@ void helper_fbld_ST0(CPUX86State *env, target_ulong ptr)
void helper_fbst_ST0(CPUX86State *env, target_ulong ptr)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
int v;
target_ulong mem_ref, mem_end;
int64_t val;
@@ -1136,7 +1136,7 @@ static const struct f2xm1_data f2xm1_table[65] = {
void helper_f2xm1(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
uint64_t sig = extractFloatx80Frac(ST0);
int32_t exp = extractFloatx80Exp(ST0);
bool sign = extractFloatx80Sign(ST0);
@@ -1369,7 +1369,7 @@ static const struct fpatan_data fpatan_table[9] = {
void helper_fpatan(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
uint64_t arg0_sig = extractFloatx80Frac(ST0);
int32_t arg0_exp = extractFloatx80Exp(ST0);
bool arg0_sign = extractFloatx80Sign(ST0);
@@ -1808,7 +1808,7 @@ void helper_fpatan(CPUX86State *env)
void helper_fxtract(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
CPU_LDoubleU temp;
temp.d = ST0;
@@ -1857,7 +1857,7 @@ void helper_fxtract(CPUX86State *env)
static void helper_fprem_common(CPUX86State *env, bool mod)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
uint64_t quotient;
CPU_LDoubleU temp0, temp1;
int exp0, exp1, expdiff;
@@ -2053,7 +2053,7 @@ static void helper_fyl2x_common(CPUX86State *env, floatx80 arg, int32_t *exp,
void helper_fyl2xp1(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
uint64_t arg0_sig = extractFloatx80Frac(ST0);
int32_t arg0_exp = extractFloatx80Exp(ST0);
bool arg0_sign = extractFloatx80Sign(ST0);
@@ -2151,7 +2151,7 @@ void helper_fyl2xp1(CPUX86State *env)
void helper_fyl2x(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
uint64_t arg0_sig = extractFloatx80Frac(ST0);
int32_t arg0_exp = extractFloatx80Exp(ST0);
bool arg0_sign = extractFloatx80Sign(ST0);
@@ -2298,7 +2298,7 @@ void helper_fyl2x(CPUX86State *env)
void helper_fsqrt(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
if (floatx80_is_neg(ST0)) {
env->fpus &= ~0x4700; /* (C3,C2,C1,C0) <-- 0000 */
env->fpus |= 0x400;
@@ -2324,14 +2324,14 @@ void helper_fsincos(CPUX86State *env)
void helper_frndint(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
ST0 = floatx80_round_to_int(ST0, &env->fp_status);
merge_exception_flags(env, old_flags);
}
void helper_fscale(CPUX86State *env)
{
- uint8_t old_flags = save_exception_flags(env);
+ int old_flags = save_exception_flags(env);
if (floatx80_invalid_encoding(ST1, &env->fp_status) ||
floatx80_invalid_encoding(ST0, &env->fp_status)) {
float_raise(float_flag_invalid, &env->fp_status);
@@ -2369,7 +2369,7 @@ void helper_fscale(CPUX86State *env)
} else {
int n;
FloatX80RoundPrec save = env->fp_status.floatx80_rounding_precision;
- uint8_t save_flags = get_float_exception_flags(&env->fp_status);
+ int save_flags = get_float_exception_flags(&env->fp_status);
set_float_exception_flags(0, &env->fp_status);
n = floatx80_to_int32_round_to_zero(ST1, &env->fp_status);
set_float_exception_flags(save_flags, &env->fp_status);
@@ -3269,7 +3269,7 @@ void update_mxcsr_status(CPUX86State *env)
void update_mxcsr_from_sse_status(CPUX86State *env)
{
- uint8_t flags = get_float_exception_flags(&env->sse_status);
+ int flags = get_float_exception_flags(&env->sse_status);
/*
* The MXCSR denormal flag has opposite semantics to
* float_flag_input_denormal_flushed (the softfloat code sets that flag
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 30/31] target/i386: Wire up MXCSR.DE and FPUS.DE correctly
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (28 preceding siblings ...)
2025-06-06 12:34 ` [PULL 29/31] target/i386: Use correct type for get_float_exception_flags() values Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 12:34 ` [PULL 31/31] tests/tcg/x86_64/fma: add test for exact-denormal output Paolo Bonzini
2025-06-06 15:27 ` [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Stefan Hajnoczi
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson
From: Peter Maydell <peter.maydell@linaro.org>
The x86 DE bit in the FPU and MXCSR status is supposed to be set
when an input denormal is consumed. We didn't previously report
this from softfloat, so the x86 code either simply didn't set
the DE bit or else incorrectly wired it up to denormal_flushed,
depending on which register you looked at.
Now we have input_denormal_used we can wire up these DE bits
with the semantics they are supposed to have.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Link: https://lore.kernel.org/r/20250519145114.2786534-4-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/tcg/fpu_helper.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/target/i386/tcg/fpu_helper.c b/target/i386/tcg/fpu_helper.c
index 4732b718129..b3b23823fda 100644
--- a/target/i386/tcg/fpu_helper.c
+++ b/target/i386/tcg/fpu_helper.c
@@ -215,7 +215,7 @@ static void merge_exception_flags(CPUX86State *env, int old_flags)
(new_flags & float_flag_overflow ? FPUS_OE : 0) |
(new_flags & float_flag_underflow ? FPUS_UE : 0) |
(new_flags & float_flag_inexact ? FPUS_PE : 0) |
- (new_flags & float_flag_input_denormal_flushed ? FPUS_DE : 0)));
+ (new_flags & float_flag_input_denormal_used ? FPUS_DE : 0)));
}
static inline floatx80 helper_fdiv(CPUX86State *env, floatx80 a, floatx80 b)
@@ -3254,6 +3254,7 @@ void update_mxcsr_status(CPUX86State *env)
/* Set exception flags. */
set_float_exception_flags((mxcsr & FPUS_IE ? float_flag_invalid : 0) |
+ (mxcsr & FPUS_DE ? float_flag_input_denormal_used : 0) |
(mxcsr & FPUS_ZE ? float_flag_divbyzero : 0) |
(mxcsr & FPUS_OE ? float_flag_overflow : 0) |
(mxcsr & FPUS_UE ? float_flag_underflow : 0) |
@@ -3270,14 +3271,8 @@ void update_mxcsr_status(CPUX86State *env)
void update_mxcsr_from_sse_status(CPUX86State *env)
{
int flags = get_float_exception_flags(&env->sse_status);
- /*
- * The MXCSR denormal flag has opposite semantics to
- * float_flag_input_denormal_flushed (the softfloat code sets that flag
- * only when flushing input denormals to zero, but SSE sets it
- * only when not flushing them to zero), so is not converted
- * here.
- */
env->mxcsr |= ((flags & float_flag_invalid ? FPUS_IE : 0) |
+ (flags & float_flag_input_denormal_used ? FPUS_DE : 0) |
(flags & float_flag_divbyzero ? FPUS_ZE : 0) |
(flags & float_flag_overflow ? FPUS_OE : 0) |
(flags & float_flag_underflow ? FPUS_UE : 0) |
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PULL 31/31] tests/tcg/x86_64/fma: add test for exact-denormal output
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (29 preceding siblings ...)
2025-06-06 12:34 ` [PULL 30/31] target/i386: Wire up MXCSR.DE and FPUS.DE correctly Paolo Bonzini
@ 2025-06-06 12:34 ` Paolo Bonzini
2025-06-06 15:27 ` [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Stefan Hajnoczi
31 siblings, 0 replies; 36+ messages in thread
From: Paolo Bonzini @ 2025-06-06 12:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Richard Henderson, Zhao Liu
From: Peter Maydell <peter.maydell@linaro.org>
Add some fma test cases that check for correct handling of FTZ and
for the flag that indicates that the input denormal was consumed.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Link: https://lore.kernel.org/r/20250519145114.2786534-5-peter.maydell@linaro.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
tests/tcg/x86_64/fma.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tests/tcg/x86_64/fma.c b/tests/tcg/x86_64/fma.c
index 46f863005ed..34219614c0a 100644
--- a/tests/tcg/x86_64/fma.c
+++ b/tests/tcg/x86_64/fma.c
@@ -82,6 +82,18 @@ static testdata tests[] = {
*/
{ 0x3fdfffffffffffff, 0x001fffffffffffff, 0x801fffffffffffff, true,
0x8010000000000000, 0x20 }, /* Enabling FTZ shouldn't change flags */
+ /*
+ * normal * 0 + a denormal. With FTZ disabled this gives an exact
+ * result (equal to the input denormal) that has consumed the denormal.
+ */
+ { 0x3cc8000000000000, 0x0000000000000000, 0x8008000000000000, false,
+ 0x8008000000000000, 0x2 }, /* Denormal */
+ /*
+ * With FTZ enabled, this consumes the denormal, returns zero (because
+ * flushed) and indicates also Underflow and Precision.
+ */
+ { 0x3cc8000000000000, 0x0000000000000000, 0x8008000000000000, true,
+ 0x8000000000000000, 0x32 }, /* Precision, Underflow, Denormal */
};
int main(void)
--
2.49.0
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06
2025-06-06 12:34 [PULL 00/31] Threading, Rust, i386 changes for 2025-06-06 Paolo Bonzini
` (30 preceding siblings ...)
2025-06-06 12:34 ` [PULL 31/31] tests/tcg/x86_64/fma: add test for exact-denormal output Paolo Bonzini
@ 2025-06-06 15:27 ` Stefan Hajnoczi
31 siblings, 0 replies; 36+ messages in thread
From: Stefan Hajnoczi @ 2025-06-06 15:27 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.1 for any user-visible changes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread