[PATCH v3 0/4] hvm/svm: Enable vm events for SVM

[PATCH v3 0/4] hvm/svm: Enable vm events for SVM

[Alexandru Isaila]

Hi all,

This series provides a skeleton for enabling vm_events on SVM. For the
first step, the MSR, CR, Breakpoint and GuestRequest have been tested
and added to the capabilities list.

Cheers,

Alexandru Isaila

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

[PATCH v3 1/4] asm-x86/monitor: Fix monitor capability reporting on SVM systems

[Alexandru Isaila]

No monitor features are available on AMD and all
capabilities are passed only to the Intel processor architecture.
This means that the arch_monitor_get_capabilities returns
capabilities = 0.

This patch is separating out features which are implemented on both
systems from those implemented only on Intel, so that we advertize the
working capabilities on AMD.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

---
Changes since V2:
	- Moved the common part of capabilities after the
	  !is_hvm_domain(d) test
	- Modified the comment in arch_monitor_get_capabilities
---
 xen/include/asm-x86/monitor.h | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index a0444d1..c339324 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -71,24 +71,28 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     uint32_t capabilities = 0;
 
     /*
-     * At the moment only Intel HVM domains are supported. However, event
-     * delivery could be extended to AMD and PV domains.
+     * At the moment only Intel and AMD HVM domains are supported. However, event
+     * delivery could be extended to PV domains.
      */
-    if ( !is_hvm_domain(d) || !cpu_has_vmx )
+    if ( !is_hvm_domain(d) )
         return capabilities;
 
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
-                   (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_EMUL_UNIMPLEMENTED);
-
-    /* Since we know this is on VMX, we can just call the hvm func */
-    if ( hvm_is_singlestep_supported() )
-        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+    if( cpu_has_vmx )
+    {
+        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
+                       (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
+
+        /* Since we know this is on VMX, we can just call the hvm func */
+        if ( hvm_is_singlestep_supported() )
+            capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+    }
 
     if ( hvm_funcs.set_descriptor_access_exiting )
         capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);
-- 
2.7.4


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

[PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Alexandru Isaila]

This commit implements the breakpoint events for svm.
At the moment, the Breakpoint vmexit is not forwarded to the monitor layer.
This patch adds the hvm_monitor_debug call to the VMEXIT_EXCEPTION_BP.
Also, the Software Breakpoint cap is moved from the Intel arch to the
common part of the code.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

---
Changes since V2:
	- Moved the comment from vmx_vmexit_handler to
	  hvm_monitor_debug
	- Moved the AMD comment up.
---
 xen/arch/x86/hvm/monitor.c    |  5 +++++
 xen/arch/x86/hvm/svm/svm.c    | 48 +++++++++++++++++++++++++++++++++++--------
 xen/arch/x86/hvm/vmx/vmx.c    |  5 -----
 xen/include/asm-x86/monitor.h |  4 ++--
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 131b852..60cb68d 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -133,6 +133,11 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
 int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
                       unsigned long trap_type, unsigned long insn_length)
 {
+    /*
+     * rc < 0 error in monitor/vm_event, crash
+     * !rc    continue normally
+     * rc > 0 paused waiting for response, work here is done
+     */
     struct vcpu *curr = current;
     struct arch_domain *ad = &curr->domain->arch;
     vm_event_request_t req = {};
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index dcbd550..0d9baf8 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -59,6 +59,7 @@
 #include <asm/hap.h>
 #include <asm/apic.h>
 #include <asm/debugger.h>
+#include <asm/hvm/monitor.h>
 #include <asm/xstate.h>
 
 void svm_asm_do_resume(void);
@@ -1079,7 +1080,8 @@ static void svm_ctxt_switch_to(struct vcpu *v)
 static void noreturn svm_do_resume(struct vcpu *v)
 {
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
-    bool_t debug_state = v->domain->debugger_attached;
+    bool debug_state = v->domain->debugger_attached
+                || v->domain->arch.monitor.software_breakpoint_enabled;
     bool_t vcpu_guestmode = 0;
     struct vlapic *vlapic = vcpu_vlapic(v);
 
@@ -2407,6 +2409,19 @@ static bool svm_get_pending_event(struct vcpu *v, struct x86_event *info)
     return true;
 }
 
+static void svm_propagate_intr(struct vcpu *v, unsigned long insn_len)
+{
+    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
+    struct x86_event event = {
+        .vector = vmcb->eventinj.fields.type,
+        .type = vmcb->eventinj.fields.type,
+        .error_code = vmcb->exitinfo1,
+    };
+
+    event.insn_len = insn_len;
+    hvm_inject_event(&event);
+}
+
 static struct hvm_function_table __initdata svm_function_table = {
     .name                 = "SVM",
     .cpu_up_prepare       = svm_cpu_up_prepare,
@@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
         break;
 
     case VMEXIT_EXCEPTION_BP:
-        if ( !v->domain->debugger_attached )
-            goto unexpected_exit_type;
-        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
-        if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
+        inst_len = __get_instruction_length(v, INSTR_INT3);
+
+        if ( inst_len == 0 )
             break;
-        __update_guest_eip(regs, inst_len);
-        current->arch.gdbsx_vcpu_event = TRAP_int3;
-        domain_pause_for_debugger();
+
+        if ( v->domain->debugger_attached )
+        {
+            /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
+            __update_guest_eip(regs, inst_len);
+            current->arch.gdbsx_vcpu_event = TRAP_int3;
+            domain_pause_for_debugger();
+        }
+        else
+        {
+           int rc;
+
+           rc = hvm_monitor_debug(regs->rip,
+                                  HVM_MONITOR_SOFTWARE_BREAKPOINT,
+                                  X86_EVENTTYPE_SW_EXCEPTION,
+                                  inst_len);
+           if ( rc < 0 )
+               goto unexpected_exit_type;
+           if ( !rc )
+               svm_propagate_intr(v, inst_len);
+        }
         break;
 
     case VMEXIT_EXCEPTION_NM:
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 3dc6a6d..c89b4b6 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3709,11 +3709,6 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
                                        HVM_MONITOR_DEBUG_EXCEPTION,
                                        trap_type, insn_len);
 
-                /*
-                 * rc < 0 error in monitor/vm_event, crash
-                 * !rc    continue normally
-                 * rc > 0 paused waiting for response, work here is done
-                 */
                 if ( rc < 0 )
                     goto exit_and_crash;
                 if ( !rc )
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c339324..11a0cae 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -77,13 +77,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     if ( !is_hvm_domain(d) )
         return capabilities;
 
-    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+    capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT));
 
     if( cpu_has_vmx )
     {
         capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
-                       (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
-- 
2.7.4


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

[PATCH v3 3/4] hvm/svm: Enable MSR events

[Alexandru Isaila]

At this moment there is no function to enable msr interception on svm.

This patch implements this function and moves the mov to msr monitor event
form the Intel arch side to the common capabilities.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 xen/arch/x86/hvm/svm/svm.c    | 9 +++++++++
 xen/include/asm-x86/monitor.h | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 0d9baf8..5092b12 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -163,6 +163,14 @@ void svm_intercept_msr(struct vcpu *v, uint32_t msr, int flags)
         __clear_bit(msr * 2 + 1, msr_bit);
 }
 
+static void svm_enable_msr_interception(struct domain *d, uint32_t msr)
+{
+    struct vcpu *v;
+
+    for_each_vcpu ( d, v )
+        svm_intercept_msr(v, msr, MSR_INTERCEPT_WRITE);
+}
+
 static void svm_save_dr(struct vcpu *v)
 {
     struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
@@ -2460,6 +2468,7 @@ static struct hvm_function_table __initdata svm_function_table = {
     .fpu_dirty_intercept  = svm_fpu_dirty_intercept,
     .msr_read_intercept   = svm_msr_read_intercept,
     .msr_write_intercept  = svm_msr_write_intercept,
+    .enable_msr_interception = svm_enable_msr_interception,
     .set_rdtsc_exiting    = svm_set_rdtsc_exiting,
     .set_descriptor_access_exiting = svm_set_descriptor_access_exiting,
     .get_insn_bytes       = svm_get_insn_bytes,
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 11a0cae..6b886af 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -78,12 +78,12 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
         return capabilities;
 
     capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT));
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR));
 
     if( cpu_has_vmx )
     {
         capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
-                       (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
-- 
2.7.4


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

[PATCH v3 4/4] hvm/svm: Enable CR events

[Alexandru Isaila]

The CR_INTERCEPT_CR3_WRITE intercept is out of the vmcb->_cr_intercepts
so the AMD arch can't intercept CR events.

This patch implements the CR intercept by adding the flag on a
write_ctrlreg event. The monitor write ctrlreg event is moved from the
Intel side to the common capabilities side.

We just need to enable the SVM intercept and then hvm_mov_to_cr() will
forward the event on to the monitor when appropriate.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
---
 xen/arch/x86/hvm/svm/svm.c    | 11 +++++++++++
 xen/include/asm-x86/monitor.h |  6 +++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 5092b12..89c628e 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -60,6 +60,7 @@
 #include <asm/apic.h>
 #include <asm/debugger.h>
 #include <asm/hvm/monitor.h>
+#include <asm/monitor.h>
 #include <asm/xstate.h>
 
 void svm_asm_do_resume(void);
@@ -560,6 +561,16 @@ void svm_update_guest_cr(struct vcpu *v, unsigned int cr)
                 svm_fpu_enter(v);
         }
 
+        if ( paging_mode_hap(v->domain) )
+        {
+            uint32_t intercepts = vmcb_get_cr_intercepts(vmcb);
+
+            /* Trap CR3 updates if CR3 memory events are enabled. */
+            if ( v->domain->arch.monitor.write_ctrlreg_enabled &
+                 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
+               vmcb_set_cr_intercepts(vmcb, intercepts | CR_INTERCEPT_CR3_WRITE);
+        }
+
         value = v->arch.hvm_vcpu.guest_cr[0] | hw_cr0_mask;
         if ( !paging_mode_hap(v->domain) )
             value |= X86_CR0_PG | X86_CR0_WP;
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 6b886af..217f3d4 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -79,12 +79,12 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 
     capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
                    (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
-                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR));
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
 
     if( cpu_has_vmx )
     {
-        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
-                       (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
+        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
                        (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);
-- 
2.7.4


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

Re: [PATCH v3 1/4] asm-x86/monitor: Fix monitor capability reporting on SVM systems

[Andrew Cooper]

On 12/02/18 15:08, Alexandru Isaila wrote:
> No monitor features are available on AMD and all
> capabilities are passed only to the Intel processor architecture.
> This means that the arch_monitor_get_capabilities returns
> capabilities = 0.
>
> This patch is separating out features which are implemented on both
> systems from those implemented only on Intel, so that we advertize the
> working capabilities on AMD.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> ---
> Changes since V2:
> 	- Moved the common part of capabilities after the
> 	  !is_hvm_domain(d) test
> 	- Modified the comment in arch_monitor_get_capabilities
> ---
>  xen/include/asm-x86/monitor.h | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index a0444d1..c339324 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -71,24 +71,28 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>      uint32_t capabilities = 0;
>  
>      /*
> -     * At the moment only Intel HVM domains are supported. However, event
> -     * delivery could be extended to AMD and PV domains.
> +     * At the moment only Intel and AMD HVM domains are supported. However, event
> +     * delivery could be extended to PV domains.
>       */
> -    if ( !is_hvm_domain(d) || !cpu_has_vmx )
> +    if ( !is_hvm_domain(d) )
>          return capabilities;
>  
> -    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> -                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
> -                   (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_EMUL_UNIMPLEMENTED);
> -
> -    /* Since we know this is on VMX, we can just call the hvm func */
> -    if ( hvm_is_singlestep_supported() )
> -        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> +    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
> +
> +    if( cpu_has_vmx )

Missing space.

> +    {
> +        capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> +                       (1U << XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED);

With an extra set of brackes around the entire expression, editors will
indent this properly.

I can fix these issues on commit if there are no other objections. 
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +
> +        /* Since we know this is on VMX, we can just call the hvm func */
> +        if ( hvm_is_singlestep_supported() )
> +            capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
> +    }
>  
>      if ( hvm_funcs.set_descriptor_access_exiting )
>          capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);


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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Tamas K Lengyel]

On Mon, Feb 12, 2018 at 8:08 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit implements the breakpoint events for svm.
> At the moment, the Breakpoint vmexit is not forwarded to the monitor layer.

This is a bit confusing as it sounds like as if you were saying that
after this patch the event is not forwarded when I think you mean that
before this patch it's not forwarded.

> This patch adds the hvm_monitor_debug call to the VMEXIT_EXCEPTION_BP.
> Also, the Software Breakpoint cap is moved from the Intel arch to the
> common part of the code.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

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

>
> ---
> Changes since V2:
>         - Moved the comment from vmx_vmexit_handler to
>           hvm_monitor_debug
>         - Moved the AMD comment up.
> ---
>  xen/arch/x86/hvm/monitor.c    |  5 +++++
>  xen/arch/x86/hvm/svm/svm.c    | 48 +++++++++++++++++++++++++++++++++++--------
>  xen/arch/x86/hvm/vmx/vmx.c    |  5 -----
>  xen/include/asm-x86/monitor.h |  4 ++--
>  4 files changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
> index 131b852..60cb68d 100644
> --- a/xen/arch/x86/hvm/monitor.c
> +++ b/xen/arch/x86/hvm/monitor.c
> @@ -133,6 +133,11 @@ static inline unsigned long gfn_of_rip(unsigned long rip)
>  int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
>                        unsigned long trap_type, unsigned long insn_length)
>  {
> +    /*
> +     * rc < 0 error in monitor/vm_event, crash
> +     * !rc    continue normally
> +     * rc > 0 paused waiting for response, work here is done
> +     */
>      struct vcpu *curr = current;
>      struct arch_domain *ad = &curr->domain->arch;
>      vm_event_request_t req = {};
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index dcbd550..0d9baf8 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -59,6 +59,7 @@
>  #include <asm/hap.h>
>  #include <asm/apic.h>
>  #include <asm/debugger.h>
> +#include <asm/hvm/monitor.h>
>  #include <asm/xstate.h>
>
>  void svm_asm_do_resume(void);
> @@ -1079,7 +1080,8 @@ static void svm_ctxt_switch_to(struct vcpu *v)
>  static void noreturn svm_do_resume(struct vcpu *v)
>  {
>      struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> -    bool_t debug_state = v->domain->debugger_attached;
> +    bool debug_state = v->domain->debugger_attached
> +                || v->domain->arch.monitor.software_breakpoint_enabled;
>      bool_t vcpu_guestmode = 0;
>      struct vlapic *vlapic = vcpu_vlapic(v);
>
> @@ -2407,6 +2409,19 @@ static bool svm_get_pending_event(struct vcpu *v, struct x86_event *info)
>      return true;
>  }
>
> +static void svm_propagate_intr(struct vcpu *v, unsigned long insn_len)
> +{
> +    struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> +    struct x86_event event = {
> +        .vector = vmcb->eventinj.fields.type,
> +        .type = vmcb->eventinj.fields.type,
> +        .error_code = vmcb->exitinfo1,
> +    };
> +
> +    event.insn_len = insn_len;
> +    hvm_inject_event(&event);
> +}
> +
>  static struct hvm_function_table __initdata svm_function_table = {
>      .name                 = "SVM",
>      .cpu_up_prepare       = svm_cpu_up_prepare,
> @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>          break;
>
>      case VMEXIT_EXCEPTION_BP:
> -        if ( !v->domain->debugger_attached )
> -            goto unexpected_exit_type;
> -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
> -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
> +        inst_len = __get_instruction_length(v, INSTR_INT3);
> +
> +        if ( inst_len == 0 )
>              break;
> -        __update_guest_eip(regs, inst_len);
> -        current->arch.gdbsx_vcpu_event = TRAP_int3;
> -        domain_pause_for_debugger();
> +
> +        if ( v->domain->debugger_attached )
> +        {
> +            /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
> +            __update_guest_eip(regs, inst_len);
> +            current->arch.gdbsx_vcpu_event = TRAP_int3;
> +            domain_pause_for_debugger();
> +        }
> +        else
> +        {
> +           int rc;
> +
> +           rc = hvm_monitor_debug(regs->rip,
> +                                  HVM_MONITOR_SOFTWARE_BREAKPOINT,
> +                                  X86_EVENTTYPE_SW_EXCEPTION,
> +                                  inst_len);
> +           if ( rc < 0 )
> +               goto unexpected_exit_type;
> +           if ( !rc )
> +               svm_propagate_intr(v, inst_len);
> +        }
>          break;
>
>      case VMEXIT_EXCEPTION_NM:
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 3dc6a6d..c89b4b6 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3709,11 +3709,6 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>                                         HVM_MONITOR_DEBUG_EXCEPTION,
>                                         trap_type, insn_len);
>
> -                /*
> -                 * rc < 0 error in monitor/vm_event, crash
> -                 * !rc    continue normally
> -                 * rc > 0 paused waiting for response, work here is done
> -                 */
>                  if ( rc < 0 )
>                      goto exit_and_crash;
>                  if ( !rc )
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index c339324..11a0cae 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -77,13 +77,13 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>      if ( !is_hvm_domain(d) )
>          return capabilities;
>
> -    capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
> +    capabilities = ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
> +                   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT));
>
>      if( cpu_has_vmx )
>      {
>          capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
>                         (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
> -                       (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>                         (1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
>                         (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>                         (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> --
> 2.7.4

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Andrew Cooper]

On 12/02/18 15:08, Alexandru Isaila wrote:
> @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>          break;
>  
>      case VMEXIT_EXCEPTION_BP:
> -        if ( !v->domain->debugger_attached )
> -            goto unexpected_exit_type;
> -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
> -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
> +        inst_len = __get_instruction_length(v, INSTR_INT3);

There are multiple ways of ending up with this vmexit, and INT3 is not
the only way.

The old code was somewhat broken (but only in the case that a debugger
was attached), but now with  this introspection hook active, executing
`0xcd 0x03` will end up crashing the domain because of a length mismatch
looking for 0xcc.

You need to inspect EXITINTINFO to work out what went on here, and
distinguish INT3 from INT $3.

Can I suggest that you run this unit test
http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
introspection an check that you get all expected events?  Every time we
touch this code, we seem to break it :(

~Andrew

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Tamas K Lengyel]

On Mon, Feb 12, 2018 at 8:54 AM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 12/02/18 15:08, Alexandru Isaila wrote:
>> @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>>          break;
>>
>>      case VMEXIT_EXCEPTION_BP:
>> -        if ( !v->domain->debugger_attached )
>> -            goto unexpected_exit_type;
>> -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
>> -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3)) == 0 )
>> +        inst_len = __get_instruction_length(v, INSTR_INT3);
>
> There are multiple ways of ending up with this vmexit, and INT3 is not
> the only way.
>
> The old code was somewhat broken (but only in the case that a debugger
> was attached), but now with  this introspection hook active, executing
> `0xcd 0x03` will end up crashing the domain because of a length mismatch
> looking for 0xcc.
>
> You need to inspect EXITINTINFO to work out what went on here, and
> distinguish INT3 from INT $3.
>
> Can I suggest that you run this unit test
> http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
> introspection an check that you get all expected events?  Every time we
> touch this code, we seem to break it :(
>

+1, this used to be an issue under vmx as well

Tamas

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Alexandru Stefan ISAILA]

On Lu, 2018-02-12 at 15:54 +0000, Andrew Cooper wrote:
> On 12/02/18 15:08, Alexandru Isaila wrote:
> >
> > @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct
> > cpu_user_regs *regs)
> >          break;
> >
> >      case VMEXIT_EXCEPTION_BP:
> > -        if ( !v->domain->debugger_attached )
> > -            goto unexpected_exit_type;
> > -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not
> > update RIP. */
> > -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3))
> > == 0 )
> > +        inst_len = __get_instruction_length(v, INSTR_INT3);
> There are multiple ways of ending up with this vmexit, and INT3 is
> not
> the only way.
>
> The old code was somewhat broken (but only in the case that a
> debugger
> was attached), but now with  this introspection hook active,
> executing
> `0xcd 0x03` will end up crashing the domain because of a length
> mismatch
> looking for 0xcc.
>
> You need to inspect EXITINTINFO to work out what went on here, and
> distinguish INT3 from INT $3.
>
> Can I suggest that you run this unit test
> http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
> introspection an check that you get all expected events?  Every time
> we
> touch this code, we seem to break it :(
>
> ~Andrew
>
Hi Andrew,

I've executed the test-swint-emulation and the domain did not crash.
The result is right here:
(d1) --- Xen Test Framework ---
(d1) Environment: HVM 64bit (Long mode 4 levels)
(d1) Software interrupt emulation
(d1) FEP support not detected - some tests will be skipped
(d1) Test cpl0: all perms ok
(d1)   Testing int3
(d1)   Testing int $3
(d1)   Testing icebp
(d1)   Testing int $1
(d1)   Testing into
(d1) Test cpl0: p=0
(d1)   Testing int3
(d1)   Testing int $3
(d1)   Testing icebp
(d1)   Testing int $1
(d1)   Testing into
(d1) Test cpl3: all perms ok
(d1)   Testing int3
(d1)   Testing int $3
(d1)   Testing icebp
(d1)   Testing int $1
(d1)   Testing into
(d1) Test cpl3: p=0
(d1)   Testing int3
(d1)   Testing int $3
(d1)   Testing icebp
(d1)   Testing int $1
(d1)   Testing into
(d1) Test cpl3: dpl=0
(d1)   Testing int3
(d1)   Testing int $3
(d1)   Testing icebp
(d1)   Testing int $1
(d1)   Testing into
(d1) Test result: SKIP


If you think we need to be safe, I can add a test like if (
exitintinfo.type !=  INSTR_INT3) break;

~Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Alexandru Stefan ISAILA]

On Lu, 2018-02-12 at 15:54 +0000, Andrew Cooper wrote:
> On 12/02/18 15:08, Alexandru Isaila wrote:
> >
> > @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct
> > cpu_user_regs *regs)
> >          break;
> >
> >      case VMEXIT_EXCEPTION_BP:
> > -        if ( !v->domain->debugger_attached )
> > -            goto unexpected_exit_type;
> > -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not
> > update RIP. */
> > -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3))
> > == 0 )
> > +        inst_len = __get_instruction_length(v, INSTR_INT3);
> There are multiple ways of ending up with this vmexit, and INT3 is
> not
> the only way.
>
> The old code was somewhat broken (but only in the case that a
> debugger
> was attached), but now with  this introspection hook active,
> executing
> `0xcd 0x03` will end up crashing the domain because of a length
> mismatch
> looking for 0xcc.
>
> You need to inspect EXITINTINFO to work out what went on here, and
> distinguish INT3 from INT $3.
>
> Can I suggest that you run this unit test
> http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
> introspection an check that you get all expected events?  Every time
> we
> touch this code, we seem to break it :(
>
> ~Andrew
>
I've tested on Intel and AMD and I only get events on int3. Further
more, I don't think there is any way to use the vmcb->exitintinfo
because all the fields are 0 on the time of VMEXIT_EXCEPTION_BP. Did I
understand the test scenario correctly?

Thanks,
Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v3 1/4] asm-x86/monitor: Fix monitor capability reporting on SVM systems

