Re: [RFC PATCH v2 3/5] rust: add PL011 device model

[Zhao Liu]

On Tue, Jun 11, 2024 at 01:33:32PM +0300, Manos Pitsidianakis wrote:
> Date: Tue, 11 Jun 2024 13:33:32 +0300
> From: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> Subject: [RFC PATCH v2 3/5] rust: add PL011 device model
> X-Mailer: git-send-email 2.44.0
> 
> This commit adds a re-implementation of hw/char/pl011.c in Rust.
> 
> It uses generated Rust bindings (produced by `ninja
> aarch64-softmmu-generated.rs`) to
> register itself as a QOM type/class.
> 
> How to build:
> 
> 1. Make sure rust, cargo and bindgen (cargo install bindgen-cli) are
>    installed
> 2. Configure a QEMU build with:
>    --enable-system --target-list=aarch64-softmmu --enable-with-rust
> 3. Launching a VM with qemu-system-aarch64 should use the Rust version
>    of the pl011 device (unless it is not set up so in hw/arm/virt.c; the
>    type of the UART device is hardcoded).
> 
>    To confirm, inspect `info qom-tree` in the monitor and look for an
>    `x-pl011-rust` device.
> 
> Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---

Hi Manos,

Thanks for your example!

> diff --git a/rust/pl011/Cargo.toml b/rust/pl011/Cargo.toml
> new file mode 100644
> index 0000000000..db74f2b59f
> --- /dev/null
> +++ b/rust/pl011/Cargo.toml
> @@ -0,0 +1,66 @@

...

> +[lints]
> +[lints.rustdoc]
> +broken_intra_doc_links = "deny"
> +redundant_explicit_links = "deny"
> +[lints.clippy]
> +# lint groups
> +correctness = { level = "deny", priority = -1 }
> +suspicious = { level = "deny", priority = -1 }
> +complexity = { level = "deny", priority = -1 }
> +perf = { level = "deny", priority = -1 }
> +cargo = { level = "deny", priority = -1 }
> +nursery = { level = "deny", priority = -1 }
> +style = { level = "deny", priority = -1 }
> +# restriction group
> +dbg_macro = "deny"
> +rc_buffer = "deny"
> +as_underscore = "deny"
> +assertions_on_result_states = "deny"
> +# pedantic group
> +doc_markdown = "deny"
> +expect_fun_call = "deny"
> +borrow_as_ptr = "deny"
> +case_sensitive_file_extension_comparisons = "deny"
> +cast_lossless = "deny"
> +cast_ptr_alignment = "allow"
> +large_futures = "deny"
> +waker_clone_wake = "deny"
> +unused_enumerate_index = "deny"
> +unnecessary_fallible_conversions = "deny"
> +struct_field_names = "deny"
> +manual_hash_one = "deny"
> +into_iter_without_iter = "deny"
> +option_if_let_else = "deny"
> +missing_const_for_fn = "deny"
> +significant_drop_tightening = "deny"
> +multiple_crate_versions = "deny"
> +significant_drop_in_scrutinee = "deny"
> +cognitive_complexity = "deny"
> +missing_safety_doc = "allow"

...

> diff --git a/rust/pl011/rustfmt.toml b/rust/pl011/rustfmt.toml
> new file mode 120000
> index 0000000000..39f97b043b
> --- /dev/null
> +++ b/rust/pl011/rustfmt.toml
> @@ -0,0 +1 @@
> +../rustfmt.toml

...

> diff --git a/rust/rustfmt.toml b/rust/rustfmt.toml
> new file mode 100644
> index 0000000000..ebecb99fe0
> --- /dev/null
> +++ b/rust/rustfmt.toml
> @@ -0,0 +1,7 @@
> +edition = "2021"
> +format_generated_files = false
> +format_code_in_doc_comments = true
> +format_strings = true
> +imports_granularity = "Crate"
> +group_imports = "StdExternalCrate"
> +wrap_comments = true
> 

About the Rust style, inspired from the discussion on my previous
simpletrace-rust [1], it looks like people prefer the default rust style
and use the default check without custom configurations.

More style requirements are also an open, especially for unstable ones,
and it would be better to split this part into a separate patch, so that
the discussion about style doesn't overshadow the focus on your example.

[1]: https://lore.kernel.org/qemu-devel/ZlnBGwk29Ds9FjUA@redhat.com/

Regards,
Zhao