On 17/02/2025 4:51 pm, Jan Beulich wrote:
> On 17.02.2025 17:12, Andrew Cooper wrote:
>> There is a corner case in the VMRUN instruction where its INTR_SHADOW state
>> leaks into guest state if a VMExit occurs before the VMRUN is complete. An
>> example of this could be taking #NPF due to event injection.
> Ouch.
Yeah. Intel go out of their way to make VM{LAUNCH,RESUME} fail if
they're executed in a shadow.
>
>> --- a/xen/arch/x86/hvm/svm/entry.S
>> +++ b/xen/arch/x86/hvm/svm/entry.S
>> @@ -57,6 +57,14 @@ __UNLIKELY_END(nsvm_hap)
>>
>> clgi
>>
>> + /*
>> + * Set EFLAGS.IF, after CLGI covers us from real interrupts, but not
>> + * immediately prior to VMRUN. AMD CPUs leak Xen's INTR_SHADOW from
>> + * the STI into guest state if a VMExit occurs during VMEntry
>> + * (e.g. taking #NPF during event injecting.)
>> + */
>> + sti
>> +
>> /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
>> /* SPEC_CTRL_EXIT_TO_SVM Req: b=curr %rsp=regs/cpuinfo, Clob:
>> acd */
>> .macro svm_vmentry_spec_ctrl
> I'm mildly worried to see it moved this high up. Any exception taken in
> this exit code would consider the system to have interrupts enabled, when
> we have more restrictive handling for the IF=0 case. Could we meet in the
> middle and have STI before we start popping registers off the stack, but
> after all the speculation machinery?
Any exception taken here is fatal, and going to fail in weird ways.
e.g. we don't clean up GIF before entering the crash kernel.
But yes, we probably should take steps to avoid the interrupted context
from looking even more weird than usual.
I'll put it above the line of pops. They're going to turn into a single
macro when I can dust off that series.
~Andrew
