[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v10 11/25] x86: refactor psr: L3 CAT: set value: implement cos finding flow.
>>> On 01.04.17 at 15:53, <yi.y.sun@xxxxxxxxxxxxxxx> wrote: > --- a/xen/arch/x86/psr.c > +++ b/xen/arch/x86/psr.c > @@ -720,8 +720,83 @@ static int find_cos(const uint32_t val[], unsigned int > array_len, > const struct psr_socket_info *info, > spinlock_t *ref_lock) > { > + unsigned int cos, i; > + const unsigned int *ref = info->cos_ref; > + const struct feat_node *feat; > + unsigned int cos_max; > + > ASSERT(spin_is_locked(ref_lock)); > > + /* cos_max is the one of the feature which is being set. */ > + feat = info->features[feat_type]; > + if ( !feat ) > + return -ENOENT; > + > + cos_max = feat->props->cos_max; > + > + for ( cos = 0; cos <= cos_max; cos++ ) > + { > + const uint32_t *val_ptr = val; > + bool found = false; > + > + if ( cos && !ref[cos] ) > + continue; > + > + /* > + * If fail to find cos in below loop, need find whole feature array > + * again from beginning. > + */ This comment is unrelated to any of the immediately surrounding code. Either move it, or get rid of it altogether. > + for ( i = 0; i < PSR_SOCKET_MAX_FEAT; i++ ) > + { > + uint32_t default_val = 0; Pointless initializer as it seems. > + feat = info->features[i]; > + if ( !feat ) > + continue; > + > + /* > + * COS ID 0 always stores the default value so input 0 to get > + * default value. > + */ > + feat->props->get_val(feat, 0, &default_val); > + > + /* > + * Compare value according to feature array order. > + * We must follow this order because value array is assembled > + * as this order. > + */ > + if ( cos > feat->props->cos_max ) > + { > + /* > + * If cos is bigger than feature's cos_max, the val should be > + * default value. Otherwise, it fails to find a COS ID. So we > + * have to exit find flow. > + */ > + if ( val[0] != default_val ) > + return -EINVAL; > + > + found = true; > + } > + else > + { > + if ( val[0] == feat->cos_reg_val[cos] ) > + found = true; > + } Same question as on previous patch- why val[0] twice above, despite cos_num possibly being larger than 1? And wouldn't this need to be val_ptr anyway? > + /* If fail to match, go to next cos to compare. */ > + if ( !found ) > + break; > + > + val_ptr += feat->props->cos_num; > + if ( val_ptr - val > array_len ) > + return -ENOSPC; This again looks suspicious - the check should once again be done before accessing the respective array element(s). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |