Re: [PATCH v8 02/13] x86: detect and initialize Intel CAT feature

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

On Thu, May 28, 2015 at 01:54:39PM +0100, Jan Beulich wrote:
> >>> On 21.05.15 at 10:41, <chao.p.peng@linux.intel.com> wrote:
> > +
> > +    if ( !cpu_has(c, X86_FEATURE_CAT) )
> > +        return;
> > +
> > +    socket = cpu_to_socket(cpu);
> > +    if ( test_bit(socket, cat_socket_enable) )
> > +        return;
> > +
> > +    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> 
> While one would hope that X86_FEATURE_CAT implies the respective
> CPUID leaf being available, I think explicitly checking this should still
> be done just like is the case elsewhere.

Against cpuid_level?

> 
> > +    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);
> > +
> > +        set_bit(socket, cat_socket_enable);
> > +        printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> > +               socket, info->cos_max, info->cbm_len);
> > +    }
> > +}
> > +
> > +static void cat_cpu_fini(unsigned int cpu)
> > +{
> > +    unsigned int socket = cpu_to_socket(cpu);
> > +
> > +    if ( !socket_cpumask[socket] || cpumask_empty(socket_cpumask[socket]) )
> > +        clear_bit(socket, cat_socket_enable);
> > +}
> 
> This being called from the CPU_DEAD notification, you now depend
> on cpu_smpboot_free) to run ahead of you. Which isn't the case
> afaict, and even if it happened to be that way you shouldn't rely
> on it without explicitly enforcing ordering between the two by
> setting the priority of on of them to a non-default value.

Yes, seems changing the priority of psr_cpu_callback to 1 is enough.

> 
> > +static void __init init_psr_cat(void)
> > +{
> > +    if ( opt_cos_max < 1 )
> > +    {
> > +        printk(XENLOG_INFO "CAT: disabled, cos_max is too small\n");
> > +        return;
> > +    }
> 
> Is opt_cos_max == 1 really useful for anything?

That means two COSes are available. cos=0 is reserved and cos=1 can
still be used anyway.

Chao