> diff --git a/rust/hw/char/pl011/src/device.rs 
> b/rust/hw/char/pl011/src/device.rs
> index 7cffb894a8..a3bcd1297a 100644
> --- a/rust/hw/char/pl011/src/device.rs
> +++ b/rust/hw/char/pl011/src/device.rs
> @@ -21,10 +21,13 @@
>      memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
>      prelude::*,
>      qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, 
> ResettablePhasesImpl},
> -    qom::{ObjectImpl, Owned, ParentField, ParentInit},
>      sysbus::{SysBusDevice, SysBusDeviceImpl},
>      vmstate_clock,
>  };
> +use qom::{
> +    qom_isa, IsA, Object, ObjectClassMethods, ObjectDeref, ObjectImpl, 
> ObjectMethods, ObjectType,
> +    Owned, ParentField, ParentInit,
> +};

These QOM parts are frequently used and very common. at least for qom,
I think prelude would help a lot.

A qom prelude could help reduce the changes in other parts (pl011/
hpet/memory...).

> diff --git a/rust/qom/meson.build b/rust/qom/meson.build
> new file mode 100644
> index 0000000000..6e95d75fa0
> --- /dev/null
> +++ b/rust/qom/meson.build
> @@ -0,0 +1,61 @@
> +_qom_cfg = run_command(rustc_args,
> +  '--config-headers', config_host_h, '--features', files('Cargo.toml'),
> +  capture: true, check: true).stdout().strip().splitlines()
> +
> +# TODO: Remove this comment when the clang/libclang mismatch issue is solved.
> +#
> +# Rust bindings generation with `bindgen` might fail in some cases where the
> +# detected `libclang` does not match the expected `clang` version/target. In
> +# this case you must pass the path to `clang` and `libclang` to your build
> +# command invocation using the environment variables CLANG_PATH and
> +# LIBCLANG_PATH
> +_qom_bindings_inc_rs = rust.bindgen(
> +  input: 'wrapper.h',
> +  dependencies: common_ss.all_dependencies(),
> +  output: 'bindings.inc.rs',

There're many binding files with the same name. What about adding a prefix
like "qom-bindings" to distinguish it? This can help search and locate
specific binding file.

> +  include_directories: bindings_incdir,
> +  bindgen_version: ['>=0.60.0'],
> +  args: bindgen_args_common,
> +)

...

> diff --git a/rust/qom/tests/tests.rs b/rust/qom/tests/tests.rs
> new file mode 100644
> index 0000000000..49f1cbecf5
> --- /dev/null
> +++ b/rust/qom/tests/tests.rs
> @@ -0,0 +1,47 @@
> +use std::{ffi::CStr, sync::LazyLock};

LazyLock is useful, but it became stable since v1.80. So if Paolo
decide pick this series after v1.83 support, it's fine.

> +use qom::{qom_isa, Object, ObjectClassMethods, ObjectImpl, ObjectType, 
> ParentField};
> +use util::bindings::{module_call_init, module_init_type};

...

> +fn init_qom() {
> +    static ONCE: LazyLock<()> = LazyLock::new(|| unsafe {
> +        module_call_init(module_init_type::MODULE_INIT_QOM);
> +    });

But for now, we can still use a static BqlCell<bool> as the workaround,
just like rust/hwcore/tests/tests.rs did, to decouple with MSRV
dependency.

And it seems rust/hwcore/tests/tests.rs has already covered this test
case, do we still need this test?

If so, then it's better to add this new test in a seperate patch, which
makes current patch focus on the splitting :-).

> +    bql::start_test();
> +    LazyLock::force(&ONCE);
> +}
> +

Otherwise, LGTM,

Reviewed-by: Zhao Liu <zhao1....@intel.com>


Reply via email to