[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v13 13/23] x86: refactor psr: CDP: implement CPU init flow.
>>> Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx> 07/06/17 4:07 AM >>> >This patch implements the CPU init flow for CDP. The flow is almost >same as L3 CAT. > >Note: CDP does NOT work until you apply the later patches of CDP. >"x86: refactor psr: CDP: implement get hw info flow." >"x86: refactor psr: CDP: implement set value callback function." This is _not_ what I did ask for in what I assume ... >v13: >- add commit message. ... this refers to: The problem isn't that it won't work, but that it'll crash the hypervisor. So what I had expected you to add is a note _after_ the first --- separator that this patch should not be applied without the other two ones. Or alternatively for this one to add stubs which the subsequent patches would then fill. >@@ -262,6 +280,29 @@ static int cat_init_feature(const struct cpuid_leaf *regs, > >break; > >+ case FEAT_TYPE_L3_CDP: >+ { >+ uint64_t val; >+ >+ if ( feat->cos_max < 3 ) >+ return -ENOENT; In the admittedly unlikely event that this return path gets taken (or any other current or future one), ... >@@ -1277,11 +1331,20 @@ static void psr_cpu_init(void) >{ >cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, ®s); > >- feat = feat_l3_cat; >- feat_l3_cat = 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) ) >+ { >+ feat = feat_l3_cdp; >+ feat_l3_cdp = NULL; >+ if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) ) >+ feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props; >+ } >+ else >+ { >+ feat = feat_l3_cat; >+ feat_l3_cat = NULL; >+ if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) ) >+ feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props; >+ } ... wouldn't it be a good idea to then try to setup plain L3 CAT here? I also notice that (already in patch 4) you leak feat here in case cat_init_feature() fails. You should put it back into the static helper variable you've taken it from. Furthermore, since you use at most one of feat_l3_cat and feat_l3_cdp, so I don't see why you need to allocate two of them. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |