[PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Petre Pircalabu]

If case of a vm_event with the emulate_flags set, if the instruction
cannot be emulated, the monitor should be notified instead of directly
injecting a hw exception.
This behavior can be used to re-execute an instruction not supported by
the emulator using the real processor (e.g. altp2m) instead of just
crashing.

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

---
Changed since v1:
  * Removed the emulation kind check when calling hvm_inject_hw_exception

Changed since v2:
  * Removed a file added by mistake

Changed since v3:
  * Removed extra stray line
  * Added the _enabled suffix to the emul_unhandleable monitor option

Changed since v4
  * Fixed return expression of hvm_monitor_emul_unhandleable handle
  monitor_traps failures.
  * Removed stray parantheses.

Changed since v5:
  * Removed unnecessary "else" when calling hvm_monitor_emul_unhandleable.
  * Added extra line in arch_monitor_domctl_event.
---
 tools/libxc/include/xenctrl.h     |  2 ++
 tools/libxc/xc_monitor.c          | 14 ++++++++++++++
 xen/arch/x86/hvm/emulate.c        |  4 ++++
 xen/arch/x86/hvm/monitor.c        | 17 +++++++++++++++++
 xen/arch/x86/monitor.c            | 13 +++++++++++++
 xen/include/asm-x86/domain.h      |  1 +
 xen/include/asm-x86/hvm/monitor.h |  1 +
 xen/include/asm-x86/monitor.h     |  3 ++-
 xen/include/public/domctl.h       |  1 +
 xen/include/public/vm_event.h     |  2 ++
 10 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index c51bb3b..8deb5ac 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2029,6 +2029,8 @@ int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
 int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
 int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
                                bool enable);
+int xc_monitor_emul_unhandleable(xc_interface *xch, domid_t domain_id,
+                                 bool enable);
 /**
  * This function enables / disables emulation for each REP for a
  * REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..8e72c6c 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -216,6 +216,20 @@ int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
     return do_domctl(xch, &domctl);
 }
 
+int xc_monitor_emul_unhandleable(xc_interface *xch, domid_t domain_id,
+                                 bool enable)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_monitor_op;
+    domctl.domain = domain_id;
+    domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
+    domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE;
+
+    return do_domctl(xch, &domctl);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index b2068ad..d518408 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -14,12 +14,14 @@
 #include <xen/sched.h>
 #include <xen/paging.h>
 #include <xen/trace.h>
+#include <xen/vm_event.h>
 #include <asm/event.h>
 #include <asm/i387.h>
 #include <asm/xstate.h>
 #include <asm/hvm/emulate.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/ioreq.h>
+#include <asm/hvm/monitor.h>
 #include <asm/hvm/trace.h>
 #include <asm/hvm/support.h>
 #include <asm/hvm/svm/svm.h>
@@ -2104,6 +2106,8 @@ void hvm_emulate_one_vm_event(enum emul_kind kind, unsigned int trapnr,
         return;
     case X86EMUL_UNHANDLEABLE:
         hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event", &ctx);
+        if ( hvm_monitor_emul_unhandleable() )
+            return;
         hvm_inject_hw_exception(trapnr, errcode);
         break;
     case X86EMUL_EXCEPTION:
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index a7ccfc4..e77b05e 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -57,6 +57,23 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
     return 0;
 }
 
+bool hvm_monitor_emul_unhandleable(void)
+{
+    struct vcpu *curr = current;
+
+    /*
+     * Send a vm_event to the monitor to signal that the current
+     * instruction couldn't be emulated.
+     */
+    vm_event_request_t req = {
+        .reason = VM_EVENT_REASON_EMUL_UNHANDLEABLE,
+        .vcpu_id  = curr->vcpu_id,
+    };
+
+    return curr->domain->arch.monitor.emul_unhandleable_enabled &&
+        monitor_traps(curr, true, &req) == 1;
+}
+
 void hvm_monitor_msr(unsigned int msr, uint64_t value)
 {
     struct vcpu *curr = current;
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 706454f..6375a4c 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -283,6 +283,19 @@ int arch_monitor_domctl_event(struct domain *d,
         break;
     }
 
+    case XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE:
+    {
+        bool old_status = ad->monitor.emul_unhandleable_enabled;
+
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;
+
+        domain_pause(d);
+        ad->monitor.emul_unhandleable_enabled = requested_status;
+        domain_unpause(d);
+        break;
+    }
+
     default:
         /*
          * Should not be reached unless arch_monitor_get_capabilities() is
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c10522b..ae2d04d 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -405,6 +405,7 @@ struct arch_domain
         unsigned int debug_exception_sync        : 1;
         unsigned int cpuid_enabled               : 1;
         unsigned int descriptor_access_enabled   : 1;
+        unsigned int emul_unhandleable_enabled   : 1;
         struct monitor_msr_bitmap *msr_bitmap;
         uint64_t write_ctrlreg_mask[4];
     } monitor;
diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-x86/hvm/monitor.h
index d9efb35..4030be7 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -47,6 +47,7 @@ int hvm_monitor_cpuid(unsigned long insn_length, unsigned int leaf,
                       unsigned int subleaf);
 void hvm_monitor_interrupt(unsigned int vector, unsigned int type,
                            unsigned int err, uint64_t cr2);
+bool hvm_monitor_emul_unhandleable(void);
 
 #endif /* __ASM_X86_HVM_MONITOR_H__ */
 
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c5c323b..38ba0ff 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -77,7 +77,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
                    (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
                    (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
                    (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT);
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE);
 
     /* Since we know this is on VMX, we can just call the hvm func */
     if ( hvm_is_singlestep_supported() )
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..6bd8666 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1083,6 +1083,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL       7
 #define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT             8
 #define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS           9
+#define XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE     10
 
 struct xen_domctl_monitor_op {
     uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index f01e471..5548afb 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -148,6 +148,8 @@
 #define VM_EVENT_REASON_INTERRUPT               12
 /* A descriptor table register was accessed. */
 #define VM_EVENT_REASON_DESCRIPTOR_ACCESS       13
+/* Current instruction couldn't be emulated */
+#define VM_EVENT_REASON_EMUL_UNHANDLEABLE       14
 
 /* Supported values for the vm_event_write_ctrlreg index. */
 #define VM_EVENT_X86_CR0    0
-- 
2.7.4


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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Paul Durrant]

> -----Original Message-----
> From: Petre Pircalabu [mailto:ppircalabu@bitdefender.com]
> Sent: 18 July 2017 10:38
> To: xen-devel@lists.xen.org
> Cc: rcojocaru@bitdefender.com; tamas@tklengyel.com; Ian Jackson
> <Ian.Jackson@citrix.com>; Wei Liu <wei.liu2@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>; jbeulich@suse.com; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Petre Pircalabu
> <ppircalabu@bitdefender.com>
> Subject: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.
> 
> If case of a vm_event with the emulate_flags set, if the instruction
> cannot be emulated, the monitor should be notified instead of directly
> injecting a hw exception.
> This behavior can be used to re-execute an instruction not supported by
> the emulator using the real processor (e.g. altp2m) instead of just
> crashing.
> 
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

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

> 
> ---
> Changed since v1:
>   * Removed the emulation kind check when calling
> hvm_inject_hw_exception
> 
> Changed since v2:
>   * Removed a file added by mistake
> 
> Changed since v3:
>   * Removed extra stray line
>   * Added the _enabled suffix to the emul_unhandleable monitor option
> 
> Changed since v4
>   * Fixed return expression of hvm_monitor_emul_unhandleable handle
>   monitor_traps failures.
>   * Removed stray parantheses.
> 
> Changed since v5:
>   * Removed unnecessary "else" when calling
> hvm_monitor_emul_unhandleable.
>   * Added extra line in arch_monitor_domctl_event.
> ---
>  tools/libxc/include/xenctrl.h     |  2 ++
>  tools/libxc/xc_monitor.c          | 14 ++++++++++++++
>  xen/arch/x86/hvm/emulate.c        |  4 ++++
>  xen/arch/x86/hvm/monitor.c        | 17 +++++++++++++++++
>  xen/arch/x86/monitor.c            | 13 +++++++++++++
>  xen/include/asm-x86/domain.h      |  1 +
>  xen/include/asm-x86/hvm/monitor.h |  1 +
>  xen/include/asm-x86/monitor.h     |  3 ++-
>  xen/include/public/domctl.h       |  1 +
>  xen/include/public/vm_event.h     |  2 ++
>  10 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index c51bb3b..8deb5ac 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2029,6 +2029,8 @@ int xc_monitor_debug_exceptions(xc_interface
> *xch, domid_t domain_id,
>  int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
>  int xc_monitor_privileged_call(xc_interface *xch, domid_t domain_id,
>                                 bool enable);
> +int xc_monitor_emul_unhandleable(xc_interface *xch, domid_t
> domain_id,
> +                                 bool enable);
>  /**
>   * This function enables / disables emulation for each REP for a
>   * REP-compatible instruction.
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index b44ce93..8e72c6c 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -216,6 +216,20 @@ int xc_monitor_privileged_call(xc_interface *xch,
> domid_t domain_id,
>      return do_domctl(xch, &domctl);
>  }
> 
> +int xc_monitor_emul_unhandleable(xc_interface *xch, domid_t
> domain_id,
> +                                 bool enable)
> +{
> +    DECLARE_DOMCTL;
> +
> +    domctl.cmd = XEN_DOMCTL_monitor_op;
> +    domctl.domain = domain_id;
> +    domctl.u.monitor_op.op = enable ?
> XEN_DOMCTL_MONITOR_OP_ENABLE
> +                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
> +    domctl.u.monitor_op.event =
> XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE;
> +
> +    return do_domctl(xch, &domctl);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index b2068ad..d518408 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -14,12 +14,14 @@
>  #include <xen/sched.h>
>  #include <xen/paging.h>
>  #include <xen/trace.h>
> +#include <xen/vm_event.h>
>  #include <asm/event.h>
>  #include <asm/i387.h>
>  #include <asm/xstate.h>
>  #include <asm/hvm/emulate.h>
>  #include <asm/hvm/hvm.h>
>  #include <asm/hvm/ioreq.h>
> +#include <asm/hvm/monitor.h>
>  #include <asm/hvm/trace.h>
>  #include <asm/hvm/support.h>
>  #include <asm/hvm/svm/svm.h>
> @@ -2104,6 +2106,8 @@ void hvm_emulate_one_vm_event(enum
> emul_kind kind, unsigned int trapnr,
>          return;
>      case X86EMUL_UNHANDLEABLE:
>          hvm_dump_emulation_state(XENLOG_G_DEBUG, "Mem event", &ctx);
> +        if ( hvm_monitor_emul_unhandleable() )
> +            return;
>          hvm_inject_hw_exception(trapnr, errcode);
>          break;
>      case X86EMUL_EXCEPTION:
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index a7ccfc4..e77b05e 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -57,6 +57,23 @@ bool_t hvm_monitor_cr(unsigned int index, unsigned
> long value, unsigned long old
>      return 0;
>  }
> 
> +bool hvm_monitor_emul_unhandleable(void)
> +{
> +    struct vcpu *curr = current;
> +
> +    /*
> +     * Send a vm_event to the monitor to signal that the current
> +     * instruction couldn't be emulated.
> +     */
> +    vm_event_request_t req = {
> +        .reason = VM_EVENT_REASON_EMUL_UNHANDLEABLE,
> +        .vcpu_id  = curr->vcpu_id,
> +    };
> +
> +    return curr->domain->arch.monitor.emul_unhandleable_enabled &&
> +        monitor_traps(curr, true, &req) == 1;
> +}
> +
>  void hvm_monitor_msr(unsigned int msr, uint64_t value)
>  {
>      struct vcpu *curr = current;
> diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
> index 706454f..6375a4c 100644
> --- a/xen/arch/x86/monitor.c
> +++ b/xen/arch/x86/monitor.c
> @@ -283,6 +283,19 @@ int arch_monitor_domctl_event(struct domain *d,
>          break;
>      }
> 
> +    case XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE:
> +    {
> +        bool old_status = ad->monitor.emul_unhandleable_enabled;
> +
> +        if ( unlikely(old_status == requested_status) )
> +            return -EEXIST;
> +
> +        domain_pause(d);
> +        ad->monitor.emul_unhandleable_enabled = requested_status;
> +        domain_unpause(d);
> +        break;
> +    }
> +
>      default:
>          /*
>           * Should not be reached unless arch_monitor_get_capabilities() is
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index c10522b..ae2d04d 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -405,6 +405,7 @@ struct arch_domain
>          unsigned int debug_exception_sync        : 1;
>          unsigned int cpuid_enabled               : 1;
>          unsigned int descriptor_access_enabled   : 1;
> +        unsigned int emul_unhandleable_enabled   : 1;
>          struct monitor_msr_bitmap *msr_bitmap;
>          uint64_t write_ctrlreg_mask[4];
>      } monitor;
> diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-
> x86/hvm/monitor.h
> index d9efb35..4030be7 100644
> --- a/xen/include/asm-x86/hvm/monitor.h
> +++ b/xen/include/asm-x86/hvm/monitor.h
> @@ -47,6 +47,7 @@ int hvm_monitor_cpuid(unsigned long insn_length,
> unsigned int leaf,
>                        unsigned int subleaf);
>  void hvm_monitor_interrupt(unsigned int vector, unsigned int type,
>                             unsigned int err, uint64_t cr2);
> +bool hvm_monitor_emul_unhandleable(void);
> 
>  #endif /* __ASM_X86_HVM_MONITOR_H__ */
> 
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-
> x86/monitor.h
> index c5c323b..38ba0ff 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -77,7 +77,8 @@ static inline uint32_t
> arch_monitor_get_capabilities(struct domain *d)
>                     (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
>                     (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
>                     (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT);
> +                   (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> +                   (1U <<
> XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE);
> 
>      /* Since we know this is on VMX, we can just call the hvm func */
>      if ( hvm_is_singlestep_supported() )
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index ff39762..6bd8666 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1083,6 +1083,7 @@
> DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
>  #define XEN_DOMCTL_MONITOR_EVENT_PRIVILEGED_CALL       7
>  #define XEN_DOMCTL_MONITOR_EVENT_INTERRUPT             8
>  #define XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS           9
> +#define XEN_DOMCTL_MONITOR_EVENT_EMUL_UNHANDLEABLE     10
> 
>  struct xen_domctl_monitor_op {
>      uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
> diff --git a/xen/include/public/vm_event.h
> b/xen/include/public/vm_event.h
> index f01e471..5548afb 100644
> --- a/xen/include/public/vm_event.h
> +++ b/xen/include/public/vm_event.h
> @@ -148,6 +148,8 @@
>  #define VM_EVENT_REASON_INTERRUPT               12
>  /* A descriptor table register was accessed. */
>  #define VM_EVENT_REASON_DESCRIPTOR_ACCESS       13
> +/* Current instruction couldn't be emulated */
> +#define VM_EVENT_REASON_EMUL_UNHANDLEABLE       14
> 
>  /* Supported values for the vm_event_write_ctrlreg index. */
>  #define VM_EVENT_X86_CR0    0
> --
> 2.7.4


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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Wei Liu]

On Tue, Jul 18, 2017 at 12:37:32PM +0300, Petre Pircalabu wrote:
> If case of a vm_event with the emulate_flags set, if the instruction
> cannot be emulated, the monitor should be notified instead of directly
> injecting a hw exception.
> This behavior can be used to re-execute an instruction not supported by
> the emulator using the real processor (e.g. altp2m) instead of just
> crashing.
> 
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Andrew Cooper]

On 18/07/17 10:37, Petre Pircalabu wrote:
> If case of a vm_event with the emulate_flags set, if the instruction
> cannot be emulated, the monitor should be notified instead of directly
> injecting a hw exception.
> This behavior can be used to re-execute an instruction not supported by
> the emulator using the real processor (e.g. altp2m) instead of just
> crashing.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

There are many situations which end up failing an emulation with
UNHANDLEABLE.

What exact scenario are you looking to catch?  Is it just instructions
which aren't implemented in the emulator?

~Andrew

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Razvan Cojocaru]

On 07/18/2017 01:09 PM, Andrew Cooper wrote:
> On 18/07/17 10:37, Petre Pircalabu wrote:
>> If case of a vm_event with the emulate_flags set, if the instruction
>> cannot be emulated, the monitor should be notified instead of directly
>> injecting a hw exception.
>> This behavior can be used to re-execute an instruction not supported by
>> the emulator using the real processor (e.g. altp2m) instead of just
>> crashing.
>>
>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> 
> There are many situations which end up failing an emulation with
> UNHANDLEABLE.
> 
> What exact scenario are you looking to catch?  Is it just instructions
> which aren't implemented in the emulator?

Instructions that are not implemented in the emulator are our main use
case for this, yes. In which case, we'd like a chance to be able to
single-step them (using altp2m), so that the guest will continue to run
even with an incomplete emulator.

We don't care about instructions that would have failed to run in both
scenarios (emulated or single-stepped). I'm not sure if there are other
cases in which an instruction, although supported by the emulator, would
fail emulation but pass single-stepping.


Thanks,
Razvan

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Tamas K Lengyel]

On Tue, Jul 18, 2017 at 3:37 AM, Petre Pircalabu
<ppircalabu@bitdefender.com> wrote:
> If case of a vm_event with the emulate_flags set, if the instruction
> cannot be emulated, the monitor should be notified instead of directly
> injecting a hw exception.
> This behavior can be used to re-execute an instruction not supported by
> the emulator using the real processor (e.g. altp2m) instead of just
> crashing.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

Acked-by: Tamas K Lengyel <tamas@tklengyel.com>

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Razvan Cojocaru]

On 07/18/2017 01:20 PM, Razvan Cojocaru wrote:
> On 07/18/2017 01:09 PM, Andrew Cooper wrote:
>> On 18/07/17 10:37, Petre Pircalabu wrote:
>>> If case of a vm_event with the emulate_flags set, if the instruction
>>> cannot be emulated, the monitor should be notified instead of directly
>>> injecting a hw exception.
>>> This behavior can be used to re-execute an instruction not supported by
>>> the emulator using the real processor (e.g. altp2m) instead of just
>>> crashing.
>>>
>>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>>
>> There are many situations which end up failing an emulation with
>> UNHANDLEABLE.
>>
>> What exact scenario are you looking to catch?  Is it just instructions
>> which aren't implemented in the emulator?
> 
> Instructions that are not implemented in the emulator are our main use
> case for this, yes. In which case, we'd like a chance to be able to
> single-step them (using altp2m), so that the guest will continue to run
> even with an incomplete emulator.
> 
> We don't care about instructions that would have failed to run in both
> scenarios (emulated or single-stepped). I'm not sure if there are other
> cases in which an instruction, although supported by the emulator, would
> fail emulation but pass single-stepping.

Is there further action required on our part the get this patch
commited? FWIW, while not ideal, from our perspective trying to
single-step an instruction that was UNHANDLEABLE when attempting to
emulate it is acceptable (even if UNHANDLEABLE doesn't mean that the
instruction is not supported by the emulator). At worst it won't run
when single-stepped either, and that'll be that.


Thanks,
Razvan

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Razvan Cojocaru]

On 07/25/2017 08:40 PM, Razvan Cojocaru wrote:
> On 07/18/2017 01:20 PM, Razvan Cojocaru wrote:
>> On 07/18/2017 01:09 PM, Andrew Cooper wrote:
>>> On 18/07/17 10:37, Petre Pircalabu wrote:
>>>> If case of a vm_event with the emulate_flags set, if the instruction
>>>> cannot be emulated, the monitor should be notified instead of directly
>>>> injecting a hw exception.
>>>> This behavior can be used to re-execute an instruction not supported by
>>>> the emulator using the real processor (e.g. altp2m) instead of just
>>>> crashing.
>>>>
>>>> Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
>>>
>>> There are many situations which end up failing an emulation with
>>> UNHANDLEABLE.
>>>
>>> What exact scenario are you looking to catch?  Is it just instructions
>>> which aren't implemented in the emulator?
>>
>> Instructions that are not implemented in the emulator are our main use
>> case for this, yes. In which case, we'd like a chance to be able to
>> single-step them (using altp2m), so that the guest will continue to run
>> even with an incomplete emulator.
>>
>> We don't care about instructions that would have failed to run in both
>> scenarios (emulated or single-stepped). I'm not sure if there are other
>> cases in which an instruction, although supported by the emulator, would
>> fail emulation but pass single-stepping.
> 
> Is there further action required on our part the get this patch
> commited? FWIW, while not ideal, from our perspective trying to
> single-step an instruction that was UNHANDLEABLE when attempting to
> emulate it is acceptable (even if UNHANDLEABLE doesn't mean that the
> instruction is not supported by the emulator). At worst it won't run
> when single-stepped either, and that'll be that.
Since this seems to be blocked as-is, I propose transforming this patch
into a series, with one patch adding a new return code specifically for
unsupported instructions (X86_EMUL_UNIMPLEMENTED or
X86_EMUL_UNSUPPORTED?), and this patch sending the vm_event out only for
that. (In which case the event's name should probably change as well to
reflect the name of the new error code.)

Thoughts?


Thanks,
Razvan

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

Re: [PATCH v6] x86/monitor: Notify monitor if an emulation fails.

[Jan Beulich]

>>> Razvan Cojocaru <rcojocaru@bitdefender.com> 08/01/17 9:22 PM >>>
>Since this seems to be blocked as-is, I propose transforming this patch
>into a series, with one patch adding a new return code specifically for
>unsupported instructions (X86_EMUL_UNIMPLEMENTED or
>X86_EMUL_UNSUPPORTED?), and this patch sending the vm_event out only for
>that. (In which case the event's name should probably change as well to
>reflect the name of the new error code.)

Yes please (and I'd favor X86EMUL_UNIMPLEMENTED fwiw).

Jan


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