Is vmxoff path really safe/correct?

[ionescu007]

Hi,

I experimented with turning VMX off by using DPC routine targeted to each processor, with synchronization barrier. If DPCs were delivered as part of SYSTEM process (such as from idle thread, or from system thread), everything worked fine. However, if a process was currently running, and the DPC interrupted the process, many times the process crashes with invalid memory access, or sometimes a kernel driver itself crashes (such as win32kfull running on the context of dwm.exe), when accessing session memory.

These problems go away if doing VMX off the way you do it, by sticking with a single thread that is already running under SYSTEM process. However, even if you have a comment that suggests you saw some strangeness.

So therefore, I have two questions:

1. Are you sure that all the register state is correctly saved and restored? You only save integer registers, but not xmm0,1, 2, 3, 4, 5 on x64, which are volatile registers (and neither the other xmm registers). Why do you only save integer registers during vmm_exit and vmxoff restore? Shouldn't FPU registers be saved too?

Must vmx_launch and vmx_off be done in the same address space? In other words, will vmx_off restore the CR3 that was saved during vmx_launch? Because if so, I can understand the danger of capturing CR3 in system process (session -1), but then doing vmx_off in dwm.exe (session 0). The totally different CR3 will certainly cause a crash. Does this mean that it's only safe to do on/off from within SYSTEM process?

Thank you.

[tandasat]

Thank you for the through description of questions.

As for the second question, VMXOFF does not change a value of CR3. However, it does not mean that VMXON and VMXOFF can only be done on the SYSTEM process. As long as a value of CR3 after VMXOFF remains the same as that of before VM-exit, it will be fine.

In case of HyperPlatform, it simply sets the same CR3 value for both the SYSTEM process (VmcsField::kGuestCr3) and the VMM (VmcsField::kHostCr3), and does not restore the CR3 that was being used at the time of VMCALL after VMXOFF. For this reason, VMXOFF can only be safely performed from the SYSTEM process in current implementation. The following are changes of CR3 w.r.t. VMXOFF:

1. Before VMCALL (CR3 == VmcsField::kGuestCr3)
2. VMCALL causing VM-exit
   CR3 is overwritten by a value stored in VmcsField::kHostCr3 during transition between VMX non-root operation, a guest context, to VMX root operation mode, a VMM context (CR3 == VmcsField::kHostCr3)
3. VMXOFF (CR3 == VmcsField::kHostCr3)
4. After VMXOFF, a guest context (CR3 == VmcsField::kHostCr3)

So, if VMCALL leading to VMXOFF is executed with a different CR3 from that of VmcsField::kHostCr3, this would cause crashes, as an original CR3 is not going to be restored even after VMXOFF.

Does this answer your question clear enough?

As for the first question, I do not have an answer right now, but it sounds good pointing out. I will look into it and will reply here shortly.

Thank you,

[tandasat]

As for the restoring registers, you are right. I believe that the VMM should save and restore non-interger registers too if they can be modified by the VMM since they are not automatically saved and restored by a processor on VM-exit and VM-enter.

In case of HyperPlatform, it does not appear to be an issue because those unhandled registers are not modified by the VMM; or VM-exit does not happen under a situation where consistency of those registers is required. For this reason, I would hold off updates unless an concrete issue is seen.

Thank you,

[ionescu007]

Hi,

Thank you for the explanation on CR3. That makes a lot of sense. It looks like the requirement for SYSTEM could go away, if, after the vmcall, the CR3 would be captured, and overwritten in the kGuestCr3 field before the vmxoff. But I don't think it's worth doing that.

As for non-integer registers, on x64, the calling convention does support the compiler auto-generating function calls which use xmm registers. This will happen automatically in certain cases, in other cases, one must use __vectorcall as the calling convention to force this. Because of this, it is certainly possible for someone to have a vector function that is using volatile XMM registers, and hit a VMM Exit. Because you are using Visual Studio and because Windows Kernel x64 allows XMM registers, I think it's technically possible for hyperplatform.sys to end up using XMM registers -- for example if a memcpy becomes inlined + vectorized.

So I think this is taking a risk that certain optimization settings/further code might hit. It may be OK now, but not if someone adds more routines without realizing them.

Thanks for your insight!

[tandasat]

Thank you for sharing your knowledge on use of XMM registers. I was unaware of most of them.

I agree with your thoughts on potential risk of not fixing it. While the issue may not hit at this time, it would be tough to figure out a bug is caused by XMM registers for someone who are not aware of it. Given the fact that this project is meant to be a platform, fix should be considered. For now, I have created #5 for reference and potential future work.

I appreciate your thorough risk assessment and suggestion.

[rianquinn]

