Re: [PATCH RFC 1/7] xen/mem_event: Cleanup of mem_event structures

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 12/11/14 15:31, Tamas K Lengyel wrote:
> diff --git a/xen/include/public/mem_event.h b/xen/include/public/mem_event.h
> index 599f9e8..c0e9394 100644
> --- a/xen/include/public/mem_event.h
> +++ b/xen/include/public/mem_event.h
> @@ -49,15 +49,19 @@
>  #define MEM_EVENT_FLAG_EMULATE_NOWRITE (1 << 6)
>  
>  /* Reasons for the memory event request */
> -#define MEM_EVENT_REASON_UNKNOWN     0    /* typical reason */
> -#define MEM_EVENT_REASON_VIOLATION   1    /* access violation, GFN is address */
> -#define MEM_EVENT_REASON_CR0         2    /* CR0 was hit: gfn is new CR0 value, gla is previous */
> -#define MEM_EVENT_REASON_CR3         3    /* CR3 was hit: gfn is new CR3 value, gla is previous */
> -#define MEM_EVENT_REASON_CR4         4    /* CR4 was hit: gfn is new CR4 value, gla is previous */
> -#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 address;
> -                                             does NOT honour HVMPME_onchangeonly */
> +typedef enum {
> +    MEM_EVENT_REASON_UNKNOWN,              /* Default case */
> +    MEM_EVENT_REASON_MEM_ACCESS_VIOLATION, /* Memory access violation */
> +    MEM_EVENT_REASON_MEM_SHARING,          /* Memory sharing event */
> +    MEM_EVENT_REASON_MEM_PAGING,           /* Memory paging event */
> +    MEM_EVENT_REASON_CR0,                  /* CR0 was updated */
> +    MEM_EVENT_REASON_CR3,                  /* CR3 was updated */
> +    MEM_EVENT_REASON_CR4,                  /* CR4 was updated */
> +    MEM_EVENT_REASON_INT3,                 /* Debug operation executed (int3) */
> +    MEM_EVENT_REASON_SINGLESTEP,           /* Single-step (MTF) */
> +    MEM_EVENT_REASON_MSR,                  /* An MSR was updated.
> +                                            * Does NOT honour HVMPME_onchangeonly */
> +} mem_event_reason_t;

Please keep these as defines.  The width of an enum is implementation
defined, meaning that different compilers are free to interpret the
width of the above definition differently, which affects the size and
alignment of mem_event_st below.

(It is completely wrong that Xen's ABI/API was ever specified using C,
rather than a document stating actual data structure widths, but that
horse has long-since bolted)

~Andrew