On 05/07/2018 11:49 AM, Andrew Cooper wrote: > On 07/05/18 16:46, Boris Ostrovsky wrote: >> On 05/07/2018 11:29 AM, Andrew Cooper wrote: >>> On 07/05/18 16:25, Jan Beulich wrote: >>>>>>> On 07.05.18 at 16:19, <andrew.coop...@citrix.com> wrote: >>>>> On 07/05/18 15:11, Jan Beulich wrote: >>>>>>>>> On 04.05.18 at 17:11, <jbeul...@suse.com> wrote: >>>>>>> --- a/xen/arch/x86/hvm/svm/entry.S >>>>>>> +++ b/xen/arch/x86/hvm/svm/entry.S >>>>>>> @@ -61,23 +61,8 @@ UNLIKELY_START(ne, nsvm_hap) >>>>>>> jmp .Lsvm_do_resume >>>>>>> __UNLIKELY_END(nsvm_hap) >>>>>>> >>>>>>> - call svm_asid_handle_vmrun >>>>>>> - >>>>>>> - cmpb $0,tb_init_done(%rip) >>>>>>> -UNLIKELY_START(nz, svm_trace) >>>>>>> - call svm_trace_vmentry >>>>>>> -UNLIKELY_END(svm_trace) >>>>>>> - >>>>>>> - mov VCPU_svm_vmcb(%rbx),%rcx >>>>>>> - mov UREGS_rax(%rsp),%rax >>>>>>> - mov %rax,VMCB_rax(%rcx) >>>>>>> - mov UREGS_rip(%rsp),%rax >>>>>>> - mov %rax,VMCB_rip(%rcx) >>>>>>> - mov UREGS_rsp(%rsp),%rax >>>>>>> - mov %rax,VMCB_rsp(%rcx) >>>>>>> - mov UREGS_eflags(%rsp),%rax >>>>>>> - or $X86_EFLAGS_MBS,%rax >>>>>>> - mov %rax,VMCB_rflags(%rcx) >>>>>>> + mov %rsp, %rdi >>>>>>> + call svm_vmenter_helper >>>>>> While I had committed this earlier today, there's one concern I've just >>>>>> come >>>>>> to think of: Now that we're calling into C land with CLGI in effect (for >>>>> more >>>>>> than just the trivial svm_trace_vmentry()) we are at risk of confusing >>>>>> parties using local_irq_is_enabled(), first and foremost >>>>>> common/spinlock.c:check_lock(). While it's some extra overhead, I wonder >>>>>> whether the call wouldn't better be framed by a CLI/STI pair. >>>>> I can't see why the SVM vmentry path uses CLGI/STGI in the first place. >>>>> >>>>> The VMX path uses plain cli/sti and our NMI/MCE handlers can cope. >>>>> Furthermore, processing NMIs/MCEs at this point will be more efficient >>>>> that taking a vmentry then immediately exiting again. >>>> Perhaps you're right, i.e. we could replace all current CLGI/STGI by >>>> CLI/STI, adding a single STGI right after VMRUN. >> The APM say "It is assumed that VMM software cleared GIF some time before >> executing the VMRUN instruction, to ensure an atomic state switch." >> >> Not sure if this is meant as suggestion or requirement. > Hmm - that can probably be tested with this proposed patch and a very > high frequency NMI perf counter.
This may only prove the we do need it, if the test without CLGI fails. If the test passes I don't think we can say anything one way or the other. I am adding Suravee and Brian, perhaps they know the answer (or can check internally). > > Basically every other hypervisor does CLGI; VMSAVE (host state); VMLOAD > (guest state); VMRUN, and Xen's lack of doing this is why we have to > play with the IDT IST settings, as well as why we can't cope cleanly > with stack overflows. > KVM manipulates both GIF and RFLAGS.IF. -boris _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
