diff mbox

[v14,13/23] x86: refactor psr: CDP: implement CPU init flow.

Message ID 1500078716-5928-14-git-send-email-yi.y.sun@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yi Sun July 15, 2017, 12:31 a.m. UTC
This patch implements the CPU init flow for CDP. The flow is almost
same as L3 CAT.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
v14:
    - remove the 'Notes' in commit message because a stub function is
      implemented to avoid potential issue.
      (suggested by Jan Beulich)
    - remove 'feat_l3_cdp' because it can be replaced by 'feat_l3'.
      (suggested by Jan Beulich)
    - implement stub callback functions for CDP to avoid system crash if
      not full CDP patches applied.
      (suggested by Jan Beulich)
    - directly assign correct value to 'alt_type' of CDP.
      (suggested by Jan Beulich)
    - goto L3 CAT init process if CDP init fails.
      (suggested by Jan Beulich)
v13:
    - add commit message.
      (suggested by Jan Beulich)
    - fix comment issue.
      (suggested by Jan Beulich)
    - set CDP default value before enabling it.
      (suggested by Jan Beulich)
    - remove unnecessary check.
      (suggested by Jan Beulich)
    - set 'alt_type' for CDP.
      (suggested by Jan Beulich)
    - check 'cos_max' and substract 1 before right shift it to get correct
      value.
      (suggested by Jan Beulich)
v12:
    - move 'type[]' assignment into l3_cdp_props declaration to make it be
      'const'.
      (suggested by Jan Beulich)
    - remove "L2 CAT" indication in printk.
      (suggested by Jan Beulich)
    - fix coding style issue.
      (suggested by Jan Beulich)
    - change 'val' type to uint64_t.
      (suggested by Jan Beulich)
    - use 1ull.
      (suggested by Jan Beulich)
    - restore mask(0) MSR to default value.
      (suggested by Jan Beulich)
v11:
    - changes about 'feat_props'.
      (suggested by Jan Beulich)
    - remove MSR restore action which is unnecessary.
      (suggested by Jan Beulich)
    - modify commit message.
v10:
    - fix comment.
      (suggested by Jan Beulich)
    - use swith in 'cat_init_feature' to handle different feature types.
      (suggested by Jan Beulich)
    - changes about 'props'.
      (suggested by Jan Beulich)
    - restore MSRs to default value when cpu online.
      (suggested by Jan Beulich)
    - remove feat_mask.
      (suggested by Jan Beulich)
v9:
    - modify commit message to describe flow clearer.
    - handle cpu offline and online again case to read MSRs registers values
      back and save them into cos array to make user can get real data.
    - modify error handling process in 'psr_cpu_prepare' to reduce redundant
      codes.
    - modify 'get_cdp_data' and 'get_cdp_code' to make them standard.
      (suggested by Roger Pau and Jan Beulich)
    - encapsulate CDP operations into 'cat_init_feature' to reduce redundant
      codes.
      (suggested by Roger Pau)
    - reuse 'cat_get_cos_max' for CDP.
      (suggested by Roger Pau)
    - handle 'PSR_CDP' in psr_presmp_init to make init work can be done when
      there is only 'psr=cdp' in cmdline.
    - remove unnecessary comment.
      (suggested by Jan Beulich)
    - move CDP related codes in 'cpu_init_work' into 'psr_cpu_init'.
      (suggested by Jan Beulich)
    - add codes to handle CDP's 'cos_num'.
      (suggested by Jan Beulich)
    - fix coding style issue.
      (suggested by Jan Beulich)
    - do not free resources when allocation fails in 'psr_cpu_prepare'.
      (suggested by Jan Beulich)
    - changes about 'uint64_t' to 'uint32_t'.
      (suggested by Jan Beulich)
v7:
    - initialize 'l3_cdp'.
      (suggested by Konrad Rzeszutek Wilk)
v6:
    - use 'cpuid_leaf'.
      (suggested by Konrad Rzeszutek Wilk and Jan Beulich)
