mbox series

[RFC,v6,0/5] rust-pl011-rfc-v6

Message ID rust-pl011-rfc-v6.git.manos.pitsidianakis@linaro.org (mailing list archive)
Headers show
Series rust-pl011-rfc-v6 | expand

Message

Manos Pitsidianakis Aug. 4, 2024, 9:04 p.m. UTC
Changes
=======

- Setting MSRV to 1.77.0:
  * cstr crate MSRV is 1.64, which is more recent than Debian bookworm
    (1.63.0) <https://github.com/upsuper/cstr/blob/master/Cargo.toml>

  * pl011's dependencies (mostly proc-macro2) don't support 1.63.0

- Dropped CI/lcitool patches.

- Dropped vendored dependencies in favor of meson subprojects.

- Added a qom ObjectImpl trait and declaration macros

- Added # SAFETY comments.

- Changed configure flag to --{enable,disable}-rust

Manos Pitsidianakis (5):
  build-sys: Add rust feature option
  rust: add bindgen step as a meson dependency
  .gitattributes: add Rust diff and merge attributes
  rust: add crate to expose bindings and interfaces
  rust: add PL011 device model

 MAINTAINERS                                   |  20 +
 configure                                     |   2 +
 meson.build                                   |  77 ++-
 rust/wrapper.h                                |  39 ++
 .gitattributes                                |   3 +
 Kconfig                                       |   1 +
 Kconfig.host                                  |   3 +
 hw/arm/Kconfig                                |  33 +-
 meson_options.txt                             |   3 +
 rust/.gitignore                               |   3 +
 rust/Kconfig                                  |   1 +
 rust/hw/Kconfig                               |   2 +
 rust/hw/char/Kconfig                          |   3 +
 rust/hw/char/meson.build                      |   1 +
 rust/hw/char/pl011/.gitignore                 |   2 +
 rust/hw/char/pl011/Cargo.lock                 | 125 ++++
 rust/hw/char/pl011/Cargo.toml                 |  26 +
 rust/hw/char/pl011/README.md                  |  31 +
 rust/hw/char/pl011/meson.build                |  28 +
 rust/hw/char/pl011/rustfmt.toml               |   1 +
 rust/hw/char/pl011/src/definitions.rs         |  26 +
 rust/hw/char/pl011/src/device.rs              | 586 ++++++++++++++++++
 rust/hw/char/pl011/src/device_class.rs        |  58 ++
 rust/hw/char/pl011/src/lib.rs                 | 584 +++++++++++++++++
 rust/hw/char/pl011/src/memory_ops.rs          |  56 ++
 rust/hw/meson.build                           |   1 +
 rust/meson.build                              |  15 +
 rust/qemu-api/.gitignore                      |   2 +
 rust/qemu-api/Cargo.lock                      |   7 +
 rust/qemu-api/Cargo.toml                      |  23 +
 rust/qemu-api/README.md                       |  17 +
 rust/qemu-api/build.rs                        |  13 +
 rust/qemu-api/meson.build                     |  19 +
 rust/qemu-api/rustfmt.toml                    |   1 +
 rust/qemu-api/src/bindings.rs                 |   7 +
 rust/qemu-api/src/definitions.rs              | 108 ++++
 rust/qemu-api/src/device_class.rs             | 128 ++++
 rust/qemu-api/src/lib.rs                      | 100 +++
 rust/qemu-api/src/tests.rs                    |  48 ++
 rust/rustfmt.toml                             |   7 +
 scripts/meson-buildoptions.sh                 |   3 +
 scripts/rustc_args.py                         |  84 +++
 subprojects/.gitignore                        |  11 +
 subprojects/arbitrary-int.wrap                |   9 +
 subprojects/bilge-impl.wrap                   |  10 +
 subprojects/bilge.wrap                        |  10 +
 subprojects/either.wrap                       |  10 +
 subprojects/itertools.wrap                    |  10 +
 .../packagefiles/arbitrary-int/meson.build    |  19 +
 .../packagefiles/bilge-impl/meson.build       |  36 ++
 subprojects/packagefiles/bilge/meson.build    |  25 +
 subprojects/packagefiles/either/meson.build   |  21 +
 .../packagefiles/itertools/meson.build        |  25 +
 .../proc-macro-error-attr/meson.build         |  27 +
 .../packagefiles/proc-macro-error/meson.build |  32 +
 .../packagefiles/proc-macro2/meson.build      |  26 +
 subprojects/packagefiles/quote/meson.build    |  25 +
 subprojects/packagefiles/syn/meson.build      |  33 +
 .../packagefiles/unicode-ident/meson.build    |  19 +
 subprojects/proc-macro-error-attr.wrap        |  10 +
 subprojects/proc-macro-error.wrap             |  11 +
 subprojects/proc-macro2.wrap                  |  10 +
 subprojects/quote.wrap                        |  10 +
 subprojects/syn.wrap                          |  11 +
 subprojects/unicode-ident.wrap                |  10 +
 65 files changed, 2695 insertions(+), 12 deletions(-)
 create mode 100644 rust/wrapper.h
 create mode 100644 rust/.gitignore
 create mode 100644 rust/Kconfig
 create mode 100644 rust/hw/Kconfig
 create mode 100644 rust/hw/char/Kconfig
 create mode 100644 rust/hw/char/meson.build
 create mode 100644 rust/hw/char/pl011/.gitignore
 create mode 100644 rust/hw/char/pl011/Cargo.lock
 create mode 100644 rust/hw/char/pl011/Cargo.toml
 create mode 100644 rust/hw/char/pl011/README.md
 create mode 100644 rust/hw/char/pl011/meson.build
 create mode 120000 rust/hw/char/pl011/rustfmt.toml
 create mode 100644 rust/hw/char/pl011/src/definitions.rs
 create mode 100644 rust/hw/char/pl011/src/device.rs
 create mode 100644 rust/hw/char/pl011/src/device_class.rs
 create mode 100644 rust/hw/char/pl011/src/lib.rs
 create mode 100644 rust/hw/char/pl011/src/memory_ops.rs
 create mode 100644 rust/hw/meson.build
 create mode 100644 rust/meson.build
 create mode 100644 rust/qemu-api/.gitignore
 create mode 100644 rust/qemu-api/Cargo.lock
 create mode 100644 rust/qemu-api/Cargo.toml
 create mode 100644 rust/qemu-api/README.md
 create mode 100644 rust/qemu-api/build.rs
 create mode 100644 rust/qemu-api/meson.build
 create mode 120000 rust/qemu-api/rustfmt.toml
 create mode 100644 rust/qemu-api/src/bindings.rs
 create mode 100644 rust/qemu-api/src/definitions.rs
 create mode 100644 rust/qemu-api/src/device_class.rs
 create mode 100644 rust/qemu-api/src/lib.rs
 create mode 100644 rust/qemu-api/src/tests.rs
 create mode 100644 rust/rustfmt.toml
 create mode 100644 scripts/rustc_args.py
 create mode 100644 subprojects/arbitrary-int.wrap
 create mode 100644 subprojects/bilge-impl.wrap
 create mode 100644 subprojects/bilge.wrap
 create mode 100644 subprojects/either.wrap
 create mode 100644 subprojects/itertools.wrap
 create mode 100644 subprojects/packagefiles/arbitrary-int/meson.build
 create mode 100644 subprojects/packagefiles/bilge-impl/meson.build
 create mode 100644 subprojects/packagefiles/bilge/meson.build
 create mode 100644 subprojects/packagefiles/either/meson.build
 create mode 100644 subprojects/packagefiles/itertools/meson.build
 create mode 100644 subprojects/packagefiles/proc-macro-error-attr/meson.build
 create mode 100644 subprojects/packagefiles/proc-macro-error/meson.build
 create mode 100644 subprojects/packagefiles/proc-macro2/meson.build
 create mode 100644 subprojects/packagefiles/quote/meson.build
 create mode 100644 subprojects/packagefiles/syn/meson.build
 create mode 100644 subprojects/packagefiles/unicode-ident/meson.build
 create mode 100644 subprojects/proc-macro-error-attr.wrap
 create mode 100644 subprojects/proc-macro-error.wrap
 create mode 100644 subprojects/proc-macro2.wrap
 create mode 100644 subprojects/quote.wrap
 create mode 100644 subprojects/syn.wrap
 create mode 100644 subprojects/unicode-ident.wrap


