diff mbox

[v2] vmx/monitor: CPUID events

Message ID 1468347198-29474-1-git-send-email-tamas.lengyel@zentific.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas Lengyel July 12, 2016, 6:13 p.m. UTC
This patch implements sending notification to a monitor subscriber when an
x86/vmx guest executes the CPUID instruction.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>

v2: add comment describing rc values in vmx
    fix typo in xen-access
---
 tools/libxc/include/xenctrl.h       |  1 +
 tools/libxc/xc_monitor.c            | 13 +++++++++++++
 tools/tests/xen-access/xen-access.c | 33 ++++++++++++++++++++++++++++++++-
 xen/arch/x86/hvm/monitor.c          | 16 ++++++++++++++++
 xen/arch/x86/hvm/vmx/vmx.c          | 28 ++++++++++++++++++++++++----
 xen/arch/x86/monitor.c              | 13 +++++++++++++
 xen/include/asm-x86/domain.h        |  1 +
 xen/include/asm-x86/hvm/monitor.h   |  1 +
 xen/include/asm-x86/monitor.h       |  3 ++-
 xen/include/public/domctl.h         |  1 +
 xen/include/public/vm_event.h       |  8 ++++++++
 11 files changed, 112 insertions(+), 6 deletions(-)

Comments

Andrew Cooper July 12, 2016, 6:22 p.m. UTC | #1
On 12/07/16 19:13, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>

From an x86 point of view, Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>, with one nit that I can fix on commit if
needs be.

> @@ -3524,10 +3526,28 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>          hvm_task_switch((uint16_t)exit_qualification, reasons[source], ecode);
>          break;
>      }
> -    case EXIT_REASON_CPUID:
> -        is_pvh_vcpu(v) ? pv_cpuid(regs) : vmx_do_cpuid(regs);
> -        update_guest_eip(); /* Safe: CPUID */
> +    case EXIT_REASON_CPUID: {

Brace on a newline here.

> +        int rc;
> +
> +        if ( is_pvh_vcpu(v) )
> +        {
> +            pv_cpuid(regs);
> +            rc = 0;
> +        }
> +        else
> +            rc = vmx_do_cpuid(regs);
> +
> +        /*
> +         * 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 )
> +            update_guest_eip(); /* Safe: CPUID */
>          break;
> +    }
>      case EXIT_REASON_HLT:
>          update_guest_eip(); /* Safe: HLT */
>          hvm_hlt(regs->eflags);
>
Razvan Cojocaru July 12, 2016, 6:25 p.m. UTC | #2
On 07/12/16 21:13, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> 
> v2: add comment describing rc values in vmx
>     fix typo in xen-access
> ---
>  tools/libxc/include/xenctrl.h       |  1 +
>  tools/libxc/xc_monitor.c            | 13 +++++++++++++
>  tools/tests/xen-access/xen-access.c | 33 ++++++++++++++++++++++++++++++++-
>  xen/arch/x86/hvm/monitor.c          | 16 ++++++++++++++++
>  xen/arch/x86/hvm/vmx/vmx.c          | 28 ++++++++++++++++++++++++----
>  xen/arch/x86/monitor.c              | 13 +++++++++++++
>  xen/include/asm-x86/domain.h        |  1 +
>  xen/include/asm-x86/hvm/monitor.h   |  1 +
>  xen/include/asm-x86/monitor.h       |  3 ++-
>  xen/include/public/domctl.h         |  1 +
>  xen/include/public/vm_event.h       |  8 ++++++++
>  11 files changed, 112 insertions(+), 6 deletions(-)

Looks good to me.

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


Thanks,
Razvan
Tamas Lengyel July 12, 2016, 6:32 p.m. UTC | #3
On Tue, Jul 12, 2016 at 12:22 PM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 12/07/16 19:13, Tamas K Lengyel wrote:
>> This patch implements sending notification to a monitor subscriber when an
>> x86/vmx guest executes the CPUID instruction.
>>
>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
>
> From an x86 point of view, Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>, with one nit that I can fix on commit if
> needs be.
>
>> @@ -3524,10 +3526,28 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>          hvm_task_switch((uint16_t)exit_qualification, reasons[source], ecode);
>>          break;
>>      }
>> -    case EXIT_REASON_CPUID:
>> -        is_pvh_vcpu(v) ? pv_cpuid(regs) : vmx_do_cpuid(regs);
>> -        update_guest_eip(); /* Safe: CPUID */
>> +    case EXIT_REASON_CPUID: {
>
> Brace on a newline here.

Sure, though both styles are used interchangeably throughout vmx.c

Tamas
Tian, Kevin July 12, 2016, 11:59 p.m. UTC | #4
> From: Tamas K Lengyel [mailto:tamas.lengyel@zentific.com]
> Sent: Wednesday, July 13, 2016 2:13 AM
> 
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>
Wei Liu July 13, 2016, 8:31 a.m. UTC | #5
On Tue, Jul 12, 2016 at 12:13:18PM -0600, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> 
> v2: add comment describing rc values in vmx
>     fix typo in xen-access
> ---
>  tools/libxc/include/xenctrl.h       |  1 +
>  tools/libxc/xc_monitor.c            | 13 +++++++++++++

Acked-by: Wei Liu <wei.liu2@citrix.com>
Andrew Cooper July 13, 2016, 1:27 p.m. UTC | #6
On 12/07/16 19:13, Tamas K Lengyel wrote:
> This patch implements sending notification to a monitor subscriber when an
> x86/vmx guest executes the CPUID instruction.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@zentific.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jun Nakajima <jun.nakajima@intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>

Committed.

~Andrew
diff mbox

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4a85b4a..e904bd5 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2167,6 +2167,7 @@  int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
                              bool enable, bool sync);
 int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
                                 bool enable, bool sync);
