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

Add support for Rust backed camkes components #17

Closed
wants to merge 12 commits into from

Conversation

podhrmic
Copy link

This commits allows camkes tool to understand Rust backed components.

Originally written by @aisamanra

@kent-mcleod
Copy link
Member

Thanks for the PRs!
I'm assuming that you are after initial feedback on what would be required to merge so I'll quickly list the main points here:

  • I would make the Firewall rust component a separate component with a separate name to prevent overwriting the C one.
  • I would move the JSON custom target files to the Rust firewall component and add parameters to the camkes makefile to allow a component to declare its target location and target file. (I can expand on this point if you want)
  • The consequence of this is that the seL4_tools pull request would be no longer needed. I think that having custom target definitions being stored and specified by a camkes component allows more fine-grain contol over target settings instead of having global targets that would have to work in every environment.
  • The custom targets should probably be updated to reflect that they aren't in a robigalia environment. I'd also only provide targets for VM configurations that can be built and run.
  • Additionally, this newer rust support requires having customised tools already installed. Are you able to add documentation about what is required to get compatible rustc and cargo binaries?
  • The camkes-vm project is in the progress of being converted to CMake. This means that if you wish for rust to still be supported in CMake builds, equivalent changes to the CAmkES CMake files will be required in the future.

The old rust rules that are hanging around in seL4_tools should probably be deleted. If you are concerned that the environment variables declared in Makefile.flags I'm happy to handle removing them myself.

@podhrmic
Copy link
Author

podhrmic commented May 4, 2018

Thanks for the feedback! Some comments below:

I would make the Firewall rust component a separate component with a separate name to prevent overwriting the C one.

Agreed.

I would move the JSON custom target files to the Rust firewall component and add parameters to the camkes makefile to allow a component to declare its target location and target file. (I can expand on this point if you want)

My concern is that the sel4 rust target should be available to multiple components - so it should probably be stored somewhere else. Not sure if it makes sense to let components choose their own target - what would be the benefit?

The custom targets should probably be updated to reflect that they aren't in a robigalia environment. I'd also only provide targets for VM configurations that can be built and run.

Agreed.

Additionally, this newer rust support requires having customised tools already installed. Are you able to add documentation about what is required to get compatible rustc and cargo binaries?

yes that is work in progress.

The camkes-vm project is in the progress of being converted to CMake. This means that if you wish for rust to still be supported in CMake builds, equivalent changes to the CAmkES CMake files will be required in the future.

Do you have examples of what needs to be changed for CMake builds?

@podhrmic
Copy link
Author

This PR is simple - just allows camkes to understand rust components

@kent-mcleod
Copy link
Member

The concern was that any changes to the Makefile template will immediately become deprecated with the migration to cmake.
I added an example rust camkes component in cmake that works the same way as in make---you build the rust part as a library and link it into the camkes component. This doesn't require any rust specific modifications to camkes-tool as you can just declare a cmake library for the rust part and the camkes component can use it the same as a C library. See the two commits for the cmake support :
seL4/camkes@3a1a4cd
seL4/seL4_tools@af348ea
Building a camkes app in cmake is:
mkdir build && cd build && ../init-build.sh -DCAMKES_APP=hellorust && ninja && ./simulate
As for the mutex library part of this pr, I have an internal pr that has been going through review, so I'll prioritise getting it merged.

@podhrmic
Copy link
Author

Oh that is great! CMake and ninja make the build process so much nicer - good work!

I updated the hellorust and cmake files to use the custom rust compiler with build-in seL4 support (rather than Xargo), so you can use some 'std' features. The PRs are here:

seL4/camkes#6
seL4/seL4_tools#16
seL4/camkes-manifest#4

You just have to download the rust compiler by calling setup_rust_env.sh before the build.

I am not sure if it makes sense to keep Xargo around, since it is not being actively developed anymore. You can do no-std builds our method as well. I am happy to add support for i686 and arm targets as well, so that should cover all use cases.

@kent-mcleod
Copy link
Member

Thanks!

