Re: [Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.
On 16-10-10 01:34:47, Jan Beulich wrote: > >>> On 09.10.16 at 08:43, wrote: > > On 16-09-30 17:18:33, Konrad Rzeszutek Wilk wrote: > >> On Thu, Sep 22, 2016 at 10:15:20AM +0800, Yi Sun wrote: > >> > Current psr.c is designed for supporting L3 CAT/CDP. It has many > >> > limitations to add new feature. Considering to support more PSR > >> > features, we need refactor PSR implementation to make it more > >> > flexible and fulfill the principle, open for extension but closed > >> > for modification. > >> > > >> > The core of the refactoring is to abstract the common actions and > >> > encapsulate them into "struct feat_ops". The detailed steps to add > >> > a new feature are described at the head of psr.c. > >> > > >> > Signed-off-by: Yi Sun > >> > > >> > --- > >> > Changed since v1: > >> > * sysctl.c > >> > - Interface change for abstraction requirement. > >> > * psr.c > >> > - Function and variables names are changed to express accurately. > >> > - Fix code style issues. > >> > - Fix imprecise comments. > >> > - Add one callback function, get_cos_num(), to fulfill the > >> > abstraction requirement. > >> > - Divide get_old_set_new() callback functions into two functions: > >> > get_old_val() and set_new_val() make it more primitive. > >> > - Change feat_info type from an array to union. > >> > - Adjust some functions to replace if to switch to make them > >> > clearer. > >> > - Replace custom list management to system. > >> > - Use 'const' to make codes more safe. > >> > * psr.h > >> > - Change 'enum mask_type' to 'enum psr_val_type' to express > >> > more accurate. > >> > - Change parameters of psr_get_info() to fulfill abstraction > >> > requirement. > >> > --- > >> > xen/arch/x86/domctl.c | 36 +- > >> > xen/arch/x86/psr.c| 1105 > >> > +++-- > >> > >> Whoa. 1K changes. This is a dense patch. > >> > > Yes, a little big because it refactors psr.c. :-) > > Please nevertheless strive to break it up some, to aid reviewing. I > have to admit that I'm on the edge of NAK-ing such a hard to review > change to code that is of no core interest (at least as long as all of > this remains Atom-only). > Very sorry for this. I will try my best to split this big patch to some small patches in V3. > Also to both of you: Please limit the quoting in your replies. Got it. Thank you for guidance! > > Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.
>>> On 09.10.16 at 08:43, wrote: > On 16-09-30 17:18:33, Konrad Rzeszutek Wilk wrote: >> On Thu, Sep 22, 2016 at 10:15:20AM +0800, Yi Sun wrote: >> > Current psr.c is designed for supporting L3 CAT/CDP. It has many >> > limitations to add new feature. Considering to support more PSR >> > features, we need refactor PSR implementation to make it more >> > flexible and fulfill the principle, open for extension but closed >> > for modification. >> > >> > The core of the refactoring is to abstract the common actions and >> > encapsulate them into "struct feat_ops". The detailed steps to add >> > a new feature are described at the head of psr.c. >> > >> > Signed-off-by: Yi Sun >> > >> > --- >> > Changed since v1: >> > * sysctl.c >> > - Interface change for abstraction requirement. >> > * psr.c >> > - Function and variables names are changed to express accurately. >> > - Fix code style issues. >> > - Fix imprecise comments. >> > - Add one callback function, get_cos_num(), to fulfill the >> > abstraction requirement. >> > - Divide get_old_set_new() callback functions into two functions: >> > get_old_val() and set_new_val() make it more primitive. >> > - Change feat_info type from an array to union. >> > - Adjust some functions to replace if to switch to make them >> > clearer. >> > - Replace custom list management to system. >> > - Use 'const' to make codes more safe. >> > * psr.h >> > - Change 'enum mask_type' to 'enum psr_val_type' to express >> > more accurate. >> > - Change parameters of psr_get_info() to fulfill abstraction >> > requirement. >> > --- >> > xen/arch/x86/domctl.c | 36 +- >> > xen/arch/x86/psr.c| 1105 >> > +++-- >> >> Whoa. 1K changes. This is a dense patch. >> > Yes, a little big because it refactors psr.c. :-) Please nevertheless strive to break it up some, to aid reviewing. I have to admit that I'm on the edge of NAK-ing such a hard to review change to code that is of no core interest (at least as long as all of this remains Atom-only). Also to both of you: Please limit the quoting in your replies. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.
Thanks for reviewing the patches! Sorry for late to reply because Oct 1 to 7
is China National Holiday.
On 16-09-30 17:18:33, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 22, 2016 at 10:15:20AM +0800, Yi Sun wrote:
> > Current psr.c is designed for supporting L3 CAT/CDP. It has many
> > limitations to add new feature. Considering to support more PSR
> > features, we need refactor PSR implementation to make it more
> > flexible and fulfill the principle, open for extension but closed
> > for modification.
> >
> > The core of the refactoring is to abstract the common actions and
> > encapsulate them into "struct feat_ops". The detailed steps to add
> > a new feature are described at the head of psr.c.
> >
> > Signed-off-by: Yi Sun
> >
> > ---
> > Changed since v1:
> > * sysctl.c
> > - Interface change for abstraction requirement.
> > * psr.c
> > - Function and variables names are changed to express accurately.
> > - Fix code style issues.
> > - Fix imprecise comments.
> > - Add one callback function, get_cos_num(), to fulfill the
> > abstraction requirement.
> > - Divide get_old_set_new() callback functions into two functions:
> > get_old_val() and set_new_val() make it more primitive.
> > - Change feat_info type from an array to union.
> > - Adjust some functions to replace if to switch to make them
> > clearer.
> > - Replace custom list management to system.
> > - Use 'const' to make codes more safe.
> > * psr.h
> > - Change 'enum mask_type' to 'enum psr_val_type' to express
> > more accurate.
> > - Change parameters of psr_get_info() to fulfill abstraction
> > requirement.
> > ---
> > xen/arch/x86/domctl.c | 36 +-
> > xen/arch/x86/psr.c| 1105
> > +++--
>
> Whoa. 1K changes. This is a dense patch.
>
Yes, a little big because it refactors psr.c. :-)
> > xen/arch/x86/sysctl.c | 14 +-
> > xen/include/asm-x86/psr.h | 20 +-
> > 4 files changed, 903 insertions(+), 272 deletions(-)
> >
> > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> > index 2a2fe04..c53d819 100644
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -1386,41 +1386,41 @@ long arch_do_domctl(
> > switch ( domctl->u.psr_cat_op.cmd )
> > {
> > case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
> > -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3);
> > +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> > + domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L3_CBM);
> > break;
> >
> > case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
> > -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3_CODE);
> > +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> > + domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L3_CODE);
> > break;
> >
> > case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
> > -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3_DATA);
> > +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> > + domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L3_DATA);
> > break;
> >
> > case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
> > -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - &domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3);
> > +ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> > + &domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L3_CBM);
> > copyback = 1;
> > break;
> >
> > case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
> > -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - &domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3_CODE);
> > +ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> > + &domctl->u.psr_cat_op.data,
> > + PSR_MASK_TYPE_L3_CODE);
> > copyback = 1;
> > break;
> >
> > case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
> > -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> > - &domctl->u.psr_cat_op.data,
> > - PSR_CBM_TYPE_L3_DATA);
> > +ret
Re: [Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.
On Thu, Sep 22, 2016 at 10:15:20AM +0800, Yi Sun wrote:
> Current psr.c is designed for supporting L3 CAT/CDP. It has many
> limitations to add new feature. Considering to support more PSR
> features, we need refactor PSR implementation to make it more
> flexible and fulfill the principle, open for extension but closed
> for modification.
>
> The core of the refactoring is to abstract the common actions and
> encapsulate them into "struct feat_ops". The detailed steps to add
> a new feature are described at the head of psr.c.
>
> Signed-off-by: Yi Sun
>
> ---
> Changed since v1:
> * sysctl.c
> - Interface change for abstraction requirement.
> * psr.c
> - Function and variables names are changed to express accurately.
> - Fix code style issues.
> - Fix imprecise comments.
> - Add one callback function, get_cos_num(), to fulfill the
> abstraction requirement.
> - Divide get_old_set_new() callback functions into two functions:
> get_old_val() and set_new_val() make it more primitive.
> - Change feat_info type from an array to union.
> - Adjust some functions to replace if to switch to make them
> clearer.
> - Replace custom list management to system.
> - Use 'const' to make codes more safe.
> * psr.h
> - Change 'enum mask_type' to 'enum psr_val_type' to express
> more accurate.
> - Change parameters of psr_get_info() to fulfill abstraction
> requirement.
> ---
> xen/arch/x86/domctl.c | 36 +-
> xen/arch/x86/psr.c| 1105
> +++--
Whoa. 1K changes. This is a dense patch.
> xen/arch/x86/sysctl.c | 14 +-
> xen/include/asm-x86/psr.h | 20 +-
> 4 files changed, 903 insertions(+), 272 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 2a2fe04..c53d819 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1386,41 +1386,41 @@ long arch_do_domctl(
> switch ( domctl->u.psr_cat_op.cmd )
> {
> case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
> -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3);
> +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> + domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_CBM);
> break;
>
> case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
> -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3_CODE);
> +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> + domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_CODE);
> break;
>
> case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
> -ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> - domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3_DATA);
> +ret = psr_set_val(d, domctl->u.psr_cat_op.target,
> + domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_DATA);
> break;
>
> case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
> -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> - &domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3);
> +ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + &domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_CBM);
> copyback = 1;
> break;
>
> case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
> -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> - &domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3_CODE);
> +ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + &domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_CODE);
> copyback = 1;
> break;
>
> case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
> -ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> - &domctl->u.psr_cat_op.data,
> - PSR_CBM_TYPE_L3_DATA);
> +ret = psr_get_val(d, domctl->u.psr_cat_op.target,
> + &domctl->u.psr_cat_op.data,
> + PSR_MASK_TYPE_L3_DATA);
> copyback = 1;
> break;
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 0b5073c..99e4c78 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -17,28 +17,159 @@
> #include
> #include
> #inc
[Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.
Current psr.c is designed for supporting L3 CAT/CDP. It has many
limitations to add new feature. Considering to support more PSR
features, we need refactor PSR implementation to make it more
flexible and fulfill the principle, open for extension but closed
for modification.
The core of the refactoring is to abstract the common actions and
encapsulate them into "struct feat_ops". The detailed steps to add
a new feature are described at the head of psr.c.
Signed-off-by: Yi Sun
---
Changed since v1:
* sysctl.c
- Interface change for abstraction requirement.
* psr.c
- Function and variables names are changed to express accurately.
- Fix code style issues.
- Fix imprecise comments.
- Add one callback function, get_cos_num(), to fulfill the
abstraction requirement.
- Divide get_old_set_new() callback functions into two functions:
get_old_val() and set_new_val() make it more primitive.
- Change feat_info type from an array to union.
- Adjust some functions to replace if to switch to make them
clearer.
- Replace custom list management to system.
- Use 'const' to make codes more safe.
* psr.h
- Change 'enum mask_type' to 'enum psr_val_type' to express
more accurate.
- Change parameters of psr_get_info() to fulfill abstraction
requirement.
---
xen/arch/x86/domctl.c | 36 +-
xen/arch/x86/psr.c| 1105 +++--
xen/arch/x86/sysctl.c | 14 +-
xen/include/asm-x86/psr.h | 20 +-
4 files changed, 903 insertions(+), 272 deletions(-)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 2a2fe04..c53d819 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1386,41 +1386,41 @@ long arch_do_domctl(
switch ( domctl->u.psr_cat_op.cmd )
{
case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
-ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
- domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3);
+ret = psr_set_val(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_CBM);
break;
case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CODE:
-ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
- domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3_CODE);
+ret = psr_set_val(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_CODE);
break;
case XEN_DOMCTL_PSR_CAT_OP_SET_L3_DATA:
-ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
- domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3_DATA);
+ret = psr_set_val(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_DATA);
break;
case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
-ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
- &domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3);
+ret = psr_get_val(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_CBM);
copyback = 1;
break;
case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CODE:
-ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
- &domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3_CODE);
+ret = psr_get_val(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_CODE);
copyback = 1;
break;
case XEN_DOMCTL_PSR_CAT_OP_GET_L3_DATA:
-ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
- &domctl->u.psr_cat_op.data,
- PSR_CBM_TYPE_L3_DATA);
+ret = psr_get_val(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data,
+ PSR_MASK_TYPE_L3_DATA);
copyback = 1;
break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 0b5073c..99e4c78 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -17,28 +17,159 @@
#include
#include
#include
+#include
#include
#define PSR_CMT(1<<0)
#define PSR_CAT(1<<1)
#define PSR_CDP(1<<2)
-struct psr_cat_cbm {
+/*
+ * To add a new PSR feature, please follow below steps:
+ * 1. Implement feature specific callback functions listed in
+ *"struct feat_ops";
+ * 2. Implement a feature spec
