ionescu007 / SimpleVisor

SimpleVisor is a simple, portable, Intel VT-x hypervisor with two specific goals: using the least amount of assembly code (10 lines), and having the smallest amount of VMX-related code to support dynamic hyperjacking and unhyperjacking (that is, virtualizing the host state from within the host). It works on Windows and UEFI.
http://ionescu007.github.io/SimpleVisor/
1.69k stars 259 forks source link

ShvVmxEntryHandler does not enable interrupts #3

Closed alexalexroro closed 8 years ago

alexalexroro commented 8 years ago

I couldn't help but notice in the ShvVmxEntryHandler function you state in your comment that you want to receive clock and IPI interrupts to occur and as a result you raise the IRQL appropriately.

However, on each VMEXIT the value of the RFLAGS register is set to 0x2 (as stated in 27.5.3 of Intel System Programming Manual Volume 3). As a result if you want to be able to receive interrupts while in VMX root mode you need to enable interrupts after raising the IRQL.

ionescu007 commented 8 years ago

You're right, the hypervisor currently runs with interrupts disabled, which actually makes the IRQL transition more of a "hey Windows, by the way...", and HIGH_LEVEL would be more appropriate. I need to think through the effects of actually allowing Clock and IPI to come in, because the OS is highly allergic to threads not running on a real OS stakck.

So I may either fix the comment and change the IRQL to HIGH_LEVEL, or, if testing is successful, enable interrupts. Thanks.

alexalexroro commented 8 years ago

I don't think you can get away with enabling interrupts in VMX root mode: the problem is that a VM exit might occur when the processor is already at a very high IRQL (>= CLOCK_LEVEL) and you will call KeRaiseIrql with a lower IRQL than the previous one.

The easiest way to replicate this behavior this is to enable RDTSC exiting and see that this exit sometimes occurs when handling IPIs (in KiIpiInterruptSubDispatch to be more precise).

ionescu007 commented 8 years ago

That seems like it could be fixed by checking the current IRQL before raising.

ionescu007 commented 8 years ago

Hey Alex,

So really there are two issues here.

First, the unconditional raise to MAX_DIRQL is always a problem, because if you do get a VMEXIT at IPI level, you are screwed, as I will be raising to a lower IRQL, regardless of interrupt state, this is illegal. One fix for this is to check the current IRQL, as CR8 will have the guest value.

The second is around enabling interrupts. I played around with this, but as soon as I enabled interrupts, I started getting bugchecks -- because we are not really on a valid NT kernel stack, there are some validation functions + PatchGuard checks + assumptions in the kernel that will break when entering/exiting interrupt context. Plus now that I think about it, it probably isn't really correct for what is essentially a guest interrupt to be running in VMX root context.

So as a fix, I will update the comment, and raise IRQL to HIGH_LEVEL. This takes care of both issues, and is more correct.