v5:
    - remove codes to free 'feat_l3_cdp' in 'free_feature'.
      (suggested by Jan Beulich)
    - encapsulate cpuid registers into 'struct cpuid_leaf_regs'.
      (suggested by Jan Beulich)
    - print socket info when 'opt_cpu_info' is true.
      (suggested by Jan Beulich)
    - rename 'l3_cdp_get_max_cos_max' to 'l3_cdp_get_cos_max'.
      (suggested by Jan Beulich)
    - rename 'dat[]' to 'data[]'.
      (suggested by Jan Beulich)
    - move 'cpu_prepare_work' contents into 'psr_cpu_prepare'.
      (suggested by Jan Beulich)
v4:
    - create this patch to make codes easier to understand.
      (suggested by Jan Beulich)
---
 xen/arch/x86/psr.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 78 insertions(+), 5 deletions(-)

Comments

Jan Beulich July 31, 2017, 2:20 p.m. UTC | #1
>>> Yi Sun <yi.y.sun@linux.intel.com> 07/15/17 2:48 AM >>>
>@@ -272,7 +312,8 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
>if ( !opt_cpu_info )
>return 0;
 >
>-    printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>+    printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>+           ((type == FEAT_TYPE_L3_CDP) ? "CDP" : "L3 CAT"),

Why is this not "L3 CDP" when the enumerator includes L3?

>@@ -1283,10 +1344,22 @@ static void psr_cpu_init(void)
>feat = feat_l3;
>feat_l3 = NULL;
 >
