update toolchain to nightly-2024-07-30 #322

spookyvision · 2024-08-10T16:58:05Z

this PR changes the toolchain to use nightly-2024-07-30 and fixes platforms/x86_64 dependencies so it uses root's Cargo.toml patch section for maitake crates.

closes #323

obsoletes #306

TODO for reviewers (roughly ordered by assumed importance):

ensure target_has_atomic is the right fix ( a9e59b1 )
warning: transmute used without annotations --> platforms/allwinner-d1/d1-core/src/drivers/smhc.rs:521:58
interior mutable const lint
VAddr::of(&DOUBLE_FAULT_STACK) ( 1aecece ) - can't use clippy's suggested ptr::addr_of! with pinned mycelium (probably best to create dedicated issue)
warning: struct GrantWriter is never constructed: platforms/allwinner-d1/d1-core/src/drivers/uart.rs:27:8
warning: struct GrantWriter is never constructed: platforms/esp32c3-buddy/src/drivers/usb_serial.rs:16:8
warning: struct GrantWriter is never constructed: platforms/esp32c3-buddy/src/drivers/uart.rs:21:8
warning: trait TryFromBits is never used: platforms/allwinner-d1/d1-core/src/plic.rs:186:7
warning: struct SingleThreadedLinkedListAllocator is never constructed: source/alloc/src/heap.rs:288:12

…t hash (cargo bug?)

…ic commit hash (cargo bug?)" This reverts commit b80729c.

…d patches instead

This reverts commit cdddfdc.

…into update-toolchain

This reverts commit d4c3505.

see rust-lang/rust#128827

jspngh · 2024-08-11T16:35:10Z

Fyi, upgrading to a recent nightly will also break the Allwinner D1 build due to a new check at link-time:

relocation R_RISCV_JAL out of range: 1633230 is not in [-1048576, 1048575]; references 'abort'

The issue is in the start-up assembly of riscv-rt and is probably only noticeable with large binaries (like mnemos) and doesn't get hit with microcontrollers.
The most recent version of riscv-rt should be okay to use (the issue is still present, but only in case of multiple harts).

Upgrading riscv and riscv-rt might be a good idea, but might also be a lot of work,
since I think we also need to upgrade to embedded-hal(-async) v1.0.0.
An alternative is to use a patched version of riscv-rt v0.11.0

hawkw · 2024-08-11T17:18:17Z

I'm looking into changing the mnemos-x86_64-bootloader build script so that we no longer use Cargo artifact deps, but I'll probably try to do that in a separate PR that we can rebase this onto.

