[PATCH] mem_event: Add support for MEM_EVENT_REASON_MSR

[PATCH] mem_event: Add support for MEM_EVENT_REASON_MSR

[Razvan Cojocaru]

Add the new MEM_EVENT_REASON_MSR event type. Works similarly
to the other register events, except event.gla always contains
the MSR type (in addition to event.gfn, which holds the value).

Signed-off-by: Razvan Cojocaru <rzvncj@gmail.com>

diff -r f50aab21f9f2 -r 46990160b4a3 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Thu Dec 13 14:39:31 2012 +0000
+++ b/xen/arch/x86/hvm/hvm.c	Mon Dec 17 16:37:18 2012 +0200
@@ -2927,6 +2927,8 @@ int hvm_msr_write_intercept(unsigned int
     hvm_cpuid(1, &cpuid[0], &cpuid[1], &cpuid[2], &cpuid[3]);
     mtrr = !!(cpuid[3] & cpufeat_mask(X86_FEATURE_MTRR));
 
+    hvm_memory_event_msr(msr, msr_content);
+
     switch ( msr )
     {
     case MSR_EFER:
@@ -3857,6 +3859,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             case HVM_PARAM_MEMORY_EVENT_CR0:
             case HVM_PARAM_MEMORY_EVENT_CR3:
             case HVM_PARAM_MEMORY_EVENT_CR4:
+            case HVM_PARAM_MEMORY_EVENT_MSR:
                 if ( d == current->domain )
                     rc = -EPERM;
                 break;
@@ -4485,6 +4488,14 @@ void hvm_memory_event_cr4(unsigned long 
                            value, old, 0, 0);
 }
 
+void hvm_memory_event_msr(unsigned long msr, unsigned long value)
+{
+    hvm_memory_event_traps(current->domain->arch.hvm_domain
+                             .params[HVM_PARAM_MEMORY_EVENT_MSR],
+                           MEM_EVENT_REASON_MSR,
+                           value, ~value, 1, msr);
+}
+
 int hvm_memory_event_int3(unsigned long gla) 
 {
     uint32_t pfec = PFEC_page_present;
diff -r f50aab21f9f2 -r 46990160b4a3 xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h	Thu Dec 13 14:39:31 2012 +0000
+++ b/xen/include/asm-x86/hvm/hvm.h	Mon Dec 17 16:37:18 2012 +0200
@@ -448,6 +448,7 @@ int hvm_x2apic_msr_write(struct vcpu *v,
 void hvm_memory_event_cr0(unsigned long value, unsigned long old);
 void hvm_memory_event_cr3(unsigned long value, unsigned long old);
 void hvm_memory_event_cr4(unsigned long value, unsigned long old);
+void hvm_memory_event_msr(unsigned long msr, unsigned long value);
 /* Called for current VCPU on int3: returns -1 if no listener */
 int hvm_memory_event_int3(unsigned long gla);
 
diff -r f50aab21f9f2 -r 46990160b4a3 xen/include/public/hvm/params.h
--- a/xen/include/public/hvm/params.h	Thu Dec 13 14:39:31 2012 +0000
+++ b/xen/include/public/hvm/params.h	Mon Dec 17 16:37:18 2012 +0200
@@ -141,6 +141,8 @@
 #define HVM_PARAM_ACCESS_RING_PFN   28
 #define HVM_PARAM_SHARING_RING_PFN  29
 
-#define HVM_NR_PARAMS          30
+#define HVM_PARAM_MEMORY_EVENT_MSR  30
+
+#define HVM_NR_PARAMS          31
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
diff -r f50aab21f9f2 -r 46990160b4a3 xen/include/public/mem_event.h
--- a/xen/include/public/mem_event.h	Thu Dec 13 14:39:31 2012 +0000
+++ b/xen/include/public/mem_event.h	Mon Dec 17 16:37:18 2012 +0200
@@ -45,6 +45,7 @@
 #define MEM_EVENT_REASON_CR4         4    /* CR4 was hit: gfn is CR4 value */
 #define MEM_EVENT_REASON_INT3        5    /* int3 was hit: gla/gfn are RIP */
 #define MEM_EVENT_REASON_SINGLESTEP  6    /* single step was invoked: gla/gfn are RIP */
+#define MEM_EVENT_REASON_MSR         7    /* MSR was hit: gfn is MSR value, gla is MSR type */
 
 typedef struct mem_event_st {
     uint32_t flags;

Re: [PATCH] mem_event: Add support for MEM_EVENT_REASON_MSR

[Tim Deegan]

Hi,

At 16:37 +0200 on 17 Dec (1355762274), Razvan Cojocaru wrote:
> Add the new MEM_EVENT_REASON_MSR event type. Works similarly
> to the other register events, except event.gla always contains
> the MSR type (in addition to event.gfn, which holds the value).
> 
> Signed-off-by: Razvan Cojocaru <rzvncj@gmail.com>

I have two minor suggestions (below).

> diff -r f50aab21f9f2 -r 46990160b4a3 xen/include/public/hvm/params.h
> --- a/xen/include/public/hvm/params.h	Thu Dec 13 14:39:31 2012 +0000
> +++ b/xen/include/public/hvm/params.h	Mon Dec 17 16:37:18 2012 +0200
> @@ -141,6 +141,8 @@
>  #define HVM_PARAM_ACCESS_RING_PFN   28
>  #define HVM_PARAM_SHARING_RING_PFN  29
>  
> -#define HVM_NR_PARAMS          30
> +#define HVM_PARAM_MEMORY_EVENT_MSR  30

Can you put this up beside the other HVM_PARAM_MEMORY_EVENT_*
definitions, please? 

> +
> +#define HVM_NR_PARAMS          31
>  
>  #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
> diff -r f50aab21f9f2 -r 46990160b4a3 xen/include/public/mem_event.h
> --- a/xen/include/public/mem_event.h	Thu Dec 13 14:39:31 2012 +0000
> +++ b/xen/include/public/mem_event.h	Mon Dec 17 16:37:18 2012 +0200
> @@ -45,6 +45,7 @@
>  #define MEM_EVENT_REASON_CR4         4    /* CR4 was hit: gfn is CR4 value */
>  #define MEM_EVENT_REASON_INT3        5    /* int3 was hit: gla/gfn are RIP */
>  #define MEM_EVENT_REASON_SINGLESTEP  6    /* single step was invoked: gla/gfn are RIP */
> +#define MEM_EVENT_REASON_MSR         7    /* MSR was hit: gfn is MSR value, gla is MSR type */

Can you add a comment here to say that MEM_EVENT_REASON_MSR doesn't
honour the HVMPME_onchangeonly bit?

With those two changes, you can add 

Acked-by: Tim Deegan <tim@xen.org> 

and repost.

Cheers,

Tim.