Re: [RFC PATCH v1 0/6] Implement ARM PL011 in Rust

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Mon, 10 Jun 2024 22:37, Pierrick Bouvier <pierrick.bouvier@linaro.org> wrote:
>Hello Manos,
>
>On 6/10/24 11:22, Manos Pitsidianakis wrote:
>> Hello everyone,
>> 
>> This is an early draft of my work on implementing a very simple device,
>> in this case the ARM PL011 (which in C code resides in hw/char/pl011.c
>> and is used in hw/arm/virt.c).
>> 
>> The device is functional, with copied logic from the C code but with
>> effort not to make a direct C to Rust translation. In other words, do
>> not write Rust as a C developer would.
>> 
>> That goal is not complete but a best-effort case. To give a specific
>> example, register values are typed but interrupt bit flags are not (but
>> could be). I will leave such minutiae for later iterations.
>> 
>> By the way, the wiki page for Rust was revived to keep track of all
>> current series on the mailing list https://wiki.qemu.org/RustInQemu
>> 
>> a #qemu-rust IRC channel was also created for rust-specific discussion
>> that might flood #qemu
>> 
>
>Excellent work, and thanks for posting this RFC!
>
>IMHO, having patches 2 and 5 splitted is a bit confusing, and exposing 
>(temporarily) the generated.rs file in patches is not a good move.
>Any reason you kept it this way?

That was my first approach, I will rework it on the second version. The 
generated code should not exist in committed code at all.

It was initally tricky setting up the dependency orders correctly, so I 
first committed it and then made it a dependency.

>
>Maybe it could be better if build.rs file was *not* needed for new 
>devices/folders, and could be abstracted as a detail of the python 
>wrapper script instead of something that should be committed.


That'd mean you cannot work on the rust files with a LanguageServer, you 
cannot run cargo build or cargo check or cargo clippy, etc. That's why I 
left the alternative choice of including a manually generated bindings 
file (generated.rs.inc)

