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

[PATCH v2 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 v2 1/4] asm-x86/monitor: Enable svm monitor events

[Alexandru Isaila]

This commit separates the svm caps from the vmx caps.

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

---
Changes since V1:
	- Removed the if ( cpu_has_svm )
---
 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..b2b4e6a 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 and 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);
+    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);
+    }
+
+    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
 
     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 v2 2/4] hvm/svm: Enable Breakpoint events

[Alexandru Isaila]

This commit enables the breakpoint events for svm.

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

---
Changes since V1:
	- Clean up bool_t
	- Removed event.insn_len = 0
	- Switched the v->domain->debugger_attached if
	- Add a extra pair of brachets for the capab var.
---
 xen/arch/x86/hvm/svm/svm.c    | 48 +++++++++++++++++++++++++++++++++++--------
 xen/include/asm-x86/monitor.h |  4 ++--
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index dcbd550..a14caab 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 )
+        {
+            __update_guest_eip(regs, inst_len);
+            current->arch.gdbsx_vcpu_event = TRAP_int3;
+            domain_pause_for_debugger();
+        }
+        else
+        {
+        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */
+           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/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index b2b4e6a..68e62bd 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -81,7 +81,6 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     {
         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) |
@@ -92,7 +91,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
             capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
     }
 
-    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+    capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
+                    (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT));
 
     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 v2 3/4] hvm/svm: Enable MSR events

[Alexandru Isaila]

This commit enables MSR events for svm.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.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 a14caab..1eadab4 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 68e62bd..138c463 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -80,7 +80,6 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
     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) |
@@ -92,7 +91,8 @@ 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_SOFTWARE_BREAKPOINT) |
+                    (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR));
 
     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 v2 4/4] hvm/svm: Enable CR events

[Alexandru Isaila]

This commit enables controlregister events for svm.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.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 1eadab4..311902f 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 138c463..b80d217 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -79,8 +79,7 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
 
     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);
@@ -92,7 +91,8 @@ 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 ( 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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[Tamas K Lengyel]

On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit separates the svm caps from the vmx caps.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> ---
> Changes since V1:
>         - Removed the if ( cpu_has_svm )
> ---
>  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..b2b4e6a 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 and PV domains.

"to and PV domains"? Remove that "and" from there too.

>       */
> -    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);
> +    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);
> +    }
> +
> +    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
>
>      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

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

[Tamas K Lengyel]

On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables the breakpoint events for svm.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> ---
> Changes since V1:
>         - Clean up bool_t
>         - Removed event.insn_len = 0
>         - Switched the v->domain->debugger_attached if
>         - Add a extra pair of brachets for the capab var.
> ---
>  xen/arch/x86/hvm/svm/svm.c    | 48 +++++++++++++++++++++++++++++++++++--------
>  xen/include/asm-x86/monitor.h |  4 ++--
>  2 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index dcbd550..a14caab 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 )
> +        {
> +            __update_guest_eip(regs, inst_len);
> +            current->arch.gdbsx_vcpu_event = TRAP_int3;
> +            domain_pause_for_debugger();
> +        }
> +        else
> +        {
> +        /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update RIP. */

This comment here looks like to belong to the code above that manually
increases the IP.

> +           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/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index b2b4e6a..68e62bd 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -81,7 +81,6 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>      {
>          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) |
> @@ -92,7 +91,8 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>              capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
>      }
>
> -    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
> +    capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST) |
> +                    (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT));
>
>      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

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

[Tamas K Lengyel]

On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables MSR events for svm.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

Acked-by: Tamas K Lengyel <tamas@tklengyel.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 a14caab..1eadab4 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 68e62bd..138c463 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -80,7 +80,6 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>      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) |
> @@ -92,7 +91,8 @@ 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_SOFTWARE_BREAKPOINT) |
> +                    (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR));
>
>      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

Re: [PATCH v2 4/4] hvm/svm: Enable CR events

[Tamas K Lengyel]

On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables controlregister events for svm.

So this patch enables the event to trigger but where is it being
handled and forwarded to the monitor ring?

>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.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 1eadab4..311902f 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 138c463..b80d217 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -79,8 +79,7 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>
>      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);
> @@ -92,7 +91,8 @@ 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 ( 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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[George Dunlap]

On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit separates the svm caps from the vmx caps.

I can see how the patch relates to the description here, but it's not
immediately clear how it relates to the title.

A good "template" to start with for any commit message is:
1. What the current situation is
2. Why that's a problem
3. How this patch fixes it

The only time you should omit any of these is if it's completely obvious.

In this case, it looks like the answers would look like:

1. Only a subset of the monitor features are available on AMD, but all
capabilities are passed regardless of the processor architecture.

2. This means that the majority of functionality advertized in
'capabilities' is actually broken when running under AMD.

3. Separate out features which are implemented on both systems from
those implemented only on Intel, so that on AMD systems we only
advertize functionality that works.

Is that about right?

 -George

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

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

[George Dunlap]

On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables the breakpoint events for svm.

s/enable/implement/;

 -George

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

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

[George Dunlap]

On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables MSR events for svm.

I'd probably say 'implement' here as well.

Also, you don't need to repeat the title in the commit message.  If
there's nothing more to put into the commit message than is said in
the title, you can just include your SoB.  (Contrariwise, if you feel
like you need something in the commit message, it should be more than
just repeating the title of the patch.)

 -George

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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[George Dunlap]

On Fri, Feb 9, 2018 at 10:28 AM, George Dunlap <dunlapg@umich.edu> wrote:
> On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
>> This commit separates the svm caps from the vmx caps.
>
> I can see how the patch relates to the description here, but it's not
> immediately clear how it relates to the title.
>
> A good "template" to start with for any commit message is:
> 1. What the current situation is
> 2. Why that's a problem
> 3. How this patch fixes it
>
> The only time you should omit any of these is if it's completely obvious.
>
> In this case, it looks like the answers would look like:
>
> 1. Only a subset of the monitor features are available on AMD, but all
> capabilities are passed regardless of the processor architecture.
>
> 2. This means that the majority of functionality advertized in
> 'capabilities' is actually broken when running under AMD.
>
> 3. Separate out features which are implemented on both systems from
> those implemented only on Intel, so that on AMD systems we only
> advertize functionality that works.

And I think a better title might be something like:

"asm-x86/monitor: Fix montior capability reporting on SVM systems"

 -George

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

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

[Alexandru Stefan ISAILA]

On Vi, 2018-02-09 at 10:37 +0000, George Dunlap wrote:
> On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
> >
> > This commit enables MSR events for svm.
> I'd probably say 'implement' here as well.
>
> Also, you don't need to repeat the title in the commit message.  If
> there's nothing more to put into the commit message than is said in
> the title, you can just include your SoB.  (Contrariwise, if you feel
> like you need something in the commit message, it should be more than
> just repeating the title of the patch.)
>
>  -George
>
 Thanks for the heads up. I will remember this for future
contributions.

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 v2 4/4] hvm/svm: Enable CR events

[Alexandru Stefan ISAILA]

On Jo, 2018-02-08 at 11:06 -0700, Tamas K Lengyel wrote:
> On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
> >
> > This commit enables controlregister events for svm.
> So this patch enables the event to trigger but where is it being
> handled and forwarded to the monitor ring?
Hi Tamas,

If I've understand your question right, this is handled, like on the
vmx side, on a special case on CR0. If this is not what you are looking
for can you please clarify the question.

Alex
>
> >
> >
> > Signed-off-by: Alexandru Isaila <aisaila@bitdefender.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 1eadab4..311902f 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 138c463..b80d217 100644
> > --- a/xen/include/asm-x86/monitor.h
> > +++ b/xen/include/asm-x86/monitor.h
> > @@ -79,8 +79,7 @@ static inline uint32_t
> > arch_monitor_get_capabilities(struct domain *d)
> >
> >      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);
> > @@ -92,7 +91,8 @@ 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 ( hvm_funcs.set_descriptor_access_exiting )
> >          capabilities |= (1U <<
> > XEN_DOMCTL_MONITOR_EVENT_DESC_ACCESS);
> > --
> > 2.7.4

________________________
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 v2 1/4] asm-x86/monitor: Enable svm monitor events

[Tamas K Lengyel]

On Fri, Feb 9, 2018 at 3:28 AM, George Dunlap <dunlapg@umich.edu> wrote:
> On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
> <aisaila@bitdefender.com> wrote:
>> This commit separates the svm caps from the vmx caps.
>
> I can see how the patch relates to the description here, but it's not
> immediately clear how it relates to the title.
>
> A good "template" to start with for any commit message is:
> 1. What the current situation is
> 2. Why that's a problem
> 3. How this patch fixes it
>
> The only time you should omit any of these is if it's completely obvious.
>
> In this case, it looks like the answers would look like:
>
> 1. Only a subset of the monitor features are available on AMD, but all
> capabilities are passed regardless of the processor architecture.
>
> 2. This means that the majority of functionality advertized in
> 'capabilities' is actually broken when running under AMD.

It is not broken under AMD. What is being reported there is that it is
no monitor option is supported - ie. capabilities = 0. The whole thing
is gated on cpu_has_vmx.

Tamas

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

Re: [PATCH v2 4/4] hvm/svm: Enable CR events

[Tamas K Lengyel]

On Fri, Feb 9, 2018 at 6:10 AM, Alexandru Stefan ISAILA
<aisaila@bitdefender.com> wrote:
> On Jo, 2018-02-08 at 11:06 -0700, Tamas K Lengyel wrote:
>> On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
>> <aisaila@bitdefender.com> wrote:
>> >
>> > This commit enables controlregister events for svm.
>> So this patch enables the event to trigger but where is it being
>> handled and forwarded to the monitor ring?
> Hi Tamas,
>
> If I've understand your question right, this is handled, like on the
> vmx side, on a special case on CR0. If this is not what you are looking
> for can you please clarify the question.

I'm sorry, I was looking for seeing a call to hvm_mov_to_crX but I
realized the whole thing is abstracted already under hvm_mov_to_cr in
the hvm layer.

Tamas

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

Re: [PATCH v2 4/4] hvm/svm: Enable CR events

[Tamas K Lengyel]

On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
<aisaila@bitdefender.com> wrote:
> This commit enables controlregister events for svm.
>
> 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 1eadab4..311902f 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 138c463..b80d217 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -79,8 +79,7 @@ static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
>
>      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);
> @@ -92,7 +91,8 @@ 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 ( 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

Re: [PATCH v2 4/4] hvm/svm: Enable CR events

[George Dunlap]

On Fri, Feb 9, 2018 at 1:10 PM, Alexandru Stefan ISAILA
<aisaila@bitdefender.com> wrote:
> On Jo, 2018-02-08 at 11:06 -0700, Tamas K Lengyel wrote:
>> On Thu, Feb 8, 2018 at 8:25 AM, Alexandru Isaila
>> <aisaila@bitdefender.com> wrote:
>> >
>> > This commit enables controlregister events for svm.
>> So this patch enables the event to trigger but where is it being
>> handled and forwarded to the monitor ring?
> Hi Tamas,
>
> If I've understand your question right, this is handled, like on the
> vmx side, on a special case on CR0. If this is not what you are looking
> for can you please clarify the question.

So I forgot #4 in my list, "4. Anything else necessary to understand the patch".

Probably a useful thing to add to the commit message would be something like:

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

 -George

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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[Boris Ostrovsky]

On 02/08/2018 10:25 AM, Alexandru Isaila wrote:
> This commit separates the svm caps from the vmx caps.
> 
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> 
> ---
> Changes since V1:
> 	- Removed the if ( cpu_has_svm )
> ---
>   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..b2b4e6a 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 and 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);
> +    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);
> +    }
> +
> +    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);


It's a nit but I'd start with setting common options and the OR in 
arch-specific ones. (i.e. move the line above to right after 
!is_hvm_domain(d) test).

-boris
	


>   
>       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 v2 2/4] hvm/svm: Enable Breakpoint events

[Boris Ostrovsky]

On 02/08/2018 10:25 AM, Alexandru Isaila wrote:

> +
> +           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);


There is a comment in vmx_vmexit_handler() where this call is made a 
couple of times that explains hvm_moonitor_debug()'s return values.

Can you move that comment to hvm_monitor_debug() definition so people 
can look there to understand how to deal with return values instead of 
searching for call sites?

-boris

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

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

[Boris Ostrovsky]

On 02/08/2018 10:25 AM, Alexandru Isaila wrote:
> This commit enables MSR events for svm.
> 
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>



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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[Boris Ostrovsky]

(Resending)

On 02/10/2018 11:30 AM, Boris Ostrovsky wrote:
>
>
> On 02/08/2018 10:25 AM, Alexandru Isaila wrote:
>> This commit separates the svm caps from the vmx caps.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>>
>> ---
>> Changes since V1:
>>     - Removed the if ( cpu_has_svm )
>> ---
>>   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..b2b4e6a 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 and 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);
>> +    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);
>> +    }
>> +
>> +    capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
>
>
> It's a nit but I'd start with setting common options and the OR in
> arch-specific ones. (i.e. move the line above to right after
> !is_hvm_domain(d) test).
>
> -boris
>     
>
>
>>         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 v2 3/4] hvm/svm: Enable MSR events

[Boris Ostrovsky]

(Resending too. Something was wrong with my client)

On 02/10/2018 11:33 AM, Boris Ostrovsky wrote:
> 
> 
> On 02/08/2018 10:25 AM, Alexandru Isaila wrote:
>> This commit enables MSR events for svm.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> 

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

Re: [PATCH v2 1/4] asm-x86/monitor: Enable svm monitor events

[George Dunlap]

On Fri, Feb 9, 2018 at 4:21 PM, Tamas K Lengyel <tamas@tklengyel.com> wrote:
> On Fri, Feb 9, 2018 at 3:28 AM, George Dunlap <dunlapg@umich.edu> wrote:
>> On Thu, Feb 8, 2018 at 3:25 PM, Alexandru Isaila
>> <aisaila@bitdefender.com> wrote:
>>> This commit separates the svm caps from the vmx caps.
>>
>> I can see how the patch relates to the description here, but it's not
>> immediately clear how it relates to the title.
>>
>> A good "template" to start with for any commit message is:
>> 1. What the current situation is
>> 2. Why that's a problem
>> 3. How this patch fixes it
>>
>> The only time you should omit any of these is if it's completely obvious.
>>
>> In this case, it looks like the answers would look like:
>>
>> 1. Only a subset of the monitor features are available on AMD, but all
>> capabilities are passed regardless of the processor architecture.
>>
>> 2. This means that the majority of functionality advertized in
>> 'capabilities' is actually broken when running under AMD.
>
> It is not broken under AMD. What is being reported there is that it is
> no monitor option is supported - ie. capabilities = 0. The whole thing
> is gated on cpu_has_vmx.

Ah, right, I missed that.

So what about a description like the following:

Currently we report no capabilities at all on AMD hardware.  However,
there are a core set of capabilities which will already work out of
the box.  Unconditionally report capabilities that currently work on
both AMD and Intel, separating out capabilities which are only
implemented on Intel cpus at the moment.

(I haven't caught up on my mail yet, so maybe this message will be
moot -- in which case just ignore it.)

 -George

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