base-commit: f9851d2ffef59b3a7f39513469263ab3b019480f

Comments

Paolo Bonzini Aug. 8, 2024, 6:10 a.m. UTC | #1
On 8/4/24 23:04, Manos Pitsidianakis wrote:
> Changes
> =======
> 
> - Setting MSRV to 1.77.0:
>    * cstr crate MSRV is 1.64, which is more recent than Debian bookworm
>      (1.63.0) <https://github.com/upsuper/cstr/blob/master/Cargo.toml>
> 
>    * pl011's dependencies (mostly proc-macro2) don't support 1.63.0

proc-macro2 is listed as supporting 1.56.0, and in general I don't see 
particularly high MSRVs for any of your dependencies.

cstr needs to use version 0.2.10 in order to work with Rust 1.63.0.

As discussed on IRC, there are obvious advantages and disadvantages to 
using meson.  The main disadvantage is the extra work when bumping the 
version of the dependencies or when adding a new one.  The advantage is 
more uniformity and less moving parts.  Overall, I think it's doable to 
use it.  Dependencies will mostly be added in the early days of QEMU, 
and won't be updated too often due to our MSRV constraints.

The automatic Cargo.toml support in Meson is promising, but it doesn't 
work right now when cross compiling build-time dependencies (which have 
to use "native: true" or Meson rightly warns about mixing build-machine 
and host-machine binaries).  So right now we'd have to write meson.build 
by hand for those.

