Re: [Xen-devel] [PATCH v11 15/23] x86: refactor psr: CDP: implement set value callback function.

[Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>]

On 17-05-31 03:44:31, Jan Beulich wrote:
> >>> On 03.05.17 at 10:44, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -352,9 +352,21 @@ static bool l3_cdp_get_feat_info(const struct 
> > feat_node *feat,
> >      return true;
> >  }
> >  
> > +static void l3_cdp_write_msr(unsigned int cos, uint32_t val, enum cbm_type 
> > type)
> > +{
> > +    /* Data */
> > +    if ( type == PSR_CBM_TYPE_L3_DATA )
> > +        wrmsrl(MSR_IA32_PSR_L3_MASK_DATA(cos), val);
> > +
> > +    /* Code */
> > +    if ( type == PSR_CBM_TYPE_L3_CODE )
> > +        wrmsrl(MSR_IA32_PSR_L3_MASK_CODE(cos), val);
> > +}
> 
> With l3_cat_write_msr() ignoring "type" altogether I think this wants
> to be an if/else pair (or even a conditional expression for the first
> wrmsrl() argument).
> 
Ok, thanks!

> > @@ -765,7 +777,8 @@ static int insert_val_into_array(uint32_t val[],
> >  
> >      /* Value setting position is same as feature array. */
> >      for ( i = 0; i < props->cos_num; i++ )
> > -        if ( type == props->type[i] )
> > +        if ( type == props->type[i] ||
> > +             (feat_type == PSR_SOCKET_L3_CDP && type == PSR_CBM_TYPE_L3) )
> 
> Didn't the earlier patch take care of doing this substitution? Non-
> feature-specific code clearly shouldn't have such special cases if
> at all avoidable.
> 
User can set both DATA and CODE to same value at same time with below command:
xl psr-cat-set dom_id 0x3ff

Because no '-c' or '-d' is input, the cbm type will be 'PSR_CBM_TYPE_L3'.

To handle this case, we have to add a special case here. If the cbm tyep is
'PSR_CBM_TYPE_L3' and the feature type is CDP, we set both DATA and CODE. This
should be the simplest way to handle this case.

> Jan

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