Re: [Xen-devel] [PATCH v6 1/7] x86: detect and initialize Cache QoS Monitoring feature

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.13 at 10:38, Dongxiao Xu <dongxiao.xu@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -230,6 +230,12 @@ static void __devinit init_intel(struct cpuinfo_x86 *c)
>            ( c->cpuid_level >= 0x00000006 ) &&
>            ( cpuid_eax(0x00000006) & (1u<<2) ) )
>               set_bit(X86_FEATURE_ARAT, c->x86_capability);
> +
> +     /* Check platform QoS monitoring capability */
> +     if ((c->cpuid_level >= 0x00000007) &&
> +         (cpuid_ebx(0x00000007) & (1u<<12)))
> +             set_bit(X86_FEATURE_QOSM, c->x86_capability);
> +

This is redundant with generic_identify() setting the respective
c->x86_capability[] element.

> +struct pqos_cqm *cqm;

__read_mostly?

> +
> +static void __init init_cqm(void)
> +{
> +    unsigned int rmid;
> +    unsigned int eax, edx;
> +
> +    if ( !opt_cqm_max_rmid )
> +        return;
> +
> +    cqm = xzalloc(struct pqos_cqm);
> +    if ( !cqm )
> +        return;
> +
> +    cpuid_count(0xf, 1, &eax, &cqm->upscaling_factor, &cqm->max_rmid, &edx);
> +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> +    {
> +        xfree(cqm);

"cqm" is a global variable and - afaict - the only way for other
entities to tell whether the functionality is enabled. Hence shouldn't
you clear the variable here (and similarly further down)? Otherwise
the variable should perhaps be static.

Jan


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