All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] x86emul: add "unblock NMI" retire flag
@ 2017-04-13  6:07 Jan Beulich
  2017-04-13 17:18 ` Andrew Cooper
  2017-04-19  9:40 ` Paul Durrant
  0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2017-04-13  6:07 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Julien Grall, Paul Durrant

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

No matter that we emulate IRET for (guest) real more only right now, we
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@suse.com>
---
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;




[-- Attachment #2: x86emul-unblock-NMI.patch --]
[-- Type: text/plain, Size: 3651 bytes --]

x86emul: add "unblock NMI" retire flag

No matter that we emulate IRET for (guest) real more only right now, we
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@suse.com>
---
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;

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] x86emul: add "unblock NMI" retire flag
  2017-04-13  6:07 [PATCH v4] x86emul: add "unblock NMI" retire flag Jan Beulich
@ 2017-04-13 17:18 ` Andrew Cooper
  2017-04-18  9:22   ` Julien Grall
  2017-04-19  9:40 ` Paul Durrant
  1 sibling, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2017-04-13 17:18 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Julien Grall, Paul Durrant

On 13/04/17 07:07, Jan Beulich wrote:
> No matter that we emulate IRET for (guest) real more only right now, we
> 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@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] x86emul: add "unblock NMI" retire flag
  2017-04-13 17:18 ` Andrew Cooper
@ 2017-04-18  9:22   ` Julien Grall
  0 siblings, 0 replies; 4+ messages in thread
From: Julien Grall @ 2017-04-18  9:22 UTC (permalink / raw)
  To: Andrew Cooper, Jan Beulich, xen-devel; +Cc: nd, Paul Durrant

Hi,

On 13/04/2017 18:18, Andrew Cooper wrote:
> On 13/04/17 07:07, Jan Beulich wrote:
>> No matter that we emulate IRET for (guest) real more only right now, we
>> 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@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] x86emul: add "unblock NMI" retire flag
  2017-04-13  6:07 [PATCH v4] x86emul: add "unblock NMI" retire flag Jan Beulich
  2017-04-13 17:18 ` Andrew Cooper
@ 2017-04-19  9:40 ` Paul Durrant
  1 sibling, 0 replies; 4+ messages in thread
From: Paul Durrant @ 2017-04-19  9:40 UTC (permalink / raw)
  To: 'Jan Beulich', xen-devel; +Cc: Andrew Cooper, Julien Grall

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 13 April 2017 07:07
> To: xen-devel <xen-devel@lists.xenproject.org>
> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
> 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@suse.com>

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

> ---
> 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@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-04-19  9:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-13  6:07 [PATCH v4] x86emul: add "unblock NMI" retire flag Jan Beulich
2017-04-13 17:18 ` Andrew Cooper
2017-04-18  9:22   ` Julien Grall
2017-04-19  9:40 ` Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.