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

2017-08-30 Thread Yi Sun 
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@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-08-30 Thread Roger Pau Monn� 
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@lists.xen.org
https://lists.xen.org/xen-devel

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

2017-08-29 Thread Yi Sun 
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.

> Roger.

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

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

2017-08-29 Thread Roger Pau Monné 
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)?

Roger.

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

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

2017-08-23 Thread Yi Sun 
This patch renames PSR sysctl/domctl interfaces and related xsm policy to
make them be general for all resource allocation features but not only
for CAT. Then, we can resuse the interfaces for all allocation features.

Basically, it changes 'cat' to 'alloc'. E.g.:
1. psr_cat_op -> psr_alloc_op
2. XEN_DOMCTL_psr_cat_op -> XEN_DOMCTL_psr_alloc_op
3. XEN_SYSCTL_psr_cat_op -> XEN_SYSCTL_psr_alloc_op

The sysctl/domctl version numbers are bumped.

Signed-off-by: Yi Sun 
Reviewed-by: Wei Liu 
Acked-by: Daniel De Graaf 
---
v1:
- add description about what to be changed in commit message.
  (suggested by Wei Liu)
- bump sysctl/domctl version numbers.
  (suggested by Wei Liu)
---
 tools/flask/policy/modules/dom0.te  |  4 ++--
 tools/libxc/xc_psr.c| 34 +-
 xen/arch/x86/domctl.c   | 36 ++--
 xen/arch/x86/sysctl.c   | 24 
 xen/include/public/domctl.h | 12 ++--
 xen/include/public/sysctl.h | 12 ++--
 xen/xsm/flask/hooks.c   |  8 
 xen/xsm/flask/policy/access_vectors |  8 
 8 files changed, 69 insertions(+), 69 deletions(-)

diff --git a/tools/flask/policy/modules/dom0.te 
b/tools/flask/policy/modules/dom0.te
index d0a4d91..1a376ac 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -14,7 +14,7 @@ allow dom0_t xen_t:xen {
tmem_control getscheduler setscheduler
 };
 allow dom0_t xen_t:xen2 {
-   resource_op psr_cmt_op psr_cat_op pmu_ctrl get_symbol
+   resource_op psr_cmt_op psr_alloc_op pmu_ctrl get_symbol
get_cpu_levelling_caps get_cpu_featureset livepatch_op
gcov_op
 };
@@ -39,7 +39,7 @@ allow dom0_t dom0_t:domain {
 };
 allow dom0_t dom0_t:domain2 {
set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
-   get_vnumainfo psr_cmt_op psr_cat_op
+   get_vnumainfo psr_cmt_op psr_alloc_op
 };
 allow dom0_t dom0_t:resource { add remove };
 
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index 039b920..73d05f2 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -274,11 +274,11 @@ int xc_psr_cat_set_domain_data(xc_interface *xch, 
uint32_t domid,
 return -1;
 }
 
-domctl.cmd = XEN_DOMCTL_psr_cat_op;
+domctl.cmd = XEN_DOMCTL_psr_alloc_op;
 domctl.domain = (domid_t)domid;
-domctl.u.psr_cat_op.cmd = cmd;
-domctl.u.psr_cat_op.target = target;
-domctl.u.psr_cat_op.data = data;
+domctl.u.psr_alloc_op.cmd = cmd;
+domctl.u.psr_alloc_op.target = target;
+domctl.u.psr_alloc_op.data = data;
 
 return do_domctl(xch, );
 }
@@ -310,15 +310,15 @@ int xc_psr_cat_get_domain_data(xc_interface *xch, 
uint32_t domid,
 return -1;
 }
 
-domctl.cmd = XEN_DOMCTL_psr_cat_op;
+domctl.cmd = XEN_DOMCTL_psr_alloc_op;
 domctl.domain = (domid_t)domid;
-domctl.u.psr_cat_op.cmd = cmd;
-domctl.u.psr_cat_op.target = target;
+domctl.u.psr_alloc_op.cmd = cmd;
+domctl.u.psr_alloc_op.target = target;
 
 rc = do_domctl(xch, );
 
 if ( !rc )
-*data = domctl.u.psr_cat_op.data;
+*data = domctl.u.psr_alloc_op.data;
 
 return rc;
 }
@@ -329,29 +329,29 @@ int xc_psr_cat_get_info(xc_interface *xch, uint32_t 
socket, unsigned int lvl,
 int rc = -1;
 DECLARE_SYSCTL;
 
-sysctl.cmd = XEN_SYSCTL_psr_cat_op;
-sysctl.u.psr_cat_op.target = socket;
+sysctl.cmd = XEN_SYSCTL_psr_alloc_op;
+sysctl.u.psr_alloc_op.target = socket;
 
 switch ( lvl )
 {
 case 2:
-sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
+sysctl.u.psr_alloc_op.cmd = XEN_SYSCTL_PSR_CAT_get_l2_info;
 rc = xc_sysctl(xch, );
 if ( !rc )
 {
-*cos_max = sysctl.u.psr_cat_op.u.cat_info.cos_max;
-*cbm_len = sysctl.u.psr_cat_op.u.cat_info.cbm_len;
+*cos_max = sysctl.u.psr_alloc_op.u.cat_info.cos_max;
+*cbm_len = sysctl.u.psr_alloc_op.u.cat_info.cbm_len;
 *cdp_enabled = false;
 }
 break;
 case 3:
-sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
+sysctl.u.psr_alloc_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
 rc = xc_sysctl(xch, );
 if ( !rc )
 {
-*cos_max = sysctl.u.psr_cat_op.u.cat_info.cos_max;
-*cbm_len = sysctl.u.psr_cat_op.u.cat_info.cbm_len;
-*cdp_enabled = sysctl.u.psr_cat_op.u.cat_info.flags &
+*cos_max = sysctl.u.psr_alloc_op.u.cat_info.cos_max;
+*cbm_len = sysctl.u.psr_alloc_op.u.cat_info.cbm_len;
+*cdp_enabled = sysctl.u.psr_alloc_op.u.cat_info.flags &
XEN_SYSCTL_PSR_CAT_L3_CDP;
 }
 break;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 127c84e..24b998b 100644
---