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

[Roger Pau Monn� <roger.pau@xxxxxxxxxx>]

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.

Roger.

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