[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v15 13/23] x86: refactor psr: CDP: implement CPU init flow.
>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 08/01/17 11:04 AM >>> >@@ -1278,15 +1339,31 @@ static void psr_cpu_init(void) >cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, ®s); >if ( regs.b & PSR_RESOURCE_TYPE_L3 ) >{ >+ bool do_l3_cat_init = true; >+ >cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, ®s); > >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; >- else >- feat_l3 = feat; >+ if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) ) >+ { >+ /* If CDP init fails, try to work as L3 CAT. */ >+ if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) ) >+ { >+ feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props; >+ /* CDP init succeeds, no need to do L3 CAT init. */ >+ do_l3_cat_init = false; >+ } >+ } The comment ahead of the inner if() now really describes the (implicit) else case. That's somewhat misleading. How about putting feat back into feat_l3 in an actual "else", and using that at once instead of the somewhat clumsily named "do_l3_cat_init" local variable? That would additionally avoid the need for me to ask you to fold the two if()s. Plus the resulting code would become more similar ... >+ if ( do_l3_cat_init ) >+ { >+ if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) ) >+ feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props; >+ else >+ feat_l3 = feat; >+ } ... to this. And btw, to avoid spamming the list with another full version of this series, I'd be fine with you just submitting v15.1 of this one patch. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |