diff mbox

[v11,06/23] x86: refactor psr: L3 CAT: implement get hw info flow.

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

Commit Message

Yi Sun May 3, 2017, 8:44 a.m. UTC
This patch implements get HW info flow including L3 CAT callback
function.

It also changes sysctl interface to make it more general.

With this patch, 'psr-hwinfo' can work for L3 CAT.

Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
v11:
    - changes about 'cos_max' and 'cbm_len'.
      (suggested by Jan Beulich)
    - change about 'feat_props'.
      (suggested by Jan Beulich)
    - declare 'PSR_SOCKET_FEAT_UNKNOWN' to handle error case.
      (suggested by Jan Beulich)
    - use 'ASSERT' to check data.
      (suggested by Jan Beulich)
    - fix comment of 'get_feat_info'.
      (suggested by Jan Beulich)
    - fix a bug when checking 'feat_type'.
      (suggested by Jan Beulich)
v10:
    - remove 'PSR_SOCKET_UNKNOWN' and use 'ASSERT_UNREACHABLE()' to handle
      this case.
      (suggested by Jan Beulich)
    - check 'feat_type'.
      (suggested by Jan Beulich)
    - adjust macros names and values to make them more appropriate.
      (suggested by Jan Beulich)
    - use 'feat_init_done'.
      (suggested by Jan Beulich)
    - changes about 'cbm_len'.
      (suggested by Jan Beulich)
v9:
    - replace feature list handling to feature array handling.
      (suggested by Roger Pau)
    - define 'PSR_INFO_SIZE'.
      (suggested by Roger Pau)
    - fix coding style issue.
      (suggested by Roger Pau and Jan Beulich)
    - use 'ARRAY_SIZE'.
      (suggested by Roger Pau)
    - rename 'l3_cat_get_feat_info' to 'cat_get_feat_info' to make it a common
      function for both L3/L2 CAT.
      (suggested by Roger Pau)
    - move constant to the right of comparison.
      (suggested by Wei Liu)
    - remove wrong comment.
      (suggested by Jan Beulich)
    - rename macros used by psr_get_info to make them meaningful.
      (suggested by Jan Beulich)
    - remove assignment for 'PSR_SOCKET_UNKNOWN'.
      (suggested by Jan Beulich)
    - retain blank line after 'case XEN_SYSCTL_PSR_CAT_get_l3_info'.
      (suggested by Jan Beulich)
    - modify patch title to indicate 'L3 CAT'.
      (suggested by Jan Beulich)
    - move common data check into common function.
      (suggested by Jan Beulich)
v6:
    - fix coding style issue.
      (suggested by Konrad Rzeszutek Wilk)
    - define 'PSR_SOCKET_UNKNOWN' in 'psr_feat_type'.
      (suggested by Konrad Rzeszutek Wilk)
    - change '-ENOTSOCK' to 'ERANGE'.
      (suggested by Konrad Rzeszutek Wilk)
    - modify position of macros to remove odd spacing in psr.h.
      (suggested by Konrad Rzeszutek Wilk)
v5:
    - change 'dat[]' to 'data[]'.
      (suggested by Jan Beulich)
    - modify parameter type to avoid fixed width type when there is no such
      intention.
      (suggested by Jan Beulich)
    - use 'const' when it is possible.
      (suggested by Jan Beulich)
    - check feature type outside callback function.
      (suggested by Jan Beulich)
    - modify macros names to add prefix 'PSR_' and change 'CDP_FLAG' to
      'PSR_FLAG'.
      (suggested by Jan Beulich)
v4:
    - create this patch to make codes easier to understand.
      (suggested by Jan Beulich)
---
 xen/arch/x86/psr.c        | 75 +++++++++++++++++++++++++++++++++++++++++++++--
 xen/arch/x86/sysctl.c     | 19 +++++++++---
 xen/include/asm-x86/psr.h | 16 ++++++----
 3 files changed, 98 insertions(+), 12 deletions(-)

Comments

Jan Beulich May 30, 2017, 1:51 p.m. UTC | #1
>>> On 03.05.17 at 10:44, <yi.y.sun@linux.intel.com> wrote:
> +int psr_get_info(unsigned int socket, enum cbm_type type,
> +                 uint32_t data[], unsigned int array_len)
> +{
> +    const struct psr_socket_info *info = get_socket_info(socket);
> +    const struct feat_node *feat;
> +    enum psr_feat_type feat_type;
> +
> +    ASSERT(data);
> +
> +    if ( IS_ERR(info) )
> +        return PTR_ERR(info);
> +
> +    feat_type = psr_cbm_type_to_feat_type(type);
> +    if ( feat_type >= ARRAY_SIZE(info->features) )
> +        return -ENOENT;
> +
> +    feat = info->features[feat_type];
> +    if ( !feat || !feat_props[feat_type] )
> +        return -ENOENT;

Isn't it a bug if feat_props[feat_type] is NULL when feat here is
non-NULL? If so, perhaps the better approach would be to have
separate checks:

    if ( !feat )
        return -ENOENT;

    if ( !feat_props[feat_type] )
    {
        ASSERT_UNREACHABLE();
        return -ENOENT;
    }

at once documenting this interdependency.

With or without this change
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox

Patch

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index bda325d..2e6595d 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -63,6 +63,7 @@ 
 enum psr_feat_type {
     PSR_SOCKET_L3_CAT,
     PSR_SOCKET_FEAT_NUM,
+    PSR_SOCKET_FEAT_UNKNOWN,
 };
 
 /*
@@ -102,6 +103,10 @@  static const struct feat_props {
      * values one by one.
      */
     enum cbm_type type[PSR_MAX_COS_NUM];
+
+    /* get_feat_info is used to return feature HW info through sysctl. */
+    bool (*get_feat_info)(const struct feat_node *feat,
+                          uint32_t data[], unsigned int array_len);
 } *feat_props[PSR_SOCKET_FEAT_NUM];
 
 /*
@@ -175,6 +180,22 @@  static void free_socket_resources(unsigned int socket)
     info->feat_init = false;
 }
 
+static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type)
+{
+    enum psr_feat_type feat_type = PSR_SOCKET_FEAT_UNKNOWN;
+
+    switch ( type )
+    {
+    case PSR_CBM_TYPE_L3:
+        feat_type = PSR_SOCKET_L3_CAT;
+        break;
+    default:
+        ASSERT_UNREACHABLE();
+    }
+
+    return feat_type;
+}
+
 /* CAT common functions implementation. */
 static void cat_init_feature(const struct cpuid_leaf *regs,
                              struct feat_node *feat,
@@ -211,9 +232,23 @@  static void cat_init_feature(const struct cpuid_leaf *regs,
            cpu_to_socket(smp_processor_id()), feat->cos_max, feat->cbm_len);
 }
 
+static bool cat_get_feat_info(const struct feat_node *feat,
+                              uint32_t data[], unsigned int array_len)
+{
+    if ( array_len != PSR_INFO_ARRAY_SIZE )
+        return false;
+
+    data[PSR_INFO_IDX_COS_MAX] = feat->cos_max;
+    data[PSR_INFO_IDX_CAT_CBM_LEN] = feat->cbm_len;
+    data[PSR_INFO_IDX_CAT_FLAG] = 0;
+
+    return true;
+}
+
 /* L3 CAT props */
 static struct feat_props l3_cat_props = {
     .cos_num = 1,
+    .get_feat_info = cat_get_feat_info,
 };
 
 static void __init parse_psr_bool(char *s, char *value, char *feature,
@@ -427,10 +462,44 @@  void psr_ctxt_switch_to(struct domain *d)
     }
 }
 
-int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
-                        uint32_t *cos_max, uint32_t *flags)
+static struct psr_socket_info *get_socket_info(unsigned int socket)
 {
-    return 0;
+    if ( !socket_info )
+        return ERR_PTR(-ENODEV);
+
+    if ( socket >= nr_sockets )
+        return ERR_PTR(-ERANGE);
+
+    if ( !socket_info[socket].feat_init )
+        return ERR_PTR(-ENOENT);
+
+    return socket_info + socket;
+}
+
+int psr_get_info(unsigned int socket, enum cbm_type type,
+                 uint32_t data[], unsigned int array_len)
+{
+    const struct psr_socket_info *info = get_socket_info(socket);
+    const struct feat_node *feat;
+    enum psr_feat_type feat_type;
+
+    ASSERT(data);
+
+    if ( IS_ERR(info) )
+        return PTR_ERR(info);
+
+    feat_type = psr_cbm_type_to_feat_type(type);
+    if ( feat_type >= ARRAY_SIZE(info->features) )
+        return -ENOENT;
+
+    feat = info->features[feat_type];
+    if ( !feat || !feat_props[feat_type] )
+        return -ENOENT;
+
+    if ( feat_props[feat_type]->get_feat_info(feat, data, array_len) )
+        return 0;
+
+    return -EINVAL;
 }
 
 int psr_get_l3_cbm(struct domain *d, unsigned int socket,
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 2f7056e..c23270d 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -175,14 +175,25 @@  long arch_do_sysctl(
         switch ( sysctl->u.psr_cat_op.cmd )
         {
         case XEN_SYSCTL_PSR_CAT_get_l3_info:
-            ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
-                                      &sysctl->u.psr_cat_op.u.l3_info.cbm_len,
-                                      &sysctl->u.psr_cat_op.u.l3_info.cos_max,
-                                      &sysctl->u.psr_cat_op.u.l3_info.flags);
+        {
+            uint32_t data[PSR_INFO_ARRAY_SIZE];
+
+            ret = psr_get_info(sysctl->u.psr_cat_op.target,
+                               PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data));
+            if ( ret )
+                break;
+
+            sysctl->u.psr_cat_op.u.l3_info.cos_max =
+                                      data[PSR_INFO_IDX_COS_MAX];
+            sysctl->u.psr_cat_op.u.l3_info.cbm_len =
+                                      data[PSR_INFO_IDX_CAT_CBM_LEN];
+            sysctl->u.psr_cat_op.u.l3_info.flags =
+                                      data[PSR_INFO_IDX_CAT_FLAG];
 
             if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
                 ret = -EFAULT;
             break;
+        }
 
         default:
             ret = -EOPNOTSUPP;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 57f47e9..af3a465 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -19,20 +19,26 @@ 
 #include <xen/types.h>
 
 /* CAT cpuid level */
-#define PSR_CPUID_LEVEL_CAT   0x10
+#define PSR_CPUID_LEVEL_CAT             0x10
 
 /* Resource Type Enumeration */
 #define PSR_RESOURCE_TYPE_L3            0x2
 
 /* L3 Monitoring Features */
-#define PSR_CMT_L3_OCCUPANCY           0x1
+#define PSR_CMT_L3_OCCUPANCY            0x1
 
 /* CDP Capability */
-#define PSR_CAT_CDP_CAPABILITY       (1u << 2)
+#define PSR_CAT_CDP_CAPABILITY          (1u << 2)
 
 /* L3 CDP Enable bit*/
 #define PSR_L3_QOS_CDP_ENABLE_BIT       0x0
 
+/* Used by psr_get_info() */
+#define PSR_INFO_IDX_COS_MAX            0
+#define PSR_INFO_IDX_CAT_CBM_LEN        1
+#define PSR_INFO_IDX_CAT_FLAG           2
+#define PSR_INFO_ARRAY_SIZE             3
+
 struct psr_cmt_l3 {
     unsigned int features;
     unsigned int upscaling_factor;
@@ -63,8 +69,8 @@  int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
 void psr_ctxt_switch_to(struct domain *d);
 
-int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
-                        uint32_t *cos_max, uint32_t *flags);
+int psr_get_info(unsigned int socket, enum cbm_type type,
+                 uint32_t data[], unsigned int array_len);
 int psr_get_l3_cbm(struct domain *d, unsigned int socket,
                    uint64_t *cbm, enum cbm_type type);
 int psr_set_l3_cbm(struct domain *d, unsigned int socket,