+int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
 /**
  * This function enables / disables emulation for each REP for a
  * REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 264992c..4298813 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -172,6 +172,19 @@  int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
     return do_domctl(xch, &domctl);
 }
 
+int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable)
+{
+    DECLARE_DOMCTL;
+
+    domctl.cmd = XEN_DOMCTL_monitor_op;
+    domctl.domain = domain_id;
+    domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+                                    : XEN_DOMCTL_MONITOR_OP_DISABLE;
+    domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_CPUID;
+
+    return do_domctl(xch, &domctl);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c
index 02655d5..ebb63b1 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -337,7 +337,7 @@  void usage(char* progname)
 {
     fprintf(stderr, "Usage: %s [-m] <domain_id> write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-            fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
+            fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug|cpuid");
 #endif
             fprintf(stderr,
             "\n"
@@ -364,6 +364,7 @@  int main(int argc, char *argv[])
     int shutting_down = 0;
     int altp2m = 0;
     int debug = 0;
+    int cpuid = 0;
     uint16_t altp2m_view_id = 0;
 
     char* progname = argv[0];
@@ -426,6 +427,10 @@  int main(int argc, char *argv[])
     {
         debug = 1;
     }
+    else if ( !strcmp(argv[0], "cpuid") )
+    {
+        cpuid = 1;
+    }
 #endif
     else
     {
@@ -548,6 +553,16 @@  int main(int argc, char *argv[])
         }
     }
 
+    if ( cpuid )
+    {
+        rc = xc_monitor_cpuid(xch, domain_id, 1);
+        if ( rc < 0 )
+        {
+            ERROR("Error %d setting cpuid listener with vm_event\n", rc);
+            goto exit;
+        }
+    }
+
     /* Wait for access */
     for (;;)
     {
@@ -560,6 +575,8 @@  int main(int argc, char *argv[])
                 rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
             if ( debug )
                 rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
+            if ( cpuid )
+                rc = xc_monitor_cpuid(xch, domain_id, 0);
 
             if ( altp2m )
             {
@@ -716,6 +733,20 @@  int main(int argc, char *argv[])
                 }
 
                 break;
+            case VM_EVENT_REASON_CPUID:
+                printf("CPUID executed: rip=%016"PRIx64", vcpu %d. Insn length: %"PRIu32" " \
+                       "EAX: 0x%"PRIx64" EBX: 0x%"PRIx64" ECX: 0x%"PRIx64" EDX: 0x%"PRIx64"\n",
+                       req.data.regs.x86.rip,
+                       req.vcpu_id,
+                       req.u.cpuid.insn_length,
+                       req.data.regs.x86.rax,
+                       req.data.regs.x86.rbx,
+                       req.data.regs.x86.rcx,
+                       req.data.regs.x86.rdx);
+                rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
+                rsp.data = req.data;
+                rsp.data.regs.x86.rip += req.u.cpuid.insn_length;
+                break;
             default:
                 fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
             }
diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index 8488e21..7277c12 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -136,6 +136,22 @@  int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
     return monitor_traps(curr, sync, &req);
 }
 