Regarding Xargo, my understanding is that it is in maintenance mode while they try and merge its functionality upstream into cargo (japaric/xargo#193 (comment)).

The only thing I am hesitant about these PRs is having to depend on a custom rust compiler as the default rather than something to opt into. I had been meaning to look into getting the Camkes issues that require changes to libstd fixed but I haven't had time. In the interest of not leaving these ignored for another few months, I'll look into getting these merged early next week.

@podhrmic
Copy link
Author

Kent, I experimented with Xargo more, and it looks like we can get the same functionality (i.e. the std/alloc support) as with our custom compiler. In which case it IMHO makes more sense to stick with Xargo for both std/no-std builds.

I will update this PR with a sample app that uses std/alloc features so it will be clearer.

What I had to do to get it working was the slightly modify the target json file: seL4/seL4_tools@dbd2af0

I am pretending that we are compiling for Redox OS, so we get the right OS bindings for free. Imagine that in this commit all sel4 references are replaced with redox: GaloisInc/sel4-rust@0f138c3

@kent-mcleod
Copy link
Member

I believe using xargo and providing different target files is enough policy freedom for anyone wanting to build rust components in camkes.
I'd also looked into using an x86_64-unknown-linux-musl target. liblibc requires that musllibc is statically linked when compiling the rlib. For the redox target, how does the C library linking work?
I also pushed out the camkes/sync.h interface. I still plan on adding libcamkesrust.

@podhrmic
Copy link
Author

The only lost feature (with Xargo) is the panic info - with Xargo you get only the output from the fault handler, while with our custom compiler you get at least some useful panic info. See GaloisInc/sel4-rust@6fb404c

This was related to missing thread-local-storage I believe, but I ll try to feature gate our changes and make a PR to the rust compiler.

Regarding hellorust/hellorust_std, I believe you can merge these two PRs:
seL4/seL4_tools#16
seL4/camkes#6

@podhrmic
Copy link
Author

I am not sure how exactly is Redox OS build, but using this little kludge seems to work fine with seL4.

@podhrmic
Copy link
Author

Not sure if the linux-musl target would work, it seems to be missing bunch of pre-linkargs` we use: https://github.com/rust-lang/rust/blob/master/src/librustc_target/spec/x86_64_unknown_linux_musl.rs

@podhrmic
Copy link
Author

Edit(you probably know that): with the x86_64-unknown-linux-musl target I can indeed compile the no-std example, but the std/alloc example is running into problems when compiling alloc:

error: could not find native static library `c`,  perhaps an -L flag is missing?

And the relevant solution is discussed here: japaric/xargo#133
But overall I am not sure if this brings any benefit over using our custom targets.

@kent-mcleod
Copy link
Member

Regarding x86_64-unknown-linux-musl, I had to use .cargo/config to specify the -L flag so that it could statically link our C library.

Not sure if the linux-musl target would work, it seems to be missing bunch of pre-linkargs` we use

To clarify, I meant a customtarget.json with os: linux and env:musl. (Interestingly, I found that I had to put these in the name of the file otherwise os and env weren't being set correctly. I haven't found out why this is)

This was related to missing thread-local-storage I believe, but I ll try to feature gate our changes and make a PR to the rust compiler.

TLS support is on our roadmap and someone is actively working on it at the moment. We also want to use unwind to get the backtrace support working. How come you prefer to use std instead of no_std and explicitly using core and alloc? (And use panic_fmt to provide a custom panic impl in no_std)

I am not sure how exactly is Redox OS build, but using this little kludge seems to work fine with seL4.

Well, we are just swapping out one unimplemented OS backend for another right? So if this target is easiest to get everything to link, it works for me.

Regarding hellorust/hellorust_std, I believe you can merge these two PRs:

I'll comment on them directly. But I agree that they are ready without requiring significant changes.

@podhrmic
Copy link
Author

podhrmic commented Aug 1, 2018

How come you prefer to use std instead of no_std and explicitly using core and alloc? (And use panic_fmt to provide a custom panic impl in no_std)

That is a good point - I think for the firewall/rustwall it was handy to have Arc for synchronization (and format! macro). I should investigate if we can get the same functionality with no-std + core + allloc...

@kent-mcleod
Copy link
Member

Arc and format are both provided by alloc and reexported by std. When I was debugging the rustwall a few months ago it was possible to run it as no_std using some of the following:

#![no_std]
#![feature(alloc)]
#![feature(libc)]
#![feature(lang_items)]
#![feature(global_allocator,allocator_api)]
#[macro_use]
extern crate alloc;
extern crate libc;
use libc::c_void;
use libc::memcpy;

extern crate camkesrust;
#[macro_use]
extern crate lazy_static;

// This is an implementation of a system allocator
use camkesrust::System;

#[global_allocator]
static A: System = System;

use core::cell::UnsafeCell;
use alloc::arc::Arc; // OK!
use alloc::Vec;
use alloc::slice;
use alloc::String;
use core::mem::transmute;

@kent-mcleod
Copy link
Member

(Closing due to stale after discussion with @podhrmic on the dev call)

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.

2 participants