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

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 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?

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

Jan


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