From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: [PATCH] x86: re-enable VCPUOP_register_vcpu_time_memory_area Date: Thu, 16 May 2013 10:44:10 +0100 Message-ID: References: <5194B93702000078000D6A7E@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5194B93702000078000D6A7E@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: George Dunlap , Tim Deegan , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 16/05/2013 09:47, "Jan Beulich" wrote: > By moving the call to update_vcpu_system_time() out of schedule() into > arch-specific context switch code, the original problem of the function > accessing the wrong domain's address space goes away (obvious even from > patch context, as update_runstate_area() does similar copying). > > Signed-off-by: Jan Beulich Acked-by: Keir Fraser > --- > Regardless of the code freeze I'd still like to propose this for > inclusion in 4.3, mainly based on the fact that this got disabled late > in the 4.0 release cycle with the expectation that it would get > re-enabled soon after. Now that upstream Linux also has, as of 3.8 at > least on x86-64, the necessary hypervisor independent support code, it > would be odd to not leverage this on Xen. > > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -232,6 +232,9 @@ static void schedule_tail(struct vcpu *p > > if ( prev != current ) > update_runstate_area(current); > + > + /* Ensure that the vcpu has an up-to-date time base. */ > + update_vcpu_system_time(current); > } > > static void continue_new_vcpu(struct vcpu *prev) > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -966,11 +966,6 @@ arch_do_vcpu_op( > > switch ( cmd ) > { > - /* > - * XXX Disable for 4.0.0: __update_vcpu_system_time() writes to the given > - * virtual address even when running in another domain's address space. > - */ > -#if 0 > case VCPUOP_register_vcpu_time_memory_area: > { > struct vcpu_register_time_memory_area area; > @@ -989,7 +984,6 @@ arch_do_vcpu_op( > > break; > } > -#endif > > case VCPUOP_get_physid: > { > @@ -1457,6 +1451,9 @@ void context_switch(struct vcpu *prev, s > if (prev != next) > update_runstate_area(next); > > + /* Ensure that the vcpu has an up-to-date time base. */ > + update_vcpu_system_time(next); > + > schedule_tail(next); > BUG(); > } > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3174,6 +3174,7 @@ static long hvm_vcpu_op( > case VCPUOP_set_singleshot_timer: > case VCPUOP_stop_singleshot_timer: > case VCPUOP_register_vcpu_info: > + case VCPUOP_register_vcpu_time_memory_area: > rc = do_vcpu_op(cmd, vcpuid, arg); > break; > default: > @@ -3232,6 +3233,7 @@ static long hvm_vcpu_op_compat32( > case VCPUOP_set_singleshot_timer: > case VCPUOP_stop_singleshot_timer: > case VCPUOP_register_vcpu_info: > + case VCPUOP_register_vcpu_time_memory_area: > rc = compat_vcpu_op(cmd, vcpuid, arg); > break; > default: > --- a/xen/common/schedule.c > +++ b/xen/common/schedule.c > @@ -1231,8 +1231,6 @@ static void schedule(void) > if ( next_slice.migrated ) > evtchn_move_pirqs(next); > > - /* Ensure that the domain has an up-to-date time base. */ > - update_vcpu_system_time(next); > vcpu_periodic_timer_work(next); > > context_switch(prev, next); > > >