-
Notifications
You must be signed in to change notification settings - Fork 139
[WIP] Enable guest tracing #695
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
base: main
Are you sure you want to change the base?
Changes from all commits
15554a8
e3f1c34
a04e125
21dd437
d3f8b63
35e044b
6a5dc64
82e08d2
ce53952
1e376fd
ce5f50e
d325ddc
b2ff420
9738dba
9218de5
8d3558e
66af14f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,11 +25,14 @@ spin = "0.10.0" | |
[features] | ||
default = ["tracing"] | ||
fuzzing = ["dep:arbitrary"] | ||
trace_guest = [] | ||
unwind_guest = [] | ||
mem_profile = [] | ||
Comment on lines
+28
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the reasoning for three separate features? From a usability standpoint it seems like a lot of things to keep track of. From a maintainer standpoint it also feels like a large additional matrix of things to test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think part of the confusion for me is that these seem like three related (as in use some base functionality) but different features but are wrapped up into a single PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jsturtevant They kind of are, since Doru built this on top of my old memory profiling PR from way back when. The idea with that was that the base tracing feature that lets you generate a file of fine-grained events quickly is useful for other things (and indeed it is being used it here for the performance events), while the full memory profiling build is extraordinarily slow, since it generates a trace event on every allocation/deallocation. Carrying around the registers to unwind the guest stack also seemed like something that would be useful in other contexts but a different amount of overhead & not always needed, e.g. the performance events here don't I believe use it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense that you may want to turn those on/off due to overhead. I think it would make for an easier review if these were split out, but I am fine with it if others are. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I recommend reviewing commit-by-commit; I put a lot of effort into making sure that each commit itself is small/atomic/reviewable. That means that we can have a PR like this one where we only merge things after they are useful (& consequently we know that the design is usable), but reviewers can understand and review each step along that path in isolation. |
||
std = [] | ||
|
||
[dev-dependencies] | ||
hyperlight-testing = { workspace = true } | ||
|
||
[lib] | ||
bench = false # see https://bheisler.github.io/criterion.rs/book/faq.html#cargo-bench-gives-unrecognized-option-errors-for-valid-command-line-options | ||
doctest = false # reduce noise in test output | ||
doctest = false # reduce noise in test output |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,17 +21,22 @@ use hyperlight_common::outb::OutBAction; | |
|
||
/// Halt the execution of the guest and returns control to the host. | ||
#[inline(never)] | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub fn halt() { | ||
// Ensure all tracing data is flushed before halting | ||
hyperlight_guest_tracing_macro::flush!(); | ||
unsafe { asm!("hlt", options(nostack)) } | ||
} | ||
|
||
/// Exits the VM with an Abort OUT action and code 0. | ||
#[unsafe(no_mangle)] | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub extern "C" fn abort() -> ! { | ||
abort_with_code(&[0, 0xFF]) | ||
} | ||
|
||
/// Exits the VM with an Abort OUT action and a specific code. | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub fn abort_with_code(code: &[u8]) -> ! { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you need to flush on an abort too? |
||
outb(OutBAction::Abort as u16, code); | ||
outb(OutBAction::Abort as u16, &[0xFF]); // send abort terminator (if not included in code) | ||
|
@@ -42,6 +47,7 @@ pub fn abort_with_code(code: &[u8]) -> ! { | |
/// | ||
/// # Safety | ||
/// This function is unsafe because it dereferences a raw pointer. | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub unsafe fn abort_with_code_and_message(code: &[u8], message_ptr: *const c_char) -> ! { | ||
unsafe { | ||
// Step 1: Send abort code (typically 1 byte, but `code` allows flexibility) | ||
|
@@ -62,7 +68,10 @@ pub unsafe fn abort_with_code_and_message(code: &[u8], message_ptr: *const c_cha | |
} | ||
|
||
/// OUT bytes to the host through multiple exits. | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub(crate) fn outb(port: u16, data: &[u8]) { | ||
// Ensure all tracing data is flushed before sending OUT bytes | ||
hyperlight_guest_tracing_macro::flush!(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the abort section this means this gets called twice, is that expected? |
||
unsafe { | ||
let mut i = 0; | ||
while i < data.len() { | ||
|
@@ -79,6 +88,7 @@ pub(crate) fn outb(port: u16, data: &[u8]) { | |
} | ||
|
||
/// OUT function for sending a 32-bit value to the host. | ||
#[hyperlight_guest_tracing_macro::trace_function] | ||
pub(crate) unsafe fn out32(port: u16, val: u32) { | ||
unsafe { | ||
asm!("out dx, eax", in("dx") port, in("eax") val, options(preserves_flags, nomem, nostack)); | ||
|
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.
This section is focused on how to use the development tools to test from a hyperlight developers perspective but it would be nice to think about it from a user of hyper light. For instance, if I am writing my own guest what do I need to do? I can of course look at these samples but having some docs about it would be useful here.