search

coconut-svsm / svsm

COCONUT-SVSM
MIT License
122 stars 42 forks source link

kernel/idt: Ensure #HV handler RIP checks are executed when interrupts are enabled #491

Closed roy-hopkins closed 1 month ago

roy-hopkins commented 1 month ago

When testing restricted injection using an APIC timer firing into VMPL0 I experienced errors reported by the GHCB.rs in the code that detects whether the switch to VMPL2 was successful. This was down to a couple of separate issues. One of which is fixed by this PR.

The current #HV handler checks EFLAGS.IF=1 at the start of the handler and immediately processes the #HV events if enabled without performing checks to see if RIP is within the VMPL switch or the iret window. The RIP windows are only checked if EFLAGS.IF=0. This means that if interrupts are enabled during either of these windows and a #HV occurs then the behaviour is undefined.

msft-jlange commented 1 month ago

The current #HV handler checks EFLAGS.IF=1 at the start of the handler and immediately processes the #HV events if enabled without performing checks to see if RIP is within the VMPL switch or the iret window. The RIP windows are only checked if EFLAGS.IF=0. This means that if interrupts are enabled during either of these windows and a #HV occurs then the behaviour is undefined.

Roy, the current behavior is explicitly by design, since those code paths are always executed with interrupts disabled, and thus there should be no point in RIP checks when interrupts are enabled. Can you explain what problem you're trying to fix?

msft-jlange commented 1 month ago

PR #492 is the correct way of handling VMPL switch interactions with #HV.

msft-jlange commented 1 month ago

PR #492 is the correct way of handling VMPL switch interactions with #HV.

This PR is no longer necessary now that #492 has merged.

roy-hopkins commented 1 month ago

So the problem I am trying to fix is to allow interrupts occurring for VMPL0 to preempt a running VMPL2. I think I've misunderstood how this was supposed to work - with the 'aborted' switch to VMPL2 from the #HV handler being used to transfer control back to VMPL0. I'm still trying to understand how the new (now merged) PR #492 works in this situation but agree this PR is not the correct answer so I'll close it.