Re: [Xen-devel] [PATCH v4] x86emul: add "unblock NMI" retire flag

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 13 April 2017 07:07
> To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: [PATCH v4] x86emul: add "unblock NMI" retire flag
> 
> No matter that we emulate IRET for (guest) real more only right now, we

s/more/mode I guess

> should get its effect on (virtual) NMI delivery right. Note that we can
> simply check the other retire flags also in the !OKAY case, as the
> insn emulator now guarantees them to only be set on OKAY.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
> v4: Except X86EMUL_RETRY from clearing MOV_SS and STI.
> v3: Drastically simplify the change to _hvm_emulate_one().
> v2: Adjust x86_emulate_wrapper() accordingly.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1955,9 +1955,6 @@ static int _hvm_emulate_one(struct hvm_e
>          memcpy(vio->mmio_insn, hvmemul_ctxt->insn_buf, vio-
> >mmio_insn_bytes);
>      }
> 
> -    if ( rc != X86EMUL_OKAY )
> -        return rc;
> -
>      if ( hvmemul_ctxt->ctxt.retire.singlestep )
>          hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
> 
> @@ -1966,15 +1963,19 @@ static int _hvm_emulate_one(struct hvm_e
>      /* MOV-SS instruction toggles MOV-SS shadow, else we just clear it. */
>      if ( hvmemul_ctxt->ctxt.retire.mov_ss )
>          new_intr_shadow ^= HVM_INTR_SHADOW_MOV_SS;
> -    else
> +    else if ( rc != X86EMUL_RETRY )
>          new_intr_shadow &= ~HVM_INTR_SHADOW_MOV_SS;
> 
>      /* STI instruction toggles STI shadow, else we just clear it. */
>      if ( hvmemul_ctxt->ctxt.retire.sti )
>          new_intr_shadow ^= HVM_INTR_SHADOW_STI;
> -    else
> +    else if ( rc != X86EMUL_RETRY )
>          new_intr_shadow &= ~HVM_INTR_SHADOW_STI;
> 
> +    /* IRET, if valid in the given context, clears NMI blocking. */
> +    if ( hvmemul_ctxt->ctxt.retire.unblock_nmi )
> +        new_intr_shadow &= ~HVM_INTR_SHADOW_NMI;
> +
>      if ( hvmemul_ctxt->intr_shadow != new_intr_shadow )
>      {
>          hvmemul_ctxt->intr_shadow = new_intr_shadow;
> @@ -1987,7 +1988,7 @@ static int _hvm_emulate_one(struct hvm_e
>          hvm_hlt(regs->eflags);
>      }
> 
> -    return X86EMUL_OKAY;
> +    return rc;
>  }
> 
>  int hvm_emulate_one(
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -4002,6 +4002,7 @@ x86_emulate(
>          uint32_t mask = X86_EFLAGS_VIP | X86_EFLAGS_VIF |
> X86_EFLAGS_VM;
> 
>          fail_if(!in_realmode(ctxt, ops));
> +        ctxt->retire.unblock_nmi = true;
>          if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes),
>                                &eip, op_bytes, ctxt, ops)) ||
>               (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes),
> @@ -7918,9 +7919,17 @@ int x86_emulate_wrapper(
> 
>      rc = x86_emulate(ctxt, ops);
> 
> -    /* Retire flags should only be set for successful instruction emulation. 
> */
> +    /*
> +     * Most retire flags should only be set for successful instruction
> +     * emulation.
> +     */
>      if ( rc != X86EMUL_OKAY )
> -        ASSERT(ctxt->retire.raw == 0);
> +    {
> +        typeof(ctxt->retire) retire = ctxt->retire;
> +
> +        retire.unblock_nmi = false;
> +        ASSERT(!retire.raw);
> +    }
> 
>      /* All cases returning X86EMUL_EXCEPTION should have fault semantics.
> */
>      if ( rc == X86EMUL_EXCEPTION )
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -490,6 +490,7 @@ struct x86_emulate_ctxt
>              bool hlt:1;          /* Instruction HLTed. */
>              bool mov_ss:1;       /* Instruction sets MOV-SS irq shadow. */
>              bool sti:1;          /* Instruction sets STI irq shadow. */
> +            bool unblock_nmi:1;  /* Instruction clears NMI blocking. */
>              bool singlestep:1;   /* Singlestepping was active. */
>          };
>      } retire;
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel