[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.