Re: [PATCH] mem_event: Add support for MEM_EVENT_REASON_MSR

[Tim Deegan <tim@xen.org>]

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.