Re: [PATCH] x86: re-enable VCPUOP_register_vcpu_time_memory_area

[Keir Fraser <keir.xen@gmail.com>]

On 16/05/2013 09:47, "Jan Beulich" <JBeulich@suse.com> 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 <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

> ---
> 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);
> 
> 
>