-
Notifications
You must be signed in to change notification settings - Fork 45
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
run tests inside SVSM #120
run tests inside SVSM #120
Conversation
Makefile
Outdated
@@ -9,7 +9,17 @@ TARGET_PATH="debug" | |||
endif | |||
|
|||
STAGE2_ELF = "target/x86_64-unknown-none/${TARGET_PATH}/stage2" | |||
ifdef TEST | |||
KERNEL_ELF = "target/x86_64-unknown-none/${TARGET_PATH}/deps/svsm-0651abaef6a489bb" |
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.
I'm not sure the metadata hash can be hardcoded here. I think it can change based on compiler version and other parameters.
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.
I think you're right in that there is no guarantee that the hash will be stable. Cargo doesn't seem to have way to fix the path: rust-lang/cargo#1924). Anecdotally, over the course of development for this pr, I've been using multiple nightly versions and the hash remained the same.
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.
I spotted in when I was trying it out - I was playing around with my own build script that deploys to our test machine and I ended up making the hash value change. Unfortunately I don't know what I did to cause it to change and it reverted back to the original when I tried to reproduce it.
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.
Addressed in ace2c55
We don't have a README specific to testing at the moment but there is reference to the unit tests in INSTALL.md. Could you maybe add some instructions on how to build and deploy the test binary in INSTALL.md? |
Thanks for doing this, it looks very promising! Somehow I have issues getting the test binary to build, it can't find the |
Sounds like the x86_64-unknown-none target on the nightly toolchain isn't installed. Try
|
That works, thanks. I can run the tests now. |
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.
In general this looks great. A few things I'd like to see changed:
- We can safely assume that the test SVSM binary will run in QEMU, so I'd like to use the QEMU exit device to get exit qemu with a given exit-code. This way running the in-kernel tests can be more easily automated.
- Please add a Makefile target to build and run the in-kernel tests.
Is there an easy way to limit certain tests to a single test-runner? So that we run some tests only in user-space and others only with the in-kernel runner?
src/testing.rs
Outdated
|
||
info!("All tests passed!"); | ||
|
||
request_termination_msr(); |
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.
I wonder if we can use the qemu exit device here?
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.
Turns out we can: 0ef34e2
3ea5805
to
f3d1e5e
Compare
Good idea! Implemented in 0ef34e2.
Implemented in 773b3b2. I added a
We can use |
A more descriptive way of specifying this would be great. Can this work with defining specific features and making tests depend on them? |
Yes, we could either use a feature or just a raw |
Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR coconut-svsm#120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR coconut-svsm#55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test
Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR coconut-svsm#120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR coconut-svsm#55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test Signed-off-by: Adam Dunlap <[email protected]>
Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR coconut-svsm#120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR coconut-svsm#55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test Signed-off-by: Adam Dunlap <[email protected]>
f3d1e5e
to
b043f5b
Compare
I added a cfg option |
b043f5b
to
bed5654
Compare
I just rebased onto the latest master branch. |
Made a few updates and stored them in this branch. Can you please look at my changes and include them into this PR? |
Looks good to me, thanks! |
Once we'll start running tests in the SVSM, we'll want to use the correct implementations and not the fake ones for tests outside the SVSM. Signed-off-by: Tom Dohrmann <[email protected]>
For one reason or another some of the tests currently fail when run inside the SVSM. Signed-off-by: Tom Dohrmann <[email protected]>
Signed-off-by: Tom Dohrmann <[email protected]>
Signed-off-by: Tom Dohrmann <[email protected]>
... and move it to `all`. This way we can use the stage1/stage1.o target with another kernel (e.g. test kernel). Signed-off-by: Tom Dohrmann <[email protected]>
To build and run a kernel that runs the tests use ``` QEMU=/path/to/qemu OVMF=/path/to/firmware/ make test-in-svsm ``` Signed-off-by: Tom Dohrmann <[email protected]>
This is cleaner than `request_terminator_msr` as that MSR is more commonly used to signal errors rather than clean exits. Signed-off-by: Tom Dohrmann <[email protected]>
Signed-off-by: Tom Dohrmann <[email protected]>
Add a helper to print the C-bit position on the current platform. Signed-off-by: Joerg Roedel <[email protected]>
Instead of having the full QEMU command line in the Makefile, extract the test-run into a separate script. Signed-off-by: Joerg Roedel <[email protected]>
aba18a1
to
845ec96
Compare
I also rebased onto the latest master branch so resolve some conflicts. |
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.
Thanks, looks good now.
Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR coconut-svsm#120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR coconut-svsm#55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test Signed-off-by: Adam Dunlap <[email protected]>
Adds tests that make sure a #VC exception is raised while executing various instructions and that the #VC handler properly handles them via the ghcb. Adds tests for cpuid, msrs, port IO, MMIO, wbinvd, rdtsc, dr7, and vmmcall. Relies on being able to run tests inside the guest VM, e.g. with PR coconut-svsm#120. The #VC handler is not yet implemented, so these tests are all marked with #[should_panic]. See PR coconut-svsm#55 for progress on its implementation. TODO: Remove added dependencies Move the tests to vc.rs or a new file Clean up comments, etc. Move assembly wrapper helper functions to more sensible places Implement page state change (PSC) test Signed-off-by: Adam Dunlap <[email protected]>
This PR makes it possible to run tests inside the SVSM. Simply setting the
TEST
environment variable before runningmake
will produce an SVSM build that runs tests instead of running the guest.This is implemented using the nightly
custom_test_frameworks
feature to specify our own tests runner. This nightly feature is only required when building tests for running inside the SVSM, non-test builds and normal test execution can continue to use a stable toolchain. The#[test]
macro expects a crate namedtest
to exist, so a small, no_std compatible subset of thetest
crate has been copied.Some tests currently fail when run inside the SVSM (AFAICT mostly for benign reasons related to setup of the allocator and file system). Those tests are currently disabled using the
ignore
attribute. Fixing those tests is probably best left to separate PRs.