My suggestion is however to name our manually-managed subprojects with 
the same convention that is used by "method = cargo" in Meson 1.5.0+, 
i.e. name-APIVER-rs:

	arbitrary-int-1-rs.wrap
	bilge-0.2-rs.wrap
	bilge-impl-0.2-rs.wrap
	either-1-rs.wrap
	itertools-0.11-rs.wrap
	proc-macro2-1-rs.wrap
	proc-macro-error-1-rs.wrap
	proc-macro-error-attr-1-rs.wrap
	quote-1-rs.wrap
	syn-2-rs.wrap
	unicode-ident-1-rs.wrap

and to access dependencies using meson.override_dependency() and 
dependency(), instead of get_variable().  This at least reduces future 
churn.

As to the individual patches:

- for patch 1, roughly the same changes I had made for cargo can be done 
for rustc, so that the cross file contains the right --target option. 
I'll reply to the individual patch.

- for patch 2, the only issue is that you are specifying 
--no-include-path-detection and that breaks for me on Fedora.  I have 
not finished testing but it seems that it's enough to remove that line.

- for patches 4 and 5, I have minimal comments on the meson.build.  For 
patch 5, however, I have already done the above renaming as part of 
getting cross compilation to work.  We can synchronize on IRC on the 
best way of getting the changes to you.

Paolo
Manos Pitsidianakis Aug. 8, 2024, 7:49 a.m. UTC | #2
On Thu, 08 Aug 2024 09:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
>On 8/4/24 23:04, Manos Pitsidianakis wrote:
>> Changes
>> =======
>> 
>> - Setting MSRV to 1.77.0:
>>    * cstr crate MSRV is 1.64, which is more recent than Debian bookworm
>>      (1.63.0) <https://github.com/upsuper/cstr/blob/master/Cargo.toml>
>> 
>>    * pl011's dependencies (mostly proc-macro2) don't support 1.63.0
>
>proc-macro2 is listed as supporting 1.56.0, and in general I don't see 
>particularly high MSRVs for any of your dependencies.

The issue was with transitive deps, proc-macro-error crates etc stopped 
compiling when lowering the version, which means we'd have to patch the 
dependency's dependency to see if that'd work; otherwise, yes!

>
>cstr needs to use version 0.2.10 in order to work with Rust 1.63.0.
>
>As discussed on IRC, there are obvious advantages and disadvantages to 
>using meson.  The main disadvantage is the extra work when bumping the 
>version of the dependencies or when adding a new one.  The advantage is 
>more uniformity and less moving parts.  Overall, I think it's doable to 
>use it.  Dependencies will mostly be added in the early days of QEMU, 
>and won't be updated too often due to our MSRV constraints.
>
>The automatic Cargo.toml support in Meson is promising, but it doesn't 
>work right now when cross compiling build-time dependencies (which have 
>to use "native: true" or Meson rightly warns about mixing build-machine 
>and host-machine binaries).  So right now we'd have to write meson.build 
>by hand for those.
>
>My suggestion is however to name our manually-managed subprojects with 
>the same convention that is used by "method = cargo" in Meson 1.5.0+, 
>i.e. name-APIVER-rs:
>
>	arbitrary-int-1-rs.wrap
>	bilge-0.2-rs.wrap
>	bilge-impl-0.2-rs.wrap
>	either-1-rs.wrap
>	itertools-0.11-rs.wrap
>	proc-macro2-1-rs.wrap
>	proc-macro-error-1-rs.wrap
>	proc-macro-error-attr-1-rs.wrap
>	quote-1-rs.wrap
>	syn-2-rs.wrap
>	unicode-ident-1-rs.wrap
>
>and to access dependencies using meson.override_dependency() and 
>dependency(), instead of get_variable().  This at least reduces future 
>churn.

