Re: [PATCH v14 13/23] x86: refactor psr: CDP: implement CPU init flow.

[Yi Sun <yi.y.sun@linux.intel.com>]

On 17-07-31 08:20:44, Jan Beulich wrote:
> >>> Yi Sun <yi.y.sun@linux.intel.com> 07/15/17 2:48 AM >>>
> >@@ -272,7 +312,8 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
> >if ( !opt_cpu_info )
> >return 0;
>  >
> >-    printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> >+    printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> >+           ((type == FEAT_TYPE_L3_CDP) ? "CDP" : "L3 CAT"),
> 
> Why is this not "L3 CDP" when the enumerator includes L3?
> 
Sure, I will change it. Thanks!

> >@@ -1283,10 +1344,22 @@ static void psr_cpu_init(void)
> >feat = feat_l3;
> >feat_l3 = NULL;
>  >
> >-        if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) )
> >-            feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
> >+        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> >+        {
> >+            if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) )
> >+                feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
> >+            else
> >+                /* If CDP init fails, try to work as L3 CAT. */
> >+                goto l3_cat_init;
> >+        }
> >else
> >-            feat_l3 = feat;
> >+        {
> >+ l3_cat_init:
> 
> I'd really like to ask to re-structure this slightly so that you won't
> need goto and a label here. As said before, goto-s are sort of okay
> for making complicated error paths readable, but they should be
> avoided in almost all other cases.
>
Got it, I will try to remove the goto. Thanks for the review!

BRs,
Sun Yi
 
> Jan

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