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
WIP: initial Wasmtime support for IceCap #311
Conversation
Currently mapping small mmaps into malloc, and leveraging Nick's large page tree hack, but it does work, and should work well enough for initial benchmarking.
This is currently necessary in order to test Wasmtime on platforms outside of the freestanding-execution-engine. Perhaps we should make this controlled by a variable so we can have both Wasmtime and Wasmi in CI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! A small number of review comments (I realise this is a WIP)...
[dependencies] | ||
icecap-std = { path = "../../../icecap/src/rust/crates/icecap/icecap-std" } | ||
icecap-start-generic = { path = "../../../icecap/src/rust/crates/icecap/icecap-start/generic" } | ||
serde = { version = "*", default-features = false, features = [ "alloc", "derive"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we constrain the version of serde
somewhat?
edition = "2018" | ||
|
||
[dependencies] | ||
serde = { version = "*", default-features = false, features = [ "alloc", "derive"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment, regarding constraining the version of serde
.
@@ -23,4 +23,34 @@ | |||
url = https://github.com/nanopb/nanopb | |||
[submodule "icecap/icecap"] | |||
path = icecap/icecap | |||
url = https://gitlab.com/arm-research/security/icecap/icecap.git | |||
url = https://gitlab.com/geky/icecap.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep this under an institutional/project workspace on Gitlab or Github, rather than a personal account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, can do, currently this is in a PR over here: https://gitlab.com/arm-research/security/icecap/icecap/-/merge_requests/8, but needs some tweaks before it can be merged.
I have the submodule pointing at my fork right now just so that this PR can be used as is.
This is ~1/2 the address space of Wasm. It would be nice to provide 4 GiB, allowing Veracruz+IceCap to run any Wasm program, but this doesn't fit on our current test platform, which has 4 GiB in total.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this PR be rebased?
Since we're patching some pretty low-level crates (libc), any changes to dependencies that change the patch-version of dependencies on these low-level crates end up creating compilation errors. For example, a change from `clang-sys v0.5.3 => clang-sys v0.5.4` may innocently bump the patch version of its dependency on `libc v0.2.86 => libc v0.2.112`, unfortunately for us, we have only patched `libc v0.2.86` so this makes cargo silently download an unpatched `libc v0.2.112`, causing a build failure later. Adding a fixed requirement to `libc =v0.2.86` doesn't solve the issue since Cargo sees the mismatch, and happily tries to use both `libc v0.2.86` and `libc v0.2.112` simultaneously. And, if there is this sort of conflict, and crates without an explicit patch-version will try to use the most recent (and unpatched) version of the crate in the tree.
Will rebase shortly. After the break, at least 5 (!) version bumps of dependent crates have caused this PR to be unreproducible. I didn't actually hunt them all down but gave up and just committed the working Cargo.locks to unblock ongoing work. But hopefully rebasing onto #312 will fix most of these issues. |
…an example Maybe this should be documented/tested in the future? Alternatively providing network access and a second machine could work, though comes with its own problems.
Based off Nick's work I'm currently ignoring both the precision and clock ids, since I think these aren't really required for most uses. These may need to be added later. Additionally this puts a dependency in platform-services on feature(llvm_asm), which locks it into requiring an unstable compiler. Also I modified the shamir-secret-sharing example to test that this work quickly, this should be reverted before merging upstream.
What is the status of this PR, @geky? |
This is immediately interesting for the IceCap port, however is in a very hacky state (I think the draft status is very appropriate). @mathias-arm and I are also working on some significant changes to how Veracruz integrates with IceCap, so I don't think bringing the Wasmtime+IceCap integration up to date is worth it yet. My current plan is to return to this after we bring in the IceCap changes and see how the integration needs to be shaped in the new context. |
This is the initial, and might I add hacky, support for Wasmtime in IceCap. I'm creating this PR to start figuring out where submodules need to go and to allow for initial benchmarking work to start.
Some of the hacks that need to be cleaned up:
runtime-manager-supervisor
that actually understand the page table structure.runtime-manager-supervisor
getrandom
is currently using a pseudo-random number generator to fake randomness. This should eventually be changed to use whatever rng the arch has available.IceCap PR: https://gitlab.com/arm-research/security/icecap/icecap/-/merge_requests/8