Yes that makes sense!

>
>As to the individual patches:
>
>- for patch 1, roughly the same changes I had made for cargo can be done 
>for rustc, so that the cross file contains the right --target option. 
>I'll reply to the individual patch.

That'd be great since I'm not familiar with how the cross file works.

>
>- for patch 2, the only issue is that you are specifying 
>--no-include-path-detection and that breaks for me on Fedora.  I have 
>not finished testing but it seems that it's enough to remove that line.

I had added that when trying to debug bindgen failing to find headers 
when dependencies were added (e.g. linux io_uring) or when compiling on 
macos, let's test again to see if it's indeed unnecessary!

>
>- for patches 4 and 5, I have minimal comments on the meson.build.  For 
>patch 5, however, I have already done the above renaming as part of 
>getting cross compilation to work.  We can synchronize on IRC on the 
>best way of getting the changes to you.
>

Sounds good to me.

Thanks,
Manos
Paolo Bonzini Aug. 8, 2024, 10:09 a.m. UTC | #3
On Thu, Aug 8, 2024 at 9:58 AM Manos Pitsidianakis
<manos.pitsidianakis@linaro.org> wrote:
>
> On Thu, 08 Aug 2024 09:10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >On 8/4/24 23:04, Manos Pitsidianakis wrote:
> >> Changes
> >> =======
> >>
> >> - Setting MSRV to 1.77.0:
> >>    * cstr crate MSRV is 1.64, which is more recent than Debian bookworm
> >>      (1.63.0) <https://github.com/upsuper/cstr/blob/master/Cargo.toml>
> >>
> >>    * pl011's dependencies (mostly proc-macro2) don't support 1.63.0
> >
> >proc-macro2 is listed as supporting 1.56.0, and in general I don't see
> >particularly high MSRVs for any of your dependencies.
>
> The issue was with transitive deps, proc-macro-error crates etc stopped
> compiling when lowering the version, which means we'd have to patch the
> dependency's dependency to see if that'd work; otherwise, yes!

Ah, I see now - you have to set the right cfg for proc-macro2 to
compile with 1.63.0. Normally (including with "method = cargo") they
are detected by build.rs:

    if rustc < 66 {
        println!("cargo:rustc-cfg=no_source_text");
    }

    if rustc < 79 {
        println!("cargo:rustc-cfg=no_literal_byte_character");
        println!("cargo:rustc-cfg=no_literal_c_string");
    }

(Meson's support for build.rs is very limited, but it does handle some
simple cases and parses rustc-cfg from the output).

And bilge-impl uses let...else; we can patch it locally via the .wrap
file's "diff_files" entry. Not great, but it's good that we can do it
and that we have an example for similar issues in the future.

I updated my rust-for-manos branch with these discoveries. Of course
it doesn't compile with 1.63.0 but it does at least configure
successfully with

../configure --rustc=$(rustup +1.63.0 which rustc) --enable-rust

and build the subprojects' rlibs.

> >The automatic Cargo.toml support in Meson is promising [...]
> >My suggestion is to name our manually-managed subprojects with
> >the same convention that is used by "method = cargo" in Meson 1.5.0+,
> >i.e. name-APIVER-rs:
>
> Yes that makes sense!

Good!

> >- for patch 2, the only issue is that you are specifying
> >--no-include-path-detection and that breaks for me on Fedora.  I have
> >not finished testing but it seems that it's enough to remove that line.
>
> I had added that when trying to debug bindgen failing to find headers
> when dependencies were added (e.g. linux io_uring) or when compiling on
> macos, let's test again to see if it's indeed unnecessary!

Ok, crossing fingers.

Paolo