Disallow MSR read/writes

phase1-kvm-handoff.md

@@ -0,0 +1,172 @@
+# Phase 1 handoff: MSR reset on KVM
+
+This brief is for an agent working on a Linux/KVM machine. It implements Phase 1
+of `plan.md` (KVM only). Read `plan.md` first, then this. This brief pins the
+starting point, the exact files, the task order, and the done criteria. Every
+factual claim here was verified against the source or the pinned crates.
+
+## Scope
+
+Phase 1 only. KVM backend. Split reset from access control:
+* Add MSR reset (capture in snapshot, write back in `reset_vcpu`).
+* Replace the `allow_msr` bool with a validated per-MSR allow list.
+* Keep the KVM default-deny filter, add allow ranges so allowed MSRs pass through.
+* Tests.
+
+Out of scope for Phase 1: mshv, WHP, the `0a` CPUID masking work, LBR/MTRR
+decisions beyond CPUID gating, persistence schema changes. Do not start those.
+
+## Why this change (one paragraph)
+
+`reset2` denies MSR access but never resets MSR state, and its `allow_msr` bool is
+an all-or-nothing `unsafe` switch that disables the filter entirely and leaks
+state across `restore()`. Reset (save in snapshot, restore on `restore()`) is the
+isolation guarantee. Access control (deny-by-default plus a per-MSR allow list)
+bounds what the guest can write so the reset set is complete by construction. See
+`plan.md` §2 for the full rationale.
+
+## Starting point
+
+* Branch: detached at `origin/reset2` @ `68ad5265` "Disallow guest from
+  read/write MSRs by default". Create a working branch from it.
+* Environment check before anything else. Confirm `/dev/kvm` exists and is
+  read/write for the current user. If a sandbox fails with "No Hypervisor was
+  found", print `ls -l /dev/kvm` and group membership and stop.
+
+## What `reset2` already ships (verified anchors)
+
+All KVM-only, under `src/hyperlight_host/src/`.
+
+* KVM deny-all filter: `enable_msr_filter()` in
+  `hypervisor/virtual_machine/kvm/x86_64.rs` (~line 357). Sets
+  `KVM_MSR_FILTER_DEFAULT_DENY` with one dummy all-zero-bit range plus
+  `enable_cap(X86UserSpaceMsr, Filter)`.
+* Filter wired at VM creation: `hypervisor/hyperlight_vm/x86_64.rs` (~line 99),
+  calls `enable_msr_filter()` unless `config.get_allow_msr()`.
+* Filter exits: `kvm/x86_64.rs` handles `X86Rdmsr`/`X86Wrmsr` (~lines 236-252 and
+  313-333) with the `error=1` plus `immediate_exit` handshake, injecting `#GP`,
+  returning `VmExit::MsrRead`/`MsrWrite`.
+* `VmExit` variants: `hypervisor/virtual_machine/mod.rs` (~lines 144, 147).
+* Violation mapping: `hypervisor/hyperlight_vm/mod.rs` maps `MsrRead`/`MsrWrite`
+  (~lines 752-757) to `RunVmError::MsrReadViolation`/`MsrWriteViolation`
+  (~lines 228, 231), then to the matching `HyperlightError` (~lines 167-173).
+* Config: `sandbox/config.rs` has `allow_msr: bool` (~lines 79, 125) and
+  `unsafe fn set_allow_msr(bool)` / `get_allow_msr()` (~lines 177, 184).
+* Config use: `sandbox/initialized_multi_use.rs` calls `set_allow_msr(true)`
+  (~line 2777). Update this call site when the API changes.
+* Reset today: `reset_vcpu` in `hypervisor/hyperlight_vm/x86_64.rs` (~line 348)
+  resets only the `sregs` MSRs. `apply_sregs` (~lines 262-269) calls `set_sregs`.
+  `get_snapshot_sregs` (~line 274).
+* Snapshot: `sandbox/snapshot/mod.rs` `Snapshot` struct (~line 70). `snapshot()`
+  in `sandbox/initialized_multi_use.rs` (~line 356).
+* Registers: `hypervisor/regs/x86_64/special_regs.rs`. `CommonSegmentRegister`
+  carries `base` (~line 379), so `fs.base`/`gs.base` restore through `set_sregs`.
+
+## Verified facts that shape the code
+
+* `FS_BASE`/`GS_BASE` reset through `sregs` on every restore. Do NOT put them in
+  `CommonMsrs`. Only `KERNEL_GS_BASE` needs MSR reset.
+* Guest CPUID is host pass-through on KVM. `kvm/x86_64.rs` (~lines 155-165) starts
+  from `get_supported_cpuid()` and modifies only leaf `0x8000_0008`. So the guest
+  sees host MTRR/CET/FRED/LBR/PMU. `KVM_GET_MSR_INDEX_LIST` tracks this, so Phase 1
+  needs no CPUID masking.
+* In-kernel LAPIC exists under the `hw-interrupts` feature (`create_irq_chip`,
+  `kvm/x86_64.rs` ~line 142). It is xAPIC, not x2APIC. LAPIC reset (§7) is part of
+  Phase 3, but keep the ordering hook in mind.
+* No TSC setup anywhere. Bare TSC is monotonic across VP reuse. Reset `TSC_ADJUST`
+  to baseline, never write raw `TSC`. Mark `TSC`/`TSC_AUX` `host_specific`.
+* `kvm-ioctls` `get_msrs`/`set_msrs` are batched. `KVM_SET_MSRS` returns a count
+  and stops at the first bad index. A short count means fail closed and poison.
+
+## API decision to make first
+
+Replace `unsafe fn set_allow_msr(bool)` on `SandboxConfiguration` with a safe,
+fallible allow list. Suggested shape, adjust to house style:
+
+```rust
+// Add each MSR the guest may read and write. Validated at init (see plan §10).
+pub fn allow_msr(&mut self, index: u32) -> &mut Self;
+pub fn allow_msrs(&mut self, indices: &[u32]) -> &mut Self;
+```
+
+Default allow list is empty (`plan.md` §0). Drop the `unsafe` bypass. Allowing an
+MSR must never disable reset. Un-gate the field from `#[cfg(kvm)]` so later phases
+reuse it (the KVM filter wiring stays `#[cfg(kvm)]`).
+
+## Task order (small, focused commits)
+
+1. Data model. Add `CommonMsrs = Vec<MsrEntry>` with `MsrEntry { index, value,
+   class, flags }` and the `MsrClass`/`MsrFlags` enums (`plan.md` §5). Put it near
+   `regs/x86_64/`. `host_specific` marks `TSC`/`TSC_AUX`.
+2. Static table plus derivation (§3, §4). Author the per-index table (class and
+   gating CPUID). Derive the resolved reset set from `KVM_GET_MSR_INDEX_LIST` minus
+   the `HostOwned` class, expanded with the table. Add MTRRs from CPUID. An
+   enumerated writable index the table cannot classify fails closed (probe get/set,
+   add if stateful, else refuse VM creation). Cache per process with `LazyLock`.
+3. Backend get/set. KVM `get_msrs`/`set_msrs` over `CommonMsrs`. Short count
+   poisons and fails closed.
+4. Snapshot wiring (§6). Add `Snapshot::msrs: Option<CommonMsrs>`. Capture in
+   `snapshot()` after `get_snapshot_sregs()`, `Stateful` entries only. In
+   `reset_vcpu`: `apply_sregs` (EFER first), then batched `SET_MSRS` of the
+   `Stateful` set (no `host_specific` entry in the batch), then `TSC_ADJUST`.
+   Init baseline for `msrs == None`.
+5. Access control (§9). Replace the `allow_msr` bool with the allow list. Build
+   `KVM_X86_SET_MSR_FILTER` DEFAULT_DENY plus allow ranges for opted-in MSRs.
+   Allowed MSRs pass through, denied MSRs keep the existing `#GP`/poison path.
+6. Validation (§10). At init, for each allowed MSR: confirm host has it (index
+   list), confirm it is `Stateful` and settable back to baseline, else hard error.
+   Add it to the reset set and the allow ranges.
+7. Tests (below).
+
+## Tests (Phase 1 subset of §12)
+
+* Fixed-list differential. For each reset-set MSR, guest writes a sentinel,
+  `restore()`, host reads back, assert baseline.
+* KVM index-list sweep. Sweep `KVM_GET_MSR_INDEX_LIST`, write sentinel, restore,
+  assert baseline.
+* Opt-out. Allow `KERNEL_GS_BASE`. Assert the guest reads and writes it, assert
+  `restore()` returns it to baseline, assert a non-resettable allow request errors.
+* Fail closed. Default-deny installs. Non-get/settable allow errors. Simulated
+  partial `SET_MSRS` poisons. An unclassifiable enumerated writable MSR fails at
+  VM creation, not silently dropped.
+* Negative leak. Write a sentinel, `restore()`, assert it is gone.
+
+## Build and test loop
+
+```bash
+just guests        # REQUIRED before tests, builds the guest binaries
+just test          # debug
+just test release  # release
+```
+
+Before pushing:
+
+```bash
+just fmt-apply
+just clippy debug
+just clippy release
+just test-like-ci
+just test-like-ci release
+```
+
+Commits must be signed and DCO signed off: `git commit -S --signoff`. Keep commits
+small and logically ordered. Rebase on `main`, no merge commits. Label the PR per
+`docs/github-labels.md`.
+
+## Done criteria
+
+* A guest MSR write to any reset-set MSR does not survive `restore()`.
+* An allowed MSR is readable and writable by the guest and still resets.
+* A non-resettable allow request is a hard init error, no `unsafe` bypass remains.
+* The default-deny filter installs, denied MSRs `#GP` and poison.
+* `just test-like-ci` and `just test-like-ci release` pass on Intel and, if
+  available, AMD.
+
+## Pitfalls
+
+* Do not write raw `TSC`. It rewinds guest time. Reset `TSC_ADJUST` only.
+* Do not add `FS_BASE`/`GS_BASE` to `CommonMsrs`. They reset via `sregs`.
+* Do not defer an unclassified writable MSR to a nightly sweep. Fail closed.
+* Do not reintroduce an `unsafe` allow-all. Allowing must never disable reset.
+* `KVM_MSR_FILTER_DEFAULT_DENY` needs at least one range. Keep the dummy all-zero
+  range and add real allow ranges alongside it.