Re: [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly

From: "Jan Beulich" <JBeulich@suse.com>
To: Corneliu ZUZU <czuzu@bitdefender.com>
Cc: Kevin Tian <kevin.tian@intel.com>,
	Tamas K Lengyel <tamas@tklengyel.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	xen-devel@lists.xen.org, Jun Nakajima <jun.nakajima@intel.com>
Subject: Re: [PATCH 4/7] vm-event/x86: use vm_event_vcpu_enter properly
Date: Thu, 16 Jun 2016 09:00:39 -0600	[thread overview]
Message-ID: <5762DB3702000078000F5BE0@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1466086187-7607-1-git-send-email-czuzu@bitdefender.com>

>>> On 16.06.16 at 16:09, <czuzu@bitdefender.com> wrote:
> @@ -2199,7 +2153,9 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
>  
>          if ( hvm_event_crX(CR0, value, old_value) )
>          {
> -            /* The actual write will occur in hvm_do_resume(), if permitted. */
> +            /* The actual write will occur in vcpu_enter_write_data(), if
> +             * permitted.
> +             */

Coding style.

> @@ -1432,18 +1430,16 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned int cr)
>          if ( paging_mode_hap(v->domain) )
>          {
>              /* Manage GUEST_CR3 when CR0.PE=0. */
> +            uint32_t old_ctls = v->arch.hvm_vmx.exec_control;
>              uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>                                   CPU_BASED_CR3_STORE_EXITING);
> +
>              v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
>              if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
>                  v->arch.hvm_vmx.exec_control |= cr3_ctls;
>  
> -            /* Trap CR3 updates if CR3 memory events are enabled. */
> -            if ( v->domain->arch.monitor.write_ctrlreg_enabled &
> -                 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> -                v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
> -
> -            vmx_update_cpu_exec_control(v);
> +            if ( old_ctls != v->arch.hvm_vmx.exec_control )
> +                vmx_update_cpu_exec_control(v);
>          }

How does this match up with the rest of this patch?

> @@ -179,6 +182,105 @@ void vm_event_fill_regs(vm_event_request_t *req)
>      req->data.regs.x86.cs_arbytes = seg.attr.bytes;
>  }
>  
> +static inline void vcpu_enter_write_data(struct vcpu *v)

Please allow the compiler to decide whether to inline such larger
functions.

> +void arch_vm_event_vcpu_enter(struct vcpu *v)
> +{
> +    /* vmx only */
> +    ASSERT( cpu_has_vmx );

Stray blanks.

Jan

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