>-        if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) )
>-            feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
>+        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
>+        {
>+            if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) )
>+                feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
>+            else
>+                /* If CDP init fails, try to work as L3 CAT. */
>+                goto l3_cat_init;
>+        }
>else
>-            feat_l3 = feat;
>+        {
>+ l3_cat_init:

I'd really like to ask to re-structure this slightly so that you won't
need goto and a label here. As said before, goto-s are sort of okay
for making complicated error paths readable, but they should be
avoided in almost all other cases.

Jan
Yi Sun Aug. 1, 2017, 12:44 a.m. UTC | #2
On 17-07-31 08:20:44, Jan Beulich wrote:
> >>> Yi Sun <yi.y.sun@linux.intel.com> 07/15/17 2:48 AM >>>
> >@@ -272,7 +312,8 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
> >if ( !opt_cpu_info )
> >return 0;
>  >
> >-    printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> >+    printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> >+           ((type == FEAT_TYPE_L3_CDP) ? "CDP" : "L3 CAT"),
> 
> Why is this not "L3 CDP" when the enumerator includes L3?
> 
Sure, I will change it. Thanks!

> >@@ -1283,10 +1344,22 @@ static void psr_cpu_init(void)
> >feat = feat_l3;
> >feat_l3 = NULL;
>  >
> >-        if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CAT) )
> >-            feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
> >+        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> >+        {
> >+            if ( !cat_init_feature(®s, feat, info, FEAT_TYPE_L3_CDP) )
> >+                feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
> >+            else
> >+                /* If CDP init fails, try to work as L3 CAT. */
> >+                goto l3_cat_init;
> >+        }
> >else
> >-            feat_l3 = feat;
> >+        {
> >+ l3_cat_init:
> 
> I'd really like to ask to re-structure this slightly so that you won't
> need goto and a label here. As said before, goto-s are sort of okay
> for making complicated error paths readable, but they should be
> avoided in almost all other cases.
>
Got it, I will try to remove the goto. Thanks for the review!

BRs,
Sun Yi
 
> Jan
diff mbox

Patch

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index a3aa1db..35b78f6 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -62,6 +62,7 @@ 
 
 enum psr_feat_type {
     FEAT_TYPE_L3_CAT,
+    FEAT_TYPE_L3_CDP,
     FEAT_TYPE_NUM,
     FEAT_TYPE_UNKNOWN,
 };
@@ -163,6 +164,22 @@  static struct feat_node *feat_l3;
 #define cat_default_val(len) (0xffffffff >> (32 - (len)))
 
 /*
+ * get_cdp_data - get DATA COS register value from input COS ID.
+ * @feat:        the feature node.
+ * @cos:         the COS ID.
+ */
+#define get_cdp_data(feat, cos)              \
+            ((feat)->cos_reg_val[(cos) * 2])
+
+/*
+ * get_cdp_code - get CODE COS register value from input COS ID.
+ * @feat:        the feature node.
+ * @cos:         the COS ID.
+ */
+#define get_cdp_code(feat, cos)              \
+            ((feat)->cos_reg_val[(cos) * 2 + 1])
+
+/*
  * Use this function to check if any allocation feature has been enabled
  * in cmdline.
  */
@@ -262,6 +279,29 @@  static int cat_init_feature(const struct cpuid_leaf *regs,
 
         break;
 
+    case FEAT_TYPE_L3_CDP:
+    {
+        uint64_t val;
+
+        if ( feat->cos_max < 3 )
+            return -ENOENT;
+
+        /* Cut half of cos_max when CDP is enabled. */
+        feat->cos_max = (feat->cos_max - 1) >> 1;
+
+        /* We reserve cos=0 as default cbm (all bits within cbm_len are 1). */
+        get_cdp_code(feat, 0) = cat_default_val(feat->cbm_len);
+        get_cdp_data(feat, 0) = cat_default_val(feat->cbm_len);
+
+        wrmsrl(MSR_IA32_PSR_L3_MASK(0), cat_default_val(feat->cbm_len));
+        wrmsrl(MSR_IA32_PSR_L3_MASK(1), cat_default_val(feat->cbm_len));
+        rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
+        wrmsrl(MSR_IA32_PSR_L3_QOS_CFG,
+               val | (1ull << PSR_L3_QOS_CDP_ENABLE_BIT));
+
+        break;
+    }
+
     default:
         return -ENOENT;
     }
@@ -272,7 +312,8 @@  static int cat_init_feature(const struct cpuid_leaf *regs,
     if ( !opt_cpu_info )
         return 0;
 
-    printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+    printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+           ((type == FEAT_TYPE_L3_CDP) ? "CDP" : "L3 CAT"),
            cpu_to_socket(smp_processor_id()), feat->cos_max, feat->cbm_len);
 
     return 0;
@@ -305,6 +346,26 @@  static const struct feat_props l3_cat_props = {
     .write_msr = l3_cat_write_msr,
 };
 
+/* L3 CDP props */
+static bool l3_cdp_get_feat_info(const struct feat_node *feat,
+                                 uint32_t data[], uint32_t array_len)
+{
+    return false;
+}
+
+static void l3_cdp_write_msr(unsigned int cos, uint32_t val, enum cbm_type type)
+{
+}
+
+static const struct feat_props l3_cdp_props = {
+    .cos_num = 2,
+    .type[0] = PSR_CBM_TYPE_L3_DATA,
+    .type[1] = PSR_CBM_TYPE_L3_CODE,
+    .alt_type = PSR_CBM_TYPE_L3,
+    .get_feat_info = l3_cdp_get_feat_info,
+    .write_msr = l3_cdp_write_msr,
+};
+
 static void __init parse_psr_bool(char *s, char *value, char *feature,
                                   unsigned int mask)
 {
@@ -1283,10 +1344,22 @@  static void psr_cpu_init(void)
         feat = feat_l3;
         feat_l3 = NULL;
 
-        if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
-            feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
+        if ( (regs.c & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
+        {
+            if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CDP) )
+                feat_props[FEAT_TYPE_L3_CDP] = &l3_cdp_props;
+            else
+                /* If CDP init fails, try to work as L3 CAT. */
+                goto l3_cat_init;
+        }
         else
-            feat_l3 = feat;
+        {
+ l3_cat_init:
+            if ( !cat_init_feature(&regs, feat, info, FEAT_TYPE_L3_CAT) )
+                feat_props[FEAT_TYPE_L3_CAT] = &l3_cat_props;
+            else
+                feat_l3 = feat;
+        }
     }
 
     info->feat_init = true;
@@ -1348,7 +1421,7 @@  static int __init psr_presmp_init(void)
     if ( (opt_psr & PSR_CMT) && opt_rmid_max )
         init_psr_cmt(opt_rmid_max);
 
-    if ( opt_psr & PSR_CAT )
+    if ( opt_psr & (PSR_CAT | PSR_CDP) )
         init_psr();
 
     if ( psr_cpu_prepare() )