Re: [Xen-devel] [PATCH v2] SVM: limit GIF=0 region

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/08/18 07:09, Jan Beulich wrote:
> Use EFLAGS.IF for all ordinary purposes; there's in particular no need
> to unduly defer NMI/#MC. Clear/set GIF solely around VMRUN itself. This
> has the additional advantage that svm_stgi_label now indeed marks the
> only place where GIF gets set.
>
> A note regarding the main STI(s) placement: Quite counterintuitively the
> host's EFLAGS.IF continues to have a meaning while the guest runs; see
> PM Vol 2 section "Physical (INTR) Interrupt Masking in EFLAGS". Hence we
> need to set the flag for the duration of time being in guest context.
> However, SPEC_CTRL_ENTRY_FROM_HVM wants to be carried out with EFLAGS.IF
> clear.
>
> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Add CLI after VMRUN. Adjust description.
>
> --- a/xen/arch/x86/hvm/svm/entry.S
> +++ b/xen/arch/x86/hvm/svm/entry.S
> @@ -43,7 +43,7 @@ ENTRY(svm_asm_do_resume)
>          lea  irq_stat+IRQSTAT_softirq_pending(%rip),%rdx
>          xor  %ecx,%ecx
>          shl  $IRQSTAT_shift,%eax
> -        CLGI
> +        cli
>          cmp  %ecx,(%rdx,%rax,1)
>          jne  .Lsvm_process_softirqs
>  
> @@ -57,7 +57,7 @@ UNLIKELY_START(ne, nsvm_hap)
>           * Someone shot down our nested p2m table; go round again
>           * and nsvm_vcpu_switch() will fix it for us.
>           */
> -        STGI
> +        sti
>          jmp  .Lsvm_do_resume
>  __UNLIKELY_END(nsvm_hap)
>  
> @@ -87,7 +87,12 @@ __UNLIKELY_END(nsvm_hap)
>          pop  %rsi
>          pop  %rdi
>  
> +        CLGI
> +        sti
>          VMRUN
> +        cli
> +        STGI
> +GLOBAL(svm_stgi_label)
>  
>          SAVE_ALL
>  
> @@ -96,13 +101,12 @@ __UNLIKELY_END(nsvm_hap)
>          SPEC_CTRL_ENTRY_FROM_HVM    /* Req: b=curr %rsp=regs/cpuinfo, Clob: 
> acd */
>          /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
>  
> -        STGI
> -GLOBAL(svm_stgi_label)
> +        sti

Nack.  As indicated in v1, moving this breaks SPEC_CTRL_ENTRY_FROM_HVM
(Even if there is an unexpected bug on the VT-x side of things which
needs fixing differently).

Furthermore, to fix LBR handling, the first thing I'd have to do is
revert this, so please leave it as it is.

~Andrew

>          mov  %rsp,%rdi
>          call svm_vmexit_handler
>          jmp  .Lsvm_do_resume
>  
>  .Lsvm_process_softirqs:
> -        STGI
> +        sti
>          call do_softirq
>          jmp  .Lsvm_do_resume
>
>
>
>


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