Re: [Xen-devel] [PATCH v12 13/23] x86: refactor psr: CDP: implement CPU init flow.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 06/14/17 3:26 AM >>>
> @@ -253,6 +271,26 @@ static void cat_init_feature(const struct cpuid_leaf 
> *regs,
>  
>          break;
>  
> +    case PSR_SOCKET_L3_CDP:
> +    {
> +        uint64_t val;
> +
> +        /* Cut half of cos_max when CDP is enabled. */
> +        feat->cos_max >>= 1;

I'm afraid this is off by one in the unusual but possible case of cos_max
being an even number.

> +        wrmsrl(MSR_IA32_PSR_L3_MASK(0), cat_default_val(feat->cbm_len));
> +        wrmsrl(MSR_IA32_PSR_L3_MASK(1), cat_default_val(feat->cbm_len));
> +        rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> +        wrmsrl(MSR_IA32_PSR_L3_QOS_CFG,
> +               val | (1ull << PSR_L3_QOS_CDP_ENABLE_BIT));
> +
> +        /* cos=0 is reserved as default cbm(all bits within cbm_len are 1). 
> */

Along the lines of a comment to an earlier patch, please add a blank ahead of
the opeing paren.

> +        get_cdp_code(feat, 0) = cat_default_val(feat->cbm_len);
> +        get_cdp_data(feat, 0) = cat_default_val(feat->cbm_len);

Wouldn't you better do this prior to enabling CDP?

> @@ -1294,11 +1344,21 @@ static void psr_cpu_init(void)
>      {
>          cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, ®s);
>  
> -        feat = feat_l3_cat;
> -        feat_l3_cat = NULL;
> -        feat_props[PSR_SOCKET_L3_CAT] = &l3_cat_props;
> -
> -        cat_init_feature(®s, feat, info, PSR_SOCKET_L3_CAT);
> +        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) &&
> +             !info->features[PSR_SOCKET_L3_CDP] )

Doesn't this last check mean you'd set up CAT in case would come here for
the 2nd CPU on a socket? In the end the check is simplky pointless afaict,
due to psr_cpu_init() calling cat_init_feature() only if ->feat_init is still
false. But please remove it as being potentially confusing (and inconsistent
with the else branch).

Jan

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