Skip to content

Add vcpu run loop hooks - #245

Merged
jserv merged 1 commit into
sysprog21:mainfrom
open-sources-port:vcpu-callback
Jul 27, 2026
Merged

Add vcpu run loop hooks#245
jserv merged 1 commit into
sysprog21:mainfrom
open-sources-port:vcpu-callback

Conversation

@doanbaotrung

@doanbaotrung doanbaotrung commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Add an optional hook API for embedders that need to run host-side work before re-entering hv_vcpu_run on the vCPU owner thread.

Keep the existing vcpu_run_loop signature as a compatibility wrapper around the extended entry point with no hooks installed.

Add a host-side compile test that pins the public signatures and verifies the hook structure can be initialized. Run it from the normal host test paths used by check and check-sanitizer.

Fix #244


Summary by cubic

Add vcpu_run_loop_with_hooks to let embedders run host work before each hv_vcpu_run re-entry. Keep vcpu_run_loop as a wrapper and add a host compile test.

  • New Features
    • New vcpu_run_loop_with_hooks(..., const vcpu_run_hooks_t *hooks) and vcpu_run_hooks_t with an optional tick.
    • tick runs on the vCPU owner thread before hv_vcpu_run(), checked only at host boundaries; nonzero return stops the loop.
    • vcpu_run_loop remains as a no-hook wrapper for API compatibility.
    • Added host compile test test-vcpu-run-hooks-host; wired into check, check-sanitizer, and make test-vcpu-run-hooks-host.

Written for commit e19686a. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv
jserv requested a review from Max042004 July 26, 2026 08:21

@jserv jserv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review of the vCPU run-loop hook API. Three points below, anchored inline.

Comment thread src/syscall/proc.c Outdated
Comment thread src/syscall/proc.h
/* Optional embedder hook. Called on the vCPU owner thread before each
* hv_vcpu_run() re-entry. Return nonzero to stop the loop.
*/
typedef int (*vcpu_run_loop_tick_fn)(guest_t *g, void *opaque);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The hook contract is underspecified in three ways:

  • Stop semantics: state what the function returns when tick returns nonzero (currently exit code 0, indistinguishable from guest exit 0).
  • Concurrency and lifetime: when one vcpu_run_hooks_t/opaque is shared across worker vCPUs, tick is called concurrently from multiple threads, so it must be thread-safe. The implementation keeps no copy and dereferences hooks/opaque every iteration, so the caller must keep both alive until the function returns.
  • Cooperative-only timing: the tick fires only at the top of the loop (host round-trips), not after hv_vcpu_run() returns. A worker (timeout_sec == 0, no alarm()) spinning entirely in EL0 never reaches the top, so a hook cannot interrupt a runaway guest -- only exit_group's hv_vcpus_exit() kick can. "before each hv_vcpu_run() re-entry" reads as periodic; clarify it is checked at host boundaries only.

.opaque = (void *) 0x1,
};

if (hooks.tick == NULL || hooks.opaque == NULL) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The runtime half of this test is tautological: given the initializer, hooks.tick/hooks.opaque are never NULL, so the if can't fire and main always reaches PASS; the tick function is defined but never called. The real value is the two _Static_assert's, which fire at compile time -- building the object is what matters, not running the binary, yet mk/tests.mk prints a "unit test" banner and executes it, implying runtime validation that does not exist. The new executable behavior (invoke tick, break on nonzero) has no coverage since no in-tree caller passes non-NULL hooks. Either make it a compile-only target, or have main actually call tick(g, opaque) and assert on both the non-NULL and NULL-opaque short-circuit results.

Add an optional hook API for embedders that need to run host-side
work before re-entering hv_vcpu_run on the vCPU owner thread.

Keep the existing vcpu_run_loop signature as a compatibility wrapper
around the extended entry point with no hooks installed.

Add a host-side compile test that pins the public signatures and
verifies the hook structure can be initialized. Run it from the normal
host test paths used by check and check-sanitizer.

Fix sysprog21#244
@doanbaotrung

Copy link
Copy Markdown
Collaborator Author

Here is a summary of my update:

  1. Loop Exit & Shutdown Logic: Modified the run loop in proc.c to propagate non-zero tick hook return values as the run loop's exit_code, and re-check for group exit requests immediately following a tick execution.
  2. Hook Contract Documentation: Documented the stop semantics, concurrency/lifetime rules, and cooperative-only timing details in the comments of proc.c.
  3. Unit Tests: Updated test-vcpu-run-hooks-host.c to verify the hook's execution and assert behavior under different combinations of NULL/non-NULL guest and opaque arguments.
  4. Verification: Ran all host unit tests and verified that they compile and execute successfully.

@jserv
jserv merged commit 6a71dd3 into sysprog21:main Jul 27, 2026
10 checks passed
@jserv

jserv commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Thank @doanbaotrung for contributing!

@doanbaotrung
doanbaotrung deleted the vcpu-callback branch July 27, 2026 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcpu_run_loop has no embedder progress callback for UI/event pumps

2 participants