Re: [Xen-devel] [PATCH v3 2/2] SVM: introduce a VM entry helper

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.05.18 at 16:19, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/05/18 15:11, Jan Beulich wrote:
>>>>> On 04.05.18 at 17:11, <JBeulich@xxxxxxxx> 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.

> As for running with interrupts disabled, that is already the case on the
> VMX side, and I don't see why the SVM side needs to be different.

Sure, as does SVM - CLGI is a superset of CLI, after all. My observation
was just that this state of interrupts being disabled can't be observed by
users of the normal infrastructure (inspecting EFLAGS.IF).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel