On Wed, Sep 20, 2017 at 12:14 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: > On 20/09/2017 10:52, Ladi Prosek wrote: >>> Therefore, storing such hidden state or a subset of it in >>> vmx->nested.smm is merely a convenience that lets you keep >>> vmx->nested.vmxon == false while in SMM. This is different from nSVM, >>> and I think it's an acceptable difference for the sake of convenience. >> So for nSVM, you'd save to and restore from the state save area? I can >> certainly do that but it would be hard to guarantee that KVM won't >> blow up if the SMM handler pokes at these fields. It feels safer to >> keep it internal or just save, not restore from it. I know that this >> comment belongs to the nSVM patch but it looks similar to your >> suggestion to protect CR4.VMXE on Intel. If we want to prevent SMM >> from wreaking havoc by trying to enable VMX, shouldn't we use the same >> measures on AMD and prevent it from switching VMCB while the nested >> guest is running. Thanks! > > The difference is that on AMD you're actually allowed to do that. The > SMM handler can also poke the general purpose register or even the > descriptor cache, so I think we should stay close to what the manual > says---even if it does two opposite things on Intel vs. AMD. The AMD manual seems to list the SVM fields as Read-Only. "Software should not modify offsets specified as read-only or reserved, otherwise unpredictable results can occur." Would this convince you to keep patch 6 as is? Saving and restoring from the save state area eliminates a bit of internal state but it adds complexity to other places (need to remember smbase before em_rsm overwrites it, for example). It's also a bit faster to restore from internal than from guest memory and it would match VMX.