while working on bareflank.org, I ran into a similar issue with XMM, as I do believe they need to be addressed. The old vmxcpu code never saved these registers because it only ran on 32bit, which did not included XMM registers in the ABI, which is why we never included them in the MoRE hypervisor (as it's based on vmxcpu). On Bareflank, we use Libc++ which touches XMM and FPU registers, so these absolutely need to be saved in our case. Our current solution is to use the fxsave and fxrstor functions. In our state save area. Figured this info might help you guys in the future if you choose to fix this problem. Here is our PR that adds this code if you want an example

Bareflank/hypervisor#123

[ionescu007]

I highly recommend you use XSAVE/XRESTORE if available, otherwise you risk to miss certain registers and have hard to debug issues :)

• Assuming LibC++ might use such registers (if you start using AES-NI or AVX).

[tandasat]

Thanks for sharing your findings. While I am not entirely familiar with the extended states, looks like an XSAVE feature set is more comprehensive ones, as far as I can see from this slides from Intel.

I originally thought a chance of hitting this issue was small and negligible, but it does not seem convincing anymore since Libc++, which one of the most common libraries, is affected. Libc++ may not going to be used inside HyperPlatform, but I would not surprise if other libraries triggered the same issue.

I will study the extended states first (ie, supported processor generations, difference between XSAVE, -OPT, -S and -C, and any implications) and then will work on implementation for fix.

[rianquinn]

Thanks for the advice, yeah we will be using XSAVE as well. The PR that is there will be updated with a couple of additional fixes as I found a nasty issue with -O3 as GCC uses SSE a lot with the heavy optimizations enabled. The problem is, GCC assumes the stack is 16 byte aligned, and so we were seeing segfaults as a result. The other thing I was missing was a restore on "promote" when coming out of VMX-non root. I should have my fixes in today for these.

[tandasat]

As far as preservation of x87, SSE and AVX are concerned (and that assumption is fine I think--need better understanding), the simplest way seems to use XSAVE and XRESTORE. It will require only checks for:

• CPUID function 1:ECX.XSAVE[bit 26] == 1 to check support of the XSAVE
• CPUID function 0DH, sub-function 0 to check if AVX state is supported

If either of them is not supported, using FXSAVE and FXRSTOR is the cleanest solution as they "save and restore the states of the XMM registers and the MXCSR register (SSE state), along with x87 state" (FXSAVE AND FXRSTOR INSTRUCTIONS), or RtlCaptureContext() as they seem to save SSE state too.

Some notes below.

Terms and Basics (XSAVE-SUPPORTED FEATURES AND STATE-COMPONENT BITMAPS)

• Each processor state managed by the "XSAVE feature set" is called state. For example, x87 state for the x87 FPU execution environment, SSE state for the streaming SIMD extensions.
• States are referred to as "state components," and managed by "state-component bitmaps."
• A state-component bitmap comprises 64 bits; each bit in such a bitmap corresponds to a single state
  component. For example, bit 0 corresponds to the x87 state and bits in the range 62:10 are not currently defined.
  • The XSAVE feature set takes state-component bitmaps from in EDX:EAX, and that value is called the "instruction mask,"
  • If the bit corresponding to a state component is clear in XCR0, instructions in the XSAVE feature set will not operate on that state component, regardless of the value of the instruction mask
• "user state components" can be managed by the entire XSAVE feature set, and XCR0 contains a state-component bitmap for them
• "supervisor state components" can be managed only by XSAVES and XRSTORS, and IA32_XSS MSR contains a state-component bitmap for them
  • All the state components corresponding to bits in the range 9:0 are user state components, except PT state
• The "requested-feature bitmap (RFBM)" is a bitmap to be actually specify which components should be saved or restored.
  • for user state components, RFBM == EDX:EAX & XCR0
  • for supervisor state components, RFBM == EDX:EAX & (XCR0 | IA32_XSS)
• The XSAVE feature set allows saving and loading processor state from a region of memory called an "XSAVE area".

Availability (ENUMERATION OF CPU SUPPORT FOR XSAVE INSTRUCTIONS AND XSAVESUPPORTED FEATURES)

• If CPUID function 1:ECX.XSAVE[bit 26] == 0, no support of the XSAVE feature set at all
• If CPUID function 1:ECX.XSAVE[bit 26] == 1,
  • at least XGETBV, XRSTOR, XSAVE, and XSETBV are supported, and
  • CR4.OSXSAVE can be set to 1
• CPUID function 0DH, sub-function 0 returns
  • supported "components," and
  • a size of an XSAVE area for all the user state components supported by this processor
• CPUID function 0DH, sub-function 1 returns availability of
  • XSAVEOPT, which is simply an optimized version of XSAVE
  • XSAVEC, which uses "compaction extensions" (ie, uses the "compacted format" for the extended region of the an XSAVE area) that are supposedly efficient than standard one (the "standard format").
  • XSAVES, which saves the "supervisor state components" apart from the "user state components".

Data structures (XSAVE AREA)

• The "legacy region" comprises the 512 bytes starting at the area’s base address. It is used to manage the state components for x87 state and SSE state
• The "XSAVE header" comprises the 64 bytes starting at an offset of 512 bytes from the area’s base address
• The "extended region" starts at an offset of 576 bytes from the area’s base address. It is used to manage the state components other than those for x87 state and SSE state