Re: [Xen-devel] [PATCH v2 02/15] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

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

On 17-08-30 08:47:42, Roger Pau Monn� wrote:
> On Wed, Aug 30, 2017 at 01:23:08PM +0800, Yi Sun wrote:
> > On 17-08-29 13:00:19, Roger Pau Monn� wrote:
> > > On Thu, Aug 24, 2017 at 09:14:36AM +0800, Yi Sun wrote:
> > > > --- a/xen/arch/x86/domctl.c
> > > > +++ b/xen/arch/x86/domctl.c
> > > > @@ -1439,60 +1439,60 @@ long arch_do_domctl(
> > > >          }
> > > >          break;
> > > >  
> > > > -    case XEN_DOMCTL_psr_cat_op:
> > > > -        switch ( domctl->u.psr_cat_op.cmd )
> > > > +    case XEN_DOMCTL_psr_alloc_op:
> > > > +        switch ( domctl->u.psr_alloc_op.cmd )
> > > >          {
> > > >              uint32_t val32;
> > > >  
> > > >          case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
> > > 
> > > In order to match the name of the structure used, shouldn't those
> > > defines be renamed to XEN_DOMCTL_PSR_ALLOC_* (instead of CAT)?
> > > 
> > These names are feature specific. E.g:
> >     XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM - L3 CAT
> >     XEN_DOMCTL_PSR_MBA_OP_SET_THRTL  - MBA
> > 
> > Could you accept them? If not, I think I may write a separate patch to 
> > rename
> > them.
> 
> Maybe I'm just lost, but in this patch you are renaming psr_cat_op to
> psr_alloc_op, hence I think those defines should also be renamed in
> this same patch.
> 
> I would also prefer that you don't introduce the '_OP_|_op_'
> everywhere, I don't think it's useful/descriptive. Hypercalls are
> operations by definition IMHO, hence the 'op' thing doesn't add
> anything useful.
> 
> Ie: the struct should be psr_alloc, and the name
> XEN_DOMCTL_psr_alloc.
> 
Ok, I will modify these things per your suggestion.

> Roger.

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