On Tue, Oct 10, 2017 at 4:56 PM, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> On 10/10/2017 14:17, Ladi Prosek wrote:
>> + /* Temporarily set the SMM flag to access the SMM state-save area */
>> + vcpu->arch.hflags |= HF_SMM_MASK;
>> + r = kvm_vcpu_read_guest(vcpu, smbase + 0xfed8, &svm_state_save,
>> + sizeof(svm_state_save));
>> + vcpu->arch.hflags &= ~HF_SMM_MASK;
>> + if (r)
>
> Isn't the flag still set here? You have:
>
> + if (ctxt->ops->post_leave_smm(ctxt, smbase, &reload_state))
> + return X86EMUL_UNHANDLEABLE;
> +
> + if (reload_state) {
> + /*
> + * post_leave_smm() made changes to the vCPU (e.g. entered
> + * guest mode) and is asking us to load the SMM state-save
> + * area again.
> + */
> + ret = rsm_load_state(ctxt, smbase);
> + if (ret != X86EMUL_CONTINUE) {
> + /* FIXME: should triple fault */
> + return X86EMUL_UNHANDLEABLE;
> + }
> + }
> +
> if ((ctxt->ops->get_hflags(ctxt) & X86EMUL_SMM_INSIDE_NMI_MASK) == 0)
> ctxt->ops->set_nmi_mask(ctxt, false);
>
> ctxt->ops->set_hflags(ctxt, ctxt->ops->get_hflags(ctxt) &
> ~(X86EMUL_SMM_INSIDE_NMI_MASK | X86EMUL_SMM_MASK));
>
> On the other hand you need to turn HF_SMM_MASK off and back on
> around nested_svm_map/enter_svm_guest_mode, so that the VMCB is
> not read from SMRAM.
Ah, yes, that's a fallout from the previous arrangement where
post_leave_smm really ran after everything else. Sorry for the mix-up.
> And the mandatory stupid question: why is the first state load
> needed at all? Could VMX's post_leave_smm callback (which would
> really become a pre_leave_smm callback) use load_vmcs12_host_state
> instead. SVM likewise could extract load_from_hsave_area from
> nested_svm_vmexit, and pre-load the SMM state-save area data in
> pre_leave_smm.
I swear I've tried load_vmcs12_host_state and spent way too much time
trying to figure out why it didn't work. I'll sleep on it and try
again tomorrow :)
> (Yes, load_vmcs12_host_state is overkill, but
> it's already there and it already knows how to do things like
> entering protected mode etc.).
And that I'm not quite sure about. If I remember correctly I had to do
a bit of prep work before I could call load_vmcs12_host_state -- is it
really supposed to be able to enter protected from real?
> Instead, post_leave_smm would run with the SMM flag clear, so
> hopefully you wouldn't need games with HF_SMM_MASK at all (except
> for the reserved VMXE flag---let's leave that out for now and
> concentrate on the right way to do L2 state save restore...).
Sounds good, thanks!
> Paolo