[Andrew Cooper]

On 12/02/18 15:13, Andrew Cooper wrote:
> On 12/02/18 15:08, Alexandru Isaila wrote:
>> No monitor features are available on AMD and all
>> capabilities are passed only to the Intel processor architecture.
>> This means that the arch_monitor_get_capabilities returns
>> capabilities = 0.
>>
>> This patch is separating out features which are implemented on both
>> systems from those implemented only on Intel, so that we advertize the
>> working capabilities on AMD.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

This patch still needs an ack from Tamas or Razvan, but there is no
comment so far that I can find.

~Andrew


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

Re: [PATCH v3 1/4] asm-x86/monitor: Fix monitor capability reporting on SVM systems

[Razvan Cojocaru]

On 02/14/2018 07:47 PM, Andrew Cooper wrote:
> On 12/02/18 15:13, Andrew Cooper wrote:
>> On 12/02/18 15:08, Alexandru Isaila wrote:
>>> No monitor features are available on AMD and all
>>> capabilities are passed only to the Intel processor architecture.
>>> This means that the arch_monitor_get_capabilities returns
>>> capabilities = 0.
>>>
>>> This patch is separating out features which are implemented on both
>>> systems from those implemented only on Intel, so that we advertize the
>>> working capabilities on AMD.
>>>
>>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> 
> This patch still needs an ack from Tamas or Razvan, but there is no
> comment so far that I can find.

