Re: [Xen-devel] [PATCH V5] xen/vm_event: Clean up control-register-write vm_events and add XCR0 event

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.05.15 at 10:33, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -19,6 +19,7 @@
>  * Place - Suite 330, Boston, MA 02111-1307 USA.
>  */
>  
> +#include <asm/monitor.h>
>  #include <xen/vm_event.h>
>  #include <xen/paging.h>

Just like almost everywhere else, please have asm/ includes follow
xen/ ones.

> @@ -3349,7 +3354,7 @@ int hvm_set_cr4(unsigned long value)
>      }
>  
>      hvm_update_cr(v, 4, value);
> -    hvm_event_cr4(value, old_cr);
> +    hvm_event_cr(CR4, value, old_cr);

Considering e.g. this one, ...

> @@ -2010,7 +2012,7 @@ static int vmx_cr_access(unsigned long 
> exit_qualification)
>          unsigned long old = curr->arch.hvm_vcpu.guest_cr[0];
>          curr->arch.hvm_vcpu.guest_cr[0] &= ~X86_CR0_TS;
>          vmx_update_guest_cr(curr, 0);
> -        hvm_event_cr0(curr->arch.hvm_vcpu.guest_cr[0], old);
> +        hvm_event_cr(VM_EVENT_X86_CR0, curr->arch.hvm_vcpu.guest_cr[0], old);

... does this build at all? Ah - you put the wrapping #define into
xen/arch/x86/hvm/hvm.c instead of right alongside the function
declaration. That's not how I would think this should be done -
either use the wrapper everywhere, or nowhere.

> @@ -156,14 +158,15 @@ struct vm_event_mem_access {
>      uint32_t _pad;
>  };
>  
> -struct vm_event_mov_to_cr {
> +struct vm_event_write_ctrlreg {
> +    uint32_t index;
>      uint64_t new_value;
>      uint64_t old_value;
> +    uint32_t _pad;
>  };

Why would you want to add padding anywhere other than between
index and new_value?

Jan


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