+int hvm_monitor_cpuid(unsigned long insn_length)
+{
+    struct vcpu *curr = current;
+    struct arch_domain *ad = &curr->domain->arch;
+    vm_event_request_t req = {};
+
+    if ( !ad->monitor.cpuid_enabled )
+        return 0;
+
+    req.reason = VM_EVENT_REASON_CPUID;
+    req.vcpu_id = curr->vcpu_id;
+    req.u.cpuid.insn_length = insn_length;
+
+    return monitor_traps(curr, 1, &req);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0776d12..ce47714 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2396,7 +2396,7 @@  static void vmx_cpuid_intercept(
     HVMTRACE_5D (CPUID, input, *eax, *ebx, *ecx, *edx);
 }
 
-static void vmx_do_cpuid(struct cpu_user_regs *regs)
+static int vmx_do_cpuid(struct cpu_user_regs *regs)
 {
     unsigned int eax, ebx, ecx, edx;
 
@@ -2411,6 +2411,8 @@  static void vmx_do_cpuid(struct cpu_user_regs *regs)
     regs->ebx = ebx;
     regs->ecx = ecx;
     regs->edx = edx;
+
+    return hvm_monitor_cpuid(get_instruction_length());
 }
 
 static void vmx_dr_access(unsigned long exit_qualification,
@@ -3524,10 +3526,28 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
         hvm_task_switch((uint16_t)exit_qualification, reasons[source], ecode);
         break;
     }
-    case EXIT_REASON_CPUID:
-        is_pvh_vcpu(v) ? pv_cpuid(regs) : vmx_do_cpuid(regs);
-        update_guest_eip(); /* Safe: CPUID */
+    case EXIT_REASON_CPUID: {
+        int rc;
+
+        if ( is_pvh_vcpu(v) )
+        {
+            pv_cpuid(regs);
+            rc = 0;
+        }
+        else
+            rc = vmx_do_cpuid(regs);
+
+        /*
+         * 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 )
+            update_guest_eip(); /* Safe: CPUID */
         break;
+    }
     case EXIT_REASON_HLT:
         update_guest_eip(); /* Safe: HLT */
         hvm_hlt(regs->eflags);
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 205df41..5f60743 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -240,6 +240,19 @@  int arch_monitor_domctl_event(struct domain *d,
         break;
     }
 
+    case XEN_DOMCTL_MONITOR_EVENT_CPUID:
+    {
+        bool_t old_status = ad->monitor.cpuid_enabled;
+
+        if ( unlikely(old_status == requested_status) )
+            return -EEXIST;
+
+        domain_pause(d);
+        ad->monitor.cpuid_enabled = requested_status;
+        domain_unpause(d);
+        break;
+    }
+
     default:
         /*
          * Should not be reached unless arch_monitor_get_capabilities() is
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 8f64ae9..5807a1f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -405,6 +405,7 @@  struct arch_domain
         unsigned int software_breakpoint_enabled : 1;
         unsigned int debug_exception_enabled     : 1;
         unsigned int debug_exception_sync        : 1;
+        unsigned int cpuid_enabled               : 1;
         struct monitor_msr_bitmap *msr_bitmap;
     } monitor;
 
diff --git a/xen/include/asm-x86/hvm/monitor.h b/xen/include/asm-x86/hvm/monitor.h
index 1c8ec6c..a92f3fc 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -40,6 +40,7 @@  bool_t hvm_monitor_cr(unsigned int index, unsigned long value,
 void hvm_monitor_msr(unsigned int msr, uint64_t value);
 int hvm_monitor_debug(unsigned long rip, enum hvm_monitor_debug_type type,
                       unsigned long trap_type, unsigned long insn_length);
+int hvm_monitor_cpuid(unsigned long insn_length);
 
 #endif /* __ASM_X86_HVM_MONITOR_H__ */
 
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0501ca2..63a994b 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -75,7 +75,8 @@  static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
                    (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_DEBUG_EXCEPTION) |
+                   (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID);
 
     /* Since we know this is on VMX, we can just call the hvm func */
     if ( hvm_is_singlestep_supported() )
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 30020ba..d6d2319 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1081,6 +1081,7 @@  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 #define XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT   3
 #define XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST         4
 #define XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION       5
+#define XEN_DOMCTL_MONITOR_EVENT_CPUID                 6
 
 struct xen_domctl_monitor_op {
     uint32_t op; /* XEN_DOMCTL_MONITOR_OP_* */
diff --git a/xen/include/public/vm_event.h b/xen/include/public/vm_event.h
index 8c29968..64e6857 100644
--- a/xen/include/public/vm_event.h
+++ b/xen/include/public/vm_event.h
@@ -122,6 +122,8 @@ 
 #define VM_EVENT_REASON_GUEST_REQUEST           8
 /* A debug exception was caught */
 #define VM_EVENT_REASON_DEBUG_EXCEPTION         9
+/* CPUID executed */
+#define VM_EVENT_REASON_CPUID                   10
 
 /* Supported values for the vm_event_write_ctrlreg index. */
 #define VM_EVENT_X86_CR0    0
@@ -222,6 +224,11 @@  struct vm_event_mov_to_msr {
     uint64_t value;
 };
 
+struct vm_event_cpuid {
+    uint32_t insn_length;
+    uint32_t _pad;
+};
+
 #define MEM_PAGING_DROP_PAGE       (1 << 0)
 #define MEM_PAGING_EVICT_FAIL      (1 << 1)
 
@@ -260,6 +267,7 @@  typedef struct vm_event_st {
         struct vm_event_singlestep            singlestep;
         struct vm_event_debug                 software_breakpoint;
         struct vm_event_debug                 debug_exception;
+        struct vm_event_cpuid                 cpuid;
     } u;
 
     union {