I think Tamas probably wouldn't object, so FWIW:

Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>


Thanks,
Razvan

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Andrew Cooper]

On 14/02/18 16:10, Alexandru Stefan ISAILA wrote:
> On Lu, 2018-02-12 at 15:54 +0000, Andrew Cooper wrote:
>> On 12/02/18 15:08, Alexandru Isaila wrote:
>>> @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct
>>> cpu_user_regs *regs)
>>>          break;
>>>
>>>      case VMEXIT_EXCEPTION_BP:
>>> -        if ( !v->domain->debugger_attached )
>>> -            goto unexpected_exit_type;
>>> -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not
>>> update RIP. */
>>> -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3))
>>> == 0 )
>>> +        inst_len = __get_instruction_length(v, INSTR_INT3);
>> There are multiple ways of ending up with this vmexit, and INT3 is
>> not
>> the only way.
>>
>> The old code was somewhat broken (but only in the case that a
>> debugger
>> was attached), but now with  this introspection hook active,
>> executing
>> `0xcd 0x03` will end up crashing the domain because of a length
>> mismatch
>> looking for 0xcc.
>>
>> You need to inspect EXITINTINFO to work out what went on here, and
>> distinguish INT3 from INT $3.
>>
>> Can I suggest that you run this unit test
>> http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
>> introspection an check that you get all expected events?  Every time
>> we
>> touch this code, we seem to break it :(
>>
>> ~Andrew
>>
> I've tested on Intel and AMD and I only get events on int3. Further
> more, I don't think there is any way to use the vmcb->exitintinfo
> because all the fields are 0 on the time of VMEXIT_EXCEPTION_BP. Did I
> understand the test scenario correctly?

Quite possibly, but now I'm even more confused.  I'll have a quick play.

~Andrew

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Andrew Cooper]

