enarx-archive / sallyport

API for the hypervisor-microkernel boundary
Apache License 2.0
7 stars 6 forks source link

Remove `asm` feature #44

Closed rvolosatovs closed 2 years ago

rvolosatovs commented 2 years ago

asm is getting stabilized upstream and the feature is not strictly necessary anymore for the implementation. Note that only the host side needs it, currently exclusively for the purpose of executing syscalls.

Unfortunately miri does not support inline assembly at the time of writing (https://github.com/rust-lang/miri/issues/1045, https://github.com/rust-lang/miri/issues/11 is the tracking issue)

Few options here:

  1. Remove miri and asm feature - we lose potential for testing against UB on CI
  2. Disable miri for tests involving host side - greatly diminishes the benefit of actually using miri
  3. Disable miri selectively for each test involving host side and inline assembly - this is going to be quite tedious to maintain
  4. It's not broken, so don't fix it - we can simply wait until inline assembly is supported by miri. That may take long and that means we have to maintain the feature within the implementation, but that is actually fairly trivial, since the host simply ignores the syscalls when feature is disabled. Note, that it is possible that will have some mechanism in place for selectively disabling/enabling host-side call handlers at runtime in the future, if we had such functionality, we could then simply disable all calls requiring asm when miri is enabled and drop the feature.

For now, I vote for 4. and suggest to revisit this in a few sprints.

Refs https://github.com/rust-lang/rust/pull/91728 https://github.com/enarx/enarx/issues/1178