>
>Having a simple rust/pl011/meson.build is nice and good taste!
>
>> ------------------------------------------------------------------------
>> A request: keep comments to Rust in relation to the QEMU project and no
>> debates on the merits of the language itself. These are valid concerns,
>> but it'd be better if they were on separate mailing list threads.
>> ------------------------------------------------------------------------
>> 
>> Table of contents: [TOC]
>> 
>> - How can I try it? [howcanItryit]
>> - What are the most important points to focus on, at this point?
>>    [whatarethemostimportant]
>>    - What are the issues with not using the compiler, rustc, directly?
>>      [whataretheissueswith]
>>      1. Tooling
>>      2. Rust dependencies
>> 
>>    - Should QEMU use third-party dependencies? [shouldqemuusethirdparty]
>>    - Should QEMU provide wrapping Rust APIs over QEMU internals?
>>      [qemuprovidewrappingrustapis]
>>    - Will QEMU now depend on Rust and thus not build on my XYZ platform?
>>      [qemudependonrustnotbuildonxyz]
>> - How is the compilation structured? [howisthecompilationstructured]
>> - The generated.rs rust file includes a bunch of junk definitions?
>>    [generatedrsincludesjunk]
>> - The staticlib artifact contains a bunch of mangled .o objects?
>>    [staticlibmangledobjects]
>> 
>> How can I try it?
>> =================
>> [howcanItryit] Back to [TOC]
>> 
>> Hopefully applying this patches (or checking out `master` branch from
>> https://gitlab.com/epilys/rust-for-qemu/ current commit
>> de81929e0e9d470deac2c6b449b7a5183325e7ee )
>> 
>> Tag for this RFC is rust-pl011-rfc-v1
>> 
>> Rustdoc documentation is hosted on
>> 
>> https://rust-for-qemu-epilys-aebb06ca9f9adfe6584811c14ae44156501d935ba4.gitlab.io/pl011/index.html
>> 
>> If `cargo` and `bindgen` is installed in your system, you should be able
>> to build qemu-system-aarch64 with configure flag --enable-rust and
>> launch an arm virt VM. One of the patches hardcodes the default UART of
>> the machine to the Rust one, so if something goes wrong you will see it
>> upon launching qemu-system-aarch64.
>> 
>> To confirm it is there for sure, run e.g. info qom-tree on the monitor
>> and look for x-pl011-rust.
>> 
>> 
>> What are the most important points to focus on, at this point?
>> ==============================================================
>> [whatarethemostimportant] Back to [TOC]
>> 
>> In my opinion, integration of the go-to Rust build system (Cargo and
>> crates.io) with the build system we use in QEMU. This is "easily" done
>> in some definition of the word with a python wrapper script.
>> 
>> What are the issues with not using the compiler, rustc, directly?
>> -----------------------------------------------------------------
>> [whataretheissueswith] Back to [TOC]
>> 
>> 1. Tooling
>>     Mostly writing up the build-sys tooling to do so. Ideally we'd
>>     compile everything without cargo but rustc directly.
>> 
>>     If we decide we need Rust's `std` library support, we could
>>     investigate whether building it from scratch is a good solution. This
>>     will only build the bits we need in our devices.
>>  > 2. Rust dependencies
>>     We could go without them completely. I chose deliberately to include
>>     one dependency in my UART implementation, `bilge`[0], because it has
>>     an elegant way of representing typed bitfields for the UART's
>>     registers.
>> 
>> [0]: Article: https://hecatia-elegua.github.io/blog/no-more-bit-fiddling/
>>       Crates.io page: https://crates.io/crates/bilge
>>       Repository: https://github.com/hecatia-elegua/bilge
>> 
>> Should QEMU use third-party dependencies?
>> -----------------------------------------
>> [shouldqemuusethirdparty] Back to [TOC]
>> 
>> In my personal opinion, if we need a dependency we need a strong
>> argument for it. A dependency needs a trusted upstream source, a QEMU
>> maintainer to make sure it us up-to-date in QEMU etc.
>> 
>> We already fetch some projects with meson subprojects, so this is not a
>> new reality. Cargo allows you to define "locked" dependencies which is
>> the same as only fetching specific commits by SHA. No suspicious
>> tarballs, and no disappearing dependencies a la left-pad in npm.
>>
>
>As a complement to this, and for other readers, in more than having a 
>lock file (fixing version you use), cargo crates system is designed to 
>be immutable (see: https://crates.io/policies), and it means there is a 
>strong guarantee that a published package will stay there, to the 
>opposite of npm, pypi, or most of other similar systems.
>
>"Crate deletion by their owners is not possible to keep the registry as 
>immutable as possible."
>
>I believe this is a *key* feature of Rust ecosystem and should be 
>emphasized regarding the policy for Rust dependencies to come.
>
>> However, I believe it's worth considering vendoring every dependency by
>> default, if they prove to be few, for the sake of having a local QEMU
>> git clone buildable without network access.
>>
>
>I would not be in favor to vendor all dependencies. Beyond the "offline 
>build" scenario, it has only downsides.
>
>Sure, we should really debate before introducing a new dependency, but 
>the technical difficulty to mirror its sources and dependencies should 
>not be an argument for or against it.
>
>What will happen the day we want to introduce something bigger than a 
>simple dependency? (let's say "serde" for instance)


Yes, vendor-the-world is a different topic than vendor e.g. two crates 
such as the dependencies I'm using here.

>
>> Should QEMU provide wrapping Rust APIs over QEMU internals?
>> -----------------------------------------------------------
>> [qemuprovidewrappingrustapis] Back to [TOC]
>> 
>> My personal opinion is no, with the reasoning being that QEMU internals
>> are not documented or stable. However I do not see why creating stable
>> opt-in interfaces is bad. It just needs someone to volunteer to maintain
>> it and ensure there are no breakages through versions.
>> 
>> Will QEMU now depend on Rust and thus not build on my XYZ platform?
>> -------------------------------------------------------------------
>> [qemudependonrustnotbuildonxyz] Back to [TOC]
>> 
>> No, worry about this in some years if this experiment takes off. Rust
>> has broad platform support and is present in most distro package
>> managers. In the future we might have gcc support for it as well.
>> 
>> For now, Rust will have an experimental status, and will be aimed to
>> those who wish to try it. I leave it to the project leaders to make
>> proper decisions and statements on this if necessary.
>> 
>> 
>> How is the compilation structured?
>> ==================================
>> [howisthecompilationstructured] Back to [TOC]
>> 
>> First, a meson target that runs `bindgen` on a bunch of header files
>> (defined in `rust/wrapper.h`) is created as a target and as a dependency
>> for any rust hardware device that needs it. You can see the generated
>> bindings by running
>> 
>>    ninja generated.rs
>> 
>> inside your build directory.
>> 
>> The devices are defined as dictionaries in rust/meson.build because they
>> depend on the bindgen dependency, which is available much later in the
>> meson process (when the static qemu lib and target emulator executables
>> are defined).
>> 
>> A cargo wrapper python script under scripts/ exists to build the crate
>> library, by providing the path to the generated.rs bindings via the
>> environment. Then, the qemu-system-aarch64 binary links against the
>> staticlib archive (i.e. libpl011.a)
>> 
>> The generated.rs rust file includes a bunch of junk definitions?
>> ================================================================
>> [generatedrsincludesjunk] Back to [TOC]
>> 
>> Yes, bindgen allows you to block certain types and identifiers from
>> being generated but they are simply too many. I have trimmed some of the
>> fat but vast improvements can be made.
>> 
>> The staticlib artifact contains a bunch of mangled .o objects?
>> ==============================================================
>> [staticlibmangledobjects] Back to [TOC]
>> 
>> Yes, until we compile without the `std` module library or we compile it
>> manually instead of linking it, we will have some junk in it.
>> 
>
>Besides the size aspect, which potential advantage would there be to 
>switch to no_std?
>We don't build a bare metal or kernel binary here, so why introduce this 
>restriction willingly?

We'll see that as we progress. Might enable more platform support, for 
example. I have no definite answers here. Also, I know binary bloat is a 
big complaint from people with dislike of Rust, so I pre-emptively 
addressed it.