Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature

[Chao Peng <chao.p.peng@linux.intel.com>]

On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -19,14 +19,26 @@
> >  #include <asm/psr.h>
> >  
> >  #define PSR_CMT        (1<<0)
> > +#define PSR_CAT        (1<<1)
> > +
> > +struct psr_cat_socket_info {
> > +    unsigned int cbm_len;
> > +    unsigned int cos_max;
> > +};
> >  
> >  struct psr_assoc {
> >      uint64_t val;
> >  };
> >  
> >  struct psr_cmt *__read_mostly psr_cmt;
> > +
> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
> 
> Didn't we agree to fold these two into one? Apart from that the
> _bitmap name tag doesn't seem very useful...

Looks like this?

static unsigned long *__read_mostly cat_socket_init,
                     *__read_mostly cat_socket_enable;

> > +    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> > +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
> > +    {
> > +        cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> > +        info = cat_socket_info + socket;
> > +        info->cbm_len = (eax & 0x1f) + 1;
> > +        info->cos_max = min(opt_cos_max, edx & 0xffff);
> 
> 
> Is opt_cos_max being zero (or even one) going to result in a useful /
> working environment? I.e. shouldn't you rather disable CAT in that
> case?

OK, I will disable CAT in init_psr_cat() for this case.

> 
> > +static void cat_cpu_fini(void)
> > +{
> > +    unsigned int cpu = smp_processor_id();
> > +    unsigned int socket = cpu_to_socket(cpu);
> 
> This is the only use of "cpu", i.e. the variable is pretty pointless.
> 
> >  static int cpu_callback(
> >      struct notifier_block *nfb, unsigned long action, void *hcpu)
> >  {
> >      if ( action == CPU_STARTING )
> >          psr_cpu_init();
> > +    else if ( action == CPU_DYING )
> > +        psr_cpu_fini();
> 
> Are these the right notifiers for doing things involving memory
> allocation / freeing?

psr_cpu_fini() does not really have memory allocation/freeing but
psr_cpu_init() does have.

While one thing to clarify is: psr_cpu_init() will not propagate
the error even when the memory allocation fails(instead it disables
the CAT on the socket).

If there is still problem then perhaps I need change CPU_STARTING back
to CPU_ONLINE and make on_selected_cpus() call on target cpu.

Chao