Re: [Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

2016-12-01 Thread Jan Beulich
>>> On 30.11.16 at 14:50,  wrote:
> The emulator needs to gain an understanding of interrupts and exceptions
> generated by its actions.
> 
> Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
> are visible to the emulator.  This removes the need for the
> inject_{hw_exception,sw_interrupt}() hooks, which are dropped and replaced
> with x86_emul_{hw_exception,software_event,reset_event}() instead.
> 
> For exceptions raised by x86_emulate() itself (rather than its callbacks), the
> shadow pagetable and PV uses of x86_emulate() previously failed with
> X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.
> 
> This behaviour has changed, and such cases will now return X86EMUL_EXCEPTION
> with event_pending set.  Until the callers of x86_emulate() have been updated
> to inject events back into the guest, divert the event_pending case back  into
> the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible behaviour.
> 
> No overall functional change.
> 
> Signed-off-by: Andrew Cooper 
> Reviewed-by: Boris Ostrovsky 
> Reviewed-by: Kevin Tian 

Reviewed-by: Jan Beulich 


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


Re: [Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

2016-12-01 Thread Tim Deegan
At 13:50 + on 30 Nov (1480513830), Andrew Cooper wrote:
> The emulator needs to gain an understanding of interrupts and exceptions
> generated by its actions.
> 
> Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
> are visible to the emulator.  This removes the need for the
> inject_{hw_exception,sw_interrupt}() hooks, which are dropped and replaced
> with x86_emul_{hw_exception,software_event,reset_event}() instead.
> 
> For exceptions raised by x86_emulate() itself (rather than its callbacks), the
> shadow pagetable and PV uses of x86_emulate() previously failed with
> X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.
> 
> This behaviour has changed, and such cases will now return X86EMUL_EXCEPTION
> with event_pending set.  Until the callers of x86_emulate() have been updated
> to inject events back into the guest, divert the event_pending case back into
> the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible behaviour.
> 
> No overall functional change.
> 
> Signed-off-by: Andrew Cooper 
> Reviewed-by: Boris Ostrovsky 
> Reviewed-by: Kevin Tian 

Acked-by: Tim Deegan 

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


Re: [Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

2016-11-30 Thread Paul Durrant
> -Original Message-
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: 30 November 2016 13:51
> To: Xen-devel 
> Cc: Andrew Cooper ; Jan Beulich
> ; Paul Durrant ; Tim
> (Xen.org) ; George Dunlap 
> Subject: [PATCH v3 13/24] x86/emul: Rework emulator event injection
> 
> The emulator needs to gain an understanding of interrupts and exceptions
> generated by its actions.
> 
> Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt
> so they
> are visible to the emulator.  This removes the need for the
> inject_{hw_exception,sw_interrupt}() hooks, which are dropped and
> replaced
> with x86_emul_{hw_exception,software_event,reset_event}() instead.
> 
> For exceptions raised by x86_emulate() itself (rather than its callbacks), the
> shadow pagetable and PV uses of x86_emulate() previously failed with
> X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.
> 
> This behaviour has changed, and such cases will now return
> X86EMUL_EXCEPTION
> with event_pending set.  Until the callers of x86_emulate() have been
> updated
> to inject events back into the guest, divert the event_pending case back into
> the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible
> behaviour.
> 
> No overall functional change.
> 
> Signed-off-by: Andrew Cooper 
> Reviewed-by: Boris Ostrovsky 
> Reviewed-by: Kevin Tian 
> ---
> CC: Jan Beulich 
> CC: Paul Durrant 

Reviewed-by: Paul Durrant 

> CC: Tim Deegan 
> CC: George Dunlap 
> 
> v3:
>  * Rework how the event_pending case is currently handled
> v2:
>  * Change x86_emul_hw_exception()'s error_code parameter to being
> signed
>  * Clarify how software interrupt injection happens.
>  * More ASSERT()'s and description of how event_pending works without the
>inject_sw_interrupt() hook
> ---
>  xen/arch/x86/hvm/emulate.c | 81 
> --
>  xen/arch/x86/hvm/hvm.c |  4 +-
>  xen/arch/x86/hvm/io.c  |  4 +-
>  xen/arch/x86/hvm/vmx/realmode.c| 16 +++
>  xen/arch/x86/mm.c  | 26 +++
>  xen/arch/x86/mm/shadow/multi.c | 17 +++
>  xen/arch/x86/x86_emulate/x86_emulate.c | 12 +++--
>  xen/arch/x86/x86_emulate/x86_emulate.h | 76
> +--
>  xen/include/asm-x86/hvm/emulate.h  |  3 --
>  9 files changed, 132 insertions(+), 107 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 91c79fa..4b8c9a0 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -568,12 +568,9 @@ static int hvmemul_virtual_to_linear(
>  return X86EMUL_UNHANDLEABLE;
> 
>  /* This is a singleton operation: fail it with an exception. */
> -hvmemul_ctxt->exn_pending = 1;
> -hvmemul_ctxt->trap.vector =
> -(seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault;
> -hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> -hvmemul_ctxt->trap.error_code = 0;
> -hvmemul_ctxt->trap.insn_len = 0;
> +x86_emul_hw_exception((seg == x86_seg_ss)
> +  ? TRAP_stack_error
> +  : TRAP_gp_fault, 0, &hvmemul_ctxt->ctxt);
>  return X86EMUL_EXCEPTION;
>  }
> 
> @@ -1562,59 +1559,6 @@ int hvmemul_cpuid(
>  return X86EMUL_OKAY;
>  }
> 
> -static int hvmemul_inject_hw_exception(
> -uint8_t vector,
> -int32_t error_code,
> -struct x86_emulate_ctxt *ctxt)
> -{
> -struct hvm_emulate_ctxt *hvmemul_ctxt =
> -container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> -
> -hvmemul_ctxt->exn_pending = 1;
> -hvmemul_ctxt->trap.vector = vector;
> -hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
> -hvmemul_ctxt->trap.error_code = error_code;
> -hvmemul_ctxt->trap.insn_len = 0;
> -
> -return X86EMUL_OKAY;
> -}
> -
> -static int hvmemul_inject_sw_interrupt(
> -enum x86_swint_type type,
> -uint8_t vector,
> -uint8_t insn_len,
> -struct x86_emulate_ctxt *ctxt)
> -{
> -struct hvm_emulate_ctxt *hvmemul_ctxt =
> -container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> -
> -switch ( type )
> -{
> -case x86_swint_icebp:
> -hvmemul_ctxt->trap.type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
> -break;
> -
> -case x86_swint_int3:
> -case x86_swint_into:
> -hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_EXCEPTION;
> -break;
> -
> -case x86_swint_int:
> -hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_INTERRUPT;
> -break;
> -
> -default:
> -return X86EMUL_UNHANDLEABLE;
> -}
> -
> -hvmemul_ctxt->exn_pending = 1;
> -hvmemul_ctxt->trap.vector = vector;
> -hvmemul_ctxt->trap.error_code = X86_EVENT_NO_EC;
> -hvmemul_ctxt->trap.insn_len = insn_len;
> -
> -return X86EMUL_OKAY;
> -}
> -
>  static int hvmemul_get_fpu(
>  void (*exception_callback)(void *, struct cpu_user_regs *),
>  void *exception_callback_arg,
> @@ -1678,8 +1622,7 @@ static int hvmemul_inv

[Xen-devel] [PATCH v3 13/24] x86/emul: Rework emulator event injection

2016-11-30 Thread Andrew Cooper
The emulator needs to gain an understanding of interrupts and exceptions
generated by its actions.

Move hvm_emulate_ctxt.{exn_pending,trap} into struct x86_emulate_ctxt so they
are visible to the emulator.  This removes the need for the
inject_{hw_exception,sw_interrupt}() hooks, which are dropped and replaced
with x86_emul_{hw_exception,software_event,reset_event}() instead.

For exceptions raised by x86_emulate() itself (rather than its callbacks), the
shadow pagetable and PV uses of x86_emulate() previously failed with
X86EMUL_UNHANDLEABLE due to the lack of inject_*() hooks.

This behaviour has changed, and such cases will now return X86EMUL_EXCEPTION
with event_pending set.  Until the callers of x86_emulate() have been updated
to inject events back into the guest, divert the event_pending case back into
the X86EMUL_UNHANDLEABLE path to maintain the same guest-visible behaviour.

No overall functional change.

Signed-off-by: Andrew Cooper 
Reviewed-by: Boris Ostrovsky 
Reviewed-by: Kevin Tian 
---
CC: Jan Beulich 
CC: Paul Durrant 
CC: Tim Deegan 
CC: George Dunlap 

v3:
 * Rework how the event_pending case is currently handled
v2:
 * Change x86_emul_hw_exception()'s error_code parameter to being signed
 * Clarify how software interrupt injection happens.
 * More ASSERT()'s and description of how event_pending works without the
   inject_sw_interrupt() hook
---
 xen/arch/x86/hvm/emulate.c | 81 --
 xen/arch/x86/hvm/hvm.c |  4 +-
 xen/arch/x86/hvm/io.c  |  4 +-
 xen/arch/x86/hvm/vmx/realmode.c| 16 +++
 xen/arch/x86/mm.c  | 26 +++
 xen/arch/x86/mm/shadow/multi.c | 17 +++
 xen/arch/x86/x86_emulate/x86_emulate.c | 12 +++--
 xen/arch/x86/x86_emulate/x86_emulate.h | 76 +--
 xen/include/asm-x86/hvm/emulate.h  |  3 --
 9 files changed, 132 insertions(+), 107 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 91c79fa..4b8c9a0 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -568,12 +568,9 @@ static int hvmemul_virtual_to_linear(
 return X86EMUL_UNHANDLEABLE;
 
 /* This is a singleton operation: fail it with an exception. */
-hvmemul_ctxt->exn_pending = 1;
-hvmemul_ctxt->trap.vector =
-(seg == x86_seg_ss) ? TRAP_stack_error : TRAP_gp_fault;
-hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
-hvmemul_ctxt->trap.error_code = 0;
-hvmemul_ctxt->trap.insn_len = 0;
+x86_emul_hw_exception((seg == x86_seg_ss)
+  ? TRAP_stack_error
+  : TRAP_gp_fault, 0, &hvmemul_ctxt->ctxt);
 return X86EMUL_EXCEPTION;
 }
 
@@ -1562,59 +1559,6 @@ int hvmemul_cpuid(
 return X86EMUL_OKAY;
 }
 
-static int hvmemul_inject_hw_exception(
-uint8_t vector,
-int32_t error_code,
-struct x86_emulate_ctxt *ctxt)
-{
-struct hvm_emulate_ctxt *hvmemul_ctxt =
-container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-
-hvmemul_ctxt->exn_pending = 1;
-hvmemul_ctxt->trap.vector = vector;
-hvmemul_ctxt->trap.type = X86_EVENTTYPE_HW_EXCEPTION;
-hvmemul_ctxt->trap.error_code = error_code;
-hvmemul_ctxt->trap.insn_len = 0;
-
-return X86EMUL_OKAY;
-}
-
-static int hvmemul_inject_sw_interrupt(
-enum x86_swint_type type,
-uint8_t vector,
-uint8_t insn_len,
-struct x86_emulate_ctxt *ctxt)
-{
-struct hvm_emulate_ctxt *hvmemul_ctxt =
-container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-
-switch ( type )
-{
-case x86_swint_icebp:
-hvmemul_ctxt->trap.type = X86_EVENTTYPE_PRI_SW_EXCEPTION;
-break;
-
-case x86_swint_int3:
-case x86_swint_into:
-hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_EXCEPTION;
-break;
-
-case x86_swint_int:
-hvmemul_ctxt->trap.type = X86_EVENTTYPE_SW_INTERRUPT;
-break;
-
-default:
-return X86EMUL_UNHANDLEABLE;
-}
-
-hvmemul_ctxt->exn_pending = 1;
-hvmemul_ctxt->trap.vector = vector;
-hvmemul_ctxt->trap.error_code = X86_EVENT_NO_EC;
-hvmemul_ctxt->trap.insn_len = insn_len;
-
-return X86EMUL_OKAY;
-}
-
 static int hvmemul_get_fpu(
 void (*exception_callback)(void *, struct cpu_user_regs *),
 void *exception_callback_arg,
@@ -1678,8 +1622,7 @@ static int hvmemul_invlpg(
  * hvmemul_virtual_to_linear() raises exceptions for type/limit
  * violations, so squash them.
  */
-hvmemul_ctxt->exn_pending = 0;
-hvmemul_ctxt->trap = (struct x86_event){};
+x86_emul_reset_event(ctxt);
 rc = X86EMUL_OKAY;
 }
 
@@ -1696,7 +1639,7 @@ static int hvmemul_vmfunc(
 
 rc = hvm_funcs.altp2m_vcpu_emulate_vmfunc(ctxt->regs);
 if ( rc != X86EMUL_OKAY )
-hvmemul_inject_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC, ctxt);
+x86_emul_hw_exception(TRAP_invalid_op, X86_EVENT_NO_EC, ctxt);