Skip to content

Guest panic context refactor #474

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

Merged
merged 4 commits into from
May 8, 2025

Conversation

danbugs
Copy link
Contributor

@danbugs danbugs commented May 8, 2025

This PR removes the guest panic context memory region and refactors the functionality to use a new OutBAction that prints to stderr.

I recommend you review this commit-by-commit.

Note: Just like #462, this PR is also part of the effort of breaking #297 into more digestible bits.

@danbugs danbugs added the kind/refactor For PRs that restructure or remove code without adding new functionality. label May 8, 2025
@danbugs danbugs force-pushed the guest-panic-context-refactor branch from 32bc862 to 5304030 Compare May 8, 2025 16:32
@simongdavies simongdavies requested a review from Copilot May 8, 2025 16:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the handling of guest panic context by removing the dedicated panic context memory region and its associated functions, and instead using a new OutBAction that prints to stderr. Key changes include:

  • Removal of the Guest Panic Context region from memory management and layout.
  • Updates to outb function signatures and related handler traits to use byte slices rather than integer values.
  • Adjustments in hypervisor and guest modules to reflect the new OutBAction API.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/hyperlight_host/src/mem/mgr.rs Removed PanicContext variant from memory region assignment.
src/hyperlight_host/src/mem/memory_region.rs Dropped the PanicContext enum variant.
src/hyperlight_host/src/mem/layout.rs Removed panic context offsets, functions, and validations from the layout.
src/hyperlight_host/src/hypervisor/* Updated outb call signatures and payload handling from u64 to Vec.
src/hyperlight_guest/src/* Adjusted panic, abort and error handling to use new OutBAction and byte slice parameters.
src/hyperlight_common/src/outb.rs Introduced new OutBAction definitions and removed GuestPanicContextData.
Comments suppressed due to low confidence (2)

src/hyperlight_host/src/mem/layout.rs:268

  • The removal of guest panic context offsets is clear; ensure that all related documentation and tests are updated to reflect the new memory layout without the Guest Panic Context.
            peb_guest_panic_context_offset,

src/hyperlight_common/src/mem.rs:76

  • The removal of GuestPanicContextData is appropriate; please verify there are no residual dependencies on this structure in other modules.
pub struct GuestPanicContextData {

@danbugs danbugs force-pushed the guest-panic-context-refactor branch 3 times, most recently from a263cc6 to 16a800a Compare May 8, 2025 18:24
danbugs added 4 commits May 8, 2025 21:04
mem/ only had one file, so it makes sense for on its own (i.e., outside a folder), which will be in agreement w/
a new mod I'll add in the next commit.

Signed-off-by: danbugs <[email protected]>
OutBAction was duplicated across the host and guest.

Signed-off-by: danbugs <[email protected]>
- added action to enable eprinting from an out instruction.
- changed out instruciton to output 4 bytes for optimized printing.

Signed-off-by: danbugs <[email protected]>
…st panic context mem region

We had a guest panic context region to store the panic msg. Now, instead of storing info in shared mem, we
use debug_print. To still have some exception info  when using OutBAction::Abort, we pass parse the
exception name, which is more readable than before and provides information that is useful as illustrated
in some of the integration tests.

Signed-off-by: danbugs <[email protected]>
@danbugs danbugs force-pushed the guest-panic-context-refactor branch from 16a800a to ce8410a Compare May 8, 2025 21:04
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danbugs danbugs merged commit ce8410a into hyperlight-dev:main May 8, 2025
27 checks passed
@danbugs danbugs deleted the guest-panic-context-refactor branch May 8, 2025 22:33
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out8, out16, and out32 to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out8, out16, and out32 to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
… panic info to abort

    In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
    (1) printing panic info w/ debug_print, and
    (2) exiting w/ abort and codes.

    This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
    aborting.

    This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
    minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

    Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 13, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 14, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 14, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 14, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
danbugs added a commit to danbugs/hyperlight that referenced this pull request May 14, 2025
…panic info to abort

In hyperlight-dev#474, I removed the guest panic context region. With that, aborting was split into two steps:
(1) printing panic info w/ debug_print, and
(2) exiting w/ abort and codes.

This PR addresses feedback I got in hyperlight-dev#474 to join the operations by buffering string output when
aborting.

This PR also cleans up our manual chunking logic w/ outb and leverages out32 w/ length prefixing to
minimize VM exits. Abort detects it should finishing buffering by a terminator code (0xFF).

Edited tests to check for panic message in aborts as they were prior to hyperlight-dev#474 merged.

Signed-off-by: danbugs <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/refactor For PRs that restructure or remove code without adding new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants