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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

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.

~Andrew

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