From 75ad88a09919ecbea05c43eab238935e8a06670b Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 15 Oct 2026 10:29:09 -0300 Subject: [PATCH] doc: retarget citations of the removed time ABI specification #529 replaced doc/design with the architecture guide and removed doc/design/time-abi.md, the time ABI specification. NVX code, tests, and documentation, and OpenVMM's time ABI comments, still cited it: its sections, its snapshot-agent and restore step numbers, and check IDs that no remaining document defines. Retarget every NVX citation to the guide, fold the specification's remaining architecture-level detail into section 3.2, leave its implementation detail to the code that defines it, and pin an OpenVMM revision whose documentation no longer depends on NVX's documents. Folded into 3.2.10 (Guest Obligations): - Which checks capture and restore rerun, instead of "the relevant subset": capture reruns the clocksource, tick-device, and watcher checks; restore reruns them, checks the declared rate in the frequency MSR again, and checks the identity leaves, CPU time bits, and CPU flags of every processor that it brings online. - What the state file reports: the outcome of each phase's checks, the declared rates, the generation, the last downtime and its source, and the wall clock's discontinuities, synchronization, offset, uncertainty, and frequency correction. - Restore repair and the discipline step the clock through the kernel, so a workload observes every step through the kernel's clock-set notification, such as a timerfd armed with TFD_TIMER_CANCEL_ON_SET. The rest of the specification's architecture-level content is already in the guide: the guest's fail-fast statuses (5.6), generations (5.5.6), the rejection of clock tokens in the command line (5.1), and capture rollback (5.3). Its implementation detail stays out of the documentation: the E_* and G_* codes, the check IDs C1 to C12, K1, and X1 to X6, the NVX-TIME-ABI line formats, the state file's path and keys, the watcher's kernel log patterns, report-only mode, and the warp probe's method. guest/common/nvx-time.c now states that it defines its check IDs, guest codes, output lines, and state file format. Renumbered snapshot-agent steps: - guest/common/nvx-time.c, guest/common/nvx-snapshot, and scripts/test_microvm_tests.py number the snapshot agent's steps as the guide does: guest preparation steps 1 to 4 (5.3.2) and guest repair steps 1 to 7 (5.5.7), instead of the removed specification's capture steps 1 to 5 and restore steps 6 to 13. Old steps 6 to 10 are repair steps 1 to 5, old step 12 is repair step 6, and old step 13, with old step 11's checks, is repair step 7. - The comment on repair_restore names repair steps 2 and 3, because cmd_capture reads the status, repair step 1, itself. - Rename step13_cpu_ns, fork_step13, and test_step13_cpu to deferred_restore_cpu_ns, fork_deferred_restore, and test_deferred_restore_cpu, and nvx-time's "step13-cpu" test hook to "deferred-restore-cpu", with its test in scripts/test_guest_time.py. Retargeted documentation: - doc/build.md: the RCU stall timeout stays at its production value because the conformance checks cover the stall-detector settings (3.2.10), rather than because of check C8. - doc/ci.md: describe the exhaustive check's MSRs and the boot check of the debug detectors in words, rather than as checks C3 and C11. Retargeted code comments and test names, with no behavior change: - scripts/nvx_tools/doctor.py: the doctor runs its checks in ID order, as the --checks help text now says, and CHECK_CPU_BUDGET_US holds the CPU-time budgets. - scripts/nvx_tools/microvm_tests.py: the debug-kernel restore cases and the long-downtime case cite doc/ci.md. - scripts/nvx_tools/time_abi.py: the CPU profile catalog cites 2.2, and the warp probe check cites the 1 us skew bound of 3.2. - scripts/test_guest_time.py: the restore packet (5.5.5) and time sample (3.2.9) fixtures follow the guide, and the exhaustive check's lines and the state file fixtures pin the formats that nvx-time.c defines. The exhaustive-report test becomes test_exhaustive_report_format. - scripts/test_time_abi.py: the CPU-time budget comments no longer cite "the spec's" budgets. OpenVMM pin: Pin nanvix/openvmm#136 at c6d97c529986c0b3b1bf1fd7f00406654cdeee67, one commit on 7f225e2c. OpenVMM's time ABI comments cited the removed specification by path, by its "Backend obligations" table, by its step numbers, and as "the specification". They now state what the code does and refer only to OpenVMM's own items, and the three tests named after the specification are renamed. There is no behavior change. Index the new pin in doc/openvmm-upstream-roadmap.md. Fixes #562 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/build.md | 6 +- doc/ci.md | 14 ++-- doc/design/3.2-clock-and-time.md | 18 +++-- doc/openvmm-upstream-roadmap.md | 7 +- guest/common/nvx-snapshot | 16 ++-- guest/common/nvx-time.c | 126 +++++++++++++++-------------- openvmm | 2 +- scripts/nvx_tools/doctor.py | 6 +- scripts/nvx_tools/microvm_tests.py | 9 ++- scripts/nvx_tools/time_abi.py | 10 ++- scripts/test_guest_time.py | 39 ++++----- scripts/test_microvm_tests.py | 3 +- scripts/test_time_abi.py | 10 +-- 13 files changed, 144 insertions(+), 122 deletions(-) diff --git a/doc/build.md b/doc/build.md index 013f37f7d..a048aa64c 100644 --- a/doc/build.md +++ b/doc/build.md @@ -284,9 +284,9 @@ the production RCU stall timeouts and turns off the debug options that detectors are missing from the generated config. CI can shorten the soft-lockup and hung-task thresholds for one boot with `watchdog_thresh=` and `sysctl.kernel.hung_task_timeout_secs=`. The RCU stall timeout stays at the -production value because the time ABI's conformance check C8, which covers -the [stall-detector settings](design/3.2-clock-and-time.md#3210-guest-obligations), -requires it. +production value because the time ABI's conformance checks cover the +[stall-detector settings](design/3.2-clock-and-time.md#3210-guest-obligations) +and require it. The variant builds in `build/linux-debug`, so it never invalidates the production object cache, and it produces: diff --git a/doc/ci.md b/doc/ci.md index 58b24ba6d..8d6edd633 100644 --- a/doc/ci.md +++ b/doc/ci.md @@ -112,12 +112,12 @@ The `time-abi-conformance` scenario boots the largest requested vCPU count and runs the guest's exhaustive CI check, `/sbin/nvx-time exhaustive`, which the boot check leaves to CI. On every online CPU it checks every leaf `0x40000006..=0x400000ff` and every base `0x40000100..=0x4000ff00` for another -hypervisor signature, every `C3` MSR, the write rules of -`HV_X64_MSR_TSC_INVARIANT_CONTROL`, writes to the read-only identity MSRs, and -reads of `IA32_TSC_ADJUST` and `IA32_TSC_DEADLINE` (checks `X1` to `X6`). The -harness requires a passing `NVX-TIME-ABI-EXHAUSTIVE` line for every check on -every CPU, a summary with `status=ok`, the requested CPU count, and no -failures, and exit status 0; a failure lists each failing check with the +hypervisor signature, every identity MSR that the boot check reads, the write +rules of `HV_X64_MSR_TSC_INVARIANT_CONTROL`, writes to the read-only identity +MSRs, and reads of `IA32_TSC_ADJUST` and `IA32_TSC_DEADLINE` (checks `X1` to +`X6`). The harness requires a passing `NVX-TIME-ABI-EXHAUSTIVE` line for every +check on every CPU, a summary with `status=ok`, the requested CPU count, and +no failures, and exit status 0; a failure lists each failing check with the guest's detail. The guest command fits on one console line, so the console's echo of it ends before the check prints. The `cpu-profile-fallback` scenario covers `--cpu-profile auto`'s fallback to a @@ -175,7 +175,7 @@ deferred checks and must report a passing restore line, so no restore leaves its checks pending. The one exception is `snapshot-tiers`' gate-timeout check, whose guest OpenVMM stops at the restore gate on purpose. The harness refuses a kernel whose `vmlinux-debug.config` lacks the detectors, because the guest's -`C11` check passes vacuously without them. To bound the cost, pull requests +boot check of those detectors passes vacuously. To bound the cost, pull requests run the debug kernel on KVM only and `dev` pushes run it on every backend; each job takes about five minutes on its own runner, in parallel with the other microVM jobs. The jobs gate the required status check, the development diff --git a/doc/design/3.2-clock-and-time.md b/doc/design/3.2-clock-and-time.md index a57d92614..5c0c3e808 100644 --- a/doc/design/3.2-clock-and-time.md +++ b/doc/design/3.2-clock-and-time.md @@ -356,7 +356,10 @@ the bound for its use. The guest agents perform conformance checks, watch for violations, discipline the wall clock, and execute the snapshot time steps in every guest mode. Production paths keep the console quiet; status goes to a state file that the -workload can read through a read-only mount. +workload can read through a read-only mount. The state file reports the +outcome of each phase's checks, the declared rates, the generation, the last +downtime and its source, and the wall clock's discontinuities, +synchronization, offset, uncertainty, and frequency correction. **Conformance.** Before readiness, the guest only takes a time sample within the bound and steps its clock to host UTC, so the workload starts on host @@ -366,9 +369,11 @@ the workload. The checks cover the identity leaves and explicit zero leaves, the CPU time bits on every CPU, the identity MSRs, the kernel's platform, clocksource, and LAPIC-timer selection, CPU flags, tick devices, the absence of forbidden clock parameters, stall-detector settings, a running violation -watcher, and kernel W+X integrity. Capture and restore rerun the relevant -subset. Every check is fail-fast: a failure powers the guest off even if the -workload is running. +watcher, and kernel W+X integrity. Capture reruns the clocksource, +tick-device, and watcher checks. Restore reruns them too, checks the declared +rate in the frequency MSR again, and checks the identity leaves, the CPU time +bits, and the CPU flags of every processor that it brings online. Every check +is fail-fast: a failure powers the guest off even if the workload is running. **Violation watcher.** The watcher follows the kernel log from before the boot checks, so no record is missed. TSC instability; clocksource instability, @@ -398,7 +403,10 @@ applications. The kernel limits the frequency correction to ±500 ppm, of which the rate tolerance uses at most 250 ppm. A sample whose generation differs from the recorded one is discarded, because a restore happened and restore repair resets the discipline. The discipline follows host wall-clock steps -and never powers off the guest. +and never powers off the guest. Restore repair and the discipline step the +clock through the kernel, so a workload can observe every step through the +kernel's clock-set notification, such as a `timerfd` armed with +`TFD_TIMER_CANCEL_ON_SET`. The [snapshot protocol](5.3-snapshot-protocol.md) and [guest repair](5.5-restore-protocol.md#557-guest-repair) define the snapshot diff --git a/doc/openvmm-upstream-roadmap.md b/doc/openvmm-upstream-roadmap.md index f37bacd7c..8e97801d6 100644 --- a/doc/openvmm-upstream-roadmap.md +++ b/doc/openvmm-upstream-roadmap.md @@ -5,7 +5,7 @@ - OpenVMM fork base: `e1cdbd916d3aacb216309c8e7779b832546759a1`. - Mapped fork tip: `2728f33ea9a44d2d0fb956d9f01768ae2100fb62`, position 166. The graph, PR summary, and commit mapping below cover positions 001-166. -- NVX's pinned OpenVMM revision: `7f225e2cd635bbd1e1839e587e659996433396c8`, 151 commits past the +- NVX's pinned OpenVMM revision: `c6d97c529986c0b3b1bf1fd7f00406654cdeee67`, 152 commits past the mapped tip on the same first-parent chain. These commits are not yet assigned to PRs; see [Commits after the mapped tip](#commits-after-the-mapped-tip). - Upstream main: `1fd455b19e7c69c72a2d375d0ed1cb35d901c688` (2026-10-01), 18 commits past @@ -199,7 +199,7 @@ e1cdbd916 -> PR18/PR23 split [162] -> PR18 fixup [163] -> PR25 fixup [164] -> PR27 [165-166] -> 2728f33ea (mapped tip) - -> unassigned [167-317] -> 7f225e2cd (NVX pin) + -> unassigned [167-318] -> c6d97c529 (NVX pin) ``` For upstream review, PR01-PR08 and PR14 can be rebased independently onto current upstream main. @@ -215,7 +215,7 @@ can be submitted independently if preferred. ## Commits after the mapped tip -The NVX pin extends the chain past position 166 without merges. The 151 later commits are not +The NVX pin extends the chain past position 166 without merges. The 152 later commits are not yet classified or assigned to PRs. This table groups them by topic in chain order; ranges are inclusive, as in the [PR summary](#pr-summary). @@ -242,6 +242,7 @@ inclusive, as in the [PR summary](#pr-summary). | 315 | `dd8def143` | 1 | Private-network ingress: `--network-ingress allow` publishes the guest's announced listeners to clients on the host's private networks | | 316 | `a44fcc2aa` | 1 | Bidirectional host-loopback access: `--host-loopback allow` publishes the guest's announced listeners on host loopback for the host alone | | 317 | `7f225e2cd` | 1 | IPv6 host peers appear inside the static IPv6 subnet, so the guest answers them from its address there | +| 318 | `c6d97c529` | 1 | Standalone time ABI documentation: comments describe the contract without citing NVX's removed specification | Some of these commits change mapped work rather than add to it. Positions 255-259 remove the legacy microVM clock paths and change files that PR10 and PR17 also change. Position 261 makes diff --git a/guest/common/nvx-snapshot b/guest/common/nvx-snapshot index 70eef4b00..7fdbe652e 100644 --- a/guest/common/nvx-snapshot +++ b/guest/common/nvx-snapshot @@ -391,10 +391,11 @@ post_restore() { finish_restore "$packet_flags" } -# Restore steps 12 and 13: the acknowledgement when the host gated its input, -# then the daemon's step 11 restore checks and grace-period release, which -# run off the restore latency path. A second argument of "quiet" omits the -# stage and completion lines of a plain untiered restore. +# Guest repair steps 6 and 7 (doc/design/5.5-restore-protocol.md): the +# acknowledgement when the host gated its input, then the daemon's restore +# checks and grace-period release, which run off the restore latency path. A +# second argument of "quiet" omits the stage and completion lines of a plain +# untiered restore. finish_restore() { [ "${2:-}" = quiet ] || console_status "NVX-POST-RESTORE-STAGE: acknowledge" @@ -440,9 +441,10 @@ elif [ "$scratch_policy" = fresh ]; then exit 1 fi -# Capture steps 1 to 3: the capture checks and the stall-detector -# suppression. They never wait, though nvx-time first lets pending boot -# checks finish: they start the daemon that the capture checks require. +# Guest preparation steps 1 and 2 (doc/design/5.3-snapshot-protocol.md): the +# capture checks and the stall-detector suppression. They never wait, though +# nvx-time first lets pending boot checks finish: they start the daemon that +# the capture checks require. /sbin/nvx-time pre-capture || exit 1 stall_detectors_suppressed=true diff --git a/guest/common/nvx-time.c b/guest/common/nvx-time.c index 3b806a68e..ce75ee163 100644 --- a/guest/common/nvx-time.c +++ b/guest/common/nvx-time.c @@ -5,9 +5,13 @@ // // doc/design/3.2-clock-and-time.md ("Guest Obligations") is the contract this // helper enforces. It runs the conformance checks, the violation watcher and -// the wall-clock discipline (one daemon), and the snapshot agent's time steps. -// It prints nothing on success: checks record their result in the state file, -// and only violation events reach the console. +// the wall-clock discipline (one daemon), and the snapshot agent's time steps: +// the guest preparation steps of doc/design/5.3-snapshot-protocol.md and the +// guest repair steps of doc/design/5.5-restore-protocol.md, which the comments +// below cite by number. It prints nothing on success: checks record their +// result in the state file, and only violation events reach the console. This +// file defines the check IDs, the guest codes, and the formats of its output +// lines and the state file. // // boot initial clock step (C12), then the other boot // checks asynchronously, which start the daemon @@ -125,7 +129,7 @@ #define TIMER_LIST_WAIT_NS (200 * NSEC_PER_MSEC) #define DEFAULT_STALL_TIMEOUT_S 21 // The asynchronous boot checks start 150 ms after the time ABI boot step, -// which init runs shortly before shell-ready, and restore step 13 150 ms +// which init runs shortly before shell-ready, and repair step 7 150 ms // after the acknowledgement: earlier, they contend with the readiness path // while the VMM still demand-faults guest memory, and with the workload's own // first activity after readiness. Both start at SCHED_IDLE and continue at @@ -186,7 +190,7 @@ static int64_t clock_ns(clockid_t clock) } // The CPU time of this process, every thread's, exited threads' included: -// what the spec's cpu_us sums. +// what a check's cpu_us reports. static int64_t cpu_time_ns(void) { return clock_ns(CLOCK_PROCESS_CPUTIME_ID); @@ -833,8 +837,8 @@ static void append_text(char *text, size_t size, size_t *length, *length += (size_t)written; } -// Formats the state in the order of the spec's state file table; the keys of -// a phase whose check never ran are absent. +// Formats the state file's key=value lines in their fixed order; the keys of a +// phase whose check never ran are absent. static void state_format(const struct time_state *state, char *text, size_t size) { @@ -989,8 +993,8 @@ static void apply_check(struct time_state *state, const void *context) check->cpu_us = record->cpu_us; check->generation = record->phase == PHASE_BOOT ? 0 : state->generation; check->failures = -1; - // Report-only mode keeps the failures in the state file, as the spec's - // keys reserved for it. + // Report-only mode keeps the failures in the state file, in keys that only + // it writes. if (g_report_only) { check->failures = record->failures; if (record->failures > 0 && strcmp(record->status, "ok") == 0) @@ -2576,7 +2580,7 @@ static pid_t daemon_pid(void) return (pid_t)pid; } -// The asynchronous boot checks and restore step 13 start at idle priority, so +// The asynchronous boot checks and repair step 7 start at idle priority, so // that on few vCPUs they never take a CPU from the workload, and continue at // normal priority after IDLE_BOUND_NS; the watcher and the discipline keep // normal priority. musl does not implement sched_setscheduler(), so this uses @@ -2666,8 +2670,8 @@ static void promoter_stop(struct promoter *promoter) pthread_join(promoter->thread, NULL); } -// Restore steps 12 and 13 on the readiness path: the acknowledgement when the -// host gated its input, then hand step 13 to DAEMON. The sigqueue() value is +// Repair steps 6 and 7 on the readiness path: the acknowledgement when the +// host gated its input, then hand step 7 to DAEMON. The sigqueue() value is // the readiness path's wall time since START_NS (CLOCK_MONOTONIC), in // microseconds clamped to 32 bits, read after the acknowledgement so that the // read does not delay it. The readiness path's CPU time is not reported: the @@ -3274,10 +3278,10 @@ static int parse_restore_record(const char *text, unsigned long long *generation // The times that restore_work records. The wall time is READINESS_US, the // readiness path's, plus the checks' since START_NS, when they were due. The -// CPU time is step 13's alone: this process's since CPU_BASE_NS, plus the -// daemon's preparation, PRIOR_CPU_NS up to the fork and the fork's own share, -// which the daemon writes to FORK_FD (-1 when there is none) once the fork -// returns. +// CPU time is repair step 7's alone: this process's since CPU_BASE_NS, plus +// the daemon's preparation, PRIOR_CPU_NS up to the fork and the fork's own +// share, which the daemon writes to FORK_FD (-1 when there is none) once the +// fork returns. struct restore_timing { int64_t start_ns; int64_t readiness_us; @@ -3286,9 +3290,9 @@ struct restore_timing { int fork_fd; }; -// Step 13's CPU time so far, in a process that TIMING describes. The read -// waits for the daemon's fork share if the daemon has not yet written it. -static int64_t step13_cpu_ns(const struct restore_timing *timing) +// Repair step 7's CPU time so far, in a process that TIMING describes. The +// read waits for the daemon's fork share if the daemon has not yet written it. +static int64_t deferred_restore_cpu_ns(const struct restore_timing *timing) { int64_t cpu = timing->prior_cpu_ns + cpu_time_ns() - timing->cpu_base_ns; int64_t fork_share = 0; @@ -3301,13 +3305,14 @@ static int64_t step13_cpu_ns(const struct restore_timing *timing) return cpu; } -// Forks the step 13 worker at SCHED_IDLE for a daemon whose preparation +// Forks the repair step 7 worker at SCHED_IDLE for a daemon whose preparation // started at CPU_START_NS, its CPU clock, and completes TIMING's CPU fields // for the child. The parent returns to normal priority as soon as the fork // does. Its fork costs CPU time after the child's copy of TIMING was taken, so // it writes that share to a pipe that the child reads. Returns fork()'s // result. -static pid_t fork_step13(int64_t cpu_start_ns, struct restore_timing *timing) +static pid_t fork_deferred_restore(int64_t cpu_start_ns, + struct restore_timing *timing) { int fds[2] = {-1, -1}; int64_t fork_share; @@ -3338,15 +3343,14 @@ static pid_t fork_step13(int64_t cpu_start_ns, struct restore_timing *timing) return pid; } -// Restore step 13, fail-fast: the step 11 restore checks, the grace-period -// release, the saved watchdog settings, the deferred C7, and the restore -// check's record. A child of the daemon runs it at SCHED_IDLE from -// DEFERRED_START_NS after the acknowledgement; the daemon raises it to normal -// priority IDLE_BOUND_NS later and releases the suppression itself at the -// stall timeout. restore-finish runs it inline, with its own deadline -// (WITH_DEADLINE), in report-only mode when no daemon runs. Neither recorded -// time counts the grace period wait or C7. Returns 0, or 1 when a report-only -// release failed. +// Repair step 7, fail-fast: the restore checks, the grace-period release, the +// saved watchdog settings, the deferred C7, and the restore check's record. A +// child of the daemon runs it at SCHED_IDLE from DEFERRED_START_NS after the +// acknowledgement; the daemon raises it to normal priority IDLE_BOUND_NS later +// and releases the suppression itself at the stall timeout. restore-finish +// runs it inline, with its own deadline (WITH_DEADLINE), in report-only mode +// when no daemon runs. Neither recorded time counts the grace period wait or +// C7. Returns 0, or 1 when a report-only release failed. static int restore_work(const bool *new_cpus, int new_count, bool with_deadline, const struct restore_timing *timing) { @@ -3362,7 +3366,7 @@ static int restore_work(const bool *new_cpus, int new_count, bool with_deadline, else run_restore_checks(&checks, new_cpus, new_count); elapsed = clock_ns(CLOCK_MONOTONIC) - timing->start_ns; - cpu = step13_cpu_ns(timing); + cpu = deferred_restore_cpu_ns(timing); if (with_deadline) { (void)read_int64(RCU_STALL_TIMEOUT, &timeout_s); released = release_with_deadline(timeout_s); @@ -3384,7 +3388,7 @@ static int restore_work(const bool *new_cpus, int new_count, bool with_deadline, return released ? 0 : 1; } -// Starts restore step 13 when it is due, DEFERRED_START_NS after the +// Starts repair step 7 when it is due, DEFERRED_START_NS after the // acknowledgement, in a child at SCHED_IDLE, so the watcher and the // discipline keep normal priority; the fork itself also runs at SCHED_IDLE. // The readiness path's wall time came with the signal. @@ -3421,11 +3425,11 @@ static void start_restore(struct daemon *daemon) } } (void)read_int64(RCU_STALL_TIMEOUT, &timeout_s); - // The worker's wall time runs from when step 13 was due. Its CPU time is - // step 13's: this preparation, the fork, and the worker's own. + // The worker's wall time runs from when step 7 was due. Its CPU time is + // step 7's: this preparation, the fork, and the worker's own. timing.start_ns = daemon->restore_due_ns; timing.readiness_us = daemon->readiness_us; - pid = fork_step13(cpu_start, &timing); + pid = fork_deferred_restore(cpu_start, &timing); if (pid == 0) _exit(restore_work(new_cpus, new_count, false, &timing)); if (pid < 0) { @@ -3472,7 +3476,7 @@ static void reap_children(struct daemon *daemon) PHASE_RESTORE, "the restore work ended with signal %d", WTERMSIG(status)); } - // Step 13 ends by restarting the discipline at the fast cadence. + // Repair step 7 ends by restarting the discipline at the fast cadence. daemon->polls = 0; (void)arm_timer(daemon); } @@ -3485,7 +3489,7 @@ static void handle_signals(struct daemon *daemon) while (read(daemon->signals, &info, sizeof(info)) == sizeof(info)) { if (info.ssi_signo == SIGTERM) _exit(0); - // sigqueue() carries the readiness path's wall time. Step 13 is due + // sigqueue() carries the readiness path's wall time. Step 7 is due // DEFERRED_START_NS later; the repair paired the clock with the // packet's UTC now. if (info.ssi_signo == SIGUSR1) { @@ -3587,7 +3591,7 @@ static void daemon_loop(struct daemon *daemon) arm_timer(daemon); } now = clock_ns(CLOCK_MONOTONIC); - // Step 13 starts DEFERRED_START_NS after the ack, and continues at + // Step 7 starts DEFERRED_START_NS after the ack, and continues at // normal priority IDLE_BOUND_NS after it starts. if (daemon->restore_pending && daemon->worker_pid == 0 && now >= daemon->restore_due_ns) { @@ -3975,8 +3979,8 @@ static int cmd_pre_capture(void) int64_t start; int64_t cpu; - // Step 1: the asynchronous boot checks start the daemon that C10 - // requires. + // Preparation step 1: the asynchronous boot checks start the daemon that + // C10 requires. if (wait_for_checks(&state, true) != 0) { memset(&checks, 0, sizeof(checks)); checks.phase = PHASE_CAPTURE; @@ -4055,10 +4059,10 @@ static void apply_repair(struct time_state *state, const void *context) state->samples++; snprintf(state->last_sample_error, sizeof(state->last_sample_error), "none"); - // Step 8 marks this restore's check pending, so nvx-time status waits for - // the daemon's step 13. Until then, restore_elapsed_us is the readiness - // path up to the clock step, at last_step_realtime_ns; restore_cpu_us - // counts only step 13, which records it. + // Repair step 3 marks this restore's check pending, so nvx-time status + // waits for the daemon's step 7. Until then, restore_elapsed_us is the + // readiness path up to the clock step, at last_step_realtime_ns; + // restore_cpu_us counts only step 7, which records it. snprintf(state->checks[PHASE_RESTORE].status, sizeof(state->checks[PHASE_RESTORE].status), "pending"); state->checks[PHASE_RESTORE].cpus = 0; @@ -4074,12 +4078,12 @@ static int repair_failed(const char *code, const char *detail) return 1; } -// Restore steps 6 to 8: read the packet with four-byte reads, validate it, +// Repair steps 2 and 3: read the packet with four-byte reads, validate it, // and set the wall clock from its bracketed UTC, then write the entropy to // ENTROPY_FD, the caller's file at ENTROPY_PATH. START_NS is when the capture // request returned. The caller holds the portb lock. With FINISH (untiered) // and a DAEMON, a restore with no processors or memory to activate needs no -// shell work before the acknowledgement, so steps 12 and 13 start here too +// shell work before the acknowledgement, so steps 6 and 7 start here too // and no second helper process starts; the metadata then begins with 2 // instead of 1. static int repair_restore(const uint8_t *previous_id, const char *entropy_path, @@ -4154,7 +4158,7 @@ static int repair_restore(const uint8_t *previous_id, const char *entropy_path, repair.elapsed_us = (clock_ns(CLOCK_MONOTONIC) - start_ns) / NSEC_PER_USEC; repair.packet = &packet; // A plain untiered restore needs no shell work: this helper also - // acknowledges and hands step 13 to the daemon. + // acknowledges and hands step 7 to the daemon. finishing = finish && daemon > 0 && packet.online_vp_count == 0 && packet.range_count == 0 && (packet.flags & PACKET_MEMORY_TARGET) == 0; @@ -4191,9 +4195,9 @@ static int repair_restore(const uint8_t *previous_id, const char *entropy_path, } // Writes the capture request to 0x605. In a restored process the write -// returns with a restore packet available, and steps 6 to 8 run here before -// any other guest work. Prints 0 when no restore happened. The portb lock is -// held from the request to the restored clock, so the snapshot never +// returns with a restore packet available, and repair steps 1 to 3 run here +// before any other guest work. Prints 0 when no restore happened. The portb +// lock is held from the request to the restored clock, so the snapshot never // contains a discipline poll that would apply a pre-capture sample after the // restore. static int cmd_capture(int argc, char **argv) @@ -4263,8 +4267,8 @@ static int cmd_capture(int argc, char **argv) return result; } -// Restore steps 12 and 13 after the shell's activation and identity work: the -// acknowledgement when the packet asked for one, then the daemon runs step 13 +// Repair steps 6 and 7 after the shell's activation and identity work: the +// acknowledgement when the packet asked for one, then the daemon runs step 7 // asynchronously. The record tells it which CPUs activation onlined. The // readiness path started restore_elapsed_us before the helper's clock step, // at last_step_realtime_ns. @@ -4323,7 +4327,7 @@ static int cmd_restore_finish(int argc, char **argv) return 1; } // Report-only runs may lack the daemon; finish the restore here, where - // step 13's CPU time is restore_work's own. + // step 7's CPU time is restore_work's own. if (ack) outb(SNAPSHOT_ACKNOWLEDGE, PORT_SNAPSHOT); timing.start_ns = clock_ns(CLOCK_MONOTONIC); @@ -4821,10 +4825,10 @@ static void test_fork_burn(void) burn_cpu(g_test_fork_burn_ms); } -// Forks a step 13 worker as start_restore does, after PRE_MS of preparation, -// with a fork that costs the parent FORK_MS; the worker spends CHILD_MS and -// prints its preparation share and step 13's CPU time. -static int test_step13_cpu(int argc, char **argv) +// Forks a repair step 7 worker as start_restore does, after PRE_MS of +// preparation, with a fork that costs the parent FORK_MS; the worker spends +// CHILD_MS and prints its preparation share and step 7's CPU time. +static int test_deferred_restore_cpu(int argc, char **argv) { struct restore_timing timing = {0}; int64_t start = cpu_time_ns(); @@ -4837,12 +4841,12 @@ static int test_step13_cpu(int argc, char **argv) if (pthread_atfork(NULL, test_fork_burn, NULL) != 0) return 1; burn_cpu(strtoll(argv[0], NULL, 10)); - pid = fork_step13(start, &timing); + pid = fork_deferred_restore(start, &timing); if (pid == 0) { burn_cpu(strtoll(argv[2], NULL, 10)); printf("prior_ms=%" PRId64 " total_ms=%" PRId64 "\n", timing.prior_cpu_ns / NSEC_PER_MSEC, - step13_cpu_ns(&timing) / NSEC_PER_MSEC); + deferred_restore_cpu_ns(&timing) / NSEC_PER_MSEC); fflush(stdout); _exit(0); } @@ -4963,8 +4967,8 @@ static int cmd_test(int argc, char **argv) } if (strcmp(name, "restore-record") == 0) return test_restore_record(argc, argv); - if (strcmp(name, "step13-cpu") == 0) - return test_step13_cpu(argc, argv); + if (strcmp(name, "deferred-restore-cpu") == 0) + return test_deferred_restore_cpu(argc, argv); if (strcmp(name, "deferral") == 0 && argc == 0) { printf("deferred_start_ms=%" PRId64 " idle_bound_ms=%" PRId64 "\n", (int64_t)(DEFERRED_START_NS / NSEC_PER_MSEC), diff --git a/openvmm b/openvmm index 7f225e2cd..c6d97c529 160000 --- a/openvmm +++ b/openvmm @@ -1 +1 @@ -Subproject commit 7f225e2cd635bbd1e1839e587e659996433396c8 +Subproject commit c6d97c529986c0b3b1bf1fd7f00406654cdeee67 diff --git a/scripts/nvx_tools/doctor.py b/scripts/nvx_tools/doctor.py index 07223175b..458c02a9d 100644 --- a/scripts/nvx_tools/doctor.py +++ b/scripts/nvx_tools/doctor.py @@ -901,7 +901,7 @@ def check_guest_warp(context: DoctorContext) -> CheckResult: boots.append(boot) rounds.extend(count_rounds) # The boot check's wall time and, where the guest reports it, its CPU - # time, which the spec budgets; both are evidence only. + # time, which CHECK_CPU_BUDGET_US budgets; both are evidence only. cpu_us = f" boot_cpu_us={boot['cpu_us']}" if "cpu_us" in boot else "" runs.append( f"vcpus={count} rounds={len(count_rounds)} " @@ -1015,7 +1015,7 @@ def check_utc(context: DoctorContext) -> CheckResult: def run_checks(context: DoctorContext, checks: Sequence[str]) -> list[CheckResult]: - """Run checks in spec order; an exception fails only its own check.""" + """Run checks in ID order; an exception fails only its own check.""" results: list[CheckResult] = [] for check in CHECK_IDS: if check not in checks: @@ -1143,7 +1143,7 @@ def configure_parser(parser: argparse.ArgumentParser) -> None: nargs="+", choices=CHECK_IDS, metavar="ID", - help="checks to run, in spec order (default: H1 to H7)", + help="checks to run, in ID order (default: H1 to H7)", ) parser.add_argument( "--openvmm", type=Path, help="OpenVMM binary for H2, H3, and H6" diff --git a/scripts/nvx_tools/microvm_tests.py b/scripts/nvx_tools/microvm_tests.py index 0da1853fa..86a21e057 100644 --- a/scripts/nvx_tools/microvm_tests.py +++ b/scripts/nvx_tools/microvm_tests.py @@ -142,8 +142,9 @@ # the guest-artifact build produces and packages omit, so they run only when # named. PUBLIC_SANDBOX_SCENARIOS = frozenset(("managed-exec-config", "sandbox-lifecycle")) -# The spec runs the same-host restore cases on the CI debug kernel, whose -# soft-lockup and hung-task detectors the guest's time ABI watcher reports. +# The same-host restore cases also run on the CI debug kernel (doc/ci.md), +# whose soft-lockup and hung-task detectors the guest's time ABI watcher +# reports. DEBUG_KERNEL_SCENARIOS = ( "smp", "smp-snapshot", @@ -302,8 +303,8 @@ # Every scenario captures a cold-booted guest (generation 0), and OpenVMM # cannot capture a restored one, so every restore carries generation 1. RESTORED_GENERATION = 1 -# The spec's long-downtime case: longer than the 21 s RCU stall timeout, at 1 -# and 8 vCPUs, with and without expedited grace periods. +# The time ABI's long-downtime case (doc/ci.md): longer than the 21 s RCU +# stall timeout, at 1 and 8 vCPUs, with and without expedited grace periods. RESTORE_DOWNTIME_SECONDS = 30.0 RESTORE_DOWNTIME_PROCESSORS = (1, 8) RESTORE_DOWNTIME_COMPLETION_MARKER = b"NVX-RESTORE-DOWNTIME-OK" diff --git a/scripts/nvx_tools/time_abi.py b/scripts/nvx_tools/time_abi.py index 24b2d0903..2b725b365 100644 --- a/scripts/nvx_tools/time_abi.py +++ b/scripts/nvx_tools/time_abi.py @@ -100,7 +100,10 @@ def describe(self) -> str: @dataclass(frozen=True) class CpuGeneration: - """One CPU generation of the spec's CPU profile catalog.""" + """One CPU generation of the CPU profile catalog. + + doc/design/2.2-cpu-and-topology.md defines the CPU profiles. + """ name: str vendor: str @@ -959,8 +962,9 @@ def check_warp_probe( ) -> list[dict[str, str]]: """Validate every warp probe summary in console output. - The spec bounds both the backward step and the ping-pong offset by 1 us - and forbids stalled pairs; an inconclusive measurement proves nothing. + Both the backward step and the ping-pong offset must stay within the time + ABI's 1 us cross-CPU skew bound (doc/design/3.2-clock-and-time.md), and no + pair may stall; an inconclusive measurement proves nothing. ``rounds``, when given, is the exact number of probe rounds expected. """ summaries: list[dict[str, str]] = [] diff --git a/scripts/test_guest_time.py b/scripts/test_guest_time.py index c60144734..7a95b82db 100644 --- a/scripts/test_guest_time.py +++ b/scripts/test_guest_time.py @@ -1,10 +1,11 @@ #!/usr/bin/env python3 -"""Spec-derived tests for the guest time component, guest/common/nvx-time.c. +"""Tests for the guest time component, guest/common/nvx-time.c. -The fixtures encode the time ABI's guest-visible formats, such as the restore -packet (doc/design/5.5-restore-protocol.md) and the time sample -(doc/design/3.2-clock-and-time.md), independently of the C decoders, so the -tests check the guest against the specification. +The fixtures encode the guest-visible formats independently of the C decoders. +The restore packet (doc/design/5.5-restore-protocol.md) and the time sample +(doc/design/3.2-clock-and-time.md) follow the guide; the other fixtures, such +as the exhaustive check's lines and the state file, pin the formats that +nvx-time.c defines. """ from __future__ import annotations @@ -47,7 +48,7 @@ def encode_packet( reserved: int = 0, entropy: bytes = ENTROPY, ) -> bytes: - """Encode a restore packet v4 (spec: "Restore packet").""" + """Encode a restore packet v4 (guide: 5.5.5 "Restore Packet").""" count = len(ranges) if range_count is None else range_count header = magic + bytes((version, flags, online, count, reserved)) header += struct.pack(" bytes: - """Encode a time sample (spec: "Time sample").""" + """Encode a time sample (guide: 3.2.9 "Time Samples").""" return struct.pack(" str: self.assertIn('(signature "KVMKVMKVM...")', signature) self.assertTrue(signature.endswith("or the highest basic leaf"), signature) - def test_exhaustive_report_lines_follow_the_spec(self): - # The spec's exhaustive check prints one line per check and CPU, with - # an escaped detail, then a summary. + def test_exhaustive_report_format(self): + # The exhaustive check prints one line per check and CPU, with an + # escaped detail, then a summary. line = re.compile( r"^NVX-TIME-ABI-EXHAUSTIVE: v=1 check=X[1-6] cpu=\d+ " r'status=(pass|fail) detail="(?:[^"\\]|\\.)*"$' @@ -734,7 +735,7 @@ def test_violation_event_format_escaping_and_limit(self): self.assertTrue(long_line.endswith('\\x02"\n')) def test_state_file_round_trip(self): - # The keys and their order follow the spec's state file table. + # The fixture lists every key, in the order that nvx-time writes them. text = ( "version=1\ngeneration=2\nboot_status=ok\ncapture_status=ok\n" "restore_status=pending\nboot_cpus=8\ncapture_cpus=8\nrestore_cpus=0\n" @@ -853,20 +854,20 @@ def test_status_prints_every_recorded_check_and_the_runtime_line(self): self.assertEqual(report[4], "ok=0") def test_deferred_work_starts_150_ms_late_and_is_promoted_100_ms_later(self): - # Step 13 and the asynchronous boot checks start 150 ms after the - # acknowledgement or the boot step and leave SCHED_IDLE 100 ms after - # they start. + # Guest repair step 7 and the asynchronous boot checks start 150 ms + # after the acknowledgement or the boot step and leave SCHED_IDLE + # 100 ms after they start. self.assertEqual( self.run_test("deferral"), "deferred_start_ms=150 idle_bound_ms=100\n" ) - def test_restore_cpu_time_is_step_13_with_the_daemons_fork(self): - # The worker's step 13 CPU time adds the daemon's preparation before - # the fork, the fork's cost to the daemon, which arrives on a pipe - # after the worker's copy of the timing was taken, and its own. + def test_restore_cpu_time_is_the_deferred_work_with_the_daemons_fork(self): + # The worker's repair step 7 CPU time adds the daemon's preparation + # before the fork, the fork's cost to the daemon, which arrives on a + # pipe after the worker's copy of the timing was taken, and its own. fields = dict( field.split("=") - for field in self.run_test("step13-cpu", "20", "30", "10").split() + for field in self.run_test("deferred-restore-cpu", "20", "30", "10").split() ) self.assertGreaterEqual(int(fields["prior_ms"]), 20) self.assertLess(int(fields["prior_ms"]), 30) diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index 335bba137..44f885a13 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -4067,7 +4067,8 @@ def test_snapshot_restore_runs_the_time_abi_steps_through_nvx_time(self): 'memtotal_kib=$memtotal_kib elapsed_us=0"', snapshot, ) - # Capture steps 1 to 3 precede the barriers and the request. + # Guest preparation steps 1 and 2 (doc/design/5.3-snapshot-protocol.md) + # precede the barriers and the request. pre_capture = snapshot.index("/sbin/nvx-time pre-capture || exit 1") freeze = snapshot.index('echo 1 >"$container_cgroup/cgroup.freeze"') capture = snapshot.index("/sbin/nvx-time capture") diff --git a/scripts/test_time_abi.py b/scripts/test_time_abi.py index d6728b853..27f2554a7 100644 --- a/scripts/test_time_abi.py +++ b/scripts/test_time_abi.py @@ -756,8 +756,8 @@ def test_cpu_profile_requests_match_openvmm_and_the_issue_form(self): self.assertIn(key, source.stdout) def test_computes_the_checks_cpu_time_budget(self): - # The spec's final budgets: a base plus an increment per additional - # CPU, one budget per backend and phase. + # The final budgets: a base plus an increment per additional CPU, one + # budget per backend and phase. for backend in ("kvm", "mshv", "whp"): self.assertEqual( set(time_abi.CHECK_CPU_BUDGET_US[backend]), @@ -768,12 +768,12 @@ def test_computes_the_checks_cpu_time_budget(self): self.assertEqual(time_abi.check_cpu_budget_us("kvm", "capture", 8), 4_300) self.assertEqual(time_abi.check_cpu_budget_us("kvm", "restore", 1), 7_000) self.assertEqual(time_abi.check_cpu_budget_us("kvm", "restore", 8), 21_000) - # The spec's binding MSHV boot cell: 4 vCPUs (4.94 ms measured). + # The binding MSHV boot cell: 4 vCPUs (4.94 ms measured). self.assertEqual(time_abi.check_cpu_budget_us("mshv", "boot", 4), 6_000) self.assertEqual(time_abi.check_cpu_budget_us("mshv", "restore", 8), 6_000) self.assertEqual(time_abi.check_cpu_budget_us("whp", "restore", 1), 35_000) - # The spec's binding WHP cells: boot at 2 vCPUs (21.84 ms measured on an - # 8573C runner), capture at 1 (0.986 ms), and restore at 4 (42.20 ms). + # The binding WHP cells: boot at 2 vCPUs (21.84 ms measured on an 8573C + # runner), capture at 1 (0.986 ms), and restore at 4 (42.20 ms). self.assertEqual(time_abi.check_cpu_budget_us("whp", "boot", 2), 26_500) self.assertEqual(time_abi.check_cpu_budget_us("whp", "capture", 1), 1_200) self.assertEqual(time_abi.check_cpu_budget_us("whp", "capture", 2), 1_600)