Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
On Tue, Jul 2, 2024 at 4:44 PM Manos Pitsidianakis wrote: > >Normally you'd see either --enable-XXX or --with-XXX and their > >corresponding --disable-XXX or --without-XXX. > > True. As the commit message says, `rust` is a reserved meson feature > name, so the auto-generated scripts/meson-buildoptions.sh > has the following args: > > --enable-with-rust > --disable-with-rust > > I used the same in `configure` even though it's not autogenerated in > order to keep the two synced. If there's a way to get around this I'd > prefer it. With the patch I posted, --with-rust/--without-rust is handled entirely in configure, Meson gleans the result from the presence of RUST_TARGET_TRIPLE in config-host.mak. Paolo
Re: [PATCH 0/5] target/i386: CCOp cleanups
On Mon, Jul 1, 2024 at 9:05 PM Richard Henderson wrote: > > no objections at all to introducing more asserts. I think keeping the > > array is a better underlying implementation for cc_op_live() however. > > Hmm. I had an implementation that would detect missing entries at runtime, > but this one > detects missing entries at compile time. But how common is it to add new CCOps? I find the array more readable (and it can also be printed quickly from gdb), while the switch statement optimizes for a really rare case. > > I'm also not very fond of mixing "sized" and "unsized" CCOps in the > > 4..7 range, there's no real reason why CC_OP_DYNAMIC and CC_OP_CLR > > must be close to CC_OP_EFLAGS and the ADCOX CCOps. I also think it's > > clearer to keep CC_OP_POPCNT[BWLQ] (even though in practice only one > > will be used because popcnt needs zero extension anyway). > > My objection to keeping the unused POPCNT* enumerators is that it interferes > with proper > cooperation with -Wswitch, to diagnose missing enumerators. This is also why > I removed > CC_OP_NB. Yes, I agree with removing CC_OP_NB. However the unused POPCNT[BWLQ] can be implemented trivially (they're all the same). > > As an aside, I'm wondering if CC_OP_CLR is particularly important; I > > expect "xor reg, reg" to be followed by more ALU operations most of > > the time and to not be followed by a jump, so it only saves a spill if > > xor reg, reg is followed by a lot or store. If gen_XOR used either > > CC_OP_LOGICn or CC_OP_EFLAGS for "xor reg, reg", the values in > > decode->cc_op_* (CC_OP_DST=0 for CC_OP_LOGICn; CC_OP_SRC=CC_Z|CC_P for > > CC_OP_EFLAGS) would be constant and wouldn't add to register pressure. > > You could easily be right. Improvements to tcg in the last 11 years may have > made it > redundant, or it might have been wishful thinking even at the time. Maybe. Just looking at the last couple years, with PCREL the cost of translation has decreased substantially, and with the new decoder the number of tcg ops has increased a bit. In both cases that means that counting the tcg ops becomes less important. BTW I found an easy way to implement X86_SPECIAL_BitTest without crashes (just use cpu_regs[op->n] when computing the displacement since you cannot have ah/bh/ch/dh). But I think it will be for 9.2. Maybe these patches can wait too? Paolo
Re: [PATCH 0/5] target/i386: CCOp cleanups
On Mon, Jul 1, 2024 at 4:51 AM Richard Henderson wrote: > While debugging #2413, I spent quite a bit of time trying to work > out if the CCOp value was incorrect. I think the following is a > worthwhile cleanup, isolating potential problems to asserts. Hi Richard, no objections at all to introducing more asserts. I think keeping the array is a better underlying implementation for cc_op_live() however. I'm also not very fond of mixing "sized" and "unsized" CCOps in the 4..7 range, there's no real reason why CC_OP_DYNAMIC and CC_OP_CLR must be close to CC_OP_EFLAGS and the ADCOX CCOps. I also think it's clearer to keep CC_OP_POPCNT[BWLQ] (even though in practice only one will be used because popcnt needs zero extension anyway). As an aside, I'm wondering if CC_OP_CLR is particularly important; I expect "xor reg, reg" to be followed by more ALU operations most of the time and to not be followed by a jump, so it only saves a spill if xor reg, reg is followed by a lot or store. If gen_XOR used either CC_OP_LOGICn or CC_OP_EFLAGS for "xor reg, reg", the values in decode->cc_op_* (CC_OP_DST=0 for CC_OP_LOGICn; CC_OP_SRC=CC_Z|CC_P for CC_OP_EFLAGS) would be constant and wouldn't add to register pressure. Paolo
Re: [PATCH 2/2] system/vl.c: parse all -accel options
On Mon, Jul 1, 2024 at 4:34 PM Philippe Mathieu-Daudé wrote: > Reviewed-by: Philippe Mathieu-Daudé In principle, a Reviewed-by tag is just stating that you don't know of any issues that would prevent the patch being included. However, as a frequent participant to the project, your Reviewed-by tag carries some weight and, to some extent, it is also a statement that you understand the area being modified. A Reviewed-by from an experienced contributor may even imply that you could take the patch in one of your pull requests. (*) That makes it even more important to understand the area. I would expect that anyone with an understanding of command line parsing would know 1) what -accel kvm -accel tcg does, and 2) what .merge_lists does; and this would be enough to flag an issue preventing the patch from being included. To be clear, I don't expect reviews to be perfect. But in this case I'm speaking up because the patch is literally a one line declarative change, and the only way to say "I've reviewed it" is by understanding the deeper effects of that line. Also, I think it's fair that the submitter didn't spot the problem; it's okay to send out broken patches, that's part of the learning experience. :) Paolo (*) as opposed to Acked-by, where your review probably has been more conceptual than technical, and that you don't really want to take the patch in a pull request. Paolo
Re: [PATCH 0/2] system/vl.c: parse all '-accel' opts
On Mon, Jul 1, 2024 at 3:30 PM Daniel Henrique Barboza wrote: > My initial intention was to fix a problem we're having with libvirt and > RISC-V where we can't set 'riscv-aia' by appending '-accel kvm,riscv-aia=val' > via in the domain XML. libvirt will add a leading > '-accel kvm' in the regular command line and ignore the second. But to > fix that (patch 2) we must first guarantee that we're not mixing different > accelerators. I think you can use -global, if you tweak qdev_prop_check_globals() to also allow descendents of TYPE_ACCEL. For example: ./qemu-system-x86_64 -accel kvm -global kvm-accel.kernel-irqchip=off Paolo
Re: [PATCH 1/2] system/vl.c: do not allow mixed -accel opts
On Mon, Jul 1, 2024 at 5:53 PM Daniel Henrique Barboza wrote: > > We use '-accel kvm -accel tcg' to allow kvm to fail (e.g. no /dev/kvm > > permission) and proceed with tcg. > > > > This patch will cause testsuite failures. > > For the issue I want to fix patch 2 alone is enough. I'll re-send. It doesn't; it effectively changes '-accel kvm -accel tcg' to just '-accel tcg'. This is why you didn't see any failures, I think. Paolo
Re: [PATCH 0/2] change some odd-looking atomic uses
On Mon, Jul 1, 2024 at 1:52 PM Wolfgang Bumiller wrote: > > I spotted the weird-looking pattern of: > atomic_set(atomic_load() N) > in a few palces and one variable in the graph-lock code which was used with > atomics except for a single case, which also seemed suspicious. > > I'm not sure if there are any known compiler-optimizations or ordering > semantics already ensuring that these operations are indeed working correctly > atomically, so I thought I'd point them out and ask about it by sending > patches. Hi Wolfgang, indeed the usage is intended - but thanks a lot for the patches, it's certainly a good way to proceed! Here's a quick explanation: - bdrv_graph_co_rdlock()/bdrv_graph_co_rdunlock(): only one coroutine can run in each AioContext at a time, so the writes to bdrv_graph->reader_count are effectively thread-local. It's just reads that need to be synchronized. - aio_poll(): similar, as the writer is the thread for the AioContext, while the readers are the other threads that call aio_notify() for that AioContext. - aio_ctx_prepare(): same idea as aio_poll(), in this case the writer is the thread running the GLib event loop (almost always the main thread, except with the Cocoa UI because macOS wants the main thread for itself). So why is this written in full instead of using fetch-add-store? From most to least important: - to make you think about who reads and who writes, i.e. to show that no atomicity is needed - because it's faster The disadvantage of course is the verbosity. Thanks, Paolo > In patch 2 the ordering is changed (see the note in its mail) > > Wolfgang Bumiller (2): > graph-lock: make sure reader_count access is atomic > atomics: replace fetch-use-store with direct atomic operations > > block/graph-lock.c | 8 +++- > util/aio-posix.c | 3 +-- > util/aio-win32.c | 3 +-- > util/async.c | 2 +- > 4 files changed, 6 insertions(+), 10 deletions(-) > > -- > 2.39.2 > >Hi
[PATCH 05/14] rust: define wrappers for Error
The wrappers for Error provide the following functionality:
- a struct that implements std::error::Error and can be filled with
information similar to what would go into a C Error* (location, message).
- functionality similar to error_prepend() via the "cause()"
member of std::error::Error.
- converting a Result<> into a value that can be returned to C,
filling in an Error** (like error_propagate() would do) if the
Result<> contains an error; useful for callbacks written in Rust
- converting a C Error* into a Result that can be returned to Rust,
useful for Rust wrappers of C functions
Signed-off-by: Paolo Bonzini
---
qemu/src/lib.rs| 2 +
qemu/src/util/error.rs | 241 +
qemu/src/util/mod.rs | 1 +
3 files changed, 244 insertions(+)
create mode 100644 qemu/src/util/error.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index c48edcf..5f926b8 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -4,7 +4,9 @@
pub mod bindings;
pub mod util;
+pub use util::error::Error;
pub use util::foreign::CloneToForeign;
pub use util::foreign::FromForeign;
pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
+pub type Result = std::result::Result;
diff --git a/qemu/src/util/error.rs b/qemu/src/util/error.rs
new file mode 100644
index 000..e7e6f2e
--- /dev/null
+++ b/qemu/src/util/error.rs
@@ -0,0 +1,241 @@
+//! Error class for QEMU Rust code
+//!
+//! @author Paolo Bonzini
+
+use crate::bindings;
+use crate::bindings::error_free;
+use crate::bindings::error_get_pretty;
+use crate::bindings::error_setg_internal;
+
+use std::ffi::CStr;
+use std::fmt::{self, Display};
+use std::ptr;
+
+use crate::util::foreign::{CloneToForeign, FromForeign, OwnedPointer};
+
+#[derive(Debug, Default)]
+pub struct Error {
+msg: Option,
+/// Appends the print string of the error to the msg if not None
+cause: Option>,
+location: Option<(String, u32)>,
+}
+
+impl std::error::Error for Error {
+fn source() -> Option<&(dyn std::error::Error + 'static)> {
+self.cause.as_deref()
+}
+
+#[allow(deprecated)]
+fn description() -> {
+self.msg
+.as_deref()
+.or_else(||
self.cause.as_deref().map(std::error::Error::description))
+.unwrap_or("error")
+}
+}
+
+impl Display for Error {
+fn fmt(, f: fmt::Formatter) -> fmt::Result {
+let mut prefix = "";
+if let Some((ref file, line)) = self.location {
+write!(f, "{}:{}", file, line)?;
+prefix = ": ";
+}
+if let Some(ref msg) = self.msg {
+write!(f, "{}{}", prefix, msg)?;
+prefix = ": ";
+}
+if let Some(ref cause) = self.cause {
+write!(f, "{}{}", prefix, cause)?;
+} else if prefix.is_empty() {
+f.write_str("unknown error")?;
+}
+Ok(())
+}
+}
+
+impl From<> for Error {
+fn from(msg: ) -> Self {
+Error {
+msg: Some(String::from(msg)),
+cause: None,
+location: None,
+}
+}
+}
+
+impl From for Error {
+fn from(error: std::io::Error) -> Self {
+Error {
+msg: None,
+cause: Some(Box::new(error)),
+location: None,
+}
+}
+}
+
+impl Error {
+/// Create a new error, prepending `msg` to the
+/// description of `cause`
+pub fn with_error(msg: , cause: E) ->
Self {
+Error {
+msg: Some(String::from(msg)),
+cause: Some(Box::new(cause)),
+location: None,
+}
+}
+
+/// Create a new error, prepending `file:line: msg` to the
+/// description of `cause`
+pub fn with_error_file_line(
+msg: ,
+cause: E,
+file: ,
+line: u32,
+) -> Self {
+Error {
+msg: Some(String::from(msg)),
+cause: Some(Box::new(cause)),
+location: Some((String::from(file), line)),
+}
+}
+
+/// Create a new error with format `file:line: msg`
+pub fn with_file_line(msg: , file: , line: u32) -> Self {
+Error {
+msg: Some(String::from(msg)),
+cause: None,
+location: Some((String::from(file), 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 valid; typically it is received from C code
+pub unsafe fn bool_or_propagate(
+result: Result<(), Self>,
+errp: *mut *mut bindings::Error,
+) -> bool {
+Self::ok_or_propagate(result, errp).is_some()
+}
+
+/// Consume a result, retu
[PATCH 14/14] rust: use version of toml_edit that does not require new Rust
toml_edit is quite aggressive in bumping the minimum required
version of Rust. Force usage of an old version that runs
with 1.63.0.
Signed-off-by: Paolo Bonzini
---
qemu/Cargo.toml | 3 +++
2 files changed, 71 insertions(+), 3 deletions(-)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 93808a5..3ce5dba 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -15,3 +15,6 @@ matches = ">=0"
[build-dependencies]
version_check = { version = "~0.9" }
+
+# pick older version in order to support Rust 1.63
+toml_edit = { version = "~0.14" }
--
2.45.2
[PATCH 12/14] rust: replace c"" literals with cstr crate
Part of what's needed to work with Rust versions prior to 1.77.
Signed-off-by: Paolo Bonzini
---
qemu/Cargo.toml| 3 +++
qemu/qom-rust.txt | 2 +-
qemu/src/hw/core/device.rs | 4 +++-
qemu/src/qom/object.rs | 4 +++-
qemu/src/util/error.rs | 4 +++-
qemu/src/util/foreign.rs | 20 +++-
qemu/tests/main.rs | 7 ---
8 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 1100725..a07a449 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -7,5 +7,8 @@ edition = "2021"
const-default = { version = "~1", features = ["derive"] }
libc = "^0"
+# pick older version in order to support Rust 1.63
+cstr = { version = "=0.2.10" }
+
[dev-dependencies]
matches = ">=0"
diff --git a/qemu/qom-rust.txt b/qemu/qom-rust.txt
index 1588445..ef4bd06 100644
--- a/qemu/qom-rust.txt
+++ b/qemu/qom-rust.txt
@@ -48,7 +48,7 @@ Bindings for C classes
struct must implement ObjectType
unsafe impl ObjectType for Object {
-const TYPE: &'static CStr = c"object";
+const TYPE: &'static CStr = cstr!("object");
}
struct must implement IsA for all superclasses T
diff --git a/qemu/src/hw/core/device.rs b/qemu/src/hw/core/device.rs
index 294251e..4edf61d 100644
--- a/qemu/src/hw/core/device.rs
+++ b/qemu/src/hw/core/device.rs
@@ -17,12 +17,14 @@ use crate::qom_isa;
use crate::Result;
+use cstr::cstr;
+
use std::ffi::CStr;
use std::ops::Deref;
use std::ptr::null_mut;
unsafe impl ObjectType for DeviceState {
-const TYPE: &'static CStr = c"device";
+const TYPE: &'static CStr = cstr!("device");
}
qom_isa!(DeviceState, Object);
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
index 4e84e29..9f6c078 100644
--- a/qemu/src/qom/object.rs
+++ b/qemu/src/qom/object.rs
@@ -7,6 +7,8 @@ use std::ffi::CStr;
use std::fmt;
use std::ops::Deref;
+use cstr::cstr;
+
use crate::bindings::object_get_typename;
use crate::bindings::object_property_add_child;
use crate::bindings::object_new;
@@ -42,7 +44,7 @@ pub unsafe trait ObjectType: Sized {
}
unsafe impl ObjectType for Object {
-const TYPE: &'static CStr = c"object";
+const TYPE: &'static CStr = cstr!("object");
}
// --
diff --git a/qemu/src/util/error.rs b/qemu/src/util/error.rs
index e7e6f2e..79c3c81 100644
--- a/qemu/src/util/error.rs
+++ b/qemu/src/util/error.rs
@@ -7,6 +7,8 @@ use crate::bindings::error_free;
use crate::bindings::error_get_pretty;
use crate::bindings::error_setg_internal;
+use cstr::cstr;
+
use std::ffi::CStr;
use std::fmt::{self, Display};
use std::ptr;
@@ -215,7 +217,7 @@ impl CloneToForeign for Error {
ptr::null_mut(), // FIXME
0,
ptr::null_mut(), // FIXME
-c"%s".as_ptr(),
+cstr!("%s").as_ptr(),
format!("{}", self),
);
OwnedPointer::new(x)
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index 464400a..7a663cc 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -167,7 +167,8 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// ```
/// # use qemu::FromForeign;
-/// let p = c"Hello, world!".as_ptr();
+/// # use cstr::cstr;
+/// let p = cstr!("Hello, world!").as_ptr();
/// let s = unsafe {
/// String::cloned_from_foreign(p as *const libc::c_char)
/// };
@@ -476,6 +477,7 @@ mod tests {
#![allow(clippy::shadow_unrelated)]
use super::*;
+use cstr::cstr;
use matches::assert_matches;
use std::ffi::c_void;
@@ -498,7 +500,7 @@ mod tests {
#[test]
fn test_cloned_from_foreign_string_cow() {
let s = "Hello, world!".to_string();
-let cstr = c"Hello, world!";
+let cstr = cstr!("Hello, world!");
let cloned = unsafe { Cow::cloned_from_foreign(cstr.as_ptr()) };
assert_eq!(s, cloned);
}
@@ -506,7 +508,7 @@ mod tests {
#[test]
fn test_cloned_from_foreign_string() {
let s = "Hello, world!".to_string();
-let cstr = c"Hello, world!";
+let cstr = cstr!("Hello, world!");
let cloned = unsafe { String::cloned_from_foreign(cstr.as_ptr()) };
assert_eq!(s, cloned);
}
@@ -570,7 +572,7 @@ mod tests {
#[test]
fn test_clone_to_foreign_str() {
let s = "Hello, world!";
-let p = c"Hello, world!".as_ptr();
+let p = cstr!("Hello, world!").as_ptr();
let cloned = s.clone_to_foreign();
unsafe {
let len = libc::strlen(cloned.as_ptr());
@@ -588,7 +590,7 @@ mod tests {
#[test]
[PATCH 00/14] rust: example of bindings code for Rust in QEMU
vice::unparent);
}
impl DeviceImpl for TestDevice {
const REALIZE: Option Result<()>> =
Some(TestDevice::realize);
}
This works and it seems like a style that (in the future) we could apply
macro or even procedural macro magic to.
- generation of qdev property tables. While only boolean properties are
implemented here, one idea that I experimented with, is that the
default value of properties is derived from the ConstDefault trait.
(ConstDefault is provided by the const_default external crate). Again,
this is material for future conversion to procedural macros.
I absolutely didn't look at vmstate, but it shouldn't be too different
from properties, at least for the common cases.
Patches 11-14 finally are an example of the changes that are needed
to respect a minimum supported Rust version consistent with what is in
Debian Bullseye. It's not too bad, especially since the current version
of the QOM bindings does not require generic associated types anymore.
Why am I posting this? Because this kind of glue code is the ultimate
source of technical debt. It is the thing that we should be scared of
when introducing a new language in QEMU. It makes it harder to change C
code, and it is hard to change once Rust code becomes more widespread.
If we think a C API is not fully baked, we probably shouldn't write
Rust code that uses it (including bindings code). If we think a Rust
API is not fully baked, we probably shouldn't add too much Rust code
that uses it.
We should have an idea of what this glue code looks like, in order to make
an informed choice. If we think we're not comfortable with reviewing it,
well, we should be ready to say so and stick with C until we are.
The alternative could be to use Rust without this kind of binding. I
think it's a bad idea. It removes many of the advantages of Rust
(which are exemplified by the above object_property_add_child one-liner),
and it also introduces _new_ kinds of memory errors, since Rust has
its own undefined behavior conditions that are not there in C/C++.
For example:
impl Struct {
pub fn f() {
call_some_c_function(Self::g, self as *const Self as *mut _);
}
fn do_g( self) {
...
}
extern "C" fn g(ptr: *mut Self) {
unsafe { *ptr }.do_g();
}
}
is invalid because a reference (exclusive) is alive at the same time
as a & reference (shared). It is left as an exercise to the reader to
figure out all the possible ways in which we can shoot our own feet,
considering the pervasive use of callbacks in QEMU.
With respect to callbacks, that's something that is missing in this
prototype. Fortunately, that's also something that will be tackled very
soon if the PL011 example is merged, because memory regions and character
devices both introduce them. Also, as I understand it, Rust code using
callbacks is not particularly nice anyway, though it is of course doable.
Instead, this exercise are about being able to write *nice* Rust code,
with all the advantages provided by the language, and the cost of
writing/maintaining the glue code that makes it possible. I expect
that we'll use a technique similar to the extern_c crate (it's 20 lines of
code; https://docs.rs/crate/extern-c/) to convert something that implements
Fn(), including a member function, into an extern "C" function.
Anyhow: I think we can do it, otherwise I would not have written 2000 lines
of code (some of it two or three times). But if people are now scared and
think we shouldn't, well, that's also a success of its own kind.
Paolo
Paolo Bonzini (14):
add skeleton
set expectations
rust: define traits and pointer wrappers to convert from/to C
representations
rust: add tests for util::foreign
rust: define wrappers for Error
rust: define wrappers for basic QOM concepts
rust: define wrappers for methods of the QOM Object class
rust: define wrappers for methods of the QOM Device class
rust: add idiomatic bindings to define Object subclasses
rust: add idiomatic bindings to define Device subclasses
rust: replace std::ffi::c_char with libc::c_char
rust: replace c"" literals with cstr crate
rust: introduce alternative to offset_of!
rust: use version of toml_edit that does not require new Rust
--
2.45.2
[PATCH 06/14] rust: define wrappers for basic QOM concepts
This provides type-safe object casts, and automatic reference counting.
Signed-off-by: Paolo Bonzini
---
qemu/qom-rust.txt | 82
qemu/src/lib.rs| 6 +
qemu/src/qom/mod.rs| 2 +
qemu/src/qom/object.rs | 34 +
qemu/src/qom/refs.rs | 274 +
5 files changed, 398 insertions(+)
create mode 100644 qemu/qom-rust.txt
create mode 100644 qemu/src/qom/mod.rs
create mode 100644 qemu/src/qom/object.rs
create mode 100644 qemu/src/qom/refs.rs
diff --git a/qemu/qom-rust.txt b/qemu/qom-rust.txt
new file mode 100644
index 000..1588445
--- /dev/null
+++ b/qemu/qom-rust.txt
@@ -0,0 +1,82 @@
+Rust QOM interoperability design
+
+
+Passing objects around
+--
+
+ObjectRef:
+-> trait for performing casts on objects
+-> upcasts safe at compile time, downcasts safe at runtime
+-> implemented by and qom::Owned
+-> casting produces , casting qom::Owned produces qom::Owned
+
+qom::Owned
+-> T is a struct for a QOM object
+-> cloning qom::Owned calls object_ref, dropping qom::Owned calls object_unref
+
+
+Calling methods
+---
+
+- all methods (interior mutability)
+ - Rust implementation needs to wrap state with Cell<>, RefCell<> or Mutex<>
+
+one struct per class; one trait per non-final class; one trait per interface
+struct: Object, Device, ...
+- defines constructors
+ example: PL011::new() (technically defined on ObjectType)
+
+- defines methods of final classes
+
+trait: ObjectMethods, DeviceMethods, UserCreatableMethods, ...
+- defines methods of non-final classes and interfaces
+ example: obj.typename()
+
+- automatically implemented by where T is a subclass
+
+
+all methods expect interior mutability
+- structs not Send/Sync by default since they contain C pointers
+ - hence and Owned also not thread-safe
+- good: QOM tree (e.g. object_unparent) not thread-safe
+- what if objects _are_ thread-safe?
+ - possibly another trait ObjectSyncMethods?
+
+Bindings for C classes
+--
+
+struct must implement ObjectType
+
+unsafe impl ObjectType for Object {
+const TYPE: &'static CStr = c"object";
+}
+
+struct must implement IsA for all superclasses T
+
+unsafe impl IsA for Object {}
+
+
+Defining QOM classes in Rust
+
+
+struct must be #[repr(C)]
+
+one traits per class + one more if it has virtual functions
+
+trait #1: ObjectTypeImpl, DeviceTypeImpl, ...
+- metadata
+type Super: ObjectType;
+- functions:
+unsafe fn instance_init(obj: *mut Self);
+...
+
+trait #2: ObjectImpl, DeviceImpl, ...
+- functions:
+fn unrealize()
+
+Rust implementation split in configuration (Default + ConstDefault) and
+state (Default)
+
+instance_init implemented automatically via Default/ConstDefault trait
+ maybe: pre_init hook that replaces memset(obj, 0, type->instance_size)?
+instance_finalize implemented automatically via Drop trait
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 5f926b8..0d91623 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,6 +2,12 @@
#![allow(dead_code)]
pub mod bindings;
+pub use bindings::Object;
+
+pub mod qom;
+pub use qom::object::ObjectType;
+pub use qom::refs::ObjectCast;
+pub use qom::refs::Owned;
pub mod util;
pub use util::error::Error;
diff --git a/qemu/src/qom/mod.rs b/qemu/src/qom/mod.rs
new file mode 100644
index 000..95489c5
--- /dev/null
+++ b/qemu/src/qom/mod.rs
@@ -0,0 +1,2 @@
+pub mod object;
+pub mod refs;
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
new file mode 100644
index 000..bd6b957
--- /dev/null
+++ b/qemu/src/qom/object.rs
@@ -0,0 +1,34 @@
+//! Bindings for the QOM Object class
+//!
+//! @author Paolo Bonzini
+
+use std::ffi::CStr;
+
+use crate::bindings::Object;
+
+use crate::qom_isa;
+
+/// Trait exposed by all structs corresponding to QOM objects.
+/// Defines "class methods" for the class. Usually these can be
+/// implemented on the class itself; here, using a trait allows
+/// each class to define `TYPE`, and it also lets `new()` return the
+/// right type.
+///
+/// # Safety
+///
+/// - the first field of the struct must be of `Object` type,
+/// or derived from it
+///
+/// - `TYPE` must match the type name used in the `TypeInfo` (no matter
+/// if it is defined in C or Rust).
+///
+/// - the struct must be `#[repr(C)]`
+pub unsafe trait ObjectType: Sized {
+const TYPE: &'static CStr;
+}
+
+unsafe impl ObjectType for Object {
+const TYPE: &'static CStr = c"object";
+}
+
+qom_isa!(Object);
diff --git a/qemu/src/qom/refs.rs b/qemu/src/qom/refs.rs
new file mode 100644
index 000..a319bde
--- /dev/null
+++ b/qemu/src/qom/refs.rs
@@ -0,0 +1,274 @@
+//! Casting and reference counting traits for QOM objects
+//!
+//! @author Paolo Bonzini
+
+use crate::bindings::object_dynamic_cast;
+use crate::bindings::Ob
[PATCH 08/14] rust: define wrappers for methods of the QOM Device class
Provide a trait that can be used to invoke methods of the QOM Device
class. The trait extends Deref and has a blanket implementation for any
type that dereferences to IsA. This way, it can be used on any
struct that dereferences to Object or a subclass.
Signed-off-by: Paolo Bonzini
---
qemu/src/hw/core/device.rs | 56 ++
qemu/src/hw/core/mod.rs| 1 +
qemu/src/hw/mod.rs | 1 +
qemu/src/lib.rs| 4 +++
4 files changed, 62 insertions(+)
create mode 100644 qemu/src/hw/core/device.rs
create mode 100644 qemu/src/hw/core/mod.rs
create mode 100644 qemu/src/hw/mod.rs
diff --git a/qemu/src/hw/core/device.rs b/qemu/src/hw/core/device.rs
new file mode 100644
index 000..294251e
--- /dev/null
+++ b/qemu/src/hw/core/device.rs
@@ -0,0 +1,56 @@
+//! Bindings for the QOM Device class
+//!
+//! @author Paolo Bonzini
+
+use crate::qom::object::ObjectType;
+
+use crate::qom::refs::IsA;
+use crate::qom::refs::ObjectCast;
+
+use crate::bindings;
+use crate::bindings::device_cold_reset;
+use crate::bindings::device_realize;
+use crate::bindings::DeviceState;
+use crate::bindings::Object;
+
+use crate::qom_isa;
+
+use crate::Result;
+
+use std::ffi::CStr;
+use std::ops::Deref;
+use std::ptr::null_mut;
+
+unsafe impl ObjectType for DeviceState {
+const TYPE: &'static CStr = c"device";
+}
+
+qom_isa!(DeviceState, Object);
+
+/// Trait for methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+pub trait DeviceMethods: Deref
+where
+Self::Target: IsA,
+{
+fn realize() -> Result<()> {
+let device = self.upcast::();
+let mut err: *mut bindings::Error = null_mut();
+// SAFETY: safety of this is the requirement for implementing IsA
+unsafe {
+device_realize(device.as_mut_ptr(), err);
+crate::Error::err_or_default(err)
+}
+}
+
+fn cold_reset() {
+let device = self.upcast::();
+// SAFETY: safety of this is the requirement for implementing IsA
+unsafe { device_cold_reset(device.as_mut_ptr()) }
+}
+}
+
+impl DeviceMethods for R where R::Target: IsA {}
diff --git a/qemu/src/hw/core/mod.rs b/qemu/src/hw/core/mod.rs
new file mode 100644
index 000..5458924
--- /dev/null
+++ b/qemu/src/hw/core/mod.rs
@@ -0,0 +1 @@
+pub mod device;
diff --git a/qemu/src/hw/mod.rs b/qemu/src/hw/mod.rs
new file mode 100644
index 000..5a7ca06
--- /dev/null
+++ b/qemu/src/hw/mod.rs
@@ -0,0 +1 @@
+pub mod core;
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index a6e7b17..b0dcce1 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,8 +2,12 @@
#![allow(dead_code)]
pub mod bindings;
+pub use bindings::DeviceState;
pub use bindings::Object;
+pub mod hw;
+pub use hw::core::device::DeviceMethods;
+
pub mod qom;
pub use qom::object::ObjectClassMethods;
pub use qom::object::ObjectMethods;
--
2.45.2
[PATCH 13/14] rust: introduce alternative to offset_of!
Allow working with Rust versions prior to 1.77. The code was
taken from Rust's Discourse platform and is used with permission of
the author.
Signed-off-by: Paolo Bonzini
---
qemu/Cargo.toml | 3 +
qemu/build.rs | 5 ++
qemu/src/hw/core/device_impl.rs | 4 +-
qemu/src/lib.rs | 4 ++
qemu/src/qom/object_impl.rs | 13 +++--
qemu/src/util/mod.rs| 1 +
qemu/src/util/offset_of.rs | 99 +
qemu/tests/main.rs | 11 +++-
9 files changed, 137 insertions(+), 10 deletions(-)
create mode 100644 qemu/build.rs
create mode 100644 qemu/src/util/offset_of.rs
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index a07a449..93808a5 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -12,3 +12,6 @@ cstr = { version = "=0.2.10" }
[dev-dependencies]
matches = ">=0"
+
+[build-dependencies]
+version_check = { version = "~0.9" }
diff --git a/qemu/build.rs b/qemu/build.rs
new file mode 100644
index 000..34f7b49
--- /dev/null
+++ b/qemu/build.rs
@@ -0,0 +1,5 @@
+fn main() {
+if let Some(true) = version_check::is_min_version("1.77.0") {
+println!("cargo:rustc-cfg=has_offset_of");
+}
+}
diff --git a/qemu/src/hw/core/device_impl.rs b/qemu/src/hw/core/device_impl.rs
index 80b0e5e..b1d2f04 100644
--- a/qemu/src/hw/core/device_impl.rs
+++ b/qemu/src/hw/core/device_impl.rs
@@ -111,7 +111,7 @@ macro_rules! qdev_prop {
$kind,
$name,
(<$crate::conf_type!($type) as ConstDefault>::DEFAULT).$field,
-<$type as $crate::DeviceTypeImpl>::CONF_OFFSET +
std::mem::offset_of!($crate::conf_type!($type), $field)
+<$type as $crate::DeviceTypeImpl>::CONF_OFFSET +
$crate::offset_of!($crate::conf_type!($type), $field)
)
};
}
@@ -126,7 +126,7 @@ macro_rules! qdev_define_type {
@extends $super $(,$supers)*, $crate::Object);
unsafe impl $crate::DeviceTypeImpl for $struct {
-const CONF_OFFSET: usize = std::mem::offset_of!($struct, conf);
+const CONF_OFFSET: usize = $crate::offset_of!($struct, conf);
fn properties() -> *const $crate::Property {
static mut PROPERTIES: &'static [$crate::Property] =
&[$($props),+];
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 3f0491c..2d43a25 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -31,3 +31,7 @@ pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
pub use util::zeroed::Zeroed;
pub type Result = std::result::Result;
+
+// with_offsets is exported directly from util::offset_of
+#[cfg(has_offset_of)]
+pub use std::mem::offset_of;
diff --git a/qemu/src/qom/object_impl.rs b/qemu/src/qom/object_impl.rs
index 61546b6..b1768b9 100644
--- a/qemu/src/qom/object_impl.rs
+++ b/qemu/src/qom/object_impl.rs
@@ -95,11 +95,14 @@ unsafe fn rust_type_register() {
#[macro_export]
macro_rules! qom_define_type {
($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty; @extends $super:ty
$(,$supers:ty)*) => {
-struct $struct {
-// self.base dropped by call to superclass instance_finalize
-base: std::mem::ManuallyDrop<$super>,
-conf: $conf_ty,
-state: $state_ty,
+$crate::with_offsets! {
+#[repr(C)]
+struct $struct {
+// self.base dropped by call to superclass instance_finalize
+base: std::mem::ManuallyDrop<$super>,
+conf: $conf_ty,
+state: $state_ty,
+}
}
// Define IsA markers for the struct itself and all the superclasses
diff --git a/qemu/src/util/mod.rs b/qemu/src/util/mod.rs
index 9c081b6..e4df7c9 100644
--- a/qemu/src/util/mod.rs
+++ b/qemu/src/util/mod.rs
@@ -1,3 +1,4 @@
pub mod error;
pub mod foreign;
+pub mod offset_of;
pub mod zeroed;
diff --git a/qemu/src/util/offset_of.rs b/qemu/src/util/offset_of.rs
new file mode 100644
index 000..4ce5188
--- /dev/null
+++ b/qemu/src/util/offset_of.rs
@@ -0,0 +1,99 @@
+#[cfg(not(has_offset_of))]
+#[macro_export]
+macro_rules! offset_of {
+($Container:ty, $field:ident) => {
+<$Container>::offset_to.$field
+};
+}
+
+/// A wrapper for struct declarations, that allows using `offset_of!` in
+/// versions of Rust prior to 1.77
+#[macro_export]
+macro_rules! with_offsets {
+// source:
https://play.rust-lang.org/?version=stable=debug=2018=10a22a9b8393abd7b541d8fc844bc0df
+// used under MIT license with permission of Yandros aka Daniel
Henry-Mantilla
+(
+#[repr(C)]
+$(#[$struct_meta:meta])*
+$struct_vis:vis
+struct $StructName:ident {
+$(
+$(#[$field_meta:meta])*
+$field_vis:vis
+$field_name:ident : $field_ty:ty
+),*
+$(,)?
+
[PATCH 03/14] rust: define traits and pointer wrappers to convert from/to C representations
The qemu::util::foreign module provides:
- A trait for structs that can be converted to a C ("foreign") representation
(CloneToForeign)
- A trait for structs that can be built from a C ("foreign") representation
(FromForeign), and the utility IntoNative that can be used with less typing
(similar to the standard library's From and Into pair)
- Automatic implementations of the above traits for Option<>, supporting NULL
pointers
- A wrapper for a pointer that automatically frees the contained data. If
a struct XYZ implements CloneToForeign, you can build an OwnedPointer
and it will free the contents automatically unless you retrieve it with
owned_ptr.into_inner()
Signed-off-by: Paolo Bonzini
---
qemu/src/lib.rs | 6 +
qemu/src/util/foreign.rs | 247 +++
qemu/src/util/mod.rs | 1 +
3 files changed, 254 insertions(+)
create mode 100644 qemu/src/util/foreign.rs
create mode 100644 qemu/src/util/mod.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index fce21d7..c48edcf 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -2,3 +2,9 @@
#![allow(dead_code)]
pub mod bindings;
+
+pub mod util;
+pub use util::foreign::CloneToForeign;
+pub use util::foreign::FromForeign;
+pub use util::foreign::IntoNative;
+pub use util::foreign::OwnedPointer;
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
new file mode 100644
index 000..a591925
--- /dev/null
+++ b/qemu/src/util/foreign.rs
@@ -0,0 +1,247 @@
+// TODO: change to use .cast() etc.
+#![allow(clippy::ptr_as_ptr)]
+
+/// Traits to map between C structs and native Rust types.
+/// Similar to glib-rs but a bit simpler and possibly more
+/// idiomatic.
+use std::borrow::Cow;
+use std::fmt;
+use std::fmt::Debug;
+use std::mem;
+use std::ptr;
+
+/// A type for which there is a canonical representation as a C datum.
+pub trait CloneToForeign {
+/// The representation of `Self` as a C datum. Typically a
+/// `struct`, though there are exceptions for example `c_char`
+/// for strings, since C strings are of `char *` type).
+type Foreign;
+
+/// Free the C datum pointed to by `p`.
+///
+/// # Safety
+///
+/// `p` must be `NULL` or point to valid data.
+unsafe fn free_foreign(p: *mut Self::Foreign);
+
+/// Convert a native Rust object to a foreign C struct, copying
+/// everything pointed to by `self` (same as `to_glib_full` in `glib-rs`)
+fn clone_to_foreign() -> OwnedPointer;
+
+/// Convert a native Rust object to a foreign C pointer, copying
+/// everything pointed to by `self`. The returned pointer must
+/// be freed with the `free_foreign` associated function.
+fn clone_to_foreign_ptr() -> *mut Self::Foreign {
+self.clone_to_foreign().into_inner()
+}
+}
+
+impl CloneToForeign for Option
+where
+T: CloneToForeign,
+{
+type Foreign = ::Foreign;
+
+unsafe fn free_foreign(x: *mut Self::Foreign) {
+T::free_foreign(x)
+}
+
+fn clone_to_foreign() -> OwnedPointer {
+// Same as the underlying implementation, but also convert `None`
+// to a `NULL` pointer.
+self.as_ref()
+.map(CloneToForeign::clone_to_foreign)
+.map(OwnedPointer::into)
+.unwrap_or_default()
+}
+}
+
+impl FromForeign for Option
+where
+T: FromForeign,
+{
+unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
+// Same as the underlying implementation, but also accept a `NULL`
pointer.
+if p.is_null() {
+None
+} else {
+Some(T::cloned_from_foreign(p))
+}
+}
+}
+
+impl CloneToForeign for Box
+where
+T: CloneToForeign,
+{
+type Foreign = ::Foreign;
+
+unsafe fn free_foreign(x: *mut Self::Foreign) {
+T::free_foreign(x)
+}
+
+fn clone_to_foreign() -> OwnedPointer {
+self.as_ref().clone_to_foreign().into()
+}
+}
+
+impl FromForeign for Box
+where
+T: FromForeign,
+{
+unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self {
+Box::new(T::cloned_from_foreign(p))
+}
+}
+
+impl<'a, B> CloneToForeign for Cow<'a, B>
+where B: 'a + ToOwned + ?Sized + CloneToForeign,
+{
+type Foreign = B::Foreign;
+
+unsafe fn free_foreign(ptr: *mut B::Foreign) {
+B::free_foreign(ptr);
+}
+
+fn clone_to_foreign() -> OwnedPointer {
+self.as_ref().clone_to_foreign().into()
+}
+}
+
+/// Convert a C datum into a native Rust object, taking ownership of
+/// the C datum. You should not need to implement this trait
+/// as long as Rust types implement `FromForeign`.
+pub trait IntoNative {
+/// Convert a C datum to a native Rust object, taking ownership of
+/// the pointer or Rust object (same as `from_glib_full` in `glib-rs`)
+///
+/// # Safety
+///
+/// `p` must point to valid data, or can be `NULL` if Self is an
+
[PATCH 11/14] rust: replace std::ffi::c_char with libc::c_char
Allow working with Rust versions prior to 1.64.
Signed-off-by: Paolo Bonzini
---
qemu/src/bindings/mod.rs | 3 ++-
qemu/src/util/foreign.rs | 7 +--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/qemu/src/bindings/mod.rs b/qemu/src/bindings/mod.rs
index a49447b..0ad3828 100644
--- a/qemu/src/bindings/mod.rs
+++ b/qemu/src/bindings/mod.rs
@@ -1,4 +1,5 @@
-use std::ffi::{c_char, c_void};
+use libc::c_char;
+use std::ffi::c_void;
#[repr(C)]
pub struct Object {
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index 0b8b708..464400a 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -4,8 +4,11 @@
/// Traits to map between C structs and native Rust types.
/// Similar to glib-rs but a bit simpler and possibly more
/// idiomatic.
+
+use libc::c_char;
+
use std::borrow::Cow;
-use std::ffi::{c_char, c_void, CStr, CString};
+use std::ffi::{c_void, CStr, CString};
use std::fmt;
use std::fmt::Debug;
use std::mem;
@@ -166,7 +169,7 @@ pub trait FromForeign: CloneToForeign + Sized {
/// # use qemu::FromForeign;
/// let p = c"Hello, world!".as_ptr();
/// let s = unsafe {
-/// String::cloned_from_foreign(p as *const std::ffi::c_char)
+/// String::cloned_from_foreign(p as *const libc::c_char)
/// };
/// assert_eq!(s, "Hello, world!");
/// ```
--
2.45.2
[PATCH 04/14] rust: add tests for util::foreign
Provide sample implementations in util::foreign for strings and
elementary integer types, and use them to test the code.
Signed-off-by: Paolo Bonzini
---
qemu/Cargo.toml | 4 +
qemu/src/util/foreign.rs | 456 +++
3 files changed, 474 insertions(+)
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
index 18d0fa4..1100725 100644
--- a/qemu/Cargo.toml
+++ b/qemu/Cargo.toml
@@ -5,3 +5,7 @@ edition = "2021"
[dependencies]
const-default = { version = "~1", features = ["derive"] }
+libc = "^0"
+
+[dev-dependencies]
+matches = ">=0"
diff --git a/qemu/src/util/foreign.rs b/qemu/src/util/foreign.rs
index a591925..0b8b708 100644
--- a/qemu/src/util/foreign.rs
+++ b/qemu/src/util/foreign.rs
@@ -5,6 +5,7 @@
/// Similar to glib-rs but a bit simpler and possibly more
/// idiomatic.
use std::borrow::Cow;
+use std::ffi::{c_char, c_void, CStr, CString};
use std::fmt;
use std::fmt::Debug;
use std::mem;
@@ -22,6 +23,14 @@ pub trait CloneToForeign {
/// # Safety
///
/// `p` must be `NULL` or point to valid data.
+///
+/// ```
+/// # use qemu::CloneToForeign;
+/// let foreign = "Hello, world!".clone_to_foreign();
+/// unsafe {
+/// String::free_foreign(foreign.into_inner());
+/// }
+/// ```
unsafe fn free_foreign(p: *mut Self::Foreign);
/// Convert a native Rust object to a foreign C struct, copying
@@ -119,6 +128,17 @@ pub trait IntoNative {
///
/// `p` must point to valid data, or can be `NULL` if Self is an
/// `Option` type. It becomes invalid after the function returns.
+///
+/// ```
+/// # use qemu::{CloneToForeign, IntoNative};
+/// let s = "Hello, world!".to_string();
+/// let foreign = s.clone_to_foreign();
+/// let native: String = unsafe {
+/// foreign.into_native()
+/// // foreign is not leaked
+/// };
+/// assert_eq!(s, native);
+/// ```
unsafe fn into_native(self) -> T;
}
@@ -141,6 +161,15 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// `p` must point to valid data, or can be `NULL` is `Self` is an
/// `Option` type.
+///
+/// ```
+/// # use qemu::FromForeign;
+/// let p = c"Hello, world!".as_ptr();
+/// let s = unsafe {
+/// String::cloned_from_foreign(p as *const std::ffi::c_char)
+/// };
+/// assert_eq!(s, "Hello, world!");
+/// ```
unsafe fn cloned_from_foreign(p: *const Self::Foreign) -> Self;
/// Convert a C datum to a native Rust object, taking ownership of
@@ -152,6 +181,16 @@ pub trait FromForeign: CloneToForeign + Sized {
///
/// `p` must point to valid data, or can be `NULL` is `Self` is an
/// `Option` type. `p` becomes invalid after the function returns.
+///
+/// ```
+/// # use qemu::{CloneToForeign, FromForeign};
+/// let s = "Hello, world!";
+/// let foreign = s.clone_to_foreign();
+/// unsafe {
+/// assert_eq!(String::from_foreign(foreign.into_inner()), s);
+/// }
+/// // foreign is not leaked
+/// ```
unsafe fn from_foreign(p: *mut Self::Foreign) -> Self {
let result = Self::cloned_from_foreign(p);
Self::free_foreign(p);
@@ -176,6 +215,12 @@ impl OwnedPointer {
/// Safely create an `OwnedPointer` from one that has the same
/// freeing function.
+/// ```
+/// # use qemu::{CloneToForeign, OwnedPointer};
+/// let s = "Hello, world!";
+/// let foreign_str = s.clone_to_foreign();
+/// let foreign_string = OwnedPointerfrom(foreign_str);
+/// # assert_eq!(foreign_string.into_native(), s);
pub fn from(x: OwnedPointer) -> Self
where
U: CloneToForeign::Foreign> + ?Sized,
@@ -189,6 +234,12 @@ impl OwnedPointer {
/// Safely convert an `OwnedPointer` into one that has the same
/// freeing function.
+/// ```
+/// # use qemu::{CloneToForeign, OwnedPointer};
+/// let s = "Hello, world!";
+/// let foreign_str = s.clone_to_foreign();
+/// let foreign_string: OwnedPointer = foreign_str.into();
+/// # assert_eq!(foreign_string.into_native(), s);
pub fn into(self) -> OwnedPointer
where
U: CloneToForeign::Foreign>,
@@ -198,6 +249,16 @@ impl OwnedPointer {
/// Return the pointer that is stored in the `OwnedPointer`. The
/// pointer is valid for as long as the `OwnedPointer` itself.
+///
+/// ```
+/// # use qemu::CloneToForeign;
+/// let s = "Hello, world!";
+/// let foreign = s.clone_to_foreign();
+/// let p = foreign.as_ptr();
+/// let len = unsafe { libc::strlen(p) };
+/// drop(foreign);
+/// # assert_eq!(len, 13);
+/// ```
pub fn as_ptr() -> *const ::Foreign {
self.ptr
}
@@ -208,6 +269,15 @@ impl OwnedPointer {
/// Ret
[PATCH 09/14] rust: add idiomatic bindings to define Object subclasses
Provide a macro to register a type and automatically define instance_init
(actually instance_mem_init) and instance_finalize functions. Subclasses
of Object must define a trait ObjectImpl, to point the type definition
machinery to the implementation of virtual functions in Object.
Signed-off-by: Paolo Bonzini
---
qemu/src/lib.rs | 4 +
qemu/src/qom/mod.rs | 1 +
qemu/src/qom/object_impl.rs | 146
qemu/src/util/mod.rs| 1 +
qemu/src/util/zeroed.rs | 21 ++
qemu/tests/main.rs | 32
6 files changed, 205 insertions(+)
create mode 100644 qemu/src/qom/object_impl.rs
create mode 100644 qemu/src/util/zeroed.rs
create mode 100644 qemu/tests/main.rs
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index b0dcce1..81abf9c 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -4,6 +4,7 @@
pub mod bindings;
pub use bindings::DeviceState;
pub use bindings::Object;
+pub use bindings::TypeInfo;
pub mod hw;
pub use hw::core::device::DeviceMethods;
@@ -12,6 +13,8 @@ pub mod qom;
pub use qom::object::ObjectClassMethods;
pub use qom::object::ObjectMethods;
pub use qom::object::ObjectType;
+pub use qom::object_impl::ObjectImpl;
+pub use qom::object_impl::TypeImpl;
pub use qom::refs::ObjectCast;
pub use qom::refs::Owned;
@@ -21,4 +24,5 @@ pub use util::foreign::CloneToForeign;
pub use util::foreign::FromForeign;
pub use util::foreign::IntoNative;
pub use util::foreign::OwnedPointer;
+pub use util::zeroed::Zeroed;
pub type Result = std::result::Result;
diff --git a/qemu/src/qom/mod.rs b/qemu/src/qom/mod.rs
index 95489c5..3f8ee6e 100644
--- a/qemu/src/qom/mod.rs
+++ b/qemu/src/qom/mod.rs
@@ -1,2 +1,3 @@
pub mod object;
+pub mod object_impl;
pub mod refs;
diff --git a/qemu/src/qom/object_impl.rs b/qemu/src/qom/object_impl.rs
new file mode 100644
index 000..61546b6
--- /dev/null
+++ b/qemu/src/qom/object_impl.rs
@@ -0,0 +1,146 @@
+//! Macros and traits to implement subclasses of Object in Rust
+//!
+//! @author Paolo Bonzini
+
+#![allow(clippy::missing_safety_doc)]
+
+use const_default::ConstDefault;
+
+use std::ffi::c_void;
+use std::mem;
+use std::mem::MaybeUninit;
+use std::ptr::drop_in_place;
+
+use crate::qom::object::ObjectType;
+
+use crate::qom::refs::ObjectCast;
+
+use crate::bindings::type_register;
+use crate::bindings::Object;
+use crate::bindings::ObjectClass;
+use crate::bindings::TypeInfo;
+
+use crate::util::zeroed::Zeroed;
+
+/// Information on which superclass methods are overridden
+/// by a Rust-implemented subclass of Object.
+pub trait ObjectImpl: ObjectType {
+/// If not `None`, a function that implements the `unparent` member
+/// of the QOM `ObjectClass`.
+const UNPARENT: Option = None;
+}
+
+impl ObjectClass {
+/// Initialize an `ObjectClass` from an `ObjectImpl`.
+pub fn class_init( self) {
+unsafe extern "C" fn rust_unparent(obj: *mut Object) {
+let f = T::UNPARENT.unwrap();
+f((&*obj).unsafe_cast::())
+}
+self.unparent = T::UNPARENT.map(|_| rust_unparent:: as _);
+}
+}
+
+impl Object {
+pub unsafe extern "C" fn rust_class_init(
+klass: *mut c_void,
+_data: *mut c_void,
+) {
+let oc: ObjectClass = *(klass.cast());
+oc.class_init::();
+}
+}
+
+/// Internal information on a Rust-implemented subclass of Object.
+/// Only public because it is used by macros.
+pub unsafe trait TypeImpl: ObjectType + ObjectImpl {
+type Super: ObjectType;
+type Conf: ConstDefault;
+type State: Default;
+
+const CLASS_INIT: unsafe extern "C" fn(klass: *mut c_void, data: *mut
c_void);
+
+fn uninit_conf(obj: MaybeUninit) ->
MaybeUninit;
+fn uninit_state(obj: MaybeUninit) ->
MaybeUninit;
+}
+
+unsafe fn rust_type_register() {
+unsafe extern "C" fn rust_instance_mem_init(obj: *mut c_void)
{
+let obj: std::mem::MaybeUninit = *(obj.cast());
+
+T::uninit_conf(obj).write(ConstDefault::DEFAULT);
+T::uninit_state(obj).write(Default::default());
+}
+
+unsafe extern "C" fn rust_instance_finalize(obj: *mut c_void)
{
+let obj: *mut T = obj.cast();
+drop_in_place(obj);
+}
+
+let ti = TypeInfo {
+name: T::TYPE.as_ptr(),
+parent: T::Super::TYPE.as_ptr(),
+instance_size: mem::size_of::(),
+instance_mem_init: Some(rust_instance_mem_init::),
+instance_finalize: Some(rust_instance_finalize::),
+class_init: Some(T::CLASS_INIT),
+
+// SAFETY: TypeInfo is defined in C and all fields are okay to be
zeroed
+..Zeroed::zeroed()
+};
+
+type_register()
+}
+
+#[macro_export]
+macro_rules! qom_define_type {
+($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty; @extends $super:ty
$(,$supers:ty)*) => {
+struct $struct {
+// self.base dropped by call to superclass ins
[PATCH 01/14] add skeleton
qemu/ is where target-independent code goes. This code should
not use constructors and will be brought in as needed.
qemu-hw/ is where the target-dependent code goes, which is going
to be built depending on Kconfig symbols.
Signed-off-by: Paolo Bonzini
---
.gitignore | 2 +
Cargo.toml | 3 +
qemu-hw/Cargo.toml | 6 ++
qemu-hw/src/lib.rs | 0
qemu-hw/src/main.rs | 3 +
qemu/Cargo.toml | 7 ++
qemu/src/bindings/mod.rs | 88 +
qemu/src/lib.rs | 4 ++
9 files changed, 249 insertions(+)
create mode 100644 .gitignore
create mode 100644 Cargo.toml
create mode 100644 qemu-hw/Cargo.toml
create mode 100644 qemu-hw/src/lib.rs
create mode 100644 qemu-hw/src/main.rs
create mode 100644 qemu/Cargo.toml
create mode 100644 qemu/src/bindings/mod.rs
create mode 100644 qemu/src/lib.rs
diff --git a/.gitignore b/.gitignore
new file mode 100644
index 000..3fbfc34
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,2 @@
+/target
+/.git-old
diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 000..f66a80e
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,3 @@
+[workspace]
+members = ["qemu"]
+resolver = "2"
diff --git a/qemu-hw/Cargo.toml b/qemu-hw/Cargo.toml
new file mode 100644
index 000..9bd6930
--- /dev/null
+++ b/qemu-hw/Cargo.toml
@@ -0,0 +1,6 @@
+[package]
+name = "qemu-hw"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
diff --git a/qemu-hw/src/lib.rs b/qemu-hw/src/lib.rs
new file mode 100644
index 000..e69de29
diff --git a/qemu-hw/src/main.rs b/qemu-hw/src/main.rs
new file mode 100644
index 000..e7a11a9
--- /dev/null
+++ b/qemu-hw/src/main.rs
@@ -0,0 +1,3 @@
+fn main() {
+println!("Hello, world!");
+}
diff --git a/qemu/Cargo.toml b/qemu/Cargo.toml
new file mode 100644
index 000..18d0fa4
--- /dev/null
+++ b/qemu/Cargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "qemu"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+const-default = { version = "~1", features = ["derive"] }
diff --git a/qemu/src/bindings/mod.rs b/qemu/src/bindings/mod.rs
new file mode 100644
index 000..a49447b
--- /dev/null
+++ b/qemu/src/bindings/mod.rs
@@ -0,0 +1,88 @@
+use std::ffi::{c_char, c_void};
+
+#[repr(C)]
+pub struct Object {
+pub klass: *mut c_void,
+pub free: extern "C" fn(c: *mut c_void),
+pub properties: *mut c_void,
+pub r#ref: u32,
+pub parent: *mut Object,
+}
+
+#[repr(C)]
+pub struct ObjectClass {
+pub unparent: Option,
+}
+
+#[repr(C)]
+pub struct DeviceState {
+pub base: Object,
+}
+
+#[repr(C)]
+#[allow(non_camel_case_types)]
+pub struct PropertyInfo {
+pub name: *const c_char,
+pub description: *const c_char,
+// ...
+}
+#[repr(C)]
+pub struct Property {
+pub name: *const c_char,
+pub offset: usize,
+pub default: u64,
+pub info: *const PropertyInfo,
+}
+
+pub struct DeviceClass {
+pub oc: ObjectClass,
+
+pub realize: Option,
+pub unrealize: Option,
+pub cold_reset: Option,
+pub properties: *const Property,
+}
+
+#[repr(C)]
+pub struct TypeInfo {
+pub name: *const c_char,
+pub parent: *const c_char,
+pub instance_mem_init: Option,
+pub instance_init: Option,
+pub instance_finalize: Option,
+pub class_init: Option,
+pub instance_size: usize,
+}
+
+#[repr(C)]
+pub struct Error {
+_unused: c_char,
+}
+
+extern "C" {
+pub fn error_setg_internal(
+errp: *mut *mut Error,
+src: *mut c_char,
+line: u32,
+func: *mut c_char,
+fmt: *const c_char,
+...
+);
+pub fn error_get_pretty(errp: *const Error) -> *mut c_char;
+pub fn error_free(errp: *mut Error);
+
+pub fn object_dynamic_cast(obj: *mut Object, typ: *const c_char) -> *mut
c_void;
+pub fn object_property_add_child(obj: *mut Object, typ: *const c_char,
+child: *mut Object);
+pub fn object_get_typename(obj: *const Object) -> *const c_char;
+pub fn object_ref(obj: *mut Object);
+pub fn object_new(typ: *const c_char) -> *const Object;
+pub fn object_unref(obj: *mut Object);
+pub fn object_unparent(obj: *mut Object);
+
+pub fn device_cold_reset(obj: *mut DeviceState);
+pub fn device_realize(obj: *mut DeviceState, err: *mut *mut Error) -> bool;
+pub fn type_register(obj: *const TypeInfo);
+
+pub static qdev_prop_bool: PropertyInfo;
+}
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
new file mode 100644
index 000..fce21d7
--- /dev/null
+++ b/qemu/src/lib.rs
@@ -0,0 +1,4 @@
+#![allow(unused_macros)]
+#![allow(dead_code)]
+
+pub mod bindings;
--
2.45.2
[PATCH 10/14] rust: add idiomatic bindings to define Device subclasses
Provide a macro to register a type and automatically define qdev
properties. Subclasses of DeviceState must define a trait DeviceImpl, to
point the type definition machinery to the implementation of virtual
functions in DeviceState.
Signed-off-by: Paolo Bonzini
---
qemu/src/hw/core/device_impl.rs | 140
qemu/src/hw/core/mod.rs | 1 +
qemu/src/lib.rs | 5 ++
qemu/tests/main.rs | 52 +++-
4 files changed, 197 insertions(+), 1 deletion(-)
create mode 100644 qemu/src/hw/core/device_impl.rs
diff --git a/qemu/src/hw/core/device_impl.rs b/qemu/src/hw/core/device_impl.rs
new file mode 100644
index 000..80b0e5e
--- /dev/null
+++ b/qemu/src/hw/core/device_impl.rs
@@ -0,0 +1,140 @@
+//! Macros and traits to implement subclasses of Device in Rust
+//!
+//! @author Paolo Bonzini
+
+#![allow(clippy::missing_safety_doc)]
+
+use std::ffi::c_void;
+
+use crate::bindings;
+use crate::bindings::DeviceClass;
+use crate::bindings::DeviceState;
+use crate::bindings::Property;
+
+use crate::qom::object_impl::ObjectImpl;
+use crate::qom::object_impl::TypeImpl;
+
+use crate::qom::refs::ObjectCast;
+
+use crate::util::error::Error;
+
+/// Information on which superclass methods are overridden
+/// by a Rust-implemented subclass of Device.
+pub trait DeviceImpl: ObjectImpl + DeviceTypeImpl {
+/// If not `None`, a function that implements the `realize` member
+/// of the QOM `DeviceClass`.
+const REALIZE: Option crate::Result<()>> = None;
+
+/// If not `None`, a function that implements the `unrealize` member
+/// of the QOM `DeviceClass`.
+const UNREALIZE: Option = None;
+
+/// If not `None`, a function that implements the `cold_reset` member
+/// of the QOM `DeviceClass`.
+const COLD_RESET: Option = None;
+}
+
+impl DeviceClass {
+pub fn class_init( self) {
+unsafe extern "C" fn rust_cold_reset(obj: *mut
DeviceState) {
+let f = T::COLD_RESET.unwrap();
+f((&*obj).unsafe_cast::())
+}
+self.cold_reset = T::COLD_RESET.map(|_| rust_cold_reset:: as _);
+
+unsafe extern "C" fn rust_realize(
+obj: *mut DeviceState,
+errp: *mut *mut bindings::Error,
+) {
+let f = T::REALIZE.unwrap();
+let result = f((&*obj).unsafe_cast::());
+Error::ok_or_propagate(result, errp);
+}
+self.realize = T::REALIZE.map(|_| rust_realize:: as _);
+
+unsafe extern "C" fn rust_unrealize(obj: *mut
DeviceState) {
+let f = T::UNREALIZE.unwrap();
+f((&*obj).unsafe_cast::())
+}
+self.unrealize = T::UNREALIZE.map(|_| rust_unrealize:: as _);
+
+self.properties = ::properties();
+
+// Now initialize the ObjectClass from the ObjectImpl.
+self.oc.class_init::();
+}
+}
+
+impl DeviceState {
+pub unsafe extern "C" fn rust_class_init(
+klass: *mut c_void,
+_data: *mut c_void,
+) {
+let dc: DeviceClass = *(klass.cast());
+dc.class_init::();
+}
+}
+
+/// Internal information on a Rust-implemented subclass of Device.
+/// Only public because it is used by macros.
+pub unsafe trait DeviceTypeImpl: TypeImpl {
+const CONF_OFFSET: usize;
+
+// This needs to be here, and not in DeviceImpl, because properties
+// reference statics (for globals defined in C, e.g. qdev_prop_bool)
+// which is unstable (see https://github.com/rust-lang/rust/issues/119618,
+// feature const_refs_to_static)
+fn properties() -> *const Property;
+}
+
+pub struct QdevPropBool;
+impl QdevPropBool {
+pub const fn convert(value: ) -> u64 {
+*value as u64
+}
+}
+
+#[macro_export]
+macro_rules! qdev_prop {
+(@internal bool, $name:expr, $default:expr, $offset:expr) => {
+$crate::Property {
+name: $name.as_ptr(),
+offset: $offset,
+default:
$crate::hw::core::device_impl::QdevPropBool::convert(&($default)),
+info: unsafe { &$crate::bindings::qdev_prop_bool },
+}
+};
+
+// Replace field with typechecking expression and offset
+($kind:tt, $name:expr, $type:ty, $default:expr, $field:ident) => {
+qdev_prop!(@internal
+$kind,
+$name,
+(<$crate::conf_type!($type) as ConstDefault>::DEFAULT).$field,
+<$type as $crate::DeviceTypeImpl>::CONF_OFFSET +
std::mem::offset_of!($crate::conf_type!($type), $field)
+)
+};
+}
+
+#[macro_export]
+macro_rules! qdev_define_type {
+($name:expr, $struct:ident, $conf_ty:ty, $state_ty:ty;
+ @extends $super:ty $(,$supers:ty)*;
+ @properties [$($props: expr),+]) => {
+$crate::qom_define_type!(
+$name, $struct, $conf_ty, $state_ty;
+@extends $super $(,$supers)*, $crate::Object);
+
+
[PATCH 02/14] set expectations
Signed-off-by: Paolo Bonzini --- README.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 README.md diff --git a/README.md b/README.md new file mode 100644 index 000..5ef6f0d --- /dev/null +++ b/README.md @@ -0,0 +1 @@ +This is very experimental and barely compiles -- 2.45.2
[PATCH 07/14] rust: define wrappers for methods of the QOM Object class
Provide a trait that can be used to invoke methods of the QOM object
class. The trait extends Deref and has a blanket implementation for any
type that dereferences to IsA. This way, it can be used on any
struct that dereferences to Object or a subclass.
Signed-off-by: Paolo Bonzini
---
qemu/src/lib.rs| 2 +
qemu/src/qom/object.rs | 92 ++
qemu/src/qom/refs.rs | 8
3 files changed, 102 insertions(+)
diff --git a/qemu/src/lib.rs b/qemu/src/lib.rs
index 0d91623..a6e7b17 100644
--- a/qemu/src/lib.rs
+++ b/qemu/src/lib.rs
@@ -5,6 +5,8 @@ pub mod bindings;
pub use bindings::Object;
pub mod qom;
+pub use qom::object::ObjectClassMethods;
+pub use qom::object::ObjectMethods;
pub use qom::object::ObjectType;
pub use qom::refs::ObjectCast;
pub use qom::refs::Owned;
diff --git a/qemu/src/qom/object.rs b/qemu/src/qom/object.rs
index bd6b957..4e84e29 100644
--- a/qemu/src/qom/object.rs
+++ b/qemu/src/qom/object.rs
@@ -2,12 +2,26 @@
//!
//! @author Paolo Bonzini
+use std::borrow::Cow;
use std::ffi::CStr;
+use std::fmt;
+use std::ops::Deref;
+use crate::bindings::object_get_typename;
+use crate::bindings::object_property_add_child;
+use crate::bindings::object_new;
+use crate::bindings::object_unparent;
use crate::bindings::Object;
use crate::qom_isa;
+use crate::qom::refs::IsA;
+use crate::qom::refs::ObjectCast;
+use crate::qom::refs::Owned;
+
+use crate::util::foreign::CloneToForeign;
+use crate::util::foreign::FromForeign;
+
/// Trait exposed by all structs corresponding to QOM objects.
/// Defines "class methods" for the class. Usually these can be
/// implemented on the class itself; here, using a trait allows
@@ -31,4 +45,82 @@ unsafe impl ObjectType for Object {
const TYPE: &'static CStr = c"object";
}
+// --
+// Object class
+
qom_isa!(Object);
+
+/// Trait for class methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+
+pub trait ObjectClassMethods: IsA {
+/// Return a new reference counted instance of this class
+fn new() -> Owned {
+// SAFETY: the object created by object_new is allocated on
+// the heap and has a reference count of 1
+unsafe {
+let obj = &*object_new(Self::TYPE.as_ptr());
+Owned::from_raw(obj.unsafe_cast::())
+}
+}
+}
+
+/// Trait for methods exposed by the Object class. The methods can be
+/// called on all objects that have the trait `IsA`.
+///
+/// The trait should only be used through the blanket implementation,
+/// which guarantees safety via `IsA`
+pub trait ObjectMethods: Deref
+where
+Self::Target: IsA,
+{
+/// Return the name of the type of `self`
+fn typename() -> Cow<'_, str> {
+let obj = self.upcast::();
+// SAFETY: safety of this is the requirement for implementing IsA
+// The result of the C API has static lifetime
+unsafe {
+Cow::cloned_from_foreign(object_get_typename(obj.as_mut_ptr()))
+}
+}
+
+/// Add an object as a child of the receiver.
+fn property_add_child(, name: , child: Owned)
+{
+let obj = self.upcast::();
+let name = name.clone_to_foreign();
+unsafe {
+// SAFETY: casting to object is always safe even if `child`'s
+// target type is an interface type
+let child = child.unsafe_cast::();
+object_property_add_child(obj.as_mut_ptr(),
+ name.as_ptr(),
+ child.as_mut_ptr());
+
+// object_property_add_child() added a reference of its own;
+// dropping the one in `child` is the common case.
+}
+}
+
+/// Remove the object from the QOM tree
+fn unparent() {
+let obj = self.upcast::();
+// SAFETY: safety of this is the requirement for implementing IsA
+unsafe {
+object_unparent(obj.as_mut_ptr());
+}
+}
+
+/// Convenience function for implementing the Debug trait
+fn debug_fmt(, f: fmt::Formatter) -> fmt::Result {
+f.debug_tuple(())
+.field(&(self as *const Self))
+.finish()
+}
+}
+
+impl ObjectClassMethods for R where R: IsA {}
+impl ObjectMethods for R where R::Target: IsA {}
diff --git a/qemu/src/qom/refs.rs b/qemu/src/qom/refs.rs
index a319bde..431ef0a 100644
--- a/qemu/src/qom/refs.rs
+++ b/qemu/src/qom/refs.rs
@@ -6,9 +6,11 @@ use crate::bindings::object_dynamic_cast;
use crate::bindings::Object;
use crate::bindings::{object_ref, object_unref};
+use crate::qom::object::ObjectMethods;
use crate::qom::object::ObjectType;
use std::borrow::Borrow;
+use std::fmt::{self, Debug};
use std::mem::ManuallyDro
Re: [PATCH 2/2] target/i386: drop AMD machine check bits from Intel CPUID
On Mon, Jul 1, 2024 at 6:08 AM Zhao Liu wrote:
> > > It seems to adjust it based on vendor in kvm_arch_get_supported_cpuid()
> > > is better than in x86_cpu_get_supported_feature_word(). Otherwise
> > > kvm_arch_get_supported_cpuid() still returns "risky" value for Intel VMs.
> >
> > But the cpuid bit is only invalid for Intel *guest* vendor, not host. It is
> > not a problem to have it if you run on Intel host but have a guest model
> > with AMD vendor.
> >
> > I will check if there are other callers of kvm_arch_get_supported_cpuid(),
> > or callers of x86_cpu_get_supported_feature_word() with NULL cpu, that
> > might care about the difference.
>
> Another example is CPUID_EXT3_TOPOEXT, though it's a no_autoenable_flags,
> it can be set by "-cpu host,+topoext" on Intel platforms.
That was done by commit 7210a02c585 ("i386: Disable TOPOEXT by default
on "-cpu host"", 2018-08-16) which however does not explain what the
bug was. It talks about missing or inconsistent cache topology
information, but that's not precise enough to decide what the problem
was.
> For this case, we have recognized that that the host/max CPU should only
> contain vender specific features, and I think it would be hard to expand
> such a rule afterwards, especially since there's other x86 vender like
> zhaoxin who implement a subset of Intel/AMD:
>
> What about a new flag "host_bare_metal_check" in FeatureWordInfo? Then
> if a feature is marked as "host_bare_metal_check", in addition to the
> current checks in x86_cpu_get_supported_feature_word(), bare-metal CPUID
> check is also needed (by host_cpuid()) for "host" CPU.
I don't see why it's needed. The bare metal vendor is not visible to
the guest, therefore it should have no bearing on whether a bit is
included in CPUID.
Paolo
Re: [RFC PATCH v3 2/5] rust: add bindgen step as a meson dependency
On Fri, Jun 28, 2024 at 9:12 PM Pierrick Bouvier wrote: > However, even tough I can build the executable, I get this error: > $ ./build/qemu-system-aarch64 -M virt > C:\w\qemu\build\qemu-system-aarch64.exe: unknown type 'x-pl011-rust' > > Any idea of what could be missing here? Maybe the underlying mechanism to invoke constructors is different? Perhaps we could use https://crates.io/crates/ctor instead? Paolo
[PULL 06/23] meson: remove dead optimization option
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 13 -
meson_options.txt | 2 --
scripts/meson-buildoptions.sh | 3 ---
3 files changed, 18 deletions(-)
diff --git a/meson.build b/meson.build
index 6e694ecd9fe..54e6b09f4fb 100644
--- a/meson.build
+++ b/meson.build
@@ -2874,18 +2874,6 @@ config_host_data.set('CONFIG_AVX2_OPT',
get_option('avx2') \
int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX2 not available').allowed())
-config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
- .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512F') \
- .require(cc.links('''
-#include
-#include
-static int __attribute__((target("avx512f"))) bar(void *a) {
- __m512i x = *(__m512i *)a;
- return _mm512_test_epi64_mask(x, x);
-}
-int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
- '''), error_message: 'AVX512F not available').allowed())
-
config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
.require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512BW') \
.require(cc.links('''
@@ -4283,7 +4271,6 @@ summary_info += {'mutex debugging':
get_option('debug_mutex')}
summary_info += {'memory allocator': get_option('malloc')}
summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')}
summary_info += {'avx512bw optimization':
config_host_data.get('CONFIG_AVX512BW_OPT')}
-summary_info += {'avx512f optimization':
config_host_data.get('CONFIG_AVX512F_OPT')}
summary_info += {'gcov': get_option('b_coverage')}
summary_info += {'thread sanitizer': get_option('tsan')}
summary_info += {'CFI support': get_option('cfi')}
diff --git a/meson_options.txt b/meson_options.txt
index 6065ed2d352..0269fa0f16e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -119,8 +119,6 @@ option('membarrier', type: 'feature', value: 'disabled',
option('avx2', type: 'feature', value: 'auto',
description: 'AVX2 optimizations')
-option('avx512f', type: 'feature', value: 'disabled',
- description: 'AVX512F optimizations')
option('avx512bw', type: 'feature', value: 'auto',
description: 'AVX512BW optimizations')
option('keyring', type: 'feature', value: 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 62842d47e88..cfadb5ea86a 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -95,7 +95,6 @@ meson_options_help() {
printf "%s\n" ' auth-pamPAM access control'
printf "%s\n" ' avx2AVX2 optimizations'
printf "%s\n" ' avx512bwAVX512BW optimizations'
- printf "%s\n" ' avx512f AVX512F optimizations'
printf "%s\n" ' blkio libblkio block device driver'
printf "%s\n" ' bochs bochs image format support'
printf "%s\n" ' bpf eBPF support'
@@ -240,8 +239,6 @@ _meson_option_parse() {
--disable-avx2) printf "%s" -Davx2=disabled ;;
--enable-avx512bw) printf "%s" -Davx512bw=enabled ;;
--disable-avx512bw) printf "%s" -Davx512bw=disabled ;;
---enable-avx512f) printf "%s" -Davx512f=enabled ;;
---disable-avx512f) printf "%s" -Davx512f=disabled ;;
--enable-gcov) printf "%s" -Db_coverage=true ;;
--disable-gcov) printf "%s" -Db_coverage=false ;;
--enable-lto) printf "%s" -Db_lto=true ;;
--
2.45.2
[PULL 03/23] Revert "host/i386: assume presence of SSSE3"
This reverts commit 433cd6d94a8256af70a5200f236dc8047c3c1468.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
util/cpuinfo-i386.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 6d474a6259a..ca74ef04f54 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -38,8 +38,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
-/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
-info |= (c & bit_AES) ? CPUINFO_AES : 0;
+/* Our AES support requires PSHUFB as well. */
+info |= ((c & bit_AES) && (c & bit_SSSE3) ? CPUINFO_AES : 0);
/* For AVX features, we must check available and usable. */
if ((c & bit_AVX) && (c & bit_OSXSAVE)) {
--
2.45.2
[PULL 07/23] block: make assertion more generic
.bdrv_needs_filename is only set for drivers that also set bdrv_file_open,
i.e. protocol drivers.
So we can make the assertion always, it will always pass for those drivers
that use bdrv_open.
Signed-off-by: Paolo Bonzini
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 468cf5e67d7..69a2905178a 100644
--- a/block.c
+++ b/block.c
@@ -1655,8 +1655,8 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->drv = drv;
bs->opaque = g_malloc0(drv->instance_size);
+assert(!drv->bdrv_needs_filename || bs->filename[0]);
if (drv->bdrv_file_open) {
-assert(!drv->bdrv_needs_filename || bs->filename[0]);
ret = drv->bdrv_file_open(bs, options, open_flags, _err);
} else if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
--
2.45.2
[PULL 15/23] target/i386: use cpu_cc_dst for CC_OP_POPCNT
It is the only CCOp, among those that compute ZF from one of the cc_op_*
registers, that uses cpu_cc_src. Do not make it the odd one off,
instead use cpu_cc_dst like the others.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 2 +-
target/i386/tcg/cc_helper.c | 2 +-
target/i386/tcg/translate.c | 4 ++--
target/i386/tcg/emit.c.inc | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 52571ababe2..1b4edbe0580 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1332,7 +1332,7 @@ typedef enum {
CC_OP_BMILGQ,
CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_SRC, all other flags clear. */
+CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/cc_helper.c b/target/i386/tcg/cc_helper.c
index f76e9cb8cfb..301ed954064 100644
--- a/target/i386/tcg/cc_helper.c
+++ b/target/i386/tcg/cc_helper.c
@@ -107,7 +107,7 @@ target_ulong helper_cc_compute_all(target_ulong dst,
target_ulong src1,
case CC_OP_CLR:
return CC_Z | CC_P;
case CC_OP_POPCNT:
-return src1 ? 0 : CC_Z;
+return dst ? 0 : CC_Z;
case CC_OP_MULB:
return compute_all_mulb(dst, src1);
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index ad1819815ab..eb353dc3c9f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -324,7 +324,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = {
[CC_OP_ADOX] = USES_CC_SRC | USES_CC_SRC2,
[CC_OP_ADCOX] = USES_CC_DST | USES_CC_SRC | USES_CC_SRC2,
[CC_OP_CLR] = 0,
-[CC_OP_POPCNT] = USES_CC_SRC,
+[CC_OP_POPCNT] = USES_CC_DST,
};
static void set_cc_op_1(DisasContext *s, CCOp op, bool dirty)
@@ -1020,7 +1020,7 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
+return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 11faa70b5e2..fc7477833bc 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -2804,10 +2804,10 @@ static void gen_POPA(DisasContext *s, X86DecodedInsn
*decode)
static void gen_POPCNT(DisasContext *s, X86DecodedInsn *decode)
{
-decode->cc_src = tcg_temp_new();
+decode->cc_dst = tcg_temp_new();
decode->cc_op = CC_OP_POPCNT;
-tcg_gen_mov_tl(decode->cc_src, s->T0);
+tcg_gen_mov_tl(decode->cc_dst, s->T0);
tcg_gen_ctpop_tl(s->T0, s->T0);
}
--
2.45.2
[PULL 19/23] target/i386: SEV: store pointer to decoded id_block in SevSnpGuest
Do not rely on finish->id_block_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 6daa8c264cd..2d4cfd41e83 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -153,6 +153,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
char *id_block_base64;
+uint8_t *id_block;
char *id_auth;
char *host_data;
@@ -2170,16 +2171,15 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
+g_free(sev_snp_guest->id_block);
g_free(sev_snp_guest->id_block_base64);
-g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_block_base64 = g_strdup(value);
+sev_snp_guest->id_block =
+qbase64_decode(sev_snp_guest->id_block_base64, -1, , errp);
-finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
-
-if (!finish->id_block_uaddr) {
+if (!sev_snp_guest->id_block) {
return;
}
@@ -2190,6 +2190,7 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
finish->id_block_en = 1;
+finish->id_block_uaddr = (uintptr_t)sev_snp_guest->id_block;
}
static char *
--
2.45.2
[PULL 18/23] target/i386: SEV: rename sev_snp_guest->id_block
Free the "id_block" name for the binary version of the data.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 30b83f1d77d..6daa8c264cd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -152,7 +152,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
-char *id_block;
+char *id_block_base64;
char *id_auth;
char *host_data;
@@ -1296,7 +1296,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2159,7 +2159,7 @@ sev_snp_guest_get_id_block(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_block);
+return g_strdup(sev_snp_guest->id_block_base64);
}
static void
@@ -2170,14 +2170,14 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
-g_free(sev_snp_guest->id_block);
+g_free(sev_snp_guest->id_block_base64);
g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_block = g_strdup(value);
+sev_snp_guest->id_block_base64 = g_strdup(value);
finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
if (!finish->id_block_uaddr) {
return;
--
2.45.2
[PULL 20/23] target/i386: SEV: rename sev_snp_guest->id_auth
Free the "id_auth" name for the binary version of the data.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2d4cfd41e83..a6b063b762c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -154,7 +154,7 @@ struct SevSnpGuestState {
char *guest_visible_workarounds;
char *id_block_base64;
uint8_t *id_block;
-char *id_auth;
+char *id_auth_base64;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -1297,7 +1297,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64,
sev_snp->id_auth_base64,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2198,7 +2198,7 @@ sev_snp_guest_get_id_auth(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_auth);
+return g_strdup(sev_snp_guest->id_auth_base64);
}
static void
@@ -2208,14 +2208,14 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
-g_free(sev_snp_guest->id_auth);
+g_free(sev_snp_guest->id_auth_base64);
g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_auth = g_strdup(value);
+sev_snp_guest->id_auth_base64 = g_strdup(value);
finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
if (!finish->id_auth_uaddr) {
return;
--
2.45.2
[PULL 17/23] target/i386: remove unused enum
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 16
1 file changed, 16 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 934c514e64f..95bad55bf46 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -282,22 +282,6 @@ enum {
JCC_LE,
};
-enum {
-/* I386 int registers */
-OR_EAX, /* MUST be even numbered */
-OR_ECX,
-OR_EDX,
-OR_EBX,
-OR_ESP,
-OR_EBP,
-OR_ESI,
-OR_EDI,
-
-OR_TMP0 = 16,/* temporary operand register */
-OR_TMP1,
-OR_A0, /* temporary register used when doing address evaluation */
-};
-
enum {
USES_CC_DST = 1,
USES_CC_SRC = 2,
--
2.45.2
[PULL 05/23] meson: allow configuring the x86-64 baseline
Add a Meson option to configure which x86-64 instruction
set to use. QEMU will now default to x86-64-v1 + cmpxchg16b for
64-bit builds (that corresponds to a Pentium 4 for 32-bit builds).
The baseline can be tuned down to Pentium Pro for 32-bit builds (with
-Dx86_version=0), or up as desired.
Acked-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 41 ---
meson_options.txt | 3 +++
scripts/meson-buildoptions.sh | 3 +++
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index 97e00d6f59b..6e694ecd9fe 100644
--- a/meson.build
+++ b/meson.build
@@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links('''
qemu_common_flags = ['-march=i486'] + qemu_common_flags
endif
-# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code)
-if host_arch == 'i386'
- qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
-endif
+# Pick x86-64 baseline version
if host_arch in ['i386', 'x86_64']
- qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags
-endif
-if host_arch == 'x86_64'
- qemu_common_flags = ['-mcx16'] + qemu_common_flags
+ if get_option('x86_version') == '0' and host_arch == 'x86_64'
+error('x86_64-v1 required for x86-64 hosts')
+ endif
+
+ # add flags for individual instruction set extensions
+ if get_option('x86_version') >= '1'
+if host_arch == 'i386'
+ qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
+else
+ # present on basically all processors but technically not part of
+ # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
+ qemu_common_flags = ['-mcx16'] + qemu_common_flags
+endif
+ endif
+ if get_option('x86_version') >= '2'
+qemu_common_flags = ['-mpopcnt'] + qemu_common_flags
+qemu_common_flags = cc.get_supported_arguments('-mneeded') +
qemu_common_flags
+ endif
+ if get_option('x86_version') >= '3'
+qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma',
'-mf16c'] + qemu_common_flags
+ endif
+
+ # add required vector instruction set (each level implies those below)
+ if get_option('x86_version') == '1'
+qemu_common_flags = ['-msse2'] + qemu_common_flags
+ elif get_option('x86_version') == '2'
+qemu_common_flags = ['-msse4.2'] + qemu_common_flags
+ elif get_option('x86_version') == '3'
+qemu_common_flags = ['-mavx2'] + qemu_common_flags
+ elif get_option('x86_version') == '4'
+qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd',
'-mavx512dq', '-mavx512vl'] + qemu_common_flags
+ endif
endif
if get_option('prefer_static')
diff --git a/meson_options.txt b/meson_options.txt
index 7a79dd89706..6065ed2d352 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -370,3 +370,6 @@ option('qemu_ga_version', type: 'string', value: '',
option('hexagon_idef_parser', type : 'boolean', value : true,
description: 'use idef-parser to automatically generate TCG code for
the Hexagon frontend')
+
+option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'],
value: '1',
+ description: 'tweak required x86_64 architecture version beyond
compiler default')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 58d49a447d5..62842d47e88 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -82,6 +82,8 @@ meson_options_help() {
printf "%s\n" ' --with-suffix=VALUE Suffix for QEMU
data/modules/config directories'
printf "%s\n" ' (can be empty) [qemu]'
printf "%s\n" ' --with-trace-file=VALUE Trace file prefix for simple
backend [trace]'
+ printf "%s\n" ' --x86-version=CHOICE tweak required x86_64 architecture
version beyond'
+ printf "%s\n" ' compiler default [1] (choices:
0/1/2/3)'
printf "%s\n" ''
printf "%s\n" 'Optional features, enabled with --enable-FEATURE and'
printf "%s\n" 'disabled with --disable-FEATURE, default is enabled if
available'
@@ -552,6 +554,7 @@ _meson_option_parse() {
--disable-werror) printf "%s" -Dwerror=false ;;
--enable-whpx) printf "%s" -Dwhpx=enabled ;;
--disable-whpx) printf "%s" -Dwhpx=disabled ;;
+--x86-version=*) quote_sh "-Dx86_version=$2" ;;
--enable-xen) printf "%s" -Dxen=enabled ;;
--disable-xen) printf "%s" -Dxen=disabled ;;
--enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
--
2.45.2
[PULL 14/23] target/i386: fix CC_OP dump
POPCNT was missing, and the entries were all out of order after
ADCX/ADOX/ADCOX were moved close to EFLAGS. Just use designated
initializers.
Fixes: 4885c3c4953 ("target-i386: Use ctpop helper", 2017-01-10)
Fixes: cc155f19717 ("target/i386: rewrite flags writeback for ADCX/ADOX",
2024-06-11)
Signed-off-by: Paolo Bonzini
---
target/i386/cpu-dump.c | 101 +
1 file changed, 51 insertions(+), 50 deletions(-)
diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index 40697064d92..3bb8e440916 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -28,69 +28,70 @@
/* x86 debug */
static const char *cc_op_str[CC_OP_NB] = {
-"DYNAMIC",
-"EFLAGS",
+[CC_OP_DYNAMIC] = "DYNAMIC",
-"MULB",
-"MULW",
-"MULL",
-"MULQ",
+[CC_OP_EFLAGS] = "EFLAGS",
+[CC_OP_ADCX] = "ADCX",
+[CC_OP_ADOX] = "ADOX",
+[CC_OP_ADCOX] = "ADCOX",
-"ADDB",
-"ADDW",
-"ADDL",
-"ADDQ",
+[CC_OP_MULB] = "MULB",
+[CC_OP_MULW] = "MULW",
+[CC_OP_MULL] = "MULL",
+[CC_OP_MULQ] = "MULQ",
-"ADCB",
-"ADCW",
-"ADCL",
-"ADCQ",
+[CC_OP_ADDB] = "ADDB",
+[CC_OP_ADDW] = "ADDW",
+[CC_OP_ADDL] = "ADDL",
+[CC_OP_ADDQ] = "ADDQ",
-"SUBB",
-"SUBW",
-"SUBL",
-"SUBQ",
+[CC_OP_ADCB] = "ADCB",
+[CC_OP_ADCW] = "ADCW",
+[CC_OP_ADCL] = "ADCL",
+[CC_OP_ADCQ] = "ADCQ",
-"SBBB",
-"SBBW",
-"SBBL",
-"SBBQ",
+[CC_OP_SUBB] = "SUBB",
+[CC_OP_SUBW] = "SUBW",
+[CC_OP_SUBL] = "SUBL",
+[CC_OP_SUBQ] = "SUBQ",
-"LOGICB",
-"LOGICW",
-"LOGICL",
-"LOGICQ",
+[CC_OP_SBBB] = "SBBB",
+[CC_OP_SBBW] = "SBBW",
+[CC_OP_SBBL] = "SBBL",
+[CC_OP_SBBQ] = "SBBQ",
-"INCB",
-"INCW",
-"INCL",
-"INCQ",
+[CC_OP_LOGICB] = "LOGICB",
+[CC_OP_LOGICW] = "LOGICW",
+[CC_OP_LOGICL] = "LOGICL",
+[CC_OP_LOGICQ] = "LOGICQ",
-"DECB",
-"DECW",
-"DECL",
-"DECQ",
+[CC_OP_INCB] = "INCB",
+[CC_OP_INCW] = "INCW",
+[CC_OP_INCL] = "INCL",
+[CC_OP_INCQ] = "INCQ",
-"SHLB",
-"SHLW",
-"SHLL",
-"SHLQ",
+[CC_OP_DECB] = "DECB",
+[CC_OP_DECW] = "DECW",
+[CC_OP_DECL] = "DECL",
+[CC_OP_DECQ] = "DECQ",
-"SARB",
-"SARW",
-"SARL",
-"SARQ",
+[CC_OP_SHLB] = "SHLB",
+[CC_OP_SHLW] = "SHLW",
+[CC_OP_SHLL] = "SHLL",
+[CC_OP_SHLQ] = "SHLQ",
-"BMILGB",
-"BMILGW",
-"BMILGL",
-"BMILGQ",
+[CC_OP_SARB] = "SARB",
+[CC_OP_SARW] = "SARW",
+[CC_OP_SARL] = "SARL",
+[CC_OP_SARQ] = "SARQ",
-"ADCX",
-"ADOX",
-"ADCOX",
+[CC_OP_BMILGB] = "BMILGB",
+[CC_OP_BMILGW] = "BMILGW",
+[CC_OP_BMILGL] = "BMILGL",
+[CC_OP_BMILGQ] = "BMILGQ",
-"CLR",
+[CC_OP_POPCNT] = "POPCNT",
+[CC_OP_CLR] = "CLR",
};
static void
--
2.45.2
[PULL 11/23] exec: avoid using C++ keywords in function parameters
From: Roman Kiryanov
to use the QEMU headers with a C++ compiler.
Signed-off-by: Roman Kiryanov
Link: https://lore.kernel.org/r/20240618224553.878869-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0903513d132..154626f9ad2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -925,7 +925,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_start)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_stop:
@@ -944,7 +944,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_sync:
--
2.45.2
[PULL 04/23] Revert "host/i386: assume presence of SSE2"
This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
util/cpuinfo-i386.c | 1 +
host/include/i386/host/bufferiszero.c.inc | 5 +++--
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
+#define CPUINFO_SSE2(1u << 7)
#define CPUINFO_AVX1(1u << 9)
#define CPUINFO_AVX2(1u << 10)
#define CPUINFO_AVX512F (1u << 11)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index ca74ef04f54..90f92a42dc8 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
if (max >= 1) {
__cpuid(1, a, b, c, d);
+info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
diff --git a/host/include/i386/host/bufferiszero.c.inc
b/host/include/i386/host/bufferiszero.c.inc
index 3b9605d806f..74ae98580f6 100644
--- a/host/include/i386/host/bufferiszero.c.inc
+++ b/host/include/i386/host/bufferiszero.c.inc
@@ -110,13 +110,14 @@ static biz_accel_fn const accel_table[] = {
static unsigned best_accel(void)
{
-#ifdef CONFIG_AVX2_OPT
unsigned info = cpuinfo_init();
+
+#ifdef CONFIG_AVX2_OPT
if (info & CPUINFO_AVX2) {
return 2;
}
#endif
-return 1;
+return info & CPUINFO_SSE2 ? 1 : 0;
}
#else
--
2.45.2
[PULL 16/23] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL
Handle it like the other arithmetic cc_ops. This simplifies a
bit the implementation of bit test instructions.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 13 +++--
target/i386/tcg/translate.c | 3 +--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 1b4edbe0580..29daf370485 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1275,6 +1275,7 @@ typedef enum {
CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest. */
CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest. */
CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest. */
+CC_OP_CLR, /* Z and P set, all other flags clear. */
CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */
CC_OP_MULW,
@@ -1331,8 +1332,16 @@ typedef enum {
CC_OP_BMILGL,
CC_OP_BMILGQ,
-CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
+/*
+ * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size)
+ * is used or implemented, because the translation needs
+ * to zero-extend CC_DST anyway.
+ */
+CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear. */
+CC_OP_POPCNTW__,
+CC_OP_POPCNTL__,
+CC_OP_POPCNTQ__,
+CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ :
CC_OP_POPCNTL__,
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index eb353dc3c9f..934c514e64f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
.imm = CC_Z };
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
-case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
@@ -3177,6 +3175,7 @@ static void disas_insn_old(DisasContext *s, CPUState
*cpu, int b)
case CC_OP_SHLB ... CC_OP_SHLQ:
case CC_OP_SARB ... CC_OP_SARQ:
case CC_OP_BMILGB ... CC_OP_BMILGQ:
+case CC_OP_POPCNT:
/* Z was going to be computed from the non-zero status of CC_DST.
We can get that same Z value (and the new C value) by leaving
CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
--
2.45.2
[PULL 02/23] Revert "host/i386: assume presence of POPCNT"
This reverts commit 45ccdbcb24baf99667997fac5cf60318e5e7db51.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
tcg/i386/tcg-target.h| 5 +++--
util/cpuinfo-i386.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index c1e94d75ce1..72f6fad61e5 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -11,6 +11,7 @@
#define CPUINFO_ALWAYS (1u << 0) /* so cpuinfo is nonzero */
#define CPUINFO_MOVBE (1u << 2)
#define CPUINFO_LZCNT (1u << 3)
+#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
#define CPUINFO_AVX1(1u << 9)
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index ecc69827287..2f67a97e059 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -111,6 +111,7 @@ typedef enum {
#endif
#define have_bmi1 (cpuinfo & CPUINFO_BMI1)
+#define have_popcnt (cpuinfo & CPUINFO_POPCNT)
#define have_avx1 (cpuinfo & CPUINFO_AVX1)
#define have_avx2 (cpuinfo & CPUINFO_AVX2)
#define have_movbe(cpuinfo & CPUINFO_MOVBE)
@@ -142,7 +143,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i32 0
#define TCG_TARGET_HAS_clz_i32 1
#define TCG_TARGET_HAS_ctz_i32 1
-#define TCG_TARGET_HAS_ctpop_i321
+#define TCG_TARGET_HAS_ctpop_i32have_popcnt
#define TCG_TARGET_HAS_deposit_i32 1
#define TCG_TARGET_HAS_extract_i32 1
#define TCG_TARGET_HAS_sextract_i32 1
@@ -177,7 +178,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i64 0
#define TCG_TARGET_HAS_clz_i64 1
#define TCG_TARGET_HAS_ctz_i64 1
-#define TCG_TARGET_HAS_ctpop_i641
+#define TCG_TARGET_HAS_ctpop_i64have_popcnt
#define TCG_TARGET_HAS_deposit_i64 1
#define TCG_TARGET_HAS_extract_i64 1
#define TCG_TARGET_HAS_sextract_i64 0
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 8f2694d88f2..6d474a6259a 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -35,6 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
__cpuid(1, a, b, c, d);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
+info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
--
2.45.2
[PULL 23/23] target/i386/sev: Fix printf formats
From: Richard Henderson
hwaddr uses HWADDR_PRIx, sizeof yields size_t so uses %zu,
and gsize uses G_GSIZE_FORMAT.
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Link:
https://lore.kernel.org/r/20240626194950.1725800-4-richard.hender...@linaro.org
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0ffdf8952c3..3ab8b3c28b7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -934,8 +934,9 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
out:
if (!ret && update.gfn_start << TARGET_PAGE_BITS != data->gpa + data->len)
{
-error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
- "got GPA range %lx-%llx",
+error_report("SEV-SNP: expected update of GPA range %"
+ HWADDR_PRIx "-%" HWADDR_PRIx ","
+ "got GPA range %" HWADDR_PRIx "-%llx",
data->gpa, data->gpa + data->len, data->gpa,
update.gfn_start << TARGET_PAGE_BITS);
ret = -EIO;
@@ -2148,7 +2149,8 @@ sev_snp_guest_set_guest_visible_workarounds(Object *obj,
const char *value,
}
if (len != sizeof(start->gosvw)) {
-error_setg(errp, "parameter length of %lu exceeds max of %lu",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " exceeds max of %zu",
len, sizeof(start->gosvw));
return;
}
@@ -2185,7 +2187,8 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
if (len != KVM_SEV_SNP_ID_BLOCK_SIZE) {
-error_setg(errp, "parameter length of %lu not equal to %u",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " not equal to %u",
len, KVM_SEV_SNP_ID_BLOCK_SIZE);
return;
}
@@ -2223,7 +2226,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
}
if (len > KVM_SEV_SNP_ID_AUTH_SIZE) {
-error_setg(errp, "parameter length:ID_AUTH %lu exceeds max of %u",
+error_setg(errp, "parameter length:ID_AUTH %" G_GSIZE_FORMAT
+ " exceeds max of %u",
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
@@ -2291,7 +2295,8 @@ sev_snp_guest_set_host_data(Object *obj, const char
*value, Error **errp)
}
if (len != sizeof(finish->host_data)) {
-error_setg(errp, "parameter length of %lu not equal to %lu",
+error_setg(errp, "parameter length of %" G_GSIZE_FORMAT
+ " not equal to %zu",
len, sizeof(finish->host_data));
return;
}
--
2.45.2
[PULL 22/23] target/i386/sev: Use size_t for object sizes
From: Richard Henderson
This code was using both uint32_t and uint64_t for len.
Consistently use size_t instead.
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Link:
https://lore.kernel.org/r/20240626194950.1725800-3-richard.hender...@linaro.org
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c| 16
target/i386/trace-events | 2 +-
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 28d6bd3adfa..0ffdf8952c3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -121,7 +121,7 @@ struct SevCommonStateClass {
Error **errp);
int (*launch_start)(SevCommonState *sev_common);
void (*launch_finish)(SevCommonState *sev_common);
-int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t
*ptr, uint64_t len);
+int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t
*ptr, size_t len);
int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
};
@@ -173,7 +173,7 @@ typedef struct SevLaunchUpdateData {
QTAILQ_ENTRY(SevLaunchUpdateData) next;
hwaddr gpa;
void *hva;
-uint64_t len;
+size_t len;
int type;
} SevLaunchUpdateData;
@@ -886,7 +886,7 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
if (!data->hva || !data->len) {
error_report("SNP_LAUNCH_UPDATE called with invalid address"
- "/ length: %p / %lx",
+ "/ length: %p / %zx",
data->hva, data->len);
return 1;
}
@@ -945,7 +945,8 @@ out:
}
static int
-sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa, uint8_t *addr,
uint64_t len)
+sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
+ uint8_t *addr, size_t len)
{
int ret, fw_error;
struct kvm_sev_launch_update_data update;
@@ -1090,8 +1091,7 @@ sev_launch_finish(SevCommonState *sev_common)
}
static int
-snp_launch_update_data(uint64_t gpa, void *hva,
- uint32_t len, int type)
+snp_launch_update_data(uint64_t gpa, void *hva, size_t len, int type)
{
SevLaunchUpdateData *data;
@@ -1108,7 +1108,7 @@ snp_launch_update_data(uint64_t gpa, void *hva,
static int
sev_snp_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
- uint8_t *ptr, uint64_t len)
+ uint8_t *ptr, size_t len)
{
int ret = snp_launch_update_data(gpa, ptr, len,
KVM_SEV_SNP_PAGE_TYPE_NORMAL);
@@ -1165,7 +1165,7 @@ sev_snp_cpuid_info_fill(SnpCpuidInfo *snp_cpuid_info,
}
static int
-snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, uint32_t cpuid_len)
+snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, size_t cpuid_len)
{
KvmCpuidInfo kvm_cpuid_info = {0};
SnpCpuidInfo snp_cpuid_info;
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 06b44ead2e2..51301673f0c 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -6,7 +6,7 @@ kvm_memcrypt_register_region(void *addr, size_t len) "addr %p
len 0x%zx"
kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%zx"
kvm_sev_change_state(const char *old, const char *new) "%s -> %s"
kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x
session %p pdh %p"
-kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIx64
+kvm_sev_launch_update_data(void *addr, size_t len) "addr %p len 0x%zx"
kvm_sev_launch_measurement(const char *value) "data %s"
kvm_sev_launch_finish(void) ""
kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len)
"hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
--
2.45.2
[PULL 21/23] target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest
Do not rely on finish->id_auth_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index a6b063b762c..28d6bd3adfa 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -155,6 +155,7 @@ struct SevSnpGuestState {
char *id_block_base64;
uint8_t *id_block;
char *id_auth_base64;
+uint8_t *id_auth;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -2208,16 +2209,16 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
+finish->id_auth_uaddr = 0;
+g_free(sev_snp_guest->id_auth);
g_free(sev_snp_guest->id_auth_base64);
-g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_auth_base64 = g_strdup(value);
+sev_snp_guest->id_auth =
+qbase64_decode(sev_snp_guest->id_auth_base64, -1, , errp);
-finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
-
-if (!finish->id_auth_uaddr) {
+if (!sev_snp_guest->id_auth) {
return;
}
@@ -2226,6 +2227,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
+
+finish->id_auth_uaddr = (uintptr_t)sev_snp_guest->id_auth;
}
static bool
--
2.45.2
[PULL 13/23] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
The typeof_strip_qual() is most useful for the atomic fetch-and-modify
operations in atomic.h, but it can be used elsewhere as well. For example,
QAPI_LIST_LENGTH() assumes that the argument is not const, which is not a
requirement.
Move the macro to compiler.h and, while at it, move it under #ifndef
__cplusplus to emphasize that it uses C-only constructs. A C++ version
of typeof_strip_qual() using type traits is possible[1], but beyond the
scope of this patch because the little C++ code that is in QEMU does not
use QAPI.
The patch was tested by changing the declaration of strv_from_str_list()
in qapi/qapi-type-helpers.c to:
char **strv_from_str_list(const strList *const list)
This is valid C code, and it fails to compile without this change.
[1] https://lore.kernel.org/qemu-devel/20240624205647.112034-1-f...@google.com/
Reviewed-by: Richard Henderson
Reviewed-by: Manos Pitsidianakis
Tested-by: Manos Pitsidianakis
Signed-off-by: Paolo Bonzini
---
include/qapi/util.h | 2 +-
include/qemu/atomic.h | 42 -
include/qemu/compiler.h | 46 +
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 20dfea8a545..b8254247b8d 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -62,7 +62,7 @@ int parse_qapi_name(const char *name, bool complete);
#define QAPI_LIST_LENGTH(list) \
({ \
size_t _len = 0;\
-typeof(list) _tail; \
+typeof_strip_qual(list) _tail; \
for (_tail = list; _tail != NULL; _tail = _tail->next) {\
_len++; \
} \
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99110abefb3..dc4118ddd9e 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -20,48 +20,6 @@
/* Compiler barrier */
#define barrier() ({ asm volatile("" ::: "memory"); (void)0; })
-/* The variable that receives the old value of an atomically-accessed
- * variable must be non-qualified, because atomic builtins return values
- * through a pointer-type argument as in __atomic_load(, , MODEL).
- *
- * This macro has to handle types smaller than int manually, because of
- * implicit promotion. int and larger types, as well as pointers, can be
- * converted to a non-qualified type just by applying a binary operator.
- */
-#define typeof_strip_qual(expr)
\
- typeof(
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const bool) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile bool),
\
-(bool)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
char),\
-(signed char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile unsigned
char), \
-(unsigned char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
short), \
-(signed short)1,
\
-
[PULL 08/23] block: do not check bdrv_file_open
The set of BlockDrivers that have .bdrv_file_open coincides with those
that have .protocol_name and guess what---checking drv->bdrv_file_open
is done to see if the driver is a protocol. So check drv->protocol_name
instead.
Signed-off-by: Paolo Bonzini
---
block.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 69a2905178a..dd14ba85fc3 100644
--- a/block.c
+++ b/block.c
@@ -926,7 +926,6 @@ BlockDriver *bdrv_find_protocol(const char *filename,
int i;
GLOBAL_STATE_CODE();
-/* TODO Drivers without bdrv_file_open must be specified explicitly */
/*
* XXX(hch): we really should not let host device detection
@@ -1983,7 +1982,7 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockBackend *file,
open_flags = bdrv_open_flags(bs, bs->open_flags);
node_name = qemu_opt_get(opts, "node-name");
-assert(!drv->bdrv_file_open || file == NULL);
+assert(!drv->protocol_name || file == NULL);
ret = bdrv_open_driver(bs, drv, node_name, options, open_flags, errp);
if (ret < 0) {
goto fail_opts;
@@ -2084,7 +2083,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
/* If the user has explicitly specified the driver, this choice should
* override the BDRV_O_PROTOCOL flag */
-protocol = drv->bdrv_file_open;
+protocol = drv->protocol_name;
}
if (protocol) {
@@ -4123,7 +4122,7 @@ bdrv_open_inherit(const char *filename, const char
*reference, QDict *options,
}
/* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */
-assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open);
+assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->protocol_name);
/* file must be NULL if a protocol BDS is about to be created
* (the inverse results in an error message from bdrv_open_common()) */
assert(!(flags & BDRV_O_PROTOCOL) || !file);
@@ -5971,7 +5970,7 @@ int64_t coroutine_fn
bdrv_co_get_allocated_file_size(BlockDriverState *bs)
return drv->bdrv_co_get_allocated_file_size(bs);
}
-if (drv->bdrv_file_open) {
+if (drv->protocol_name) {
/*
* Protocol drivers default to -ENOTSUP (most of their data is
* not stored in any of their children (if they even have any),
@@ -8030,7 +8029,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* Both of these conditions are represented by
generate_json_filename.
*/
if (primary_child_bs->exact_filename[0] &&
-primary_child_bs->drv->bdrv_file_open &&
+primary_child_bs->drv->protocol_name &&
!drv->is_filter && !generate_json_filename)
{
strcpy(bs->exact_filename, primary_child_bs->exact_filename);
--
2.45.2
[PULL 09/23] block: remove separate bdrv_file_open callback
bdrv_file_open and bdrv_open are completely equivalent, they are
never checked except to see which one to invoke. So merge them
into a single one.
Signed-off-by: Paolo Bonzini
---
include/block/block_int-common.h | 3 ---
block.c | 4 +---
block/blkdebug.c | 2 +-
block/blkio.c| 2 +-
block/blkverify.c| 2 +-
block/curl.c | 8
block/file-posix.c | 8
block/file-win32.c | 4 ++--
block/gluster.c | 6 +++---
block/iscsi.c| 4 ++--
block/nbd.c | 6 +++---
block/nfs.c | 2 +-
block/null.c | 4 ++--
block/nvme.c | 2 +-
block/rbd.c | 3 ++-
block/ssh.c | 2 +-
block/vvfat.c| 2 +-
17 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 761276127ed..ebb4e56a503 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -248,9 +248,6 @@ struct BlockDriver {
int GRAPH_UNLOCKED_PTR (*bdrv_open)(
BlockDriverState *bs, QDict *options, int flags, Error **errp);
-/* Protocol drivers should implement this instead of bdrv_open */
-int GRAPH_UNLOCKED_PTR (*bdrv_file_open)(
-BlockDriverState *bs, QDict *options, int flags, Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)(
diff --git a/block.c b/block.c
index dd14ba85fc3..c1cc313d216 100644
--- a/block.c
+++ b/block.c
@@ -1655,9 +1655,7 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->opaque = g_malloc0(drv->instance_size);
assert(!drv->bdrv_needs_filename || bs->filename[0]);
-if (drv->bdrv_file_open) {
-ret = drv->bdrv_file_open(bs, options, open_flags, _err);
-} else if (drv->bdrv_open) {
+if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
} else {
ret = 0;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 9da8c9eddc2..c95c818c388 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -1073,7 +1073,7 @@ static BlockDriver bdrv_blkdebug = {
.is_filter = true,
.bdrv_parse_filename= blkdebug_parse_filename,
-.bdrv_file_open = blkdebug_open,
+.bdrv_open = blkdebug_open,
.bdrv_close = blkdebug_close,
.bdrv_reopen_prepare= blkdebug_reopen_prepare,
.bdrv_child_perm= blkdebug_child_perm,
diff --git a/block/blkio.c b/block/blkio.c
index 882e1c297b4..1a38064ce76 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_file_open = blkio_file_open, \
+.bdrv_open = blkio_file_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/blkverify.c b/block/blkverify.c
index ec45d8335ed..5a9bf674d9c 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -321,7 +321,7 @@ static BlockDriver bdrv_blkverify = {
.instance_size= sizeof(BDRVBlkverifyState),
.bdrv_parse_filename = blkverify_parse_filename,
-.bdrv_file_open = blkverify_open,
+.bdrv_open= blkverify_open,
.bdrv_close = blkverify_close,
.bdrv_child_perm = bdrv_default_perms,
.bdrv_co_getlength= blkverify_co_getlength,
diff --git a/block/curl.c b/block/curl.c
index 419f7c89ef2..ef5252d00b5 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -1034,7 +1034,7 @@ static BlockDriver bdrv_http = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1053,7 +1053,7 @@ static BlockDriver bdrv_https = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1072,7 +1072,7 @@ static BlockDriver bdrv_ftp = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open =
[PULL 10/23] block: rename former bdrv_file_open callbacks
Since there is no bdrv_file_open callback anymore, rename the implementations
so that they end with "_open" instead of "_file_open". NFS is the exception
because all the functions are named nfs_file_*.
Suggested-by: Kevin Wolf
Signed-off-by: Paolo Bonzini
---
block/blkio.c | 8
block/null.c | 8
block/nvme.c | 8
block/ssh.c | 6 +++---
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/block/blkio.c b/block/blkio.c
index 1a38064ce76..3d9a2e764c3 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -713,7 +713,7 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
* for example will fail.
*
* In order to open the device read-only, we are using the `read-only`
- * property of the libblkio driver in blkio_file_open().
+ * property of the libblkio driver in blkio_open().
*/
fd = qemu_open(path, O_RDWR, NULL);
if (fd < 0) {
@@ -791,8 +791,8 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
return 0;
}
-static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int blkio_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *blkio_driver = bs->drv->protocol_name;
BDRVBlkioState *s = bs->opaque;
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_open = blkio_file_open, \
+.bdrv_open = blkio_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/null.c b/block/null.c
index 6fa64d20d86..4730acc1eb2 100644
--- a/block/null.c
+++ b/block/null.c
@@ -77,8 +77,8 @@ static void null_aio_parse_filename(const char *filename,
QDict *options,
}
}
-static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int null_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
QemuOpts *opts;
BDRVNullState *s = bs->opaque;
@@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = {
.protocol_name = "null-co",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_co_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
@@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = {
.protocol_name = "null-aio",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_aio_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
diff --git a/block/nvme.c b/block/nvme.c
index c84914af6dd..3b588b139f6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -889,7 +889,7 @@ out:
qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)regs, 0, sizeof(NvmeBar));
}
-/* Cleaning up is done in nvme_file_open() upon error. */
+/* Cleaning up is done in nvme_open() upon error. */
return ret;
}
@@ -967,8 +967,8 @@ static void nvme_close(BlockDriverState *bs)
g_free(s->device);
}
-static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int nvme_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *device;
QemuOpts *opts;
@@ -1630,7 +1630,7 @@ static BlockDriver bdrv_nvme = {
.create_opts = _create_opts_simple,
.bdrv_parse_filename = nvme_parse_filename,
-.bdrv_open= nvme_file_open,
+.bdrv_open= nvme_open,
.bdrv_close = nvme_close,
.bdrv_co_getlength= nvme_co_getlength,
.bdrv_probe_blocksizes= nvme_probe_blocksizes,
diff --git a/block/ssh.c b/block/ssh.c
index 1344822ed85..27d582e0e3d 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -837,8 +837,8 @@ static int connect_to_ssh(BDRVSSHState *s,
BlockdevOptionsSsh *opts,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
- Error **errp)
+static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+Error **errp)
{
BDRVSSHState *s = bs->opaque;
BlockdevOptionsSsh *opts;
@@ -1362,7 +1362,7 @@ static BlockDriver bdrv_ssh = {
[PULL 12/23] exec: don't use void* in pointer arithmetic in headers
From: Roman Kiryanov
void* pointer arithmetic is a GCC extentension which could not be
available in other build tools (e.g. C++). This changes removes this
assumption.
Signed-off-by: Roman Kiryanov
Suggested-by: Paolo Bonzini
Link: https://lore.kernel.org/r/20240620201654.598024-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 154626f9ad2..c26ede33d21 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2764,7 +2764,7 @@ MemTxResult address_space_write_rom(AddressSpace *as,
hwaddr addr,
#include "exec/memory_ldst_phys.h.inc"
struct MemoryRegionCache {
-void *ptr;
+uint8_t *ptr;
hwaddr xlat;
hwaddr len;
FlatView *fv;
--
2.45.2
[PULL v3 00/23] Misc changes for 2024-06-28
The following changes since commit 28b8a57ad63670aa0ce90334523dc552b13b4336: Merge tag 'pull-riscv-to-apply-20240627-1' of https://github.com/alistair23/qemu into staging (2024-06-27 07:36:16 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to b31d386781cf85c193f3b1355dd0604cd6a59943: target/i386/sev: Fix printf formats (2024-06-28 19:26:54 +0200) I dropped the bit test instructions and the rest of the decoder updates, because they were buggy and I didn't like any of the fixes I could come up with. Supersedes: <20240624135939.632257-1-pbonz...@redhat.com> * configure: detect --cpu=mipsisa64r6 * target/i386: decode address before going back to translate.c * meson: allow configuring the x86-64 baseline * meson: remove dead optimization option * exec: small changes to allow compilation with C++ in Android emulator * fix SEV compilation on 32-bit systems ---- Paolo Bonzini (19): configure: detect --cpu=mipsisa64r6 Revert "host/i386: assume presence of POPCNT" Revert "host/i386: assume presence of SSSE3" Revert "host/i386: assume presence of SSE2" meson: allow configuring the x86-64 baseline meson: remove dead optimization option block: make assertion more generic block: do not check bdrv_file_open block: remove separate bdrv_file_open callback block: rename former bdrv_file_open callbacks include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH() target/i386: fix CC_OP dump target/i386: use cpu_cc_dst for CC_OP_POPCNT target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL target/i386: remove unused enum target/i386: SEV: rename sev_snp_guest->id_block target/i386: SEV: store pointer to decoded id_block in SevSnpGuest target/i386: SEV: rename sev_snp_guest->id_auth target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest Richard Henderson (2): target/i386/sev: Use size_t for object sizes target/i386/sev: Fix printf formats Roman Kiryanov (2): exec: avoid using C++ keywords in function parameters exec: don't use void* in pointer arithmetic in headers configure | 2 +- meson.build | 54 +--- host/include/i386/host/cpuinfo.h | 2 + include/block/block_int-common.h | 3 - include/exec/memory.h | 6 +- include/qapi/util.h | 2 +- include/qemu/atomic.h | 42 - include/qemu/compiler.h | 46 ++ target/i386/cpu.h | 13 +++- tcg/i386/tcg-target.h | 5 +- block.c | 17 +++-- block/blkdebug.c | 2 +- block/blkio.c | 8 +-- block/blkverify.c | 2 +- block/curl.c | 8 +-- block/file-posix.c| 8 +-- block/file-win32.c| 4 +- block/gluster.c | 6 +- block/iscsi.c | 4 +- block/nbd.c | 6 +- block/nfs.c | 2 +- block/null.c | 8 +-- block/nvme.c | 8 +-- block/rbd.c | 3 +- block/ssh.c | 6 +- block/vvfat.c | 2 +- target/i386/cpu-dump.c| 101 +++--- target/i386/sev.c | 71 - target/i386/tcg/cc_helper.c | 2 +- target/i386/tcg/translate.c | 21 +-- util/cpuinfo-i386.c | 6 +- host/include/i386/host/bufferiszero.c.inc | 5 +- target/i386/tcg/emit.c.inc| 4 +- meson_options.txt | 5 +- scripts/meson-buildoptions.sh | 6 +- target/i386/trace-events | 2 +- 36 files changed, 256 insertions(+), 236 deletions(-) -- 2.45.2
[PULL 01/23] configure: detect --cpu=mipsisa64r6
Treat it as a MIPS64 machine. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 5ad1674ca5f..8b6a2f16ceb 100755 --- a/configure +++ b/configure @@ -450,7 +450,7 @@ case "$cpu" in linux_arch=loongarch ;; - mips64*) + mips64*|mipsisa64*) cpu=mips64 host_arch=mips linux_arch=mips -- 2.45.2
Re: [PATCH 2/2] target/i386: drop AMD machine check bits from Intel CPUID
Il ven 28 giu 2024, 10:32 Xiaoyao Li ha scritto:
> On 6/27/2024 10:06 PM, Paolo Bonzini wrote:
> > The recent addition of the SUCCOR bit to kvm_arch_get_supported_cpuid()
> > causes the bit to be visible when "-cpu host" VMs are started on Intel
> > processors.
> >
> > While this should in principle be harmless, it's not tidy and we don't
> > even know for sure that it doesn't cause any guest OS to take unexpected
> > paths. Since x86_cpu_get_supported_feature_word() can return different
> > different values depending on the guest, adjust it to hide the SUCCOR
>
> superfluous different
>
> > bit if the guest has non-AMD vendor.
>
> It seems to adjust it based on vendor in kvm_arch_get_supported_cpuid()
> is better than in x86_cpu_get_supported_feature_word(). Otherwise
> kvm_arch_get_supported_cpuid() still returns "risky" value for Intel VMs.
>
But the cpuid bit is only invalid for Intel *guest* vendor, not host. It is
not a problem to have it if you run on Intel host but have a guest model
with AMD vendor.
I will check if there are other callers of kvm_arch_get_supported_cpuid(),
or callers of x86_cpu_get_supported_feature_word() with NULL cpu, that
might care about the difference.
Paolo
>
> > Suggested-by: Xiaoyao Li
> > Cc: John Allen
> > Signed-off-by: Paolo Bonzini
> > ---
> > target/i386/cpu.c | 16 +++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index deb58670651..f3e9b543682 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -6064,8 +6064,10 @@ uint64_t
> x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w)
> > } else {
> > return ~0;
> > }
> > +
> > +switch (w) {
> > #ifndef TARGET_X86_64
> > -if (w == FEAT_8000_0001_EDX) {
> > +case FEAT_8000_0001_EDX:
> > /*
> >* 32-bit TCG can emulate 64-bit compatibility mode. If there
> is no
> >* way for userspace to get out of its 32-bit jail, we can
> leave
> > @@ -6077,6 +6079,18 @@ uint64_t
> x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w)
> > r &= ~unavail;
> > break;
> > #endif
> > +
> > +case FEAT_8000_0007_EBX:
> > +if (cpu && !IS_AMD_CPU(>env)) {
> > +/* Disable AMD machine check architecture for Intel CPU. */
> > +r = 0;
> > +}
> > +break;
> > +
> > +default:
> > +break;
> > +}
> > +
> > if (cpu && cpu->migratable) {
> > r &= x86_cpu_get_migratable_flags(w);
> > }
>
>
Re: [PATCH v3] timer: Fix a race condition between timer's callback and destroying code
On Thu, Jun 27, 2024 at 6:12 PM Roman Kiryanov wrote:
>
> On Thu, Jun 27, 2024 at 6:27 AM Paolo Bonzini wrote:
> >
> > On Thu, Jun 27, 2024 at 2:32 AM Roman Kiryanov wrote:
> > > +if (qatomic_read(>cb_running)) {
> > > +qemu_event_wait(_list->timers_done_ev);
> > > +}
> >
> > qemu_event_wait() already has the right atomic magic, and
> > ts->cb_running is both redundant (in general), and I think racy (as
> > implemented in this patch).
>
> I added cb_running to avoid waiting for timers_done_ev if we know our
> cb is done.
Yes, but it's very tricky. Assuming we want to fix it in the timer
core, the QemuEvent should be enough, no need to optimize it. On the
other hand, I'm still worried about deadlocks (more below).
> > But especially, you haven't justified in the commit message _why_ you
> > need this.
>
> I mentioned the problem of cleanup racing with the timer's callback function
> in the current shape of QEMU.
Yes, but it was not clear what are the involved threads. It is clear
now that you have a function in a separate thread, creating a timer in
the main QEMU event loop.
> > using
> > aio_bh_schedule_oneshot() or aio_wait_bh_oneshot() to synchronize
> > everything with the AioContext thread seems like a superior solution
> > to me.
>
> Could you please elaborate? The problem we want to solve is this:
>
> void myThreadFunc() {
> CallbackState callbackState;
> QEMUTimer timer;
>
> timer_init(, myClockType, myScale, ,
> );
> ...
> timer_del();
> }
>
> Currently, myTimerCallbackFunc could fire after myThreadFunc exits
> (if timer_del runs between qemu_mutex_unlock and cb(opaque) in
> timerlist_run_timers) and callbackState gets destroyed.
Ok, got it now. I agree that qemu_event_wait() is safe for you here
because you are in a completely separate thread. But I'm worried that
it causes deadlocks in QEMU where the timer callback and the timer_del
run in the same thread.
I think the easiest options would be:
1) if possible, allocate the timer and the callbackState statically in
the device.
2) use "aio_wait_bh_oneshot(qemu_get_aio_context(), [](void
*opaque){}, NULL);" after timer_del(). You can also put the timer and
the callbackState in a RAII wrapper, so that aio_wait_bh_oneshot() is
executed when the RAII wrapper is destructed
Another thing that you could do is to use a shared_ptr<> for the
timer+callbackState combo, and pass a weak_ptr<> to the timer. Then:
- at the beginning of the timer, you upgrade the weak_ptr with lock()
and if it fails, return
- at the end of myThreadfunc, you destruct the shared_ptr before
deleting the timer.
I'm not sure how you'd pass the weak_ptr/shared_ptr to a callback
(Rust has Weak::into_raw/Weak::from_raw, but I don't know C++ well
enough). That may be overkill.
Paolo
Re: [PATCH 0/3] target/i386/sev: Fix 32-bit host build issues
On Wed, Jun 26, 2024 at 9:49 PM Richard Henderson wrote: > I separated the fixes into 3 smaller patches > that may be easier to review. Oops, I missed this. I queued patches 2-3, while for the first one I prefer the version I sent at https://lore.kernel.org/qemu-devel/20240627145357.1038664-1-pbonz...@redhat.com/ (patches 1-4) Paolo > r~ > > Richard Henderson (3): > target/i386/sev: Cast id_auth_uaddr through uintptr_t > target/i386/sev: Use size_t for object sizes > target/i386/sev: Fix printf formats > > target/i386/sev.c| 41 ++-- > target/i386/trace-events | 2 +- > 2 files changed, 24 insertions(+), 19 deletions(-) > > -- > 2.34.1 >
[PATCH 3/5] target/i386: SEV: rename sev_snp_guest->id_auth
Free the "id_auth" name for the binary version of the data.
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2d4cfd41e83..a6b063b762c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -154,7 +154,7 @@ struct SevSnpGuestState {
char *guest_visible_workarounds;
char *id_block_base64;
uint8_t *id_block;
-char *id_auth;
+char *id_auth_base64;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -1297,7 +1297,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64,
sev_snp->id_auth_base64,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2198,7 +2198,7 @@ sev_snp_guest_get_id_auth(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_auth);
+return g_strdup(sev_snp_guest->id_auth_base64);
}
static void
@@ -2208,14 +2208,14 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
-g_free(sev_snp_guest->id_auth);
+g_free(sev_snp_guest->id_auth_base64);
g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_auth = g_strdup(value);
+sev_snp_guest->id_auth_base64 = g_strdup(value);
finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
if (!finish->id_auth_uaddr) {
return;
--
2.45.2
[PATCH 1/5] target/i386: SEV: rename sev_snp_guest->id_block
Free the "id_block" name for the binary version of the data.
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 30b83f1d77d..6daa8c264cd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -152,7 +152,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
-char *id_block;
+char *id_block_base64;
char *id_auth;
char *host_data;
@@ -1296,7 +1296,7 @@ sev_snp_launch_finish(SevCommonState *sev_common)
}
}
-trace_kvm_sev_snp_launch_finish(sev_snp->id_block, sev_snp->id_auth,
+trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth,
sev_snp->host_data);
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH,
finish, );
@@ -2159,7 +2159,7 @@ sev_snp_guest_get_id_block(Object *obj, Error **errp)
{
SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj);
-return g_strdup(sev_snp_guest->id_block);
+return g_strdup(sev_snp_guest->id_block_base64);
}
static void
@@ -2170,14 +2170,14 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
-g_free(sev_snp_guest->id_block);
+g_free(sev_snp_guest->id_block_base64);
g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
-sev_snp_guest->id_block = g_strdup(value);
+sev_snp_guest->id_block_base64 = g_strdup(value);
finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block, -1, , errp);
+(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
if (!finish->id_block_uaddr) {
return;
--
2.45.2
[PATCH 4/5] target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest
Do not rely on finish->id_auth_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index a6b063b762c..28d6bd3adfa 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -155,6 +155,7 @@ struct SevSnpGuestState {
char *id_block_base64;
uint8_t *id_block;
char *id_auth_base64;
+uint8_t *id_auth;
char *host_data;
struct kvm_sev_snp_launch_start kvm_start_conf;
@@ -2208,16 +2209,16 @@ sev_snp_guest_set_id_auth(Object *obj, const char
*value, Error **errp)
struct kvm_sev_snp_launch_finish *finish = _snp_guest->kvm_finish_conf;
gsize len;
+finish->id_auth_uaddr = 0;
+g_free(sev_snp_guest->id_auth);
g_free(sev_snp_guest->id_auth_base64);
-g_free((guchar *)finish->id_auth_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_auth_base64 = g_strdup(value);
+sev_snp_guest->id_auth =
+qbase64_decode(sev_snp_guest->id_auth_base64, -1, , errp);
-finish->id_auth_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, ,
errp);
-
-if (!finish->id_auth_uaddr) {
+if (!sev_snp_guest->id_auth) {
return;
}
@@ -2226,6 +2227,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
+
+finish->id_auth_uaddr = (uintptr_t)sev_snp_guest->id_auth;
}
static bool
--
2.45.2
[PATCH 5/5] target/i386: SEV: fix format strings for 32-bit hosts
Use PRIx64 for uint64_t and %zu for size_t/gsize.
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 28d6bd3adfa..77ff908ab17 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -886,7 +886,7 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
if (!data->hva || !data->len) {
error_report("SNP_LAUNCH_UPDATE called with invalid address"
- "/ length: %p / %lx",
+ "/ length: %p / %" PRIx64,
data->hva, data->len);
return 1;
}
@@ -934,8 +934,8 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest,
out:
if (!ret && update.gfn_start << TARGET_PAGE_BITS != data->gpa + data->len)
{
-error_report("SEV-SNP: expected update of GPA range %lx-%lx,"
- "got GPA range %lx-%llx",
+error_report("SEV-SNP: expected update of GPA range %" PRIx64 "-%"
PRIx64 ","
+ "got GPA range %" PRIx64 "-%llx",
data->gpa, data->gpa + data->len, data->gpa,
update.gfn_start << TARGET_PAGE_BITS);
ret = -EIO;
@@ -2148,7 +2148,7 @@ sev_snp_guest_set_guest_visible_workarounds(Object *obj,
const char *value,
}
if (len != sizeof(start->gosvw)) {
-error_setg(errp, "parameter length of %lu exceeds max of %lu",
+error_setg(errp, "parameter length of %zu exceeds max of %zu",
len, sizeof(start->gosvw));
return;
}
@@ -2185,7 +2185,7 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
if (len != KVM_SEV_SNP_ID_BLOCK_SIZE) {
-error_setg(errp, "parameter length of %lu not equal to %u",
+error_setg(errp, "parameter length of %zu not equal to %u",
len, KVM_SEV_SNP_ID_BLOCK_SIZE);
return;
}
@@ -2223,7 +2223,7 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value,
Error **errp)
}
if (len > KVM_SEV_SNP_ID_AUTH_SIZE) {
-error_setg(errp, "parameter length:ID_AUTH %lu exceeds max of %u",
+error_setg(errp, "parameter length:ID_AUTH %zu exceeds max of %u",
len, KVM_SEV_SNP_ID_AUTH_SIZE);
return;
}
@@ -2291,7 +2291,7 @@ sev_snp_guest_set_host_data(Object *obj, const char
*value, Error **errp)
}
if (len != sizeof(finish->host_data)) {
-error_setg(errp, "parameter length of %lu not equal to %lu",
+error_setg(errp, "parameter length of %zu not equal to %zu",
len, sizeof(finish->host_data));
return;
}
--
2.45.2
[PATCH 2/5] target/i386: SEV: store pointer to decoded id_block in SevSnpGuest
Do not rely on finish->id_block_uaddr, so that there are no casts from
pointer to uint64_t. They break on 32-bit hosts.
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 6daa8c264cd..2d4cfd41e83 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -153,6 +153,7 @@ struct SevSnpGuestState {
/* configuration parameters */
char *guest_visible_workarounds;
char *id_block_base64;
+uint8_t *id_block;
char *id_auth;
char *host_data;
@@ -2170,16 +2171,15 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
gsize len;
finish->id_block_en = 0;
+g_free(sev_snp_guest->id_block);
g_free(sev_snp_guest->id_block_base64);
-g_free((guchar *)finish->id_block_uaddr);
/* store the base64 str so we don't need to re-encode in getter */
sev_snp_guest->id_block_base64 = g_strdup(value);
+sev_snp_guest->id_block =
+qbase64_decode(sev_snp_guest->id_block_base64, -1, , errp);
-finish->id_block_uaddr =
-(uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, ,
errp);
-
-if (!finish->id_block_uaddr) {
+if (!sev_snp_guest->id_block) {
return;
}
@@ -2190,6 +2190,7 @@ sev_snp_guest_set_id_block(Object *obj, const char
*value, Error **errp)
}
finish->id_block_en = 1;
+finish->id_block_uaddr = (uintptr_t)sev_snp_guest->id_block;
}
static char *
--
2.45.2
[PATCH 0/5] target/i386: SEV: fix compiler warnings on 32-bit hosts
While SEV in practice is unlikely to be used when compiling for 32-bit environments (it is not even supported by 32-bit kernels), it is easier and/or nicer to clean up the warts that block compilation, than to add conditionals in the build system to limit it to 64-bit hosts. Paolo Paolo Bonzini (5): target/i386: SEV: rename sev_snp_guest->id_block target/i386: SEV: store pointer to decoded id_block in SevSnpGuest target/i386: SEV: rename sev_snp_guest->id_auth target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest target/i386: SEV: fix format strings for 32-bit hosts target/i386/sev.c | 52 +-- 1 file changed, 28 insertions(+), 24 deletions(-) -- 2.45.2
[PATCH 0/2] target/i386: drop AMD machine check bits from Intel CPUID
The recent addition of the SUCCOR bit to kvm_arch_get_supported_cpuid() causes the bit to be visible when "-cpu host" VMs are started on Intel processors. While this should in principle be harmless, it's not tidy and we don't even know for sure that it doesn't cause any guest OS to take unexpected paths. So plumb in a mechanism for x86_cpu_get_supported_feature_word() to return different values depending on the *guest* CPU vendor (which, for KVM, is by default the same as the host vendor); and then use it to hide the SUCCOR bit if the guest has non-AMD vendor. Paolo Bonzini (2): target/i386: pass X86CPU to x86_cpu_get_supported_feature_word target/i386: drop AMD machine check bits from Intel CPUID target/i386/cpu.h | 3 +-- target/i386/cpu.c | 29 + target/i386/kvm/kvm-cpu.c | 2 +- 3 files changed, 23 insertions(+), 11 deletions(-) -- 2.45.2
[PATCH 1/2] target/i386: pass X86CPU to x86_cpu_get_supported_feature_word
This allows modifying the bits in "-cpu max"/"-cpu host" depending on
the guest CPU vendor (which, at least by default, is the host vendor in
the case of KVM).
For example, machine check architecture differs between Intel and AMD,
and bits from AMD should be dropped when configuring the guest for
an Intel model.
Cc: Xiaoyao Li
Cc: John Allen
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 3 +--
target/i386/cpu.c | 13 ++---
target/i386/kvm/kvm-cpu.c | 2 +-
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 29daf370485..9bea7142bf4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -666,8 +666,7 @@ typedef enum FeatureWord {
} FeatureWord;
typedef uint64_t FeatureWordArray[FEATURE_WORDS];
-uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
-bool migratable_only);
+uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w);
/* cpuid_features bits */
#define CPUID_FP87 (1U << 0)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4c2e6f3a71e..deb58670651 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6035,8 +6035,7 @@ CpuDefinitionInfoList *qmp_query_cpu_definitions(Error
**errp)
#endif /* !CONFIG_USER_ONLY */
-uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
-bool migratable_only)
+uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu, FeatureWord w)
{
FeatureWordInfo *wi = _word_info[w];
uint64_t r = 0;
@@ -6076,9 +6075,9 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
? CPUID_EXT2_LM & ~CPUID_EXT2_KERNEL_FEATURES
: CPUID_EXT2_LM;
r &= ~unavail;
-}
+break;
#endif
-if (migratable_only) {
+if (cpu && cpu->migratable) {
r &= x86_cpu_get_migratable_flags(w);
}
return r;
@@ -7371,7 +7370,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp)
* by the user.
*/
env->features[w] |=
-x86_cpu_get_supported_feature_word(w, cpu->migratable) &
+x86_cpu_get_supported_feature_word(cpu, w) &
~env->user_features[w] &
~feature_word_info[w].no_autoenable_flags;
}
@@ -7497,7 +7496,7 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool
verbose)
for (w = 0; w < FEATURE_WORDS; w++) {
uint64_t host_feat =
-x86_cpu_get_supported_feature_word(w, false);
+x86_cpu_get_supported_feature_word(NULL, w);
uint64_t requested_features = env->features[w];
uint64_t unavailable_features = requested_features & ~host_feat;
mark_unavailable_features(cpu, w, unavailable_features, prefix);
@@ -7617,7 +7616,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error
**errp)
env->features[FEAT_PERF_CAPABILITIES] & PERF_CAP_LBR_FMT;
if (requested_lbr_fmt && kvm_enabled()) {
uint64_t host_perf_cap =
-x86_cpu_get_supported_feature_word(FEAT_PERF_CAPABILITIES, false);
+x86_cpu_get_supported_feature_word(NULL, FEAT_PERF_CAPABILITIES);
unsigned host_lbr_fmt = host_perf_cap & PERF_CAP_LBR_FMT;
if (!cpu->enable_pmu) {
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index f9b99b5f50d..b2735d6efee 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -136,7 +136,7 @@ static void kvm_cpu_xsave_init(void)
if (!esa->size) {
continue;
}
-if ((x86_cpu_get_supported_feature_word(esa->feature, false) &
esa->bits)
+if ((x86_cpu_get_supported_feature_word(NULL, esa->feature) &
esa->bits)
!= esa->bits) {
continue;
}
--
2.45.2
[PATCH 2/2] target/i386: drop AMD machine check bits from Intel CPUID
The recent addition of the SUCCOR bit to kvm_arch_get_supported_cpuid()
causes the bit to be visible when "-cpu host" VMs are started on Intel
processors.
While this should in principle be harmless, it's not tidy and we don't
even know for sure that it doesn't cause any guest OS to take unexpected
paths. Since x86_cpu_get_supported_feature_word() can return different
different values depending on the guest, adjust it to hide the SUCCOR
bit if the guest has non-AMD vendor.
Suggested-by: Xiaoyao Li
Cc: John Allen
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index deb58670651..f3e9b543682 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6064,8 +6064,10 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w)
} else {
return ~0;
}
+
+switch (w) {
#ifndef TARGET_X86_64
-if (w == FEAT_8000_0001_EDX) {
+case FEAT_8000_0001_EDX:
/*
* 32-bit TCG can emulate 64-bit compatibility mode. If there is no
* way for userspace to get out of its 32-bit jail, we can leave
@@ -6077,6 +6079,18 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w)
r &= ~unavail;
break;
#endif
+
+case FEAT_8000_0007_EBX:
+if (cpu && !IS_AMD_CPU(>env)) {
+/* Disable AMD machine check architecture for Intel CPU. */
+r = 0;
+}
+break;
+
+default:
+break;
+}
+
if (cpu && cpu->migratable) {
r &= x86_cpu_get_migratable_flags(w);
}
--
2.45.2
Re: [PULL 39/42] i386: Add support for SUCCOR feature
On 6/13/24 11:50, Xiaoyao Li wrote:
On 6/8/2024 4:34 PM, Paolo Bonzini wrote:
From: John Allen
Add cpuid bit definition for the SUCCOR feature. This cpuid bit is
required to
be exposed to guests to allow them to handle machine check exceptions
on AMD
hosts.
v2:
- Add "succor" feature word.
- Add case to kvm_arch_get_supported_cpuid for the SUCCOR feature.
Reported-by: William Roche
Reviewed-by: Joao Martins
Signed-off-by: John Allen
Message-ID: <20240603193622.47156-3-john.al...@amd.com>
Signed-off-by: Paolo Bonzini
[snip]
...
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 55a9e8a70cf..56d8e2a89ec 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -532,6 +532,8 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
uint32_t function,
*/
cpuid_1_edx = kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
ret |= cpuid_1_edx & CPUID_EXT2_AMD_ALIASES;
+ } else if (function == 0x8007 && reg == R_EBX) {
+ ret |= CPUID_8000_0007_EBX_SUCCOR;
IMO, this is incorrect.
Why make it supported unconditionally? Shouldn't there be a KVM patch to
report it in KVM_GET_SUPPORTED_CPUID?
Yes, but since the bit doesn't need any hypervisor support it is common
to also add it in QEMU.
If make it supported unconditionally, all VMs boot with "-cpu host/max"
will see the CPUID bits, even if it is Intel VMs.
It should be harmless, but you're right, it's not tidy and we don't know
for sure that it doesn't trigger weird paths in guest OSes. I've send a
series to fix this.
Paolo
Re: [PATCH v3] timer: Fix a race condition between timer's callback and destroying code
On Thu, Jun 27, 2024 at 2:32 AM Roman Kiryanov wrote:
> +if (qatomic_read(>cb_running)) {
> +qemu_event_wait(_list->timers_done_ev);
> +}
qemu_event_wait() already has the right atomic magic, and
ts->cb_running is both redundant (in general), and I think racy (as
implemented in this patch). This stuff is really hard to get right. At
the very least you need a store-release when clearing the flag, and I
think it also has to be read under the mutex (I'm not sure if there's
anything else, I haven't done a full analysis).
But especially, you haven't justified in the commit message _why_ you
need this. Since this problem is not timer-specific, using
aio_bh_schedule_oneshot() or aio_wait_bh_oneshot() to synchronize
everything with the AioContext thread seems like a superior solution
to me.
Paolo
> }
> }
>
> @@ -571,9 +576,15 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
> cb = ts->cb;
> opaque = ts->opaque;
>
> +/* prevent timer_del from returning while cb(opaque)
> + * is still running (by waiting for timers_done_ev).
> + */
> +qatomic_set(>cb_running, true);
> +
> /* run the callback (the timer list can be modified) */
> qemu_mutex_unlock(_list->active_timers_lock);
> cb(opaque);
> +qatomic_set(>cb_running, false);
> qemu_mutex_lock(_list->active_timers_lock);
>
> progress = true;
> --
> 2.45.2.741.gdbec12cfda-goog
>
[PATCH] target/i386/tcg: remove unused enum
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 16
1 file changed, 16 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 257110ac703..aeb7bc4d51b 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -282,22 +282,6 @@ enum {
JCC_LE,
};
-enum {
-/* I386 int registers */
-OR_EAX, /* MUST be even numbered */
-OR_ECX,
-OR_EDX,
-OR_EBX,
-OR_ESP,
-OR_EBP,
-OR_ESI,
-OR_EDI,
-
-OR_TMP0 = 16,/* temporary operand register */
-OR_TMP1,
-OR_A0, /* temporary register used when doing address evaluation */
-};
-
enum {
USES_CC_DST = 1,
USES_CC_SRC = 2,
--
2.45.2
Re: [PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
On Thu, Jun 27, 2024 at 10:38 AM Manos Pitsidianakis wrote: > > On Thu, 27 Jun 2024 00:32, Paolo Bonzini wrote: > >On Tue, Jun 25, 2024 at 9:17 PM Manos Pitsidianakis > > wrote: > >> >Move the macro to compiler.h and, while at it, move it under #ifndef > >> >__cplusplus to emphasize that it uses C-only constructs. A C++ version > >> >of typeof_strip_qual() using type traits is possible[1], but beyond the > >> >scope of this patch because the little C++ code that is in QEMU does not > >> >use QAPI. > >> > >> Should we add an #else to provide a fallback for cplusplus until the > >> alternative is merged? > > > >As the commit message says, I don't think we need to include the C++ > >code in QEMU since it would be essentially dead. > > The #ifndef __cplusplus part stood out for me, if it's not required for > the qemu build then it's similarly unnecessary. Thinking out loud here. Yeah, the patch had actually two purposes, only one of which is explicit in the commit message because the other is more relevant to Google than to the upstream project: 1) Google wants some help making QEMU headers compilable with C++, which is generally not a goal of the project but not something we want to hinder. And if that brings to our attention things that could be improved in C as well, why not. 2) it is true that typeof_strip_qual() is useful in other places than qemu/atomic.h Since qemu/compiler.h already has #ifdef __cplusplus in various places, and could reasonably be included in the few C++ files that QEMU has, it makes sense to have a #ifndef __cplusplus in qemu/compiler.h (unlike in qemu/atomic.h). Personally I prefer to have a little more cognitive load in common headers such as qemu/osdep.h and qemu/compiler.h, than to have different styles (e.g. no qemu/osdep.h inclusion) in C++ sources, even though most likely I'll never touch those C++ sources. The #ifndef is not absolutely necessary---if we removed it, Google could work around that with "#ifdef __cplusplus / #undef typeof_strip_qual". It is mostly for documentation purposes to point out the C-only compiler builtins, and because if anyone ended up using typeof_strip_qual in C++ code the error message would be very very long. Paolo
Re: [RFC PATCH] target/i386: restrict SEV to 64 bit host builds
On 6/26/24 16:03, Alex Bennée wrote:
Re-enabling the 32 bit host build on i686 showed the recently merged
SEV code doesn't take enough care over its types. While the format
strings could use more portable types there isn't much we can do about
casting uint64_t into a pointer. The easiest solution seems to be just
to disable SEV for a 32 bit build. It's highly unlikely anyone would
want this functionality anyway.
It's better style to just fix the compilation issues. I'll send a small
series once I test it.
Paolo
Signed-off-by: Alex Bennée
---
target/i386/sev.h | 2 +-
target/i386/meson.build | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/i386/sev.h b/target/i386/sev.h
index 858005a119..b0cb9dd7ed 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -45,7 +45,7 @@ typedef struct SevKernelLoaderContext {
size_t cmdline_size;
} SevKernelLoaderContext;
-#ifdef CONFIG_SEV
+#if defined(CONFIG_SEV) && defined(HOST_X86_64)
bool sev_enabled(void);
bool sev_es_enabled(void);
bool sev_snp_enabled(void);
diff --git a/target/i386/meson.build b/target/i386/meson.build
index 075117989b..d2a008926c 100644
--- a/target/i386/meson.build
+++ b/target/i386/meson.build
@@ -6,7 +6,7 @@ i386_ss.add(files(
'xsave_helper.c',
'cpu-dump.c',
))
-i386_ss.add(when: 'CONFIG_SEV', if_true: files('host-cpu.c',
'confidential-guest.c'))
+i386_ss.add(when: ['CONFIG_SEV', 'HOST_X86_64'], if_true: files('host-cpu.c',
'confidential-guest.c'))
# x86 cpu type
i386_ss.add(when: 'CONFIG_KVM', if_true: files('host-cpu.c'))
@@ -21,7 +21,7 @@ i386_system_ss.add(files(
'cpu-apic.c',
'cpu-sysemu.c',
))
-i386_system_ss.add(when: 'CONFIG_SEV', if_true: files('sev.c'), if_false:
files('sev-sysemu-stub.c'))
+i386_system_ss.add(when: ['CONFIG_SEV', 'HOST_X86_64'], if_true:
files('sev.c'), if_false: files('sev-sysemu-stub.c'))
i386_user_ss = ss.source_set()
Re: [PATCH v2] timer: Fix a race condition between timer's callback and destroying code
On 6/26/24 23:52, Roman Kiryanov wrote:
`timerlist_run_timers` provides no mechanism to
make sure the data pointed by `opaque` is valid
when calling timer's callback: there could be
another thread running which is destroying
timer's opaque data.
With this change `timer_del` becomes blocking if
timer's callback is running and it will be safe
to destroy timer's data once `timer_del` returns.
Signed-off-by: Roman Kiryanov
Generally QEMU is running event loop code under the "big QEMU lock"
(BQL). If both timer_del() and the callback run under the BQL, the race
cannot happen.
If you're using multiple threads, however, this code is generally very
performance sensitive; and adding a mutex and broadcast on every timer
that fires is probably too much. A simpler possibility (and the
QemuEvent is already there, even) could be to use
qemu_event_wait(_list->timers_done_ev);
but I think this situation is not specific to timers, and tied more to
the code that creates the timer; therefore it's easier to handle it
outside common code.
If you only need to synchronize freeing against callbacks, you can use
aio_bh_schedule_oneshot() and do the free in the bottom half. If
instead you need the cleanup to be synchronous, the same idea of the
bottom half can be used, via aio_wait_bh_oneshot().
Paolo
---
v2: rebased to the right branch and removed
Google specific tags from the commit message.
include/qemu/timer.h | 4
util/qemu-timer.c| 21 +
2 files changed, 25 insertions(+)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5ce83c7911..efd0e95d20 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -3,6 +3,7 @@
#include "qemu/bitops.h"
#include "qemu/notify.h"
+#include "qemu/thread.h"
#include "qemu/host-utils.h"
#define NANOSECONDS_PER_SECOND 10LL
@@ -86,9 +87,12 @@ struct QEMUTimer {
QEMUTimerList *timer_list;
QEMUTimerCB *cb;
void *opaque;
+QemuMutex opaque_lock;
+QemuCond cb_done;
QEMUTimer *next;
int attributes;
int scale;
+bool cb_running;
};
extern QEMUTimerListGroup main_loop_tlg;
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 213114be68..95af255519 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -369,7 +369,10 @@ void timer_init_full(QEMUTimer *ts,
ts->opaque = opaque;
ts->scale = scale;
ts->attributes = attributes;
+qemu_mutex_init(>opaque_lock);
+qemu_cond_init(>cb_done);
ts->expire_time = -1;
+ts->cb_running = false;
}
void timer_deinit(QEMUTimer *ts)
@@ -394,6 +397,12 @@ static void timer_del_locked(QEMUTimerList *timer_list,
QEMUTimer *ts)
}
pt = >next;
}
+
+qemu_mutex_lock(>opaque_lock);
+while (ts->cb_running) {
+qemu_cond_wait(>cb_done, >opaque_lock);
+}
+qemu_mutex_unlock(>opaque_lock);
}
static bool timer_mod_ns_locked(QEMUTimerList *timer_list,
@@ -571,11 +580,23 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
cb = ts->cb;
opaque = ts->opaque;
+/* Mark the callback as running to prevent
+ * destroying `opaque` in another thread.
+ */
+qemu_mutex_lock(>opaque_lock);
+ts->cb_running = true;
+qemu_mutex_unlock(>opaque_lock);
+
/* run the callback (the timer list can be modified) */
qemu_mutex_unlock(_list->active_timers_lock);
cb(opaque);
qemu_mutex_lock(_list->active_timers_lock);
+qemu_mutex_lock(>opaque_lock);
+ts->cb_running = false;
+qemu_cond_broadcast(>cb_done);
+qemu_mutex_unlock(>opaque_lock);
+
progress = true;
}
qemu_mutex_unlock(_list->active_timers_lock);
Re: [PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
On Tue, Jun 25, 2024 at 9:17 PM Manos Pitsidianakis wrote: > >Move the macro to compiler.h and, while at it, move it under #ifndef > >__cplusplus to emphasize that it uses C-only constructs. A C++ version > >of typeof_strip_qual() using type traits is possible[1], but beyond the > >scope of this patch because the little C++ code that is in QEMU does not > >use QAPI. > > Should we add an #else to provide a fallback for cplusplus until the > alternative is merged? As the commit message says, I don't think we need to include the C++ code in QEMU since it would be essentially dead. Paolo
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
On 6/25/24 23:47, Manos Pitsidianakis wrote:
On Mon, 24 Jun 2024 20:14, Paolo Bonzini wrote:
Yes, I agree. However, considering we haven't even checked the situation
with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.
FWIW, I just ran
`cargo msrv -- cargo check --target x86_64-unknown-linux-gnu`
And the result was `The MSRV is: 1.77.2`
and the most important issue was that the mem::offset_of! macro was
unstable till then.
I looked for a way to avoid it and the most promising is
https://play.rust-lang.org/?version=stable=debug=2018=10a22a9b8393abd7b541d8fc844bc0df
Basically, you replace
pub struct Foo {
foo: i32,
bar: i32
}
with
with_offsets! {
#[repr(C)] // mandatory
pub struct Foo {
foo: i32,
bar: i32,
}
}
The macro walks the struct twice, once to actually declare it and once
to determine the offsets using mem::size_of and mem::align_of. The
result is something like
#[repr(C)] // mandatory
pub struct Foo {
foo: i32,
bar: i32,
}
pub struct FooOffsets {
foo: usize,
bar: usize,
}
impl Foo {
pub const offset_to: FooOffsets = FooOffsets {
foo: 0,
bar: 4,
}
}
(where 0 and 4 are actually the aforementioned computation based on
size_of and align_of).
There are some limitations but the trick is really well done; the need
for #[repr(C)] is not a problem for us (C<->Rust interoperability needs
it anyway), and the implementation is fully "const". And though it only
works for structs that use "with_offsets!", and with just one level of
fields, the implementation of offset_of is trivial:
macro_rules! offset_of {
($Container:ty, $field:ident) => {
<$Container>::offset_to.$field
};
}
Anyhow, this should _not_ be in the first version that is
committed---which, as you remarked in the v2, should focus on working
build system integration. As long as we know it is doable, it can be
left for later.
Paolo
[PATCH] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH()
The typeof_strip_qual() is most useful for the atomic fetch-and-modify
operations in atomic.h, but it can be used elsewhere as well. For example,
QAPI_LIST_LENGTH() assumes that the argument is not const, which is not a
requirement.
Move the macro to compiler.h and, while at it, move it under #ifndef
__cplusplus to emphasize that it uses C-only constructs. A C++ version
of typeof_strip_qual() using type traits is possible[1], but beyond the
scope of this patch because the little C++ code that is in QEMU does not
use QAPI.
The patch was tested by changing the declaration of strv_from_str_list()
in qapi/qapi-type-helpers.c to:
char **strv_from_str_list(const strList *const list)
This is valid C code, and it fails to compile without this change.
[1] https://lore.kernel.org/qemu-devel/20240624205647.112034-1-f...@google.com/
Signed-off-by: Paolo Bonzini
---
include/qapi/util.h | 2 +-
include/qemu/atomic.h | 42 -
include/qemu/compiler.h | 46 +
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 20dfea8a545..b8254247b8d 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -62,7 +62,7 @@ int parse_qapi_name(const char *name, bool complete);
#define QAPI_LIST_LENGTH(list) \
({ \
size_t _len = 0;\
-typeof(list) _tail; \
+typeof_strip_qual(list) _tail; \
for (_tail = list; _tail != NULL; _tail = _tail->next) {\
_len++; \
} \
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99110abefb3..dc4118ddd9e 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -20,48 +20,6 @@
/* Compiler barrier */
#define barrier() ({ asm volatile("" ::: "memory"); (void)0; })
-/* The variable that receives the old value of an atomically-accessed
- * variable must be non-qualified, because atomic builtins return values
- * through a pointer-type argument as in __atomic_load(, , MODEL).
- *
- * This macro has to handle types smaller than int manually, because of
- * implicit promotion. int and larger types, as well as pointers, can be
- * converted to a non-qualified type just by applying a binary operator.
- */
-#define typeof_strip_qual(expr)
\
- typeof(
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const bool) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile bool) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile bool),
\
-(bool)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
char),\
-(signed char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile unsigned char) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile unsigned
char), \
-(unsigned char)1,
\
-__builtin_choose_expr(
\
- __builtin_types_compatible_p(typeof(expr), signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), volatile signed short) ||
\
-__builtin_types_compatible_p(typeof(expr), const volatile signed
short), \
-(signed short)1,
\
-__builtin_choose_expr(
\
- __builtin_types_c
Re: [PATCH 1/1] include/qemu: Provide a C++ compatible version of typeof_strip_qual
Il mar 25 giu 2024, 04:32 Roman Kiryanov ha scritto: > Hi Philippe, thank you for looking. > > On Mon, Jun 24, 2024 at 7:27 PM Philippe Mathieu-Daudé > wrote: > > In particular this patch seems contained well enough > > to be carried in forks were C++ _is_ used. > > Will you agree to take #ifdef __cplusplus and #error to the QEMU side > in atomic.h and > we will keep atomic.hpp on our side? The error message looks better > when atomic.hpp > is somewhere near. > I think we should also move typeof_strip_qual elsewhere; I will take a look. I think there are a couple headers that already have #ifdef __cplusplus, but I need to check (no source code around right now). But another good thing to do would be to avoid having atomic.h as a rebuild-the-world header, and any steps towards that would be very welcome. Paolo > Regards, > Roman. > >
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
Il lun 24 giu 2024, 18:52 Daniel P. Berrangé ha
scritto:
> On Wed, Jun 19, 2024 at 11:13:58PM +0300, Manos Pitsidianakis wrote:
> > Add options for Rust in meson_options.txt, meson.build, configure to
> > prepare for adding Rust code in the followup commits.
> >
> > `rust` is a reserved meson name, so we have to use an alternative.
> > `with_rust` was chosen.
> >
> > A cargo_wrapper.py script is added that is heavily based on the work of
> > Marc-André Lureau from 2021.
> >
> >
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> >
> > Signed-off-by: Marc-André Lureau
> > Signed-off-by: Manos Pitsidianakis
> > ---
> > MAINTAINERS | 5 +
> > configure | 11 ++
> > meson.build | 11 ++
> > meson_options.txt | 4 +
> > scripts/cargo_wrapper.py | 279 ++
> > scripts/meson-buildoptions.sh | 6 +
> > 6 files changed, 316 insertions(+)
> > create mode 100644 scripts/cargo_wrapper.py
>
> > diff --git a/configure b/configure
> > index 38ee257701..6894d7c2d1 100755
> > --- a/configure
> > +++ b/configure
> > @@ -302,6 +302,9 @@ else
> >objcc="${objcc-${cross_prefix}clang}"
> > fi
> >
> > +with_rust="auto"
>
> On last week's call one of the things we discussed is the expectations
> for consumers of QEMU around the usage of Rust & its optionality (or
> not) long term.
>
> I'm of the view that to be valuable for QEMU, we need to consider
> Rust to become a mandatory feature. There's a question of how quickly
> we move, however, before declaring it mandatory. The longer we take
> to declare it mandatory, the longer we are limiting the value we
> can take from Rust.
>
Yes, I agree. However, considering we haven't even checked the situation
with what language features are required by any idiomatic bindings vs the
1.63 version that we need to support for Debian, I think it's a bit
premature to make it mandatory.
As soon as the PL011 device is a reasonable example of how to write good
Rust device models, however, we should switch to making it default-on, and
--without-rust can go away within one or two releases.
> + --with-rust-target-triple=*) with_rust_target_triple="$optarg"
>
As with previous posting, IMHO, this should ideally not exist. We should
> honour the --cross-prefix= values, re-mapping to to Rust targets for all
> the combos we know about
More precisely it should be based not on the cross prefix, but on the same
OS and CPU values that are detected from cpp symbols and used for the meson
cross file. I have already made a rough list of differences between these
and the Rust target triples, but I haven't yet turned them to code.
It would however be the first patch I send after these are in a good shape
for inclusion.
The priority right now should be to investigate the build tree workspace
design that I sketched last week in my reply to Richard. Once that can be
considered a stable build system integration, further improvements can be
done in tree.
--with-rust-target-triple should only be needed
> as a workaround for where we might have missed a mapping.
>
Agreed.
Paolo
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
Il lun 24 giu 2024, 10:36 Zhao Liu ha scritto:
> [snip]
>
> > diff --git a/meson.build b/meson.build
> > index c5360fbd299..ad7dbc0d641 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -290,6 +290,11 @@ foreach lang : all_languages
> >endif
> > endforeach
> > +cargo = not_found
> > +if 'RUST_TARGET_TRIPLE' in config_host
> > + cargo = find_program('cargo', required: true)
> > +endif
> > +
>
> As with the original Manos version, it looks like there's no need to
> check cargo here? Since patch 2 checks cargo and others in
> rust/meson.build.
>
> Otherwise, cargo was checked twice.
>
Yes, I would check it here though because it's used in the summary, already
in this patch.
Paolo
> > # default flags for all hosts
> > # We use -fwrapv to tell the compiler that we require a C dialect where
> > # left shift of signed integers is well defined and has the expected
> > @@ -4239,6 +4244,10 @@ if 'objc' in all_languages
> > else
> >summary_info += {'Objective-C compiler': false}
> > endif
> > +summary_info += {'Rust support': cargo.found()}
> > +if cargo.found() and config_host['RUST_TARGET_TRIPLE']) !=
> config_host['RUST_HOST_TRIPLE']
> > + summary_info += {'Rust target': config_host['RUST_TARGET_TRIPLE']}
> > +endif
> > option_cflags = (get_option('debug') ? ['-g'] : [])
> > if get_option('optimization') != 'plain'
> >option_cflags += ['-O' + get_option('optimization')]
> >
> >
>
>
[PULL v2 15/23] Revert "host/i386: assume presence of SSE2"
This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
util/cpuinfo-i386.c | 1 +
host/include/i386/host/bufferiszero.c.inc | 5 +++--
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
+#define CPUINFO_SSE2(1u << 7)
#define CPUINFO_AVX1(1u << 9)
#define CPUINFO_AVX2(1u << 10)
#define CPUINFO_AVX512F (1u << 11)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index ca74ef04f54..90f92a42dc8 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
if (max >= 1) {
__cpuid(1, a, b, c, d);
+info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
diff --git a/host/include/i386/host/bufferiszero.c.inc
b/host/include/i386/host/bufferiszero.c.inc
index 3b9605d806f..74ae98580f6 100644
--- a/host/include/i386/host/bufferiszero.c.inc
+++ b/host/include/i386/host/bufferiszero.c.inc
@@ -110,13 +110,14 @@ static biz_accel_fn const accel_table[] = {
static unsigned best_accel(void)
{
-#ifdef CONFIG_AVX2_OPT
unsigned info = cpuinfo_init();
+
+#ifdef CONFIG_AVX2_OPT
if (info & CPUINFO_AVX2) {
return 2;
}
#endif
-return 1;
+return info & CPUINFO_SSE2 ? 1 : 0;
}
#else
--
2.45.2
[PULL v2 00/23] Misc changes for 2024-06-22
The following changes since commit c9ba79baca7c673098361e3a687f72d458e0d18a: Merge tag 'pull-target-arm-20240622' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-06-22 09:56:49 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 0753fbb4750ad6709c43b4263c3d29b00f7cd0bb: exec: don't use void* in pointer arithmetic in headers (2024-06-24 09:17:16 +0200) * configure: detect --cpu=mipsisa64r6 * target/i386: decode address before going back to translate.c * meson: allow configuring the x86-64 baseline * meson: remove dead optimization option * exec: small changes to allow compilation with C++ in Android emulator Paolo Bonzini (21): configure: detect --cpu=mipsisa64r6 target/i386: fix CC_OP dump target/i386: use cpu_cc_dst for CC_OP_POPCNT target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL target/i386: convert bit test instructions to new decoder target/i386: try not to force EFLAGS computation for CC_OP_ADOX/ADCX target/i386: decode address before going back to translate.c target/i386: convert CMPXCHG8B/CMPXCHG16B to new decoder target/i386: do not check PREFIX_LOCK in old-style decoder target/i386: list instructions still in translate.c target/i386: assert that cc_op* and pc_save are preserved target/i386: remove gen_ext_tl Revert "host/i386: assume presence of POPCNT" Revert "host/i386: assume presence of SSSE3" Revert "host/i386: assume presence of SSE2" meson: allow configuring the x86-64 baseline meson: remove dead optimization option block: make assertion more generic block: do not check bdrv_file_open block: remove separate bdrv_file_open callback block: rename former bdrv_file_open callbacks Roman Kiryanov (2): exec: avoid using C++ keywords in function parameters exec: don't use void* in pointer arithmetic in headers configure | 2 +- meson.build | 54 ++-- host/include/i386/host/cpuinfo.h | 2 + include/block/block_int-common.h | 3 - include/exec/memory.h | 6 +- target/i386/cpu.h | 13 +- target/i386/tcg/decode-new.h | 19 +- tcg/i386/tcg-target.h | 5 +- block.c | 17 +- block/blkdebug.c | 2 +- block/blkio.c | 8 +- block/blkverify.c | 2 +- block/curl.c | 8 +- block/file-posix.c| 8 +- block/file-win32.c| 4 +- block/gluster.c | 6 +- block/iscsi.c | 4 +- block/nbd.c | 6 +- block/nfs.c | 2 +- block/null.c | 8 +- block/nvme.c | 8 +- block/rbd.c | 3 +- block/ssh.c | 6 +- block/vvfat.c | 2 +- target/i386/cpu-dump.c| 101 +++--- target/i386/tcg/cc_helper.c | 2 +- target/i386/tcg/translate.c | 492 ++ util/cpuinfo-i386.c | 6 +- host/include/i386/host/bufferiszero.c.inc | 5 +- target/i386/tcg/decode-new.c.inc | 136 ++--- target/i386/tcg/emit.c.inc| 249 ++- meson_options.txt | 5 +- scripts/meson-buildoptions.sh | 6 +- 33 files changed, 619 insertions(+), 581 deletions(-) -- 2.45.2
[PULL 21/23] block: rename former bdrv_file_open callbacks
Since there is no bdrv_file_open callback anymore, rename the implementations
so that they end with "_open" instead of "_file_open". NFS is the exception
because all the functions are named nfs_file_*.
Suggested-by: Kevin Wolf
Signed-off-by: Paolo Bonzini
---
block/blkio.c | 8
block/null.c | 8
block/nvme.c | 8
block/ssh.c | 6 +++---
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/block/blkio.c b/block/blkio.c
index 1a38064ce76..3d9a2e764c3 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -713,7 +713,7 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
* for example will fail.
*
* In order to open the device read-only, we are using the `read-only`
- * property of the libblkio driver in blkio_file_open().
+ * property of the libblkio driver in blkio_open().
*/
fd = qemu_open(path, O_RDWR, NULL);
if (fd < 0) {
@@ -791,8 +791,8 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs,
QDict *options,
return 0;
}
-static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int blkio_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *blkio_driver = bs->drv->protocol_name;
BDRVBlkioState *s = bs->opaque;
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_open = blkio_file_open, \
+.bdrv_open = blkio_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/null.c b/block/null.c
index 6fa64d20d86..4730acc1eb2 100644
--- a/block/null.c
+++ b/block/null.c
@@ -77,8 +77,8 @@ static void null_aio_parse_filename(const char *filename,
QDict *options,
}
}
-static int null_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int null_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
QemuOpts *opts;
BDRVNullState *s = bs->opaque;
@@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = {
.protocol_name = "null-co",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_co_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
@@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = {
.protocol_name = "null-aio",
.instance_size = sizeof(BDRVNullState),
-.bdrv_open = null_file_open,
+.bdrv_open = null_open,
.bdrv_parse_filename= null_aio_parse_filename,
.bdrv_co_getlength = null_co_getlength,
.bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size,
diff --git a/block/nvme.c b/block/nvme.c
index c84914af6dd..3b588b139f6 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -889,7 +889,7 @@ out:
qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)regs, 0, sizeof(NvmeBar));
}
-/* Cleaning up is done in nvme_file_open() upon error. */
+/* Cleaning up is done in nvme_open() upon error. */
return ret;
}
@@ -967,8 +967,8 @@ static void nvme_close(BlockDriverState *bs)
g_free(s->device);
}
-static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags,
- Error **errp)
+static int nvme_open(BlockDriverState *bs, QDict *options, int flags,
+ Error **errp)
{
const char *device;
QemuOpts *opts;
@@ -1630,7 +1630,7 @@ static BlockDriver bdrv_nvme = {
.create_opts = _create_opts_simple,
.bdrv_parse_filename = nvme_parse_filename,
-.bdrv_open= nvme_file_open,
+.bdrv_open= nvme_open,
.bdrv_close = nvme_close,
.bdrv_co_getlength= nvme_co_getlength,
.bdrv_probe_blocksizes= nvme_probe_blocksizes,
diff --git a/block/ssh.c b/block/ssh.c
index 1344822ed85..27d582e0e3d 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -837,8 +837,8 @@ static int connect_to_ssh(BDRVSSHState *s,
BlockdevOptionsSsh *opts,
return ret;
}
-static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
- Error **errp)
+static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
+Error **errp)
{
BDRVSSHState *s = bs->opaque;
BlockdevOptionsSsh *opts;
@@ -1362,7 +1362,7 @@ static BlockDriver bdrv_ssh = {
[PULL 15/23] Revert "host/i386: assume presence of SSE2"
This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
util/bufferiszero.c | 4 ++--
util/cpuinfo-i386.c | 1 +
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index 72f6fad61e5..81771733eaa 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -14,6 +14,7 @@
#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
+#define CPUINFO_SSE2(1u << 7)
#define CPUINFO_AVX1(1u << 9)
#define CPUINFO_AVX2(1u << 10)
#define CPUINFO_AVX512F (1u << 11)
diff --git a/util/bufferiszero.c b/util/bufferiszero.c
index 11c080e02cf..74864f7b782 100644
--- a/util/bufferiszero.c
+++ b/util/bufferiszero.c
@@ -188,14 +188,14 @@ static biz_accel_fn const accel_table[] = {
static unsigned best_accel(void)
{
-#ifdef CONFIG_AVX2_OPT
unsigned info = cpuinfo_init();
+#ifdef CONFIG_AVX2_OPT
if (info & CPUINFO_AVX2) {
return 2;
}
#endif
-return 1;
+return info & CPUINFO_SSE2 ? 1 : 0;
}
#elif defined(__aarch64__) && defined(__ARM_NEON)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index ca74ef04f54..90f92a42dc8 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
if (max >= 1) {
__cpuid(1, a, b, c, d);
+info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
--
2.45.2
[PULL 17/23] meson: remove dead optimization option
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 13 -
meson_options.txt | 2 --
scripts/meson-buildoptions.sh | 3 ---
3 files changed, 18 deletions(-)
diff --git a/meson.build b/meson.build
index 6e694ecd9fe..54e6b09f4fb 100644
--- a/meson.build
+++ b/meson.build
@@ -2874,18 +2874,6 @@ config_host_data.set('CONFIG_AVX2_OPT',
get_option('avx2') \
int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX2 not available').allowed())
-config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
- .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512F') \
- .require(cc.links('''
-#include
-#include
-static int __attribute__((target("avx512f"))) bar(void *a) {
- __m512i x = *(__m512i *)a;
- return _mm512_test_epi64_mask(x, x);
-}
-int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
- '''), error_message: 'AVX512F not available').allowed())
-
config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \
.require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable
AVX512BW') \
.require(cc.links('''
@@ -4283,7 +4271,6 @@ summary_info += {'mutex debugging':
get_option('debug_mutex')}
summary_info += {'memory allocator': get_option('malloc')}
summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')}
summary_info += {'avx512bw optimization':
config_host_data.get('CONFIG_AVX512BW_OPT')}
-summary_info += {'avx512f optimization':
config_host_data.get('CONFIG_AVX512F_OPT')}
summary_info += {'gcov': get_option('b_coverage')}
summary_info += {'thread sanitizer': get_option('tsan')}
summary_info += {'CFI support': get_option('cfi')}
diff --git a/meson_options.txt b/meson_options.txt
index 6065ed2d352..0269fa0f16e 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -119,8 +119,6 @@ option('membarrier', type: 'feature', value: 'disabled',
option('avx2', type: 'feature', value: 'auto',
description: 'AVX2 optimizations')
-option('avx512f', type: 'feature', value: 'disabled',
- description: 'AVX512F optimizations')
option('avx512bw', type: 'feature', value: 'auto',
description: 'AVX512BW optimizations')
option('keyring', type: 'feature', value: 'auto',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 62842d47e88..cfadb5ea86a 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -95,7 +95,6 @@ meson_options_help() {
printf "%s\n" ' auth-pamPAM access control'
printf "%s\n" ' avx2AVX2 optimizations'
printf "%s\n" ' avx512bwAVX512BW optimizations'
- printf "%s\n" ' avx512f AVX512F optimizations'
printf "%s\n" ' blkio libblkio block device driver'
printf "%s\n" ' bochs bochs image format support'
printf "%s\n" ' bpf eBPF support'
@@ -240,8 +239,6 @@ _meson_option_parse() {
--disable-avx2) printf "%s" -Davx2=disabled ;;
--enable-avx512bw) printf "%s" -Davx512bw=enabled ;;
--disable-avx512bw) printf "%s" -Davx512bw=disabled ;;
---enable-avx512f) printf "%s" -Davx512f=enabled ;;
---disable-avx512f) printf "%s" -Davx512f=disabled ;;
--enable-gcov) printf "%s" -Db_coverage=true ;;
--disable-gcov) printf "%s" -Db_coverage=false ;;
--enable-lto) printf "%s" -Db_lto=true ;;
--
2.45.2
[PULL 02/23] target/i386: fix CC_OP dump
POPCNT was missing, and the entries were all out of order after
ADCX/ADOX/ADCOX were moved close to EFLAGS. Just use designated
initializers.
Fixes: 4885c3c4953 ("target-i386: Use ctpop helper", 2017-01-10)
Fixes: cc155f19717 ("target/i386: rewrite flags writeback for ADCX/ADOX",
2024-06-11)
Signed-off-by: Paolo Bonzini
---
target/i386/cpu-dump.c | 101 +
1 file changed, 51 insertions(+), 50 deletions(-)
diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c
index 40697064d92..3bb8e440916 100644
--- a/target/i386/cpu-dump.c
+++ b/target/i386/cpu-dump.c
@@ -28,69 +28,70 @@
/* x86 debug */
static const char *cc_op_str[CC_OP_NB] = {
-"DYNAMIC",
-"EFLAGS",
+[CC_OP_DYNAMIC] = "DYNAMIC",
-"MULB",
-"MULW",
-"MULL",
-"MULQ",
+[CC_OP_EFLAGS] = "EFLAGS",
+[CC_OP_ADCX] = "ADCX",
+[CC_OP_ADOX] = "ADOX",
+[CC_OP_ADCOX] = "ADCOX",
-"ADDB",
-"ADDW",
-"ADDL",
-"ADDQ",
+[CC_OP_MULB] = "MULB",
+[CC_OP_MULW] = "MULW",
+[CC_OP_MULL] = "MULL",
+[CC_OP_MULQ] = "MULQ",
-"ADCB",
-"ADCW",
-"ADCL",
-"ADCQ",
+[CC_OP_ADDB] = "ADDB",
+[CC_OP_ADDW] = "ADDW",
+[CC_OP_ADDL] = "ADDL",
+[CC_OP_ADDQ] = "ADDQ",
-"SUBB",
-"SUBW",
-"SUBL",
-"SUBQ",
+[CC_OP_ADCB] = "ADCB",
+[CC_OP_ADCW] = "ADCW",
+[CC_OP_ADCL] = "ADCL",
+[CC_OP_ADCQ] = "ADCQ",
-"SBBB",
-"SBBW",
-"SBBL",
-"SBBQ",
+[CC_OP_SUBB] = "SUBB",
+[CC_OP_SUBW] = "SUBW",
+[CC_OP_SUBL] = "SUBL",
+[CC_OP_SUBQ] = "SUBQ",
-"LOGICB",
-"LOGICW",
-"LOGICL",
-"LOGICQ",
+[CC_OP_SBBB] = "SBBB",
+[CC_OP_SBBW] = "SBBW",
+[CC_OP_SBBL] = "SBBL",
+[CC_OP_SBBQ] = "SBBQ",
-"INCB",
-"INCW",
-"INCL",
-"INCQ",
+[CC_OP_LOGICB] = "LOGICB",
+[CC_OP_LOGICW] = "LOGICW",
+[CC_OP_LOGICL] = "LOGICL",
+[CC_OP_LOGICQ] = "LOGICQ",
-"DECB",
-"DECW",
-"DECL",
-"DECQ",
+[CC_OP_INCB] = "INCB",
+[CC_OP_INCW] = "INCW",
+[CC_OP_INCL] = "INCL",
+[CC_OP_INCQ] = "INCQ",
-"SHLB",
-"SHLW",
-"SHLL",
-"SHLQ",
+[CC_OP_DECB] = "DECB",
+[CC_OP_DECW] = "DECW",
+[CC_OP_DECL] = "DECL",
+[CC_OP_DECQ] = "DECQ",
-"SARB",
-"SARW",
-"SARL",
-"SARQ",
+[CC_OP_SHLB] = "SHLB",
+[CC_OP_SHLW] = "SHLW",
+[CC_OP_SHLL] = "SHLL",
+[CC_OP_SHLQ] = "SHLQ",
-"BMILGB",
-"BMILGW",
-"BMILGL",
-"BMILGQ",
+[CC_OP_SARB] = "SARB",
+[CC_OP_SARW] = "SARW",
+[CC_OP_SARL] = "SARL",
+[CC_OP_SARQ] = "SARQ",
-"ADCX",
-"ADOX",
-"ADCOX",
+[CC_OP_BMILGB] = "BMILGB",
+[CC_OP_BMILGW] = "BMILGW",
+[CC_OP_BMILGL] = "BMILGL",
+[CC_OP_BMILGQ] = "BMILGQ",
-"CLR",
+[CC_OP_POPCNT] = "POPCNT",
+[CC_OP_CLR] = "CLR",
};
static void
--
2.45.2
[PULL 07/23] target/i386: decode address before going back to translate.c
There are now relatively few unconverted opcodes in translate.c (there
are 13 of them including 8 for x87), and all of them have the same
format with a mod/rm byte and no immediate. A good next step is
to remove the early bail out to disas_insn_x87/disas_insn_old,
instead giving these legacy translator functions the same prototype
as the other gen_* functions.
To do this, the X86DecodeInsn can be passed down to the places that
used to fetch address bytes from the instruction stream. To make
sure that everything is done cleanly, the CPUX86State* argument is
removed.
As part of the unification, the gen_lea_modrm() name is now free,
so rename gen_load_ea() to gen_lea_modrm(). This is as good a name
and it makes the changes to translate.c easier to review.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/decode-new.h | 14 ++-
target/i386/tcg/translate.c | 152 +--
target/i386/tcg/decode-new.c.inc | 53 ++-
target/i386/tcg/emit.c.inc | 2 +-
4 files changed, 103 insertions(+), 118 deletions(-)
diff --git a/target/i386/tcg/decode-new.h b/target/i386/tcg/decode-new.h
index e4cdf5e3c4f..bebc77bd54b 100644
--- a/target/i386/tcg/decode-new.h
+++ b/target/i386/tcg/decode-new.h
@@ -264,12 +264,13 @@ typedef enum X86VEXSpecial {
typedef struct X86OpEntry X86OpEntry;
typedef struct X86DecodedInsn X86DecodedInsn;
+struct DisasContext;
/* Decode function for multibyte opcodes. */
-typedef void (*X86DecodeFunc)(DisasContext *s, CPUX86State *env, X86OpEntry
*entry, uint8_t *b);
+typedef void (*X86DecodeFunc)(struct DisasContext *s, CPUX86State *env,
X86OpEntry *entry, uint8_t *b);
/* Code generation function. */
-typedef void (*X86GenFunc)(DisasContext *s, X86DecodedInsn *decode);
+typedef void (*X86GenFunc)(struct DisasContext *s, X86DecodedInsn *decode);
struct X86OpEntry {
/* Based on the is_decode flags. */
@@ -316,6 +317,14 @@ typedef struct X86DecodedOp {
};
} X86DecodedOp;
+typedef struct AddressParts {
+int def_seg;
+int base;
+int index;
+int scale;
+target_long disp;
+} AddressParts;
+
struct X86DecodedInsn {
X86OpEntry e;
X86DecodedOp op[3];
@@ -333,3 +342,4 @@ struct X86DecodedInsn {
uint8_t b;
};
+static void gen_lea_modrm(struct DisasContext *s, X86DecodedInsn *decode);
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 08db40681fa..1d845ff66bb 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -29,6 +29,7 @@
#include "exec/helper-proto.h"
#include "exec/helper-gen.h"
#include "helper-tcg.h"
+#include "decode-new.h"
#include "exec/log.h"
@@ -1529,14 +1530,6 @@ static inline uint64_t x86_ldq_code(CPUX86State *env,
DisasContext *s)
/* Decompose an address. */
-typedef struct AddressParts {
-int def_seg;
-int base;
-int index;
-int scale;
-target_long disp;
-} AddressParts;
-
static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
int modrm)
{
@@ -1695,24 +1688,11 @@ static TCGv gen_lea_modrm_1(DisasContext *s,
AddressParts a, bool is_vsib)
return ea;
}
-static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
-{
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
-TCGv ea = gen_lea_modrm_1(s, a, false);
-gen_lea_v_seg(s, ea, a.def_seg, s->override);
-}
-
-static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
-{
-(void)gen_lea_modrm_0(env, s, modrm);
-}
-
/* Used for BNDCL, BNDCU, BNDCN. */
-static void gen_bndck(CPUX86State *env, DisasContext *s, int modrm,
+static void gen_bndck(DisasContext *s, X86DecodedInsn *decode,
TCGCond cond, TCGv_i64 bndv)
{
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
-TCGv ea = gen_lea_modrm_1(s, a, false);
+TCGv ea = gen_lea_modrm_1(s, decode->mem, false);
tcg_gen_extu_tl_i64(s->tmp1_i64, ea);
if (!CODE64(s)) {
@@ -1724,8 +1704,9 @@ static void gen_bndck(CPUX86State *env, DisasContext *s,
int modrm,
}
/* generate modrm load of memory or register. */
-static void gen_ld_modrm(CPUX86State *env, DisasContext *s, int modrm, MemOp
ot)
+static void gen_ld_modrm(DisasContext *s, X86DecodedInsn *decode, MemOp ot)
{
+int modrm = s->modrm;
int mod, rm;
mod = (modrm >> 6) & 3;
@@ -1733,14 +1714,15 @@ static void gen_ld_modrm(CPUX86State *env, DisasContext
*s, int modrm, MemOp ot)
if (mod == 3) {
gen_op_mov_v_reg(s, ot, s->T0, rm);
} else {
-gen_lea_modrm(env, s, modrm);
+gen_lea_modrm(s, decode);
gen_op_ld_v(s, ot, s->T0, s->A0);
}
}
/* generate modrm store of memory or register. */
-static void gen_st_modrm(CPUX86State *env, DisasContext *s, int modrm, MemOp
ot)
+static void gen_st_modrm(DisasContext *s, X86DecodedI
[PULL 01/23] configure: detect --cpu=mipsisa64r6
Treat it as a MIPS64 machine. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 5ad1674ca5f..8b6a2f16ceb 100755 --- a/configure +++ b/configure @@ -450,7 +450,7 @@ case "$cpu" in linux_arch=loongarch ;; - mips64*) + mips64*|mipsisa64*) cpu=mips64 host_arch=mips linux_arch=mips -- 2.45.2
[PULL 14/23] Revert "host/i386: assume presence of SSSE3"
This reverts commit 433cd6d94a8256af70a5200f236dc8047c3c1468.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
util/cpuinfo-i386.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 6d474a6259a..ca74ef04f54 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -38,8 +38,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
-/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
-info |= (c & bit_AES) ? CPUINFO_AES : 0;
+/* Our AES support requires PSHUFB as well. */
+info |= ((c & bit_AES) && (c & bit_SSSE3) ? CPUINFO_AES : 0);
/* For AVX features, we must check available and usable. */
if ((c & bit_AVX) && (c & bit_OSXSAVE)) {
--
2.45.2
[PULL 22/23] exec: avoid using C++ keywords in function parameters
From: Roman Kiryanov
to use the QEMU headers with a C++ compiler.
Signed-off-by: Roman Kiryanov
Link: https://lore.kernel.org/r/20240618224553.878869-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1be58f694c9..d7591a60d9f 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -945,7 +945,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_start)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_stop:
@@ -964,7 +964,7 @@ struct MemoryListener {
* the current transaction.
*/
void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section,
- int old, int new);
+ int old_val, int new_val);
/**
* @log_sync:
--
2.45.2
[PULL 10/23] target/i386: list instructions still in translate.c
Group them so that it is easier to figure out which two-byte opcodes to
tackle together.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/decode-new.c.inc | 31 +++
1 file changed, 31 insertions(+)
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index fa51aadfcf2..f01a4f1f1fe 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -129,6 +129,37 @@
*
*(^) these are the two cases in which Intel and AMD disagree on the
* primary exception class
+ *
+ * Instructions still in translate.c
+ * -
+ * Generation of TCG opcodes for almost all instructions is in emit.c.inc;
+ * this file interprets the prefixes and opcode bytes down to individual
+ * instruction mnemonics. There is only a handful of opcodes still using
+ * a switch statement to decode modrm bits 3-5 and prefixes after decoding
+ * is complete; these are relics of the older x86 decoder and their code
+ * generation is performed in translate.c.
+ *
+ * These unconverted opcodes also perform their own effective address
+ * generation using the gen_lea_modrm() function.
+ *
+ * There is nothing particularly complicated about them; simply, they don't
+ * need any nasty hacks in the decoder, and they shouldn't get in the way
+ * of the implementation of new x86 instructions, so they are left alone
+ * for the time being.
+ *
+ * x87:
+ * 0xD8 - 0xDF
+ *
+ * privileged/system:
+ * 0x0F 0x00 group 6 (SLDT, STR, LLDT, LTR, VERR, VERW)
+ * 0x0F 0x01 group 7 (SGDT, SIDT, LGDT, LIDT, SMSW, LMSW, INVLPG,
+ * MONITOR, MWAIT, CLAC, STAC, XGETBV, XSETBV,
+ * SWAPGS, RDTSCP)
+ * 0x0F 0xC7 (reg operand) group 9 (RDRAND, RDSEED, RDPID)
+ *
+ * MPX:
+ * 0x0F 0x1A BNDLDX, BNDMOV, BNDCL, BNDCU
+ * 0x0F 0x1B BNDSTX, BNDMOV, BNDMK, BNDCN
*/
#define X86_OP_NONE { 0 },
--
2.45.2
[PULL 04/23] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL
Handle it like the other arithmetic cc_ops. This simplifies a
bit the implementation of bit test instructions.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 13 +++--
target/i386/tcg/translate.c | 3 +--
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index f54cd93b3f9..8504a7998fd 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1275,6 +1275,7 @@ typedef enum {
CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest. */
CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest. */
CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest. */
+CC_OP_CLR, /* Z and P set, all other flags clear. */
CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */
CC_OP_MULW,
@@ -1331,8 +1332,16 @@ typedef enum {
CC_OP_BMILGL,
CC_OP_BMILGQ,
-CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
+/*
+ * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size)
+ * is used or implemented, because the translation needs
+ * to zero-extend CC_DST anyway.
+ */
+CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear. */
+CC_OP_POPCNTW__,
+CC_OP_POPCNTL__,
+CC_OP_POPCNTQ__,
+CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ :
CC_OP_POPCNTL__,
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index eb353dc3c9f..934c514e64f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
.imm = CC_Z };
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
-case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
@@ -3177,6 +3175,7 @@ static void disas_insn_old(DisasContext *s, CPUState
*cpu, int b)
case CC_OP_SHLB ... CC_OP_SHLQ:
case CC_OP_SARB ... CC_OP_SARQ:
case CC_OP_BMILGB ... CC_OP_BMILGQ:
+case CC_OP_POPCNT:
/* Z was going to be computed from the non-zero status of CC_DST.
We can get that same Z value (and the new C value) by leaving
CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
--
2.45.2
[PULL 00/23] Misc changes for 2024-06-22
The following changes since commit 223696363bb117241ad9c2facbff0c474afa4104: Merge tag 'edgar/xilinx-queue-2024-06-17.for-upstream' of https://gitlab.com/edgar.iglesias/qemu into staging (2024-06-18 13:08:01 -0700) are available in the Git repository at: https://gitlab.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to b9b51004033983589e00fb4697f620b903cfcf0e: exec: don't use void* in pointer arithmetic in headers (2024-06-21 18:32:18 +0200) * configure: detect --cpu=mipsisa64r6 * target/i386: decode address before going back to translate.c * meson: allow configuring the x86-64 baseline * meson: remove dead optimization option * exec: small changes to allow compilation with C++ in Android emulator Paolo Bonzini (21): configure: detect --cpu=mipsisa64r6 target/i386: fix CC_OP dump target/i386: use cpu_cc_dst for CC_OP_POPCNT target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL target/i386: convert bit test instructions to new decoder target/i386: try not to force EFLAGS computation for CC_OP_ADOX/ADCX target/i386: decode address before going back to translate.c target/i386: convert CMPXCHG8B/CMPXCHG16B to new decoder target/i386: do not check PREFIX_LOCK in old-style decoder target/i386: list instructions still in translate.c target/i386: assert that cc_op* and pc_save are preserved target/i386: remove gen_ext_tl Revert "host/i386: assume presence of POPCNT" Revert "host/i386: assume presence of SSSE3" Revert "host/i386: assume presence of SSE2" meson: allow configuring the x86-64 baseline meson: remove dead optimization option block: make assertion more generic block: do not check bdrv_file_open block: remove separate bdrv_file_open callback block: rename former bdrv_file_open callbacks Roman Kiryanov (2): exec: avoid using C++ keywords in function parameters exec: don't use void* in pointer arithmetic in headers configure| 2 +- meson.build | 54 +++-- host/include/i386/host/cpuinfo.h | 2 + include/block/block_int-common.h | 3 - include/exec/memory.h| 6 +- target/i386/cpu.h| 13 +- target/i386/tcg/decode-new.h | 19 +- tcg/i386/tcg-target.h| 5 +- block.c | 17 +- block/blkdebug.c | 2 +- block/blkio.c| 8 +- block/blkverify.c| 2 +- block/curl.c | 8 +- block/file-posix.c | 8 +- block/file-win32.c | 4 +- block/gluster.c | 6 +- block/iscsi.c| 4 +- block/nbd.c | 6 +- block/nfs.c | 2 +- block/null.c | 8 +- block/nvme.c | 8 +- block/rbd.c | 3 +- block/ssh.c | 6 +- block/vvfat.c| 2 +- target/i386/cpu-dump.c | 101 target/i386/tcg/cc_helper.c | 2 +- target/i386/tcg/translate.c | 492 --- util/bufferiszero.c | 4 +- util/cpuinfo-i386.c | 6 +- target/i386/tcg/decode-new.c.inc | 136 --- target/i386/tcg/emit.c.inc | 249 +++- meson_options.txt| 5 +- scripts/meson-buildoptions.sh| 6 +- 33 files changed, 618 insertions(+), 581 deletions(-) -- 2.45.2
[PULL 05/23] target/i386: convert bit test instructions to new decoder
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/decode-new.h | 3 +
target/i386/tcg/translate.c | 147 +-
target/i386/tcg/decode-new.c.inc | 40 ++---
target/i386/tcg/emit.c.inc | 149 ++-
4 files changed, 181 insertions(+), 158 deletions(-)
diff --git a/target/i386/tcg/decode-new.h b/target/i386/tcg/decode-new.h
index f9bf9a60411..e4cdf5e3c4f 100644
--- a/target/i386/tcg/decode-new.h
+++ b/target/i386/tcg/decode-new.h
@@ -190,6 +190,9 @@ typedef enum X86InsnSpecial {
/* Always locked if it has a memory operand (XCHG) */
X86_SPECIAL_Locked,
+/* Like HasLock, but also operand 2 provides bit displacement into memory.
*/
+X86_SPECIAL_BitTest,
+
/* Do not load effective address in s->A0 */
X86_SPECIAL_NoLoadEA,
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 934c514e64f..257110ac703 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -708,11 +708,6 @@ static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size,
bool sign)
return dst;
}
-static void gen_exts(MemOp ot, TCGv reg)
-{
-gen_ext_tl(reg, reg, ot, true);
-}
-
static void gen_op_j_ecx(DisasContext *s, TCGCond cond, TCGLabel *label1)
{
TCGv tmp = gen_ext_tl(NULL, cpu_regs[R_ECX], s->aflag, false);
@@ -2985,7 +2980,7 @@ static void disas_insn_old(DisasContext *s, CPUState
*cpu, int b)
int prefixes = s->prefix;
MemOp dflag = s->dflag;
MemOp ot;
-int modrm, reg, rm, mod, op, val;
+int modrm, reg, rm, mod, op;
/* now check op code */
switch (b) {
@@ -3051,146 +3046,6 @@ static void disas_insn_old(DisasContext *s, CPUState
*cpu, int b)
}
break;
-//
-/* bit operations */
-case 0x1ba: /* bt/bts/btr/btc Gv, im */
-ot = dflag;
-modrm = x86_ldub_code(env, s);
-op = (modrm >> 3) & 7;
-mod = (modrm >> 6) & 3;
-rm = (modrm & 7) | REX_B(s);
-if (mod != 3) {
-s->rip_offset = 1;
-gen_lea_modrm(env, s, modrm);
-if (!(s->prefix & PREFIX_LOCK)) {
-gen_op_ld_v(s, ot, s->T0, s->A0);
-}
-} else {
-gen_op_mov_v_reg(s, ot, s->T0, rm);
-}
-/* load shift */
-val = x86_ldub_code(env, s);
-tcg_gen_movi_tl(s->T1, val);
-if (op < 4)
-goto unknown_op;
-op -= 4;
-goto bt_op;
-case 0x1a3: /* bt Gv, Ev */
-op = 0;
-goto do_btx;
-case 0x1ab: /* bts */
-op = 1;
-goto do_btx;
-case 0x1b3: /* btr */
-op = 2;
-goto do_btx;
-case 0x1bb: /* btc */
-op = 3;
-do_btx:
-ot = dflag;
-modrm = x86_ldub_code(env, s);
-reg = ((modrm >> 3) & 7) | REX_R(s);
-mod = (modrm >> 6) & 3;
-rm = (modrm & 7) | REX_B(s);
-gen_op_mov_v_reg(s, MO_32, s->T1, reg);
-if (mod != 3) {
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
-/* specific case: we need to add a displacement */
-gen_exts(ot, s->T1);
-tcg_gen_sari_tl(s->tmp0, s->T1, 3 + ot);
-tcg_gen_shli_tl(s->tmp0, s->tmp0, ot);
-tcg_gen_add_tl(s->A0, gen_lea_modrm_1(s, a, false), s->tmp0);
-gen_lea_v_seg(s, s->A0, a.def_seg, s->override);
-if (!(s->prefix & PREFIX_LOCK)) {
-gen_op_ld_v(s, ot, s->T0, s->A0);
-}
-} else {
-gen_op_mov_v_reg(s, ot, s->T0, rm);
-}
-bt_op:
-tcg_gen_andi_tl(s->T1, s->T1, (1 << (3 + ot)) - 1);
-tcg_gen_movi_tl(s->tmp0, 1);
-tcg_gen_shl_tl(s->tmp0, s->tmp0, s->T1);
-if (s->prefix & PREFIX_LOCK) {
-switch (op) {
-case 0: /* bt */
-/* Needs no atomic ops; we suppressed the normal
- memory load for LOCK above so do it now. */
-gen_op_ld_v(s, ot, s->T0, s->A0);
-break;
-case 1: /* bts */
-tcg_gen_atomic_fetch_or_tl(s->T0, s->A0, s->tmp0,
- s->mem_index, ot | MO_LE);
-break;
-case 2: /* btr */
-tcg_gen_not_tl(s->tmp0, s->tmp0);
-tcg_gen_atomic_fetch_and_tl(s->T0, s->A0, s->tmp0,
-s->mem_index, ot | MO_LE);
-break;
-default:
-case 3: /* btc */
-tcg_gen_atomic_fetch_xor_tl(s->T0, s->A0, s->tmp0,
-s->mem_index, ot | MO_LE);
-break;
-
[PULL 08/23] target/i386: convert CMPXCHG8B/CMPXCHG16B to new decoder
This moves the last LOCK-enabled instructions to the new decoder. It is now
possible to assume that PREFIX_LOCK gen_multi0F is called only after checking
that LOCK was not specified.
The gen_cmpxchg8b and gen_cmpxchg16b functions even have the correct
prototype already; the only thing that needs to be done is removing the
gen_lea_modrm() call.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/decode-new.h | 2 +
target/i386/tcg/translate.c | 121 +--
target/i386/tcg/decode-new.c.inc | 34 ++---
target/i386/tcg/emit.c.inc | 96
4 files changed, 124 insertions(+), 129 deletions(-)
diff --git a/target/i386/tcg/decode-new.h b/target/i386/tcg/decode-new.h
index bebc77bd54b..7f23d373ea7 100644
--- a/target/i386/tcg/decode-new.h
+++ b/target/i386/tcg/decode-new.h
@@ -114,6 +114,8 @@ typedef enum X86CPUIDFeature {
X86_FEAT_CLWB,
X86_FEAT_CMOV,
X86_FEAT_CMPCCXADD,
+X86_FEAT_CX8,
+X86_FEAT_CX16,
X86_FEAT_F16C,
X86_FEAT_FMA,
X86_FEAT_FSGSBASE,
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 1d845ff66bb..c60f18c7482 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2298,104 +2298,6 @@ static void gen_sty_env_A0(DisasContext *s, int offset,
bool align)
tcg_gen_qemu_st_i128(t, s->tmp0, mem_index, mop);
}
-static void gen_cmpxchg8b(DisasContext *s, X86DecodedInsn *decode)
-{
-TCGv_i64 cmp, val, old;
-TCGv Z;
-
-gen_lea_modrm(s, decode);
-
-cmp = tcg_temp_new_i64();
-val = tcg_temp_new_i64();
-old = tcg_temp_new_i64();
-
-/* Construct the comparison values from the register pair. */
-tcg_gen_concat_tl_i64(cmp, cpu_regs[R_EAX], cpu_regs[R_EDX]);
-tcg_gen_concat_tl_i64(val, cpu_regs[R_EBX], cpu_regs[R_ECX]);
-
-/* Only require atomic with LOCK; non-parallel handled in generator. */
-if (s->prefix & PREFIX_LOCK) {
-tcg_gen_atomic_cmpxchg_i64(old, s->A0, cmp, val, s->mem_index,
MO_TEUQ);
-} else {
-tcg_gen_nonatomic_cmpxchg_i64(old, s->A0, cmp, val,
- s->mem_index, MO_TEUQ);
-}
-
-/* Set tmp0 to match the required value of Z. */
-tcg_gen_setcond_i64(TCG_COND_EQ, cmp, old, cmp);
-Z = tcg_temp_new();
-tcg_gen_trunc_i64_tl(Z, cmp);
-
-/*
- * Extract the result values for the register pair.
- * For 32-bit, we may do this unconditionally, because on success (Z=1),
- * the old value matches the previous value in EDX:EAX. For x86_64,
- * the store must be conditional, because we must leave the source
- * registers unchanged on success, and zero-extend the writeback
- * on failure (Z=0).
- */
-if (TARGET_LONG_BITS == 32) {
-tcg_gen_extr_i64_tl(cpu_regs[R_EAX], cpu_regs[R_EDX], old);
-} else {
-TCGv zero = tcg_constant_tl(0);
-
-tcg_gen_extr_i64_tl(s->T0, s->T1, old);
-tcg_gen_movcond_tl(TCG_COND_EQ, cpu_regs[R_EAX], Z, zero,
- s->T0, cpu_regs[R_EAX]);
-tcg_gen_movcond_tl(TCG_COND_EQ, cpu_regs[R_EDX], Z, zero,
- s->T1, cpu_regs[R_EDX]);
-}
-
-/* Update Z. */
-gen_compute_eflags(s);
-tcg_gen_deposit_tl(cpu_cc_src, cpu_cc_src, Z, ctz32(CC_Z), 1);
-}
-
-#ifdef TARGET_X86_64
-static void gen_cmpxchg16b(DisasContext *s, X86DecodedInsn *decode)
-{
-MemOp mop = MO_TE | MO_128 | MO_ALIGN;
-TCGv_i64 t0, t1;
-TCGv_i128 cmp, val;
-
-gen_lea_modrm(s, decode);
-
-cmp = tcg_temp_new_i128();
-val = tcg_temp_new_i128();
-tcg_gen_concat_i64_i128(cmp, cpu_regs[R_EAX], cpu_regs[R_EDX]);
-tcg_gen_concat_i64_i128(val, cpu_regs[R_EBX], cpu_regs[R_ECX]);
-
-/* Only require atomic with LOCK; non-parallel handled in generator. */
-if (s->prefix & PREFIX_LOCK) {
-tcg_gen_atomic_cmpxchg_i128(val, s->A0, cmp, val, s->mem_index, mop);
-} else {
-tcg_gen_nonatomic_cmpxchg_i128(val, s->A0, cmp, val, s->mem_index,
mop);
-}
-
-tcg_gen_extr_i128_i64(s->T0, s->T1, val);
-
-/* Determine success after the fact. */
-t0 = tcg_temp_new_i64();
-t1 = tcg_temp_new_i64();
-tcg_gen_xor_i64(t0, s->T0, cpu_regs[R_EAX]);
-tcg_gen_xor_i64(t1, s->T1, cpu_regs[R_EDX]);
-tcg_gen_or_i64(t0, t0, t1);
-
-/* Update Z. */
-gen_compute_eflags(s);
-tcg_gen_setcondi_i64(TCG_COND_EQ, t0, t0, 0);
-tcg_gen_deposit_tl(cpu_cc_src, cpu_cc_src, t0, ctz32(CC_Z), 1);
-
-/*
- * Extract the result values for the register pair. We may do this
- * unconditionally, because on success (Z=1), the old value matches
- * the previous value in RDX:RAX.
- */
-tcg_gen_mov_i64(cpu_regs[R_EAX], s->T0);
-tcg_gen_mov_i64(cpu_regs[R_EDX], s->T1);
-}
-#endif
-
#include "emit.c.inc"
static void gen_x87(DisasContext *s, X86
[PULL 23/23] exec: don't use void* in pointer arithmetic in headers
From: Roman Kiryanov
void* pointer arithmetic is a GCC extentension which could not be
available in other build tools (e.g. C++). This changes removes this
assumption.
Signed-off-by: Roman Kiryanov
Suggested-by: Paolo Bonzini
Link: https://lore.kernel.org/r/20240620201654.598024-1-r...@google.com
Signed-off-by: Paolo Bonzini
---
include/exec/memory.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index d7591a60d9f..08ecd7e195d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2796,7 +2796,7 @@ MemTxResult address_space_write_rom(AddressSpace *as,
hwaddr addr,
#include "exec/memory_ldst_phys.h.inc"
struct MemoryRegionCache {
-void *ptr;
+uint8_t *ptr;
hwaddr xlat;
hwaddr len;
FlatView *fv;
--
2.45.2
[PULL 20/23] block: remove separate bdrv_file_open callback
bdrv_file_open and bdrv_open are completely equivalent, they are
never checked except to see which one to invoke. So merge them
into a single one.
Signed-off-by: Paolo Bonzini
---
include/block/block_int-common.h | 3 ---
block.c | 4 +---
block/blkdebug.c | 2 +-
block/blkio.c| 2 +-
block/blkverify.c| 2 +-
block/curl.c | 8
block/file-posix.c | 8
block/file-win32.c | 4 ++--
block/gluster.c | 6 +++---
block/iscsi.c| 4 ++--
block/nbd.c | 6 +++---
block/nfs.c | 2 +-
block/null.c | 4 ++--
block/nvme.c | 2 +-
block/rbd.c | 3 ++-
block/ssh.c | 2 +-
block/vvfat.c| 2 +-
17 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 761276127ed..ebb4e56a503 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -248,9 +248,6 @@ struct BlockDriver {
int GRAPH_UNLOCKED_PTR (*bdrv_open)(
BlockDriverState *bs, QDict *options, int flags, Error **errp);
-/* Protocol drivers should implement this instead of bdrv_open */
-int GRAPH_UNLOCKED_PTR (*bdrv_file_open)(
-BlockDriverState *bs, QDict *options, int flags, Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)(
diff --git a/block.c b/block.c
index dd14ba85fc3..c1cc313d216 100644
--- a/block.c
+++ b/block.c
@@ -1655,9 +1655,7 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->opaque = g_malloc0(drv->instance_size);
assert(!drv->bdrv_needs_filename || bs->filename[0]);
-if (drv->bdrv_file_open) {
-ret = drv->bdrv_file_open(bs, options, open_flags, _err);
-} else if (drv->bdrv_open) {
+if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
} else {
ret = 0;
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 9da8c9eddc2..c95c818c388 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -1073,7 +1073,7 @@ static BlockDriver bdrv_blkdebug = {
.is_filter = true,
.bdrv_parse_filename= blkdebug_parse_filename,
-.bdrv_file_open = blkdebug_open,
+.bdrv_open = blkdebug_open,
.bdrv_close = blkdebug_close,
.bdrv_reopen_prepare= blkdebug_reopen_prepare,
.bdrv_child_perm= blkdebug_child_perm,
diff --git a/block/blkio.c b/block/blkio.c
index 882e1c297b4..1a38064ce76 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs,
Error **errp)
*/
#define BLKIO_DRIVER_COMMON \
.instance_size = sizeof(BDRVBlkioState), \
-.bdrv_file_open = blkio_file_open, \
+.bdrv_open = blkio_file_open, \
.bdrv_close = blkio_close, \
.bdrv_co_getlength = blkio_co_getlength, \
.bdrv_co_truncate= blkio_truncate, \
diff --git a/block/blkverify.c b/block/blkverify.c
index ec45d8335ed..5a9bf674d9c 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -321,7 +321,7 @@ static BlockDriver bdrv_blkverify = {
.instance_size= sizeof(BDRVBlkverifyState),
.bdrv_parse_filename = blkverify_parse_filename,
-.bdrv_file_open = blkverify_open,
+.bdrv_open= blkverify_open,
.bdrv_close = blkverify_close,
.bdrv_child_perm = bdrv_default_perms,
.bdrv_co_getlength= blkverify_co_getlength,
diff --git a/block/curl.c b/block/curl.c
index 419f7c89ef2..ef5252d00b5 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -1034,7 +1034,7 @@ static BlockDriver bdrv_http = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1053,7 +1053,7 @@ static BlockDriver bdrv_https = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
+.bdrv_open = curl_open,
.bdrv_close = curl_close,
.bdrv_co_getlength = curl_co_getlength,
@@ -1072,7 +1072,7 @@ static BlockDriver bdrv_ftp = {
.instance_size = sizeof(BDRVCURLState),
.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open =
[PULL 19/23] block: do not check bdrv_file_open
The set of BlockDrivers that have .bdrv_file_open coincides with those
that have .protocol_name and guess what---checking drv->bdrv_file_open
is done to see if the driver is a protocol. So check drv->protocol_name
instead.
Signed-off-by: Paolo Bonzini
---
block.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 69a2905178a..dd14ba85fc3 100644
--- a/block.c
+++ b/block.c
@@ -926,7 +926,6 @@ BlockDriver *bdrv_find_protocol(const char *filename,
int i;
GLOBAL_STATE_CODE();
-/* TODO Drivers without bdrv_file_open must be specified explicitly */
/*
* XXX(hch): we really should not let host device detection
@@ -1983,7 +1982,7 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockBackend *file,
open_flags = bdrv_open_flags(bs, bs->open_flags);
node_name = qemu_opt_get(opts, "node-name");
-assert(!drv->bdrv_file_open || file == NULL);
+assert(!drv->protocol_name || file == NULL);
ret = bdrv_open_driver(bs, drv, node_name, options, open_flags, errp);
if (ret < 0) {
goto fail_opts;
@@ -2084,7 +2083,7 @@ static int bdrv_fill_options(QDict **options, const char
*filename,
}
/* If the user has explicitly specified the driver, this choice should
* override the BDRV_O_PROTOCOL flag */
-protocol = drv->bdrv_file_open;
+protocol = drv->protocol_name;
}
if (protocol) {
@@ -4123,7 +4122,7 @@ bdrv_open_inherit(const char *filename, const char
*reference, QDict *options,
}
/* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */
-assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open);
+assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->protocol_name);
/* file must be NULL if a protocol BDS is about to be created
* (the inverse results in an error message from bdrv_open_common()) */
assert(!(flags & BDRV_O_PROTOCOL) || !file);
@@ -5971,7 +5970,7 @@ int64_t coroutine_fn
bdrv_co_get_allocated_file_size(BlockDriverState *bs)
return drv->bdrv_co_get_allocated_file_size(bs);
}
-if (drv->bdrv_file_open) {
+if (drv->protocol_name) {
/*
* Protocol drivers default to -ENOTSUP (most of their data is
* not stored in any of their children (if they even have any),
@@ -8030,7 +8029,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
* Both of these conditions are represented by
generate_json_filename.
*/
if (primary_child_bs->exact_filename[0] &&
-primary_child_bs->drv->bdrv_file_open &&
+primary_child_bs->drv->protocol_name &&
!drv->is_filter && !generate_json_filename)
{
strcpy(bs->exact_filename, primary_child_bs->exact_filename);
--
2.45.2
[PULL 18/23] block: make assertion more generic
.bdrv_needs_filename is only set for drivers that also set bdrv_file_open,
i.e. protocol drivers.
So we can make the assertion always, it will always pass for those drivers
that use bdrv_open.
Signed-off-by: Paolo Bonzini
---
block.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 468cf5e67d7..69a2905178a 100644
--- a/block.c
+++ b/block.c
@@ -1655,8 +1655,8 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name,
bs->drv = drv;
bs->opaque = g_malloc0(drv->instance_size);
+assert(!drv->bdrv_needs_filename || bs->filename[0]);
if (drv->bdrv_file_open) {
-assert(!drv->bdrv_needs_filename || bs->filename[0]);
ret = drv->bdrv_file_open(bs, options, open_flags, _err);
} else if (drv->bdrv_open) {
ret = drv->bdrv_open(bs, options, open_flags, _err);
--
2.45.2
[PULL 12/23] target/i386: remove gen_ext_tl
With the introduction of tcg_gen_ext_tl, most uses can be converted directly
because they do not have a NULL destination. tcg_gen_ext_tl is able to drop
no-ops like "tcg_gen_ext_tl(tcgv, tcgv, MO_TL)" just fine, and the only thing
that gen_ext_tl was adding on top was avoiding the creation of a useless
temporary. This can be done in the only place where it matters, which is
gen_op_j_ecx.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 41 +++--
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index d11c5e1dc13..5c9c992400e 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -697,23 +697,16 @@ static inline TCGv gen_compute_Dshift(DisasContext *s,
MemOp ot)
return dshift;
};
-static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
-{
-if (size == MO_TL) {
-return src;
-}
-if (!dst) {
-dst = tcg_temp_new();
-}
-tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
-return dst;
-}
-
static void gen_op_j_ecx(DisasContext *s, TCGCond cond, TCGLabel *label1)
{
-TCGv tmp = gen_ext_tl(NULL, cpu_regs[R_ECX], s->aflag, false);
-
-tcg_gen_brcondi_tl(cond, tmp, 0, label1);
+TCGv lhs;
+if (s->aflag == MO_TL) {
+lhs = cpu_regs[R_ECX];
+} else {
+lhs = tcg_temp_new();
+tcg_gen_ext_tl(lhs, cpu_regs[R_ECX], s->aflag);
+}
+tcg_gen_brcondi_tl(cond, lhs, 0, label1);
}
static inline void gen_op_jz_ecx(DisasContext *s, TCGLabel *label1)
@@ -886,16 +879,16 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s,
TCGv reg)
case CC_OP_SUBB ... CC_OP_SUBQ:
/* (DATA_TYPE)CC_SRCT < (DATA_TYPE)CC_SRC */
size = s->cc_op - CC_OP_SUBB;
-gen_ext_tl(s->cc_srcT, s->cc_srcT, size, false);
-gen_ext_tl(cpu_cc_src, cpu_cc_src, size, false);
+tcg_gen_ext_tl(s->cc_srcT, s->cc_srcT, size);
+tcg_gen_ext_tl(cpu_cc_src, cpu_cc_src, size);
return (CCPrepare) { .cond = TCG_COND_LTU, .reg = s->cc_srcT,
.reg2 = cpu_cc_src, .use_reg2 = true };
case CC_OP_ADDB ... CC_OP_ADDQ:
/* (DATA_TYPE)CC_DST < (DATA_TYPE)CC_SRC */
size = s->cc_op - CC_OP_ADDB;
-gen_ext_tl(cpu_cc_dst, cpu_cc_dst, size, false);
-gen_ext_tl(cpu_cc_src, cpu_cc_src, size, false);
+tcg_gen_ext_tl(cpu_cc_dst, cpu_cc_dst, size);
+tcg_gen_ext_tl(cpu_cc_src, cpu_cc_src, size);
return (CCPrepare) { .cond = TCG_COND_LTU, .reg = cpu_cc_dst,
.reg2 = cpu_cc_src, .use_reg2 = true };
@@ -920,7 +913,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv
reg)
case CC_OP_BMILGB ... CC_OP_BMILGQ:
size = s->cc_op - CC_OP_BMILGB;
-gen_ext_tl(cpu_cc_src, cpu_cc_src, size, false);
+tcg_gen_ext_tl(cpu_cc_src, cpu_cc_src, size);
return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
case CC_OP_ADCX:
@@ -1050,8 +1043,8 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b,
TCGv reg)
size = s->cc_op - CC_OP_SUBB;
switch (jcc_op) {
case JCC_BE:
-gen_ext_tl(s->cc_srcT, s->cc_srcT, size, false);
-gen_ext_tl(cpu_cc_src, cpu_cc_src, size, false);
+tcg_gen_ext_tl(s->cc_srcT, s->cc_srcT, size);
+tcg_gen_ext_tl(cpu_cc_src, cpu_cc_src, size);
cc = (CCPrepare) { .cond = TCG_COND_LEU, .reg = s->cc_srcT,
.reg2 = cpu_cc_src, .use_reg2 = true };
break;
@@ -1061,8 +1054,8 @@ static CCPrepare gen_prepare_cc(DisasContext *s, int b,
TCGv reg)
case JCC_LE:
cond = TCG_COND_LE;
fast_jcc_l:
-gen_ext_tl(s->cc_srcT, s->cc_srcT, size, true);
-gen_ext_tl(cpu_cc_src, cpu_cc_src, size, true);
+tcg_gen_ext_tl(s->cc_srcT, s->cc_srcT, size | MO_SIGN);
+tcg_gen_ext_tl(cpu_cc_src, cpu_cc_src, size | MO_SIGN);
cc = (CCPrepare) { .cond = cond, .reg = s->cc_srcT,
.reg2 = cpu_cc_src, .use_reg2 = true };
break;
--
2.45.2
[PULL 16/23] meson: allow configuring the x86-64 baseline
Add a Meson option to configure which x86-64 instruction
set to use. QEMU will now default to x86-64-v1 + cmpxchg16b for
64-bit builds (that corresponds to a Pentium 4 for 32-bit builds).
The baseline can be tuned down to Pentium Pro for 32-bit builds (with
-Dx86_version=0), or up as desired.
Acked-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
meson.build | 41 ---
meson_options.txt | 3 +++
scripts/meson-buildoptions.sh | 3 +++
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/meson.build b/meson.build
index 97e00d6f59b..6e694ecd9fe 100644
--- a/meson.build
+++ b/meson.build
@@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links('''
qemu_common_flags = ['-march=i486'] + qemu_common_flags
endif
-# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code)
-if host_arch == 'i386'
- qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
-endif
+# Pick x86-64 baseline version
if host_arch in ['i386', 'x86_64']
- qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags
-endif
-if host_arch == 'x86_64'
- qemu_common_flags = ['-mcx16'] + qemu_common_flags
+ if get_option('x86_version') == '0' and host_arch == 'x86_64'
+error('x86_64-v1 required for x86-64 hosts')
+ endif
+
+ # add flags for individual instruction set extensions
+ if get_option('x86_version') >= '1'
+if host_arch == 'i386'
+ qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags
+else
+ # present on basically all processors but technically not part of
+ # x86-64-v1, so only include -mneeded for x86-64 version 2 and above
+ qemu_common_flags = ['-mcx16'] + qemu_common_flags
+endif
+ endif
+ if get_option('x86_version') >= '2'
+qemu_common_flags = ['-mpopcnt'] + qemu_common_flags
+qemu_common_flags = cc.get_supported_arguments('-mneeded') +
qemu_common_flags
+ endif
+ if get_option('x86_version') >= '3'
+qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma',
'-mf16c'] + qemu_common_flags
+ endif
+
+ # add required vector instruction set (each level implies those below)
+ if get_option('x86_version') == '1'
+qemu_common_flags = ['-msse2'] + qemu_common_flags
+ elif get_option('x86_version') == '2'
+qemu_common_flags = ['-msse4.2'] + qemu_common_flags
+ elif get_option('x86_version') == '3'
+qemu_common_flags = ['-mavx2'] + qemu_common_flags
+ elif get_option('x86_version') == '4'
+qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd',
'-mavx512dq', '-mavx512vl'] + qemu_common_flags
+ endif
endif
if get_option('prefer_static')
diff --git a/meson_options.txt b/meson_options.txt
index 7a79dd89706..6065ed2d352 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -370,3 +370,6 @@ option('qemu_ga_version', type: 'string', value: '',
option('hexagon_idef_parser', type : 'boolean', value : true,
description: 'use idef-parser to automatically generate TCG code for
the Hexagon frontend')
+
+option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'],
value: '1',
+ description: 'tweak required x86_64 architecture version beyond
compiler default')
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 58d49a447d5..62842d47e88 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -82,6 +82,8 @@ meson_options_help() {
printf "%s\n" ' --with-suffix=VALUE Suffix for QEMU
data/modules/config directories'
printf "%s\n" ' (can be empty) [qemu]'
printf "%s\n" ' --with-trace-file=VALUE Trace file prefix for simple
backend [trace]'
+ printf "%s\n" ' --x86-version=CHOICE tweak required x86_64 architecture
version beyond'
+ printf "%s\n" ' compiler default [1] (choices:
0/1/2/3)'
printf "%s\n" ''
printf "%s\n" 'Optional features, enabled with --enable-FEATURE and'
printf "%s\n" 'disabled with --disable-FEATURE, default is enabled if
available'
@@ -552,6 +554,7 @@ _meson_option_parse() {
--disable-werror) printf "%s" -Dwerror=false ;;
--enable-whpx) printf "%s" -Dwhpx=enabled ;;
--disable-whpx) printf "%s" -Dwhpx=disabled ;;
+--x86-version=*) quote_sh "-Dx86_version=$2" ;;
--enable-xen) printf "%s" -Dxen=enabled ;;
--disable-xen) printf "%s" -Dxen=disabled ;;
--enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;;
--
2.45.2
[PULL 13/23] Revert "host/i386: assume presence of POPCNT"
This reverts commit 45ccdbcb24baf99667997fac5cf60318e5e7db51.
The x86-64 instruction set can now be tuned down to x86-64 v1
or i386 Pentium Pro.
Signed-off-by: Paolo Bonzini
---
host/include/i386/host/cpuinfo.h | 1 +
tcg/i386/tcg-target.h| 5 +++--
util/cpuinfo-i386.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h
index c1e94d75ce1..72f6fad61e5 100644
--- a/host/include/i386/host/cpuinfo.h
+++ b/host/include/i386/host/cpuinfo.h
@@ -11,6 +11,7 @@
#define CPUINFO_ALWAYS (1u << 0) /* so cpuinfo is nonzero */
#define CPUINFO_MOVBE (1u << 2)
#define CPUINFO_LZCNT (1u << 3)
+#define CPUINFO_POPCNT (1u << 4)
#define CPUINFO_BMI1(1u << 5)
#define CPUINFO_BMI2(1u << 6)
#define CPUINFO_AVX1(1u << 9)
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index ecc69827287..2f67a97e059 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -111,6 +111,7 @@ typedef enum {
#endif
#define have_bmi1 (cpuinfo & CPUINFO_BMI1)
+#define have_popcnt (cpuinfo & CPUINFO_POPCNT)
#define have_avx1 (cpuinfo & CPUINFO_AVX1)
#define have_avx2 (cpuinfo & CPUINFO_AVX2)
#define have_movbe(cpuinfo & CPUINFO_MOVBE)
@@ -142,7 +143,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i32 0
#define TCG_TARGET_HAS_clz_i32 1
#define TCG_TARGET_HAS_ctz_i32 1
-#define TCG_TARGET_HAS_ctpop_i321
+#define TCG_TARGET_HAS_ctpop_i32have_popcnt
#define TCG_TARGET_HAS_deposit_i32 1
#define TCG_TARGET_HAS_extract_i32 1
#define TCG_TARGET_HAS_sextract_i32 1
@@ -177,7 +178,7 @@ typedef enum {
#define TCG_TARGET_HAS_nor_i64 0
#define TCG_TARGET_HAS_clz_i64 1
#define TCG_TARGET_HAS_ctz_i64 1
-#define TCG_TARGET_HAS_ctpop_i641
+#define TCG_TARGET_HAS_ctpop_i64have_popcnt
#define TCG_TARGET_HAS_deposit_i64 1
#define TCG_TARGET_HAS_extract_i64 1
#define TCG_TARGET_HAS_sextract_i64 0
diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c
index 8f2694d88f2..6d474a6259a 100644
--- a/util/cpuinfo-i386.c
+++ b/util/cpuinfo-i386.c
@@ -35,6 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void)
__cpuid(1, a, b, c, d);
info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0);
+info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0);
info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0);
/* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */
--
2.45.2
[PULL 09/23] target/i386: do not check PREFIX_LOCK in old-style decoder
It is already checked before getting there.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 26 --
1 file changed, 8 insertions(+), 18 deletions(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index c60f18c7482..501a1ef9313 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2878,7 +2878,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
switch ((modrm >> 3) & 7) {
case 7:
if (mod != 3 ||
-(s->prefix & (PREFIX_LOCK | PREFIX_REPNZ))) {
+(s->prefix & PREFIX_REPNZ)) {
goto illegal_op;
}
if (s->prefix & PREFIX_REPZ) {
@@ -2898,7 +2898,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
case 6: /* RDRAND */
if (mod != 3 ||
-(s->prefix & (PREFIX_LOCK | PREFIX_REPZ | PREFIX_REPNZ)) ||
+(s->prefix & (PREFIX_REPZ | PREFIX_REPNZ)) ||
!(s->cpuid_ext_features & CPUID_EXT_RDRAND)) {
goto illegal_op;
}
@@ -3058,8 +3058,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
case 0xd0: /* xgetbv */
if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
-|| (s->prefix & (PREFIX_LOCK | PREFIX_DATA
- | PREFIX_REPZ | PREFIX_REPNZ))) {
+|| (s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ))) {
goto illegal_op;
}
tcg_gen_trunc_tl_i32(s->tmp2_i32, cpu_regs[R_ECX]);
@@ -3069,8 +3068,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
case 0xd1: /* xsetbv */
if ((s->cpuid_ext_features & CPUID_EXT_XSAVE) == 0
-|| (s->prefix & (PREFIX_LOCK | PREFIX_DATA
- | PREFIX_REPZ | PREFIX_REPNZ))) {
+|| (s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ))) {
goto illegal_op;
}
gen_svm_check_intercept(s, SVM_EXIT_XSETBV);
@@ -3237,8 +3235,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
gen_st_modrm(s, decode, ot);
break;
case 0xee: /* rdpkru */
-if (s->prefix & (PREFIX_LOCK | PREFIX_DATA
- | PREFIX_REPZ | PREFIX_REPNZ)) {
+if (s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ)) {
goto illegal_op;
}
tcg_gen_trunc_tl_i32(s->tmp2_i32, cpu_regs[R_ECX]);
@@ -3246,8 +3243,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
tcg_gen_extr_i64_tl(cpu_regs[R_EAX], cpu_regs[R_EDX], s->tmp1_i64);
break;
case 0xef: /* wrpkru */
-if (s->prefix & (PREFIX_LOCK | PREFIX_DATA
- | PREFIX_REPZ | PREFIX_REPNZ)) {
+if (s->prefix & (PREFIX_DATA | PREFIX_REPZ | PREFIX_REPNZ)) {
goto illegal_op;
}
tcg_gen_concat_tl_i64(s->tmp1_i64, cpu_regs[R_EAX],
@@ -3323,7 +3319,6 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
if (prefixes & PREFIX_REPZ) {
/* bndcl */
if (reg >= 4
-|| (prefixes & PREFIX_LOCK)
|| s->aflag == MO_16) {
goto illegal_op;
}
@@ -3331,7 +3326,6 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
} else if (prefixes & PREFIX_REPNZ) {
/* bndcu */
if (reg >= 4
-|| (prefixes & PREFIX_LOCK)
|| s->aflag == MO_16) {
goto illegal_op;
}
@@ -3345,7 +3339,7 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
}
if (mod == 3) {
int reg2 = (modrm & 7) | REX_B(s);
-if (reg2 >= 4 || (prefixes & PREFIX_LOCK)) {
+if (reg2 >= 4) {
goto illegal_op;
}
if (s->flags & HF_MPX_IU_MASK) {
@@ -3374,7 +3368,6 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
/* bndldx */
AddressParts a = decode->mem;
if (reg >= 4
-|| (prefixes & PREFIX_LOCK)
|| s->aflag == MO_16
|| a.base < -1) {
goto illegal_op;
@@ -3410,7 +3403,6 @@ static void gen_multi0F(DisasContext *s, X86DecodedInsn
*decode)
[PULL 03/23] target/i386: use cpu_cc_dst for CC_OP_POPCNT
It is the only CCOp, among those that compute ZF from one of the cc_op_*
registers, that uses cpu_cc_src. Do not make it the odd one off,
instead use cpu_cc_dst like the others.
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 2 +-
target/i386/tcg/cc_helper.c | 2 +-
target/i386/tcg/translate.c | 4 ++--
target/i386/tcg/emit.c.inc | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 7e2a9b56aea..f54cd93b3f9 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1332,7 +1332,7 @@ typedef enum {
CC_OP_BMILGQ,
CC_OP_CLR, /* Z set, all other flags clear. */
-CC_OP_POPCNT, /* Z via CC_SRC, all other flags clear. */
+CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */
CC_OP_NB,
} CCOp;
diff --git a/target/i386/tcg/cc_helper.c b/target/i386/tcg/cc_helper.c
index f76e9cb8cfb..301ed954064 100644
--- a/target/i386/tcg/cc_helper.c
+++ b/target/i386/tcg/cc_helper.c
@@ -107,7 +107,7 @@ target_ulong helper_cc_compute_all(target_ulong dst,
target_ulong src1,
case CC_OP_CLR:
return CC_Z | CC_P;
case CC_OP_POPCNT:
-return src1 ? 0 : CC_Z;
+return dst ? 0 : CC_Z;
case CC_OP_MULB:
return compute_all_mulb(dst, src1);
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index ad1819815ab..eb353dc3c9f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -324,7 +324,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = {
[CC_OP_ADOX] = USES_CC_SRC | USES_CC_SRC2,
[CC_OP_ADCOX] = USES_CC_DST | USES_CC_SRC | USES_CC_SRC2,
[CC_OP_CLR] = 0,
-[CC_OP_POPCNT] = USES_CC_SRC,
+[CC_OP_POPCNT] = USES_CC_DST,
};
static void set_cc_op_1(DisasContext *s, CCOp op, bool dirty)
@@ -1020,7 +1020,7 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s,
TCGv reg)
case CC_OP_CLR:
return (CCPrepare) { .cond = TCG_COND_ALWAYS };
case CC_OP_POPCNT:
-return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src };
+return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst };
default:
{
MemOp size = (s->cc_op - CC_OP_ADDB) & 3;
diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc
index 11faa70b5e2..fc7477833bc 100644
--- a/target/i386/tcg/emit.c.inc
+++ b/target/i386/tcg/emit.c.inc
@@ -2804,10 +2804,10 @@ static void gen_POPA(DisasContext *s, X86DecodedInsn
*decode)
static void gen_POPCNT(DisasContext *s, X86DecodedInsn *decode)
{
-decode->cc_src = tcg_temp_new();
+decode->cc_dst = tcg_temp_new();
decode->cc_op = CC_OP_POPCNT;
-tcg_gen_mov_tl(decode->cc_src, s->T0);
+tcg_gen_mov_tl(decode->cc_dst, s->T0);
tcg_gen_ctpop_tl(s->T0, s->T0);
}
--
2.45.2
[PULL 11/23] target/i386: assert that cc_op* and pc_save are preserved
Now all decoding has been done before any code generation. There is no need anymore to save and restore cc_op* and pc_save but, for the time being, assert that this is indeed the case. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 501a1ef9313..d11c5e1dc13 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -3709,15 +3709,9 @@ static void i386_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) case 2: /* Restore state that may affect the next instruction. */ dc->pc = dc->base.pc_next; -/* - * TODO: These save/restore can be removed after the table-based - * decoder is complete; we will be decoding the insn completely - * before any code generation that might affect these variables. - */ -dc->cc_op_dirty = orig_cc_op_dirty; -dc->cc_op = orig_cc_op; -dc->pc_save = orig_pc_save; -/* END TODO */ +assert(dc->cc_op_dirty == orig_cc_op_dirty); +assert(dc->cc_op == orig_cc_op); +assert(dc->pc_save == orig_pc_save); dc->base.num_insns--; tcg_remove_ops_after(dc->prev_insn_end); dc->base.insn_start = dc->prev_insn_start; -- 2.45.2
[PULL 06/23] target/i386: try not to force EFLAGS computation for CC_OP_ADOX/ADCX
When computing the "other" flag (CF for CC_OP_ADOX, OF for CC_OP_ADCX),
take into account that it is already in the right position of cpu_cc_src,
just like for CC_OP_EFLAGS. There is no need to call gen_compute_eflags().
Reviewed-by: Richard Henderson
Signed-off-by: Paolo Bonzini
---
target/i386/tcg/translate.c | 4
1 file changed, 4 insertions(+)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 257110ac703..08db40681fa 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -928,6 +928,7 @@ static CCPrepare gen_prepare_eflags_c(DisasContext *s, TCGv
reg)
.no_setcond = true };
case CC_OP_EFLAGS:
+case CC_OP_ADOX:
case CC_OP_SARB ... CC_OP_SARQ:
/* CC_SRC & 1 */
return (CCPrepare) { .cond = TCG_COND_TSTNE,
@@ -994,6 +995,9 @@ static CCPrepare gen_prepare_eflags_o(DisasContext *s, TCGv
reg)
return (CCPrepare) { .cond = TCG_COND_NE, .reg = cpu_cc_src };
default:
gen_compute_eflags(s);
+/* fallthrough */
+case CC_OP_EFLAGS:
+case CC_OP_ADCX:
return (CCPrepare) { .cond = TCG_COND_TSTNE, .reg = cpu_cc_src,
.imm = CC_O };
}
--
2.45.2
Re: [PATCH v2] exec: don't use void* in pointer arithmetic in headers
On 6/20/24 22:16, Roman Kiryanov wrote:
void* pointer arithmetic is a GCC extentension
which could not be available in other build
tools (e.g. C++). This changes removes this
assumption.
Google-Bug-Id: 331190993
Change-Id: I5a064853429f627c17a9213910811dea4ced6174
Signed-off-by: Roman Kiryanov
Suggested-by: Paolo Bonzini
---
v2: renamed from "use char* for pointer arithmetic"
and removed all explicit extra cast with
one typedef in memory.h.
include/exec/memory.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index b1713f30b8..b616338f05 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2795,8 +2795,10 @@ MemTxResult address_space_write_rom(AddressSpace *as,
hwaddr addr,
#define ARG1_DECLAddressSpace *as
#include "exec/memory_ldst_phys.h.inc"
+typedef uint8_t *MemoryRegionCachePtr;
+
struct MemoryRegionCache {
-void *ptr;
+MemoryRegionCachePtr ptr;
Just "uint8_t *ptr" is enough; thanks for testing that it's enough.
Queued for the next pull request, thanks.
Paolo
hwaddr xlat;
hwaddr len;
FlatView *fv;
Re: [RFC PATCH v3 1/5] build-sys: Add rust feature option
Il gio 20 giu 2024, 20:13 Manos Pitsidianakis <
manos.pitsidiana...@linaro.org> ha scritto:
> On Thu, 20 Jun 2024 16:21, Paolo Bonzini wrote:
> >On 6/19/24 22:13, Manos Pitsidianakis wrote:
> >> Add options for Rust in meson_options.txt, meson.build, configure to
> >> prepare for adding Rust code in the followup commits.
> >>
> >> `rust` is a reserved meson name, so we have to use an alternative.
> >> `with_rust` was chosen.
> >>
> >> A cargo_wrapper.py script is added that is heavily based on the work of
> >> Marc-André Lureau from 2021.
> >>
> >>
> https://patchew.org/QEMU/20210907121943.3498701-1-marcandre.lur...@redhat.com/
> >>
> >> Signed-off-by: Marc-André Lureau
> >> Signed-off-by: Manos Pitsidianakis
> >
> >The cargo_wrapper.py script is not used yet, so it should be
> >delayed until it's used.
>
> That's true, I just wanted to make review easier by splitting it out.
> Can we squash them later or do you think I should I do it for the next
> series version?
>
I guess it depends on what the next version looks like. If you start
working on the workspace/build tree/Kconfig parts, it might not have a lot
of cargo_wrapper.py code surviving.
Feel free to take this patch and add
Signed-off-by: Paolo Bonzini
Paolo
>For the detection of the toolchain, I'd rather do everything in
> >configure since that's where the cross file is built. Something like:
> >
> >diff --git a/configure b/configure
> >index 8b6a2f16ceb..6412a1021c3 100755
> >--- a/configure
> >+++ b/configure
> >@@ -173,6 +173,8 @@ fi
> >
> > # default parameters
> > container_engine="auto"
> >+rust_target_triple=""
> >+with_rust="no"
> > cpu=""
> > cross_compile="no"
> > cross_prefix=""
> >@@ -201,6 +202,8 @@ for opt do
> >--cross-prefix=*) cross_prefix="$optarg"
> > cross_compile="yes"
> >;;
> >+ --cargo=*) CARGO="$optarg"
> >+ ;;
> >--cc=*) CC="$optarg"
> >;;
> >--cxx=*) CXX="$optarg"
> >@@ -317,6 +322,8 @@ windmc="${WINDMC-${cross_prefix}windmc}"
> > pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
> > sdl2_config="${SDL2_CONFIG-${cross_prefix}sdl2-config}"
> >
> >+cargo="${CARGO-cargo}"
> >+
> > check_define() {
> > cat > $TMPC < > #if !defined($1)
> >@@ -628,6 +635,8 @@ for opt do
> >;;
> >--cross-prefix=*)
> >;;
> >+ --cargo=*)
> >+ ;;
> >--cc=*)
> >;;
> >--host-cc=*) host_cc="$optarg"
> >@@ -755,8 +764,14 @@ for opt do
> >;;
> >--container-engine=*) container_engine="$optarg"
> >;;
> >+ --rust-target-triple=*) rust_target_triple="$optarg"
> >+ ;;
> >--gdb=*) gdb_bin="$optarg"
> >;;
> >+ --with-rust) with_rust=yes
> >+ ;;
> >+ --without-rust) with_rust=no
> >+ ;;
> ># everything else has the same name in configure and meson
> >--*) meson_option_parse "$opt" "$optarg"
> >;;
> >@@ -854,6 +869,7 @@ $(echo Available targets: $default_target_list | \
> > Advanced options (experts only):
> >-Dmesonoptname=val passthrough option to meson unmodified
> >--cross-prefix=PREFIXuse PREFIX for compile tools, PREFIX can be
> blank [$cross_prefix]
> >+ --cargo=CARGOuse Cargo binary CARGO [$cargo]
> >--cc=CC use C compiler CC [$cc]
> >--host-cc=CC when cross compiling, use C compiler CC for
> code run
> > at build time [$host_cc]
> >@@ -869,11 +885,13 @@ Advanced options (experts only):
> >--python=PYTHON use specified python [$python]
> >--ninja=NINJAuse specified ninja [$ninja]
> >--static enable static build [$static]
> >- --without-default-features default all --enable-* options to "disabled"
> >- --without-default-devices do not include any device that is not
> needed to
> >+ --rust-target-triple=TRIPLE target for Rust cross compilation
> >+ --without-default-features default all --enable-* options to
> "disabled"
> >+ --without-default-devicesdo not include any device that is not
> needed to
> > start the emulator (only use if you are
> including
> >
