Fix duplicate guest trace batches#1619
Open
ludfjig wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes duplicate guest trace span batches by moving trace-batch delivery to a dedicated TraceBatch OUT port, so the host only reads the trace registers on the specific VM exit that carries a new batch (instead of polling sticky registers on every exit).
Changes:
- Host: only reads guest registers and parses trace data on
VmExit::IoOutwhere the port isOutBAction::TraceBatch. - Guest: trace batches are emitted via a dedicated
TraceBatchOUT in the tracing subsystem; abort paths now callhyperlight_guest_tracing::flush()before emitting abort bytes. - Cleanup/docs: removes the old “piggyback trace batch on arbitrary OUT” API surface and updates tracing documentation accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_host/src/sandbox/trace/context.rs | Removes the old “check registers for trace data” helper; trace handling is now driven by dedicated exits. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/mod.rs | Parses trace batches only on TraceBatch port exits, avoiding re-reading sticky registers on unrelated exits. |
| src/hyperlight_guest/src/exit.rs | Flushes trace batches before deliberate abort sequences so the final buffered batch can be delivered. |
| src/hyperlight_guest/src/arch/amd64/exit.rs | Makes out32 a pure I/O primitive again (no embedded trace-batch side effects). |
| src/hyperlight_guest_tracing/src/state.rs | Sends trace batches to the host via a dedicated TraceBatch OUT and tightens inline-asm options. |
| src/hyperlight_guest_tracing/src/lib.rs | Trims exported API to the remaining supported tracing entrypoints. |
| docs/hyperlight-metrics-logs-and-traces.md | Documents the dedicated TraceBatch exit and new batching/flush behavior (with a couple clarifications needed per review comments). |
The host stamped the TraceBatch magic in r8 on every OUT and polled r8 on every VM exit. r8 is sticky, so the host re-read the same batch on later exits and emitted duplicate spans. Deliver batches on a dedicated TraceBatch port. The host reads guest registers only on that exit, so each batch is read once. Aborts flush the buffer over the same port so the final batch still reaches the host. Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
a2fcbca to
9c871f5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The host stamped the TraceBatch magic in r8 on every OUT and polled r8 on every VM exit. r8 is sticky, so the host re-read the same batch on later exits and emitted duplicate spans.
Deliver batches on a dedicated TraceBatch port. The host reads guest registers only on that exit, so each batch is read once. Aborts flush the buffer over the same port so the final batch still reaches the host.
Closes #1230