Re: [PATCH V2] xen/x86: Clean up vm_event-related code in asm-x86/domain.h

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

On 15/08/15 07:23, Razvan Cojocaru wrote:
> As suggested by Jan Beulich, moved struct monitor_write_data from
> struct arch_domain to struct arch_vcpu, as well as moving all
> vm_event-related data from asm-x86/domain.h to struct vm_event,
> and allocating it dynamically only when needed.
>
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Apologies - this fell off my radar while travelling.

Looks fine, and also looks like a candidate for 4.6 as well?

Just one comment.

> diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
> index 0ae5952..17ba4eb 100644
> --- a/xen/include/asm-x86/vm_event.h
> +++ b/xen/include/asm-x86/vm_event.h
> @@ -22,6 +22,18 @@
>  #include <xen/sched.h>
>  #include <xen/vm_event.h>
>  
> +/*
> + * Should we emulate the next matching instruction on VCPU resume
> + * after a vm_event?
> + */
> +struct vm_event {

This should be named arch_vm_event as it is an architecture specific
header file, and lives in arch_vcpu.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> +    uint32_t emulate_flags;
> +    unsigned long gpa;
> +    unsigned long eip;
> +    struct vm_event_emul_read_data emul_read_data;
> +    struct monitor_write_data write_data;
> +};
> +
>  int vm_event_init_domain(struct domain *d);
>  
>  void vm_event_cleanup_domain(struct domain *d);