virt_whp: Add a 1 second periodic unstick timer

[Steven Malis (smalis-msft)]

We've recently discovered what appears to be a hypervisor bug, that can leave a guest stranded despite a pending interrupt. While debugging and fixing that is ongoing, add a workaround here: A periodic 1 second timer that checks for this condition and manually unsticks the VP when it is found. Hopefully this lets us keep moving and gets rid of a bunch of test flakiness.

[Copilot]

Pull request overview

This PR adds a workaround in the virt_whp backend to mitigate a suspected hypervisor issue where a VP can remain halted despite a pending interrupt in an offloaded APIC, by periodically checking for the stuck condition and forcibly clearing the halt state.

Changes:

• Add a per-VP, 1-second periodic VM-time timer used to trigger the “unstick” check.
• Implement APIC-state inspection helpers and a unhalt_for_pending_interrupt routine to clear the hypervisor halt/idle suspend state when an interrupt is ready.
• Wire the periodic check into the VP run loop (offloaded APIC only).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
vmm_core/virt_whp/src/vp.rs	Adds a periodic 1s timer poll in the VP run loop and triggers the unhalt workaround when it expires.
vmm_core/virt_whp/src/lib.rs	Extends RunState to hold an optional unhalt_check_vmtime accessor and initializes it when using an offloaded APIC.
vmm_core/virt_whp/src/apic.rs	Adds APIC bitmap/priority helpers and implements the “unhalt if pending interrupt should have woken VP” logic.

Suppressed comments (1)

vmm_core/virt_whp/src/apic.rs:510

• This path both unwraps get_apic() / set_register() and emits an un-rate-limited tracing::warn!. If the underlying WHP calls fail or the condition repeats, this can either crash the VMM or spam logs every second per VP. Prefer graceful error handling plus tracelimit::warn_ratelimited!.

        let apic = vp::ApicRegisters::from_page(&whp.get_apic().unwrap());
        if !apic_interrupt_ready(&apic) {
            return;
        }

        tracing::warn!(?vtl, "unhalting vp for pending apic interrupt");

[Chris Oo (chris-oo)]

probably a HACK: prefix on these changes is more searchable, since i think we use that along with TODO: etc?

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (4)

vmm_core/virt_whp/src/apic.rs:495

• This new periodic workaround uses get_registers!(...).unwrap(). If WHP returns an error (e.g. transient hypervisor failure), this would crash the VMM in a path intended to reduce flakiness. Prefer graceful handling + rate-limited logging.

        .unwrap();

vmm_core/virt_whp/src/apic.rs:518

• This warning + set_register(...).unwrap() can spam logs and crash the VMM if the hypervisor rejects the write. Since this runs on a periodic poll, it’s safer to use rate-limited logging and handle the error instead of panicking.

        tracing::warn!(?vtl, "unhalting vp for pending apic interrupt");
        whp.set_register(
            whp::Register64::InternalActivityState,
            activity
                .with_halt_suspend(false)
                .with_idle_suspend(false)
                .into(),
        )
        .unwrap();

vmm_core/virt_whp/src/vp.rs:362

• On non-x86_64 builds this block can still reach highest_set().unwrap() when the timer expires, but the actual unhalt call is cfg-gated away. That makes this workaround a potential panic point (and extra work) on guest_arch != "x86_64" even though it can’t do anything there.

                    if expired {
                        let _vtl = self.state.runnable_vtls.highest_set().unwrap();
                        #[cfg(guest_arch = "x86_64")]
                        self.unhalt_for_pending_interrupt(_vtl);
                    }

vmm_core/virt_whp/src/apic.rs:505

• whp.get_apic().unwrap() in this periodic workaround can also crash the VMM on a recoverable WHP error. Handling the error and returning keeps the workaround from becoming a new reliability hazard.

This issue also appears on line 510 of the same file.

        let apic = vp::ApicRegisters::from_page(&whp.get_apic().unwrap());

[github-actions]

1 Petri tests failed (0 unstable)

[John Starks (jstarks)]

I think this is too complicated. As I mentioned offline, I think you should just clear+set IF every second.

[Copilot]

Pull request overview

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

Suppressed comments (2)

vmm_core/virt_whp/src/vp.rs:367

• The unhalt workaround currently runs on a 1s period regardless of whether the VP is halted, so it will still toggle RFLAGS.IF every second even while the VP is actively running. That adds avoidable WHP register traffic and could perturb normal execution. Consider arming/polling this timer only when the VP is effectively halted, and cancel any pending timeout when the VP becomes runnable again.

                #[cfg(guest_arch = "x86_64")]
                {
                    const UNHALT_CHECK_PERIOD: std::time::Duration =
                        std::time::Duration::from_secs(1);

vmm_core/virt_whp/src/lib.rs:332

• RunState::reset currently ignores unhalt_check_vmtime, so any previously-armed timeout can carry across resets/scrubs and fire immediately afterward. Since this is a periodic workaround timer, it should be returned to a clean state on reset by canceling any pending timeout.

            ref mut halted,
            unhalt_check_vmtime: _,
            exits: _,
            vtl2_wakeup_vmtime: _,
            vmtime: _,

[github-actions]

6 Petri tests failed (0 unstable)

[Steven Malis (smalis-msft)]

Just toggling IF was not enough, we hit a timeout in this PR's run. Now we just reassert the stuck interrupt.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

vmm_core/virt_whp/src/apic.rs:476

• This workaround is intended to improve resilience in the presence of a hypervisor bug, but unwrap() here will panic the whole VMM if WHP fails to return registers. Please handle the error and return (ideally with a rate-limited log) instead of panicking.

        let (activity, rflags) = get_registers!(
            whp,
            [
                whp::Register64::InternalActivityState,
                whp::Register64::Rflags,
            ]
        )
        .unwrap();

vmm_core/virt_whp/src/vp.rs:370

• The 1s timer can expire even while the VP is actively running; in that case this will still call into unstick_halted_vp, which reads WHP registers/APIC state once per second per VP. Since this workaround is only meaningful when the VP is effectively halted, gate the unstick attempt on !ready to avoid unnecessary hypervisor calls in the common case.

                    if expired {

vmm_core/virt_whp/src/apic.rs:492

• whp.get_apic() can fail (and this code path exists specifically because the hypervisor is misbehaving). Using unwrap() here risks turning a recoverable WHP error into a process abort. Prefer handling the error and returning after logging (rate-limited), similar to other get_apic().for_op("get apic state")? call sites in this file.

        let apic = vp::ApicRegisters::from_page(&whp.get_apic().unwrap());

[github-actions]

33 Petri tests failed (4 unstable)