Re: [PATCH v8 04/24] x86: refactor psr: implement CPU init and free flow.

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-03-10 01:56:04, Jan Beulich wrote:
> >>> On 10.03.17 at 02:32, <yi.y.sun@linux.intel.com> wrote:
> > On 17-03-08 07:56:54, Jan Beulich wrote:
> >> >>> On 15.02.17 at 09:49, <yi.y.sun@linux.intel.com> wrote:

[...]
> >> > +    if ( !cpu_has(&current_cpu_data, X86_FEATURE_PQE) )
> >> 
> >> Do you really mean to not universally check the global (boot CPU)
> >> flag? I.e. possibly differing behavior on different CPUs?
> >> 
> > Yes, different sockets may have different configurations. E.g. sockt 0 has
> > PQE support but socket 1 does not.
> 
> For all other CPU features we assume symmetry. Why would we
> not do so here? And how would things even work in that case,
> when a vCPU gets moved (by the scheduler) from a more capable
> to a less capable pCPU?
> 
Hmm, ok, I will use boot cpu to check PQE here. Thanks!

> >> >  static void psr_cpu_init(void)
> >> >  {
> >> > +    if ( socket_info )
> >> > +        cpu_init_work();
> >> > +
> >> >      psr_assoc_init();
> >> >  }
> >> >  
> >> >  static void psr_cpu_fini(unsigned int cpu)
> >> >  {
> >> > +    if ( socket_info )
> >> > +        cpu_fini_work(cpu);
> >> >      return;
> >> >  }
> >> 
> >> Is it really useful to use another layer of helper functions here?
> >> 
> > The reason we define 'cpu_fini_work' is to match 'cpu_init_work'.
> 
> And the question was for both of them.
> 
> > If we move
> > codes of 'cpu_init_work' into 'psr_cpu_init', the codes look messy.
> 
> I don't think that's the case; I could see this as a valid argument if
> the calling functions above were already quite complex.
> 
> > That is
> > the reason we define 'cpu_init_work'. Do you think if it is acceptable to 
> > you?
> 
> Well, I won't NAK the patch if you keep them, but I'd prefer the
> functions to be folded into their callers.
> 
I will move the codes.

> Jan

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