Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use stable Rust toolchain #81

Merged
merged 5 commits into from
Aug 31, 2023

Conversation

00xc
Copy link
Member

@00xc 00xc commented Aug 29, 2023

Switch to using the stable Rust toolchain instead of nightly. To do this, we must not need to rebuild parts of the core and standard library, which is an unstable feature. In order to do so, switch to a supported build target (x86_64-unknown-none) instead of our own (svsm-target.json).

Fixes #15

@00xc 00xc marked this pull request as draft August 29, 2023 13:25
@00xc
Copy link
Member Author

00xc commented Aug 29, 2023

Keeping this as a draft until I figure out how to fix tests, as they need to build the standard library.

@stefano-garzarella
Copy link
Contributor

In order to successfully build it on my Fedora 38, I needed to run rustup target add x86_64-unknown-none, should we add an hint about it in the INSTALL.md?

Related to INSTALL.md I'd also suggest updating the gdb example:

diff --git a/INSTALL.md b/INSTALL.md
index b79cdbf..8301fca 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -341,7 +341,7 @@ If you have the source code available on the host system then you can add the
 debug symbols and use source-level debugging:
 

-(gdb) symbol-file target/svsm-target/debug/svsm
+(gdb) symbol-file target/x86_64-unknown-none/debug/svsm

I'd also suggest updating script/docker/build.sh in something like this:

diff --git a/scripts/docker/opensuse-rust.docker b/scripts/docker/opensuse-rust.docker
index e46cbbb..13fd729 100644
--- a/scripts/docker/opensuse-rust.docker
+++ b/scripts/docker/opensuse-rust.docker
@@ -19,4 +19,4 @@ RUN zypper ref && \
 USER $USER_NAME
 
 RUN curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs > /tmp/rustup-init.sh && \
-    sh /tmp/rustup-init.sh --default-toolchain nightly-x86_64-unknown-linux-gnu --component rust-src -y
+    sh /tmp/rustup-init.sh --default-toolchain stable-x86_64-unknown-linux-gnu --target x86_64-unknown-none -y
(END)

Anyway, cool PR ;-)

@00xc
Copy link
Member Author

00xc commented Aug 29, 2023

In order to successfully build it on my Fedora 38, I needed to run rustup target add x86_64-unknown-none, should we add an hint about it in the INSTALL.md?

Related to INSTALL.md I'd also suggest updating the gdb example

Thanks! I'll add these changes to v2

@stefano-garzarella
Copy link
Contributor

Keeping this as a draft until I figure out how to fix tests, as they need to build the standard library.

What about use nightly only for tests?
I mean something like: cargo +nightly test ...

@00xc
Copy link
Member Author

00xc commented Aug 30, 2023

Keeping this as a draft until I figure out how to fix tests, as they need to build the standard library.

What about use nightly only for tests? I mean something like: cargo +nightly test ...

That would be my last resource. However, I managed to run tests by doing:

RUSTFLAGS="-C code-model=kernel" cargo test --target=x86_64-unknown-linux-gnu

So rebuilding the standard library is not needed at all. And in fact, I've just realized that code-model=kernel also works for the regular build, and it's the default for the x86_64-unknown-none target.

@stefano-garzarella
Copy link
Contributor

Keeping this as a draft until I figure out how to fix tests, as they need to build the standard library.

What about use nightly only for tests? I mean something like: cargo +nightly test ...

That would be my last resource. However, I managed to run tests by doing:

RUSTFLAGS="-C code-model=kernel" cargo test --target=x86_64-unknown-linux-gnu

So rebuilding the standard library is not needed at all. And in fact, I've just realized that code-model=kernel also works for the regular build, and it's the default for the x86_64-unknown-none target.

Cool! So what about adding it in .cargo/config.toml:

diff --git a/.cargo/config.toml b/.cargo/config.toml
index 4e38721..1a006b6 100644
--- a/.cargo/config.toml
+++ b/.cargo/config.toml
@@ -6,3 +6,7 @@ rustflags = [
        "-C", "code-model=large"
 ]
 
+[target.x86_64-unknown-linux-gnu]
+rustflags = [
+       "-C", "code-model=kernel",
+]

Then cargo test --target=x86_64-unknown-linux-gnu should work.

@00xc
Copy link
Member Author

00xc commented Aug 30, 2023

Cool! So what about adding it in .cargo/config.toml:

diff --git a/.cargo/config.toml b/.cargo/config.toml
index 4e38721..1a006b6 100644
--- a/.cargo/config.toml
+++ b/.cargo/config.toml
@@ -6,3 +6,7 @@ rustflags = [
        "-C", "code-model=large"
 ]
 
+[target.x86_64-unknown-linux-gnu]
+rustflags = [
+       "-C", "code-model=kernel",
+]

Then cargo test --target=x86_64-unknown-linux-gnu should work.

Yep, I was trying this just now. I will add it to v2 as well.

Switch to using the stable Rust toolchain instead of nightly. To do
this, we must not need to rebuild parts of the core and standard
library, which is an unstable feature. In order to do so, switch to a
supported build target (x86_64-unknown-none) instead of our own
(svsm-target.json).

Signed-off-by: Carlos López <[email protected]>
Use stable toolchain and the correct target.

Signed-off-by: Carlos López <[email protected]>
Add appropriate build flags so that tests can be run as usual. These
stopped working because rebuilding the standard library
(via -Z build-std) is an unstable feature. However, this is not
needed since the x86_64-unknown-linux-gnu target already provides a
built standard library.

Signed-off-by: Carlos López <[email protected]>
@00xc 00xc marked this pull request as ready for review August 30, 2023 10:07
@stefano-garzarella
Copy link
Contributor

LGTM!

I'd just suggest the following changes in INSTALL.md:

diff --git a/INSTALL.md b/INSTALL.md
index 8301fca..af72adf 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -171,8 +171,9 @@ Building the COCONUT-SVSM
 -------------------------
 
 Building the SVSM itself requires:
-- a recent Rust-nightly compiler and build environment installed. Please refer to
+- a recent Rust compiler and build environment installed. Please refer to
   [https://rustup.rs/](https://rustup.rs/) on how to get this environment installed.
+- `x86_64-unknown-none` target toolchain installed (`rustup target add x86_64-unknown-none`)
 - `binutils` >= 2.39

@00xc
Copy link
Member Author

00xc commented Aug 30, 2023

LGTM!

I'd just suggest the following changes in INSTALL.md:

diff --git a/INSTALL.md b/INSTALL.md
index 8301fca..af72adf 100644
--- a/INSTALL.md
+++ b/INSTALL.md
@@ -171,8 +171,9 @@ Building the COCONUT-SVSM
 -------------------------
 
 Building the SVSM itself requires:
-- a recent Rust-nightly compiler and build environment installed. Please refer to
+- a recent Rust compiler and build environment installed. Please refer to
   [https://rustup.rs/](https://rustup.rs/) on how to get this environment installed.
+- `x86_64-unknown-none` target toolchain installed (`rustup target add x86_64-unknown-none`)
 - `binutils` >= 2.39

Done, I merged this with the other change we had in INSTALL.md.

@joergroedel joergroedel merged commit 987dc24 into coconut-svsm:main Aug 31, 2023
2 checks passed
@osteffenrh
Copy link
Contributor

That's awesome! Thanks!

@berrange
Copy link

In order to successfully build it on my Fedora 38, I needed to run rustup target add x86_64-unknown-none, should we add an hint about it in the INSTALL.md?

FYI, I've got a proposal open for Fedora to add a sub-RPM for 'rust-std-static-x86_64-unknown-none' which enabled me to build coconut SVSM, with this PR merged, using the standard Rust toolchain in Fedora:

https://src.fedoraproject.org/rpms/rust/pull-request/23

liuyangxy pushed a commit to fedora-riscv/rust that referenced this pull request Nov 15, 2023
The coconut-svsm project which provides a low level firmware for AMD
SEV-SNP virtual machines uses 'x86_64-unknown-none' as its build
target and has recently removed the requirement to use rust nightly[1].

Thus adding 'x86_64-unknown-none' as a build target will enable us to
build coconut-svsm in Fedora using the standard Rust toolchain packages.

[1] coconut-svsm/svsm#81
Signed-off-by: Daniel P. Berrangé <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert to use stable Rust
5 participants