The `mnemos-x86_64-bootloader` image is built by a separate crate `mnemos-x86_64-bootloader`, which includes a build script that runs the post-build workflow to take the `bootloader_api`-compatible kernel binary and turn it into a bootable disk image that links with the actual bootloader. At present, we use a Cargo artifact dependency to build the actual kernel and expose it to the bootloader image build script. This is the ideal solution for this, since it allows the dependency between the build script and the kernel binary to be expressed as a normal Cargo dependency. However, using an artifact dep on a binary that's built for a different target than the crate with the artifact dep is sadly broken: see rust-lang/cargo#12358. We've somehow managed to find a way to avoid this issue in the past, but updating to a recent nightly seems to have brought it back (see #322). Therefore, this PR changes the build script to instead shell out to a new `cargo` invocation that builds the kernel. This is a bit unfortunate, since it means that the build output from building the actual kernel isn't exposed to the user as nicely, and there's probably less build caching. However, it works for now. I've modified the `just build-x86` Just recipe to invoke `cargo check` for the actual x86_64 kernel binary before building the bootable image, to help ensure that the cargo build output is made visible to the user. A nicer long-term solution would be to switch from a build script to using a cargo `xtask`-like builder, that's invoked as a cargo `runner` for that target, with the path to the builder binary passed in on the CLI. This is what [mycelium does][1]. That approach is nicer because it allows the kernel to be built using a normal cargo invocation that's actually visible to the user, instead of secretly in a build script. [1]: https://github.com/hawkw/mycelium/blob/e51eb8aa98e7609490fa674f408db32fd51caa70/.cargo/config.toml#L1-L2

This reverts commit 653d459.

This reverts commit 9074d0d.

spookyvision · 2024-08-13T15:39:28Z

Fyi, upgrading to a recent nightly will also break the Allwinner D1 build due to a new check at link-time

that's good to know, thank you! Though at least on my system I can build, flash and run as-is (Lichee RV). Any idea why it doesn't break?

hawkw

Overall, this looks good to me! I left a few comments, mostly about the lint fixes.

.github/workflows/ci.yml

Cargo.toml

hawkw · 2024-08-13T15:46:40Z

platforms/allwinner-d1/README.md

+After flashing, you should be able to see the hello server outputting `hello\n`
+repeatedly on the UART port (which is not port used for FEL!).
+Note that mnemOS changes the baud rate to 115200.
+


This seems good to mention, but I might also suggest using crowtty to see kernel traces (and it selects the correct baud rate by default).

hawkw · 2024-08-13T15:46:56Z

platforms/allwinner-d1/README.md

@@ -94,11 +94,20 @@ xfel write 0x40000000 platforms/allwinner-d1/boards/target/riscv64imac-unknown-n
 xfel exec 0x40000000
 ```

+After flashing, you should be able to see the hello server outputting `hello\n`
+repeatedly on the UART port (which is not port used for FEL!).


tiny nit:

Suggested change

repeatedly on the UART port (which is not port used for FEL!).

repeatedly on the UART port (which is not the port used for FEL!).

sorry for the mess, I had changed the text multiple times and maybe forgot to push my latest version…
currently it reads:

After flashing you can connect using just crowtty serial <UART-DEVICE>
(this is not the FEL device!). Alternatively any serial terminal will do - note
that we use 115200 baud unlike the builtin bootloader which is at 9600.

platforms/x86_64/core/src/bin/bootloader/main.rs

platforms/x86_64/core/src/interrupt.rs

source/abi/src/bbqueue_ipc/bbbuffer.rs

hawkw · 2024-08-13T15:57:54Z

source/abi/src/bbqueue_ipc/bbbuffer.rs

-#[cfg(feature = "thumbv6")]
+#[cfg(not(target_has_atomic))]


perhaps we should just be using the portable-atomic crate here --- cc @jamesmunns ?

but, we can do that in a separate PR. AFAICT none of this matters since we aren't currently compiling MnemOS for thumbv6m or any other target without atomics...

source/spitebuf/src/lib.rs

jspngh · 2024-08-13T19:56:40Z

Fyi, upgrading to a recent nightly will also break the Allwinner D1 build due to a new check at link-time

that's good to know, thank you! Though at least on my system I can build, flash and run as-is (Lichee RV). Any idea why it doesn't break?

Interesting 😅
I'm running cargo build -p mnemos-d1 --bin mq-pro (same for lichee-rv) on your update-toolchain branch using the latest nightly, and I do run into the issue I described.

If you haven't done a cargo clean recently, could you give that a try?

spookyvision · 2024-08-13T20:38:33Z

Interesting 😅 I'm running cargo build -p mnemos-d1 --bin mq-pro (same for lichee-rv) on your update-toolchain branch using the latest nightly, and I do run into the issue I described.
If you haven't done a cargo clean recently, could you give that a try?

ah, with your build command I do get the error you described. Good news though, this one works for me:
just build-d1 mq-pro

the difference seems to be --profile release; here's the fully expanded command:

❯ just build-d1 mq-pro
       Found cargo-objcopy
cargo build --profile release --package mnemos-d1 --bin mq-pro

(side note - are you using the latest nightly or the one specified by the project root's rust-toolchain.toml?)

Co-authored-by: Eliza Weisman <eliza@elizas.website>

…into update-toolchain

jspngh · 2024-08-14T06:19:51Z

ah, with your build command I do get the error you described. Good news though, this one works for me: just build-d1 mq-pro

the difference seems to be --profile release; here's the fully expanded command:
❯ just build-d1 mq-pro
       Found cargo-objcopy
cargo build --profile release --package mnemos-d1 --bin mq-pro

Just tried in release mode and indeed that works.
I wonder if it's due to the binary being smaller because of optimization or the abort symbol being placed at a different location. In any case, I think we can postpone upgrading riscv(-rt) to a different PR.

(side note - are you using the latest nightly or the one specified by the project root's rust-toolchain.toml?)

I tend to use the "default" nightly toolchain instead of a specific version from rust-toolchain.toml files (shame on me)
and that was the latest nightly at the time of writing.

Anatol Ulrich added 5 commits August 10, 2024 18:56

update nightly and change maitake patches to not use a specific commi…

b80729c

…t hash (cargo bug?)

Revert "update nightly and change maitake patches to not use a specif…

37d5abd

…ic commit hash (cargo bug?)" This reverts commit b80729c.

reintroduce maitake patch section, update platforms/x86_64 to use sai…

9a8fe7c

…d patches instead

cargo update

72ef0da

add clippy exception and TODO note

84b33de

spookyvision mentioned this pull request Aug 10, 2024

AFIT has been stabilized #323

Closed

Anatol Ulrich and others added 20 commits August 10, 2024 20:07

replace the ?UNUSED? thumbv6 with target_has_atomic

a9e59b1

update nix flake to pick up latest rust

5e91c3a

fix bindep-related build failure

b45865f

fix clippy lints - further action recommended (see TODOs)

80928e4

use ptr::addr_of

cdddfdc

add rust-src component to clippy action

874e8d0

add SPECIFIC rust-src component to clippy action

5eb245d

Revert "use ptr::addr_of"

5bc3123

This reverts commit cdddfdc.

add clippy exception for static_mut_refs

1aecece

remove component-src again, red herring

8dd6bc8

experiment: remove forced-target and see what breaks

d4c3505

Merge branch 'main' into update-toolchain

195d52c

update to stabilized panic_info_message

ce04211

Merge branch 'update-toolchain' of https://github.com/tosc-rs/mnemos …

fe3018d

…into update-toolchain

Revert "experiment: remove forced-target and see what breaks"

089e142

This reverts commit d4c3505.

justfile: fix _x86_pkg referring to bootloader instead of core

9799a49

add dep:spin, seems to help with bindeps b0rkage

85f6125

cargo update

57732bf

go back to a nightly where build-std works

0096f32

see rust-lang/rust#128827

more dead code

5acf3a7

hawkw mentioned this pull request Aug 11, 2024

chore(x86_64): change build script to invoke cargo #326

Merged

Anatol Ulrich added 5 commits August 13, 2024 04:18

Revert "try to debug CI borkage with more verbose env settings"

71993e9

This reverts commit 653d459.

*properly* set debug environment

9074d0d

Revert "*properly* set debug environment"

7131be2

This reverts commit 9074d0d.

document running melpo under macos with homebrew-installed sdl2

cc6e5ae

update nextest

290e0fa

spookyvision marked this pull request as ready for review August 13, 2024 02:59

spookyvision marked this pull request as draft August 13, 2024 03:02

Anatol Ulrich added 4 commits August 13, 2024 16:53

add Lichee RV FEL flash instructions

14a8b20

wording

80eb1f9

fix justfile path

c145098

additional info re: D1 running/verifying successful install

753e464

spookyvision mentioned this pull request Aug 13, 2024

update to embedded-hal 1.0 #327

Open

spookyvision marked this pull request as ready for review August 13, 2024 15:41

D1: change docs to mention crowtty

cdb25d4

hawkw reviewed Aug 13, 2024

View reviewed changes

spookyvision and others added 9 commits August 13, 2024 22:42

Update source/spitebuf/src/lib.rs

6d77149

Co-authored-by: Eliza Weisman <eliza@elizas.website>

Update .github/workflows/ci.yml

fc69b5f

Co-authored-by: Eliza Weisman <eliza@elizas.website>

revert toml autoformat

67e2091

Update platforms/allwinner-d1/d1-core/src/drivers/smhc.rs

e0df9cf

Co-authored-by: Eliza Weisman <eliza@elizas.website>

Update source/abi/src/bbqueue_ipc/bbbuffer.rs

1d1fb96

Co-authored-by: Eliza Weisman <eliza@elizas.website>

Update platforms/melpomene/README.md

a21981a

Co-authored-by: Eliza Weisman <eliza@elizas.website>

Update platforms/x86_64/core/src/interrupt.rs

e69c2bc

Co-authored-by: Eliza Weisman <eliza@elizas.website>

Merge branch 'update-toolchain' of https://github.com/tosc-rs/mnemos …

afdd704

…into update-toolchain

placate rustfmt

a7b8cc2

spookyvision merged commit 8a1ccea into main Aug 22, 2024
10 checks passed

spookyvision deleted the update-toolchain branch August 22, 2024 14:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update toolchain to nightly-2024-07-30 #322

update toolchain to nightly-2024-07-30 #322

spookyvision commented Aug 10, 2024 •

edited

Loading

jspngh commented Aug 11, 2024 •

edited

Loading

hawkw commented Aug 11, 2024

spookyvision commented Aug 13, 2024

hawkw left a comment

hawkw Aug 13, 2024

hawkw Aug 13, 2024

spookyvision Aug 13, 2024

hawkw Aug 13, 2024

jspngh commented Aug 13, 2024

spookyvision commented Aug 13, 2024

jspngh commented Aug 14, 2024

	repeatedly on the UART port (which is not port used for FEL!).
	repeatedly on the UART port (which is not the port used for FEL!).

		#[cfg(feature = "thumbv6")]
		#[cfg(not(target_has_atomic))]

update toolchain to nightly-2024-07-30 #322

update toolchain to nightly-2024-07-30 #322

Conversation

spookyvision commented Aug 10, 2024 • edited Loading

jspngh commented Aug 11, 2024 • edited Loading

hawkw commented Aug 11, 2024

spookyvision commented Aug 13, 2024

hawkw left a comment

Choose a reason for hiding this comment

hawkw Aug 13, 2024

Choose a reason for hiding this comment

hawkw Aug 13, 2024

Choose a reason for hiding this comment

spookyvision Aug 13, 2024

Choose a reason for hiding this comment

hawkw Aug 13, 2024

Choose a reason for hiding this comment

jspngh commented Aug 13, 2024

spookyvision commented Aug 13, 2024

jspngh commented Aug 14, 2024

spookyvision commented Aug 10, 2024 •

edited

Loading

jspngh commented Aug 11, 2024 •

edited

Loading