[PATCH] vmx/monitor: CPUID events

[PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

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>
---
 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          | 23 +++++++++++++++++++----
 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, 107 insertions(+), 6 deletions(-)

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..d525b82 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 = 1;
     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 df19579..f7439ed 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2397,7 +2397,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;
 
@@ -2412,6 +2412,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,
@@ -3525,10 +3527,23 @@ 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);
+
+        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 8e1426f..b8ce8a7 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -41,6 +41,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 a9db3c0..0051e85 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -78,7 +78,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 7bfe6cc..0bbe62f 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 {
-- 
2.8.1


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

Re: [PATCH] vmx/monitor: CPUID events

[Razvan Cojocaru]

On 07/08/16 05:31, 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>
> ---
>  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          | 23 +++++++++++++++++++----
>  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, 107 insertions(+), 6 deletions(-)
> 
> 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..d525b82 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 = 1;

Should this be on by default? All the rest of the options are 0, and ...

>      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

... you also set it to 1 here.


Thanks,
Razvan

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

Re: [PATCH] vmx/monitor: CPUID events

[Andrew Cooper]

On 08/07/16 03:31, 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>

Is it wise having an on/off control without any further filtering?  (I
suppose that it is at least a fine first start).

cpuid is usually the serialising instruction used with rdtsc for timing
loops.  This is bad enough in VMs because of the VMExit, but becomes
even worse if there is a monitor delay as well.

~Andrew

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

Re: [PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

On Fri, Jul 8, 2016 at 1:03 AM, Razvan Cojocaru
<rcojocaru@bitdefender.com> wrote:
> On 07/08/16 05:31, 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>
>> ---
>>  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          | 23 +++++++++++++++++++----
>>  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, 107 insertions(+), 6 deletions(-)
>>
>> 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..d525b82 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 = 1;
>
> Should this be on by default? All the rest of the options are 0, and ...

No, it's just a typo.

Thanks,
Tamas

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

Re: [PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

On Fri, Jul 8, 2016 at 3:33 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 08/07/16 03:31, 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>
>
> Is it wise having an on/off control without any further filtering?  (I
> suppose that it is at least a fine first start).

What type of extra filtering do you have in mind?

>
> cpuid is usually the serialising instruction used with rdtsc for timing
> loops.  This is bad enough in VMs because of the VMExit, but becomes
> even worse if there is a monitor delay as well.

Yes, going the extra route of sending a monitor event out will add to
that delay (how much delay will depend on the subscriber and what it
decides to do with the event). Wouldn't we be able to mask some of
that with tsc offsetting though?

Thanks,
Tamas

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

Re: [PATCH] vmx/monitor: CPUID events

[Andrew Cooper]

On 08/07/16 16:44, Tamas K Lengyel wrote:
> On Fri, Jul 8, 2016 at 3:33 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 08/07/16 03:31, 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>
>> Is it wise having an on/off control without any further filtering?  (I
>> suppose that it is at least a fine first start).
> What type of extra filtering do you have in mind?

Not sure.  What are you intending to use this facility for?

Given that the hypervisor is already in complete control of what a guest
gets to see via cpuid, mutating the results via the monitor framework
doesn't seem like a useful thing to do.

>
>> cpuid is usually the serialising instruction used with rdtsc for timing
>> loops.  This is bad enough in VMs because of the VMExit, but becomes
>> even worse if there is a monitor delay as well.
> Yes, going the extra route of sending a monitor event out will add to
> that delay (how much delay will depend on the subscriber and what it
> decides to do with the event). Wouldn't we be able to mask some of
> that with tsc offsetting though?

I am going to go out on a limb and say that that is a very large can of
worms which you don't want to open.

The problem is not that time skews from the point of view of the guest,
but that the timing loop with a fixed number of iterations takes
proportionally longer.

~Andrew

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

Re: [PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

On Fri, Jul 8, 2016 at 10:49 AM, Andrew Cooper
<andrew.cooper3@citrix.com> wrote:
> On 08/07/16 16:44, Tamas K Lengyel wrote:
>> On Fri, Jul 8, 2016 at 3:33 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> On 08/07/16 03:31, 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>
>>> Is it wise having an on/off control without any further filtering?  (I
>>> suppose that it is at least a fine first start).
>> What type of extra filtering do you have in mind?
>
> Not sure.  What are you intending to use this facility for?

Primarily to detect malware that is fingerprinting it's environment by
looking for hypervisor leafs and/or doing timing based detection by
benchmarking cpuid with rdtsc.

>
> Given that the hypervisor is already in complete control of what a guest
> gets to see via cpuid, mutating the results via the monitor framework
> doesn't seem like a useful thing to do.

Indeed, the hypervisor is in control and to a certain extant the user
is via overriding some leafs in the domain config. However, there are
CPUID leafs Xen adds that the user is unable to override with the
domain config. For example in malware analysis it may be very useful
to be able to hide all hypervisor leafs from the guest, which
currently requires us to recompile Xen completely. By being able to
put the monitor system inline of CPUID it can decide which process it
wants to allow to see what leafs and when. It's very handy.

>
>>
>>> cpuid is usually the serialising instruction used with rdtsc for timing
>>> loops.  This is bad enough in VMs because of the VMExit, but becomes
>>> even worse if there is a monitor delay as well.
>> Yes, going the extra route of sending a monitor event out will add to
>> that delay (how much delay will depend on the subscriber and what it
>> decides to do with the event). Wouldn't we be able to mask some of
>> that with tsc offsetting though?
>
> I am going to go out on a limb and say that that is a very large can of
> worms which you don't want to open.

Yea, I'm well aware. However, we might have to go down that rabbit
hole eventually..

>
> The problem is not that time skews from the point of view of the guest,
> but that the timing loop with a fixed number of iterations takes
> proportionally longer.
>

Yes, there is overhead inevitably. For our use-case what would be the
goal is make the detection of this overhead as hard as possible so as
long as the overhead is reasonable (ie. we don't make network
connections drop and such) we can live with the overhead.

Cheers,
Tamas

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

Re: [PATCH] vmx/monitor: CPUID events

[Andrew Cooper]

On 08/07/16 17:59, Tamas K Lengyel wrote:
> On Fri, Jul 8, 2016 at 10:49 AM, Andrew Cooper
> <andrew.cooper3@citrix.com> wrote:
>> On 08/07/16 16:44, Tamas K Lengyel wrote:
>>> On Fri, Jul 8, 2016 at 3:33 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>>> On 08/07/16 03:31, 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>
>>>> Is it wise having an on/off control without any further filtering?  (I
>>>> suppose that it is at least a fine first start).
>>> What type of extra filtering do you have in mind?
>> Not sure.  What are you intending to use this facility for?
> Primarily to detect malware that is fingerprinting it's environment by
> looking for hypervisor leafs and/or doing timing based detection by
> benchmarking cpuid with rdtsc.
>
>> Given that the hypervisor is already in complete control of what a guest
>> gets to see via cpuid, mutating the results via the monitor framework
>> doesn't seem like a useful thing to do.
> Indeed, the hypervisor is in control and to a certain extant the user
> is via overriding some leafs in the domain config. However, there are
> CPUID leafs Xen adds that the user is unable to override with the
> domain config. For example in malware analysis it may be very useful
> to be able to hide all hypervisor leafs from the guest, which
> currently requires us to recompile Xen completely. By being able to
> put the monitor system inline of CPUID it can decide which process it
> wants to allow to see what leafs and when. It's very handy.

Fair enough.

For the record, my planned further work for cpuid will make things far
more configurable.  The current abilities of a toolstack, and the
in-hypervisor auditing are woeful.

~Andrew

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

Re: [PATCH] vmx/monitor: CPUID events

[Tian, Kevin]

> From: Tamas K Lengyel [mailto:tamas.lengyel@zentific.com]
> Sent: Friday, July 08, 2016 10:32 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>


> @@ -3525,10 +3527,23 @@ 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);
> +
> +        if ( rc < 0 )
> +            goto exit_and_crash;
> +        if ( !rc )
> +            update_guest_eip(); /* Safe: CPUID */

favor a simple comment to explain policies on various
rc values, as you have done in other places. 

Thanks
Kevin

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

Re: [PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

On Sun, Jul 10, 2016 at 9:00 PM, Tian, Kevin <kevin.tian@intel.com> wrote:
>> From: Tamas K Lengyel [mailto:tamas.lengyel@zentific.com]
>> Sent: Friday, July 08, 2016 10:32 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>
>
>
>> @@ -3525,10 +3527,23 @@ 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);
>> +
>> +        if ( rc < 0 )
>> +            goto exit_and_crash;
>> +        if ( !rc )
>> +            update_guest_eip(); /* Safe: CPUID */
>
> favor a simple comment to explain policies on various
> rc values, as you have done in other places.

Certainly.

Thanks,
Tamas

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