[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v10 08/25] x86: refactor psr: L3 CAT: implement get value flow.
On 17-04-06 02:40:01, Jan Beulich wrote: > >>> On 06.04.17 at 08:10, <yi.y.sun@xxxxxxxxxxxxxxx> wrote: > > On 17-04-05 09:51:44, Jan Beulich wrote: > >> >>> On 01.04.17 at 15:53, <yi.y.sun@xxxxxxxxxxxxxxx> wrote: [...] > >> > --- a/xen/arch/x86/psr.c > >> > +++ b/xen/arch/x86/psr.c > >> > @@ -97,6 +97,10 @@ struct feat_node { > >> > /* get_feat_info is used to get feature HW info. */ > >> > bool (*get_feat_info)(const struct feat_node *feat, > >> > uint32_t data[], unsigned int array_len); > >> > + > >> > + /* get_val is used to get feature COS register value. */ > >> > + void (*get_val)(const struct feat_node *feat, unsigned int cos, > >> > + uint32_t *val); > >> > } *props; > >> > > >> > uint32_t cos_reg_val[MAX_COS_REG_CNT]; > >> > @@ -265,10 +269,17 @@ static bool cat_get_feat_info(const struct > >> > feat_node *feat, > >> > return true; > >> > } > >> > > >> > +static void cat_get_val(const struct feat_node *feat, unsigned int cos, > >> > + uint32_t *val) > >> > +{ > >> > + *val = feat->cos_reg_val[cos]; > >> > +} > >> > >> This can be done by the caller - there's nothing feature specific in > >> here, so there's no need for a hook. > >> > > Hmm, CDP's 'get_val' is different so that we need this hook. Do you mean I > > should create this CAT's 'get_val' hook when implementing CDP patch? > > No, not really - doesn't the type-to-index mapping array (or > whichever way it ends up being) all take care of the feature > specific aspects here? > For CDP case, the value getting depends on type. If we don't have this hook, we have to do special handling in main flow. Still use 'fits_cos_max' as example: /* For CDP, DATA is the first item in val[], CODE is the second. */ for ( j = 0; j < feat->props->cos_num; j++ ) { feat->props->get_val(feat, 0, feat->props->type[j], &default_val); if ( val[j] != default_val ) return false; } We want to get CDP DATA and CODE one by one to compare with val[] as the order. If we do not have 'get_val', how can we handle this case? Getting DATA is different with getting CODE which is shown below. Even we have type-to-index, we still need the hook to help I think. So far, I cannot figure out a generic way. Get data: (feat)->cos_reg_val[(cos) * 2] Get code: (feat)->cos_reg_val[(cos) * 2 + 1] Furthermore, 'get_val' is straightforward. I think it loses the pithiness if we remove the function. > >> > @@ -494,24 +505,34 @@ static struct psr_socket_info > >> > *get_socket_info(unsigned int socket) > >> > return socket_info + socket; > >> > } > >> > > >> > -int psr_get_info(unsigned int socket, enum cbm_type type, > >> > - uint32_t data[], unsigned int array_len) > >> > +static struct feat_node * psr_get_feat(unsigned int socket, > >> > >> Stray blank after *. > >> > >> > + enum cbm_type type) > >> > { > >> > const struct psr_socket_info *info = get_socket_info(socket); > >> > - const struct feat_node *feat; > >> > enum psr_feat_type feat_type; > >> > > >> > if ( IS_ERR(info) ) > >> > - return PTR_ERR(info); > >> > + return ERR_PTR(PTR_ERR(info)); > >> > >> Urgh. But yes, a cast would seem to be the worse alternative. > >> > > Then, any suggestion for this? Shall I add a parameter into the function to > > get this error number back? > > Once again, excuse me: Didn't my previous reply make clear that > while this looks ugly, I can't see a better alternative, and hence > it can remain as is unless someone comes up with a better > suggestion? > Oh, sorry, I mis-understood you. I thought you cannot accept current codes. But I was wrong. > Jan > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > https://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |