Re: [Xen-devel] [PATCH v2 for 4.5] x86/viridian: Freeze time reference counter when domain is paused

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 16 October 2014 09:07
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> Subject: Re: [PATCH v2 for 4.5] x86/viridian: Freeze time reference counter
> when domain is paused
> 
> >>> On 13.10.14 at 15:38, <paul.durrant@xxxxxxxxxx> wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -43,6 +43,10 @@
> >  #include <xen/trace.h>
> >  #include <xen/tmem.h>
> >
> > +#ifdef CONFIG_X86
> > +#include <asm/hvm/viridian.h>
> > +#endif
> > +
> >  /* Linux config option: propageted to domain0 */
> >  /* xen_processor_pmbits: xen control Cx, Px, ... */
> >  unsigned int xen_processor_pmbits = XEN_PROCESSOR_PM_PX;
> > @@ -706,6 +710,11 @@ void domain_shutdown(struct domain *d, u8
> reason)
> >          v->paused_for_shutdown = 1;
> >      }
> >
> > +#ifdef CONFIG_X86
> > +    if ( has_viridian_time_ref_count(d) )
> > +        viridian_time_ref_count_freeze(d);
> > +#endif
> > +
> >      __domain_finalise_shutdown(d);
> >
> >      spin_unlock(&d->shutdown_lock);
> > @@ -925,32 +934,42 @@ int vcpu_unpause_by_systemcontroller(struct
> vcpu *v)
> >      return 0;
> >  }
> >
> > -void domain_pause(struct domain *d)
> > +void do_domain_pause(struct domain *d,
> > +                     void (*sleep_fn)(struct vcpu *v))
> 
> static
> 
> >  {
> >      struct vcpu *v;
> >
> > -    ASSERT(d != current->domain);
> > -
> >      atomic_inc(&d->pause_count);
> >
> >      for_each_vcpu( d, v )
> > -        vcpu_sleep_sync(v);
> > +        sleep_fn(v);
> > +
> > +#ifdef CONFIG_X86
> > +    if ( has_viridian_time_ref_count(d) )
> > +        viridian_time_ref_count_freeze(d);
> > +#endif
> >  }
> >
> > -void domain_pause_nosync(struct domain *d)
> > +void domain_pause(struct domain *d)
> >  {
> > -    struct vcpu *v;
> > -
> > -    atomic_inc(&d->pause_count);
> > +    ASSERT(d != current->domain);
> > +    do_domain_pause(d, vcpu_sleep_sync);
> > +}
> >
> > -    for_each_vcpu( d, v )
> > -        vcpu_sleep_nosync(v);
> > +void domain_pause_nosync(struct domain *d)
> > +{
> > +    do_domain_pause(d, vcpu_sleep_nosync);
> >  }
> >
> >  void domain_unpause(struct domain *d)
> >  {
> >      struct vcpu *v;
> >
> > +#ifdef CONFIG_X86
> > +    if ( has_viridian_time_ref_count(d) )
> > +        viridian_time_ref_count_thaw(d);
> > +#endif
> > +
> >      if ( atomic_dec_and_test(&d->pause_count) )
> >          for_each_vcpu( d, v )
> >              vcpu_wake(v);
> 
> All these CONFIG_X86 uses should be replaced imo:
> arch_domain_{shutdown,pause,unpause}() should be used instead,
> either defining empty stubs for ARM or using #ifdef on them instead
> of CONFIG_X86. That's then also avoid the inclusion of an x86-
> specific header in this common file.
> 

That makes sense. I didn't want to make the leap, but now you've sanctioned it 
I'll do it :-)

  Paul

> Jan


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