| Message ID | 1489662495-5375-16-git-send-email-yi.y.sun@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 16.03.17 at 12:08, <yi.y.sun@linux.intel.com> wrote: > --- a/xen/arch/x86/sysctl.c > +++ b/xen/arch/x86/sysctl.c > @@ -180,10 +180,36 @@ long arch_do_sysctl( > > ret = psr_get_info(sysctl->u.psr_cat_op.target, > PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data)); > - > - sysctl->u.psr_cat_op.u.l3_info.cbm_len = data[PSR_INFO_IDX_CBM_LEN]; > - sysctl->u.psr_cat_op.u.l3_info.cos_max = data[PSR_INFO_IDX_COS_MAX]; > - sysctl->u.psr_cat_op.u.l3_info.flags = data[PSR_INFO_IDX_FLAG]; > + if ( !ret ) > + { > + sysctl->u.psr_cat_op.u.l3_info.cbm_len = > + data[PSR_INFO_IDX_CBM_LEN]; > + sysctl->u.psr_cat_op.u.l3_info.cos_max = > + data[PSR_INFO_IDX_COS_MAX]; > + sysctl->u.psr_cat_op.u.l3_info.flags = > + data[PSR_INFO_IDX_FLAG]; > + } > + else > + { > + /* > + * Check if CDP is enabled. > + * > + * Per spec, L3 CAT and CDP cannot co-exist. So, we need replace > + * output values to CDP's if it is enabled. > + */ > + ret = psr_get_info(sysctl->u.psr_cat_op.target, > + PSR_CBM_TYPE_L3_CODE, data, > + ARRAY_SIZE(data)); I think this could/should be done without such a strange retry mechanism. You can find out which of the features is available, can't you? > + if ( !ret ) > + { > + sysctl->u.psr_cat_op.u.l3_info.cbm_len = > + data[PSR_INFO_IDX_CBM_LEN]; > + sysctl->u.psr_cat_op.u.l3_info.cos_max = > + data[PSR_INFO_IDX_COS_MAX]; > + sysctl->u.psr_cat_op.u.l3_info.flags = > + data[PSR_INFO_IDX_FLAG]; > + } > + } > > if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) ) > ret = -EFAULT; So where is PSR_CBM_TYPE_L3_DATA being handled? Jan
On 17-03-27 08:08:13, Jan Beulich wrote: > >>> On 16.03.17 at 12:08, <yi.y.sun@linux.intel.com> wrote: > > --- a/xen/arch/x86/sysctl.c > > +++ b/xen/arch/x86/sysctl.c > > @@ -180,10 +180,36 @@ long arch_do_sysctl( > > > > ret = psr_get_info(sysctl->u.psr_cat_op.target, > > PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data)); > > - > > - sysctl->u.psr_cat_op.u.l3_info.cbm_len = data[PSR_INFO_IDX_CBM_LEN]; > > - sysctl->u.psr_cat_op.u.l3_info.cos_max = data[PSR_INFO_IDX_COS_MAX]; > > - sysctl->u.psr_cat_op.u.l3_info.flags = data[PSR_INFO_IDX_FLAG]; > > + if ( !ret ) > > + { > > + sysctl->u.psr_cat_op.u.l3_info.cbm_len = > > + data[PSR_INFO_IDX_CBM_LEN]; > > + sysctl->u.psr_cat_op.u.l3_info.cos_max = > > + data[PSR_INFO_IDX_COS_MAX]; > > + sysctl->u.psr_cat_op.u.l3_info.flags = > > + data[PSR_INFO_IDX_FLAG]; > > + } > > + else > > + { > > + /* > > + * Check if CDP is enabled. > > + * > > + * Per spec, L3 CAT and CDP cannot co-exist. So, we need replace > > + * output values to CDP's if it is enabled. > > + */ > > + ret = psr_get_info(sysctl->u.psr_cat_op.target, > > + PSR_CBM_TYPE_L3_CODE, data, > > + ARRAY_SIZE(data)); > > I think this could/should be done without such a strange retry > mechanism. You can find out which of the features is available, > can't you? > Ok, I can handle it inside psr_get_info. If L3 CAT feature node is NULL, will use CDP feature node. > > + if ( !ret ) > > + { > > + sysctl->u.psr_cat_op.u.l3_info.cbm_len = > > + data[PSR_INFO_IDX_CBM_LEN]; > > + sysctl->u.psr_cat_op.u.l3_info.cos_max = > > + data[PSR_INFO_IDX_COS_MAX]; > > + sysctl->u.psr_cat_op.u.l3_info.flags = > > + data[PSR_INFO_IDX_FLAG]; > > + } > > + } > > > > if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) ) > > ret = -EFAULT; > > So where is PSR_CBM_TYPE_L3_DATA being handled? > This interface is to get L3 CAT/CDP HW info. So, either PSR_CBM_TYPE_L3_DATA or PSR_CBM_TYPE_L3_CODE can be used to get CDP's HW info. Either of them will be converted to feature type 'PSR_SOCKET_L3_CDP'. > Jan
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c index e2a2643..ba5c7a4 100644 --- a/xen/arch/x86/psr.c +++ b/xen/arch/x86/psr.c @@ -275,6 +275,10 @@ static enum psr_feat_type psr_cbm_type_to_feat_type(enum cbm_type type) case PSR_CBM_TYPE_L3: feat_type = PSR_SOCKET_L3_CAT; break; + case PSR_CBM_TYPE_L3_DATA: + case PSR_CBM_TYPE_L3_CODE: + feat_type = PSR_SOCKET_L3_CDP; + break; default: feat_type = PSR_SOCKET_UNKNOWN; break; @@ -506,8 +510,20 @@ static const struct feat_ops l3_cat_ops = { }; /* L3 CDP ops */ +static bool l3_cdp_get_feat_info(const struct feat_node *feat, + uint32_t data[], uint32_t array_len) +{ + if ( !cat_get_feat_info(feat, data, array_len) ) + return false; + + data[PSR_INFO_IDX_FLAG] |= XEN_SYSCTL_PSR_CAT_L3_CDP; + + return true; +} + struct feat_ops l3_cdp_ops = { .get_cos_max = cat_get_cos_max, + .get_feat_info = l3_cdp_get_feat_info, }; static void __init parse_psr_bool(char *s, char *value, char *feature, diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c index aab3d2d..3bf51a4 100644 --- a/xen/arch/x86/sysctl.c +++ b/xen/arch/x86/sysctl.c @@ -180,10 +180,36 @@ long arch_do_sysctl( ret = psr_get_info(sysctl->u.psr_cat_op.target, PSR_CBM_TYPE_L3, data, ARRAY_SIZE(data)); - - sysctl->u.psr_cat_op.u.l3_info.cbm_len = data[PSR_INFO_IDX_CBM_LEN]; - sysctl->u.psr_cat_op.u.l3_info.cos_max = data[PSR_INFO_IDX_COS_MAX]; - sysctl->u.psr_cat_op.u.l3_info.flags = data[PSR_INFO_IDX_FLAG]; + if ( !ret ) + { + sysctl->u.psr_cat_op.u.l3_info.cbm_len = + data[PSR_INFO_IDX_CBM_LEN]; + sysctl->u.psr_cat_op.u.l3_info.cos_max = + data[PSR_INFO_IDX_COS_MAX]; + sysctl->u.psr_cat_op.u.l3_info.flags = + data[PSR_INFO_IDX_FLAG]; + } + else + { + /* + * Check if CDP is enabled. + * + * Per spec, L3 CAT and CDP cannot co-exist. So, we need replace + * output values to CDP's if it is enabled. + */ + ret = psr_get_info(sysctl->u.psr_cat_op.target, + PSR_CBM_TYPE_L3_CODE, data, + ARRAY_SIZE(data)); + if ( !ret ) + { + sysctl->u.psr_cat_op.u.l3_info.cbm_len = + data[PSR_INFO_IDX_CBM_LEN]; + sysctl->u.psr_cat_op.u.l3_info.cos_max = + data[PSR_INFO_IDX_COS_MAX]; + sysctl->u.psr_cat_op.u.l3_info.flags = + data[PSR_INFO_IDX_FLAG]; + } + } if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) ) ret = -EFAULT;
This patch implements get HW info flow for CDP including L3 CDP callback function. The flow is almost same as L3 CAT. It also changes sysctl function to make it work for CDP. With this patch, 'psr-hwinfo' can work for L3 CDP. Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com> --- v9: - modify commit message to explain flow more clearly. - reuse 'cat_get_feat_info' for CDP to reduce redundant codes. (suggested by Roger Pau) - fix coding style issues. (suggested by Wei Liu and Roger Pau) - rename macros used by psr_get_info to make them meaningful. (suggested by Jan Beulich) v5: - rename 'dat[]' to 'data[]'. (suggested by Jan Beulich) - remove type check in callback function. (suggested by Jan Beulich) v4: - create this patch to make codes easier to understand. (suggested by Jan Beulich) --- xen/arch/x86/psr.c | 16 ++++++++++++++++ xen/arch/x86/sysctl.c | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 4 deletions(-)