On 14/02/18 18:22, Andrew Cooper wrote:
> On 14/02/18 16:10, Alexandru Stefan ISAILA wrote:
>> On Lu, 2018-02-12 at 15:54 +0000, Andrew Cooper wrote:
>>> On 12/02/18 15:08, Alexandru Isaila wrote:
>>>> @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct
>>>> cpu_user_regs *regs)
>>>>          break;
>>>>
>>>>      case VMEXIT_EXCEPTION_BP:
>>>> -        if ( !v->domain->debugger_attached )
>>>> -            goto unexpected_exit_type;
>>>> -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not
>>>> update RIP. */
>>>> -        if ( (inst_len = __get_instruction_length(v, INSTR_INT3))
>>>> == 0 )
>>>> +        inst_len = __get_instruction_length(v, INSTR_INT3);
>>> There are multiple ways of ending up with this vmexit, and INT3 is
>>> not
>>> the only way.
>>>
>>> The old code was somewhat broken (but only in the case that a
>>> debugger
>>> was attached), but now with  this introspection hook active,
>>> executing
>>> `0xcd 0x03` will end up crashing the domain because of a length
>>> mismatch
>>> looking for 0xcc.
>>>
>>> You need to inspect EXITINTINFO to work out what went on here, and
>>> distinguish INT3 from INT $3.
>>>
>>> Can I suggest that you run this unit test
>>> http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under debug
>>> introspection an check that you get all expected events?  Every time
>>> we
>>> touch this code, we seem to break it :(
>>>
>>> ~Andrew
>>>
>> I've tested on Intel and AMD and I only get events on int3. Further
>> more, I don't think there is any way to use the vmcb->exitintinfo
>> because all the fields are 0 on the time of VMEXIT_EXCEPTION_BP. Did I
>> understand the test scenario correctly?
> Quite possibly, but now I'm even more confused.  I'll have a quick play.

Ok - after some investigation, executing `int $3` triggers VMEXIT_SWINT,
with the vector in EXITINFO1, as opposed to triggering VMEXIT_EXCP3,
except that we don't have INTERCEPT_SWINT active, so it completes
internally.

Therefore, in your patch, we do expect only ever to find an int3
triggering VMEXIT_EXCEPTION_BP.  Sorry for the noise.

However, do you mind rebasing the remainder of your series onto
staging?  It doesn't apply cleanly any more.

~Andrew

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

Re: [PATCH v3 1/4] asm-x86/monitor: Fix monitor capability reporting on SVM systems

[Tamas K Lengyel]

On Wed, Feb 14, 2018 at 10:56 AM, Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
> On 02/14/2018 07:47 PM, Andrew Cooper wrote:
>> On 12/02/18 15:13, Andrew Cooper wrote:
>>> On 12/02/18 15:08, Alexandru Isaila wrote:
>>>> No monitor features are available on AMD and all
>>>> capabilities are passed only to the Intel processor architecture.
>>>> This means that the arch_monitor_get_capabilities returns
>>>> capabilities = 0.
>>>>
>>>> This patch is separating out features which are implemented on both
>>>> systems from those implemented only on Intel, so that we advertize the
>>>> working capabilities on AMD.
>>>>
>>>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>>
>> This patch still needs an ack from Tamas or Razvan, but there is no
>> comment so far that I can find.
>
> I think Tamas probably wouldn't object, so FWIW:
>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Yes, this looks fine to me too:

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

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

Re: [PATCH v3 2/4] hvm/svm: Enable Breakpoint events

[Alexandru Stefan ISAILA]

On Mi, 2018-02-14 at 19:11 +0000, Andrew Cooper wrote:
> On 14/02/18 18:22, Andrew Cooper wrote:
> >
> > On 14/02/18 16:10, Alexandru Stefan ISAILA wrote:
> > >
> > > On Lu, 2018-02-12 at 15:54 +0000, Andrew Cooper wrote:
> > > >
> > > > On 12/02/18 15:08, Alexandru Isaila wrote:
> > > > >
> > > > > @@ -2619,14 +2634,31 @@ void svm_vmexit_handler(struct
> > > > > cpu_user_regs *regs)
> > > > >          break;
> > > > >
> > > > >      case VMEXIT_EXCEPTION_BP:
> > > > > -        if ( !v->domain->debugger_attached )
> > > > > -            goto unexpected_exit_type;
> > > > > -        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do
> > > > > not
> > > > > update RIP. */
> > > > > -        if ( (inst_len = __get_instruction_length(v,
> > > > > INSTR_INT3))
> > > > > == 0 )
> > > > > +        inst_len = __get_instruction_length(v, INSTR_INT3);
> > > > There are multiple ways of ending up with this vmexit, and INT3
> > > > is
> > > > not
> > > > the only way.
> > > >
> > > > The old code was somewhat broken (but only in the case that a
> > > > debugger
> > > > was attached), but now with  this introspection hook active,
> > > > executing
> > > > `0xcd 0x03` will end up crashing the domain because of a length
> > > > mismatch
> > > > looking for 0xcc.
> > > >
> > > > You need to inspect EXITINTINFO to work out what went on here,
> > > > and
> > > > distinguish INT3 from INT $3.
> > > >
> > > > Can I suggest that you run this unit test
> > > > http://xenbits.xen.org/docs/xtf/test-swint-emulation.html under
> > > > debug
> > > > introspection an check that you get all expected events?  Every
> > > > time
> > > > we
> > > > touch this code, we seem to break it :(
> > > >
> > > > ~Andrew
> > > >
> > > I've tested on Intel and AMD and I only get events on int3.
> > > Further
> > > more, I don't think there is any way to use the vmcb->exitintinfo
> > > because all the fields are 0 on the time of VMEXIT_EXCEPTION_BP.
> > > Did I
> > > understand the test scenario correctly?
> > Quite possibly, but now I'm even more confused.  I'll have a quick
> > play.
> Ok - after some investigation, executing `int $3` triggers
> VMEXIT_SWINT,
> with the vector in EXITINFO1, as opposed to triggering VMEXIT_EXCP3,
> except that we don't have INTERCEPT_SWINT active, so it completes
> internally.
>
> Therefore, in your patch, we do expect only ever to find an int3
> triggering VMEXIT_EXCEPTION_BP.  Sorry for the noise.
>
> However, do you mind rebasing the remainder of your series onto
> staging?  It doesn't apply cleanly any more.
>
> ~Andrew
>
Nice to hear that. Ok, I will re base to staging and address your other
comments  as well.

Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel