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

[Tamas K Lengyel <tamas@tklengyel.com>]

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