|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 04/15] x86: implement data structure and CPU init flow for MBA
On Tue, Sep 05, 2017 at 05:32:26PM +0800, Yi Sun wrote:
> This patch implements main data structures of MBA.
>
> Like CAT features, MBA HW info has cos_max which means the max thrtl
> register number, and thrtl_max which means the max throttle value
> (delay value). It also has a flag to represent if the throttle
> value is linear or not.
>
> One thrtl register of MBA stores a throttle value for one or more
> domains. The throttle value means the transaction time between L2
> cache and next level memory to be delayed.
"The throttle value contains the delay between L2 cache and the next
cache level."
Seems better, but I'm not a native speaker anyway.
>
> This patch also implements init flow for MBA and register stub
> callback functions.
>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> v3:
> - replace 'psr_val_type' to 'psr_type'. Also, change 'PSR_VAL_TYPE_MBA' to
> 'PSR_TYPE_MBA_THRTL'.
> (suggested by Roger Pau Monné)
> - replace 'MBA_LINEAR' to 'MBA_LINEAR_MASK' to make the name more clear.
> (suggested by Roger Pau Monné)
> - replase 'cat_info'/'mba_info' to 'cat'/'mba' to make the names shorter.
> (suggested by Roger Pau Monné)
> - change type of 'linear' to 'bool'.
> (suggested by Roger Pau Monné)
> - make format string of printf in one line.
> (suggested by Roger Pau Monné)
> v2:
> - modify commit message to replace 'cos register' to 'thrtl register' to
> make it accurate.
> (suggested by Chao Peng)
> - restore the place of the sentence to assign value to 'feat->cbm_len'
> because the MBA init flow is splitted out as a separate function in v1.
> (suggested by Chao Peng)
> - add comment to explain what the MBA thrtl defaul value '0' stands for.
> (suggested by Chao Peng)
> - check 'thrtl_max' under linear mode. It could not be euqal or larger
> than
> 100.
> (suggested by Chao Peng)
> v1:
> - rebase codes onto L2 CAT v15.
> - move comment to appropriate place.
> (suggested by Chao Peng)
> - implement 'mba_init_feature' and keep 'cat_init_feature'.
> (suggested by Chao Peng)
> - keep 'regs.b' into a local variable to avoid reading CPUID every time.
> (suggested by Chao Peng)
> ---
> xen/arch/x86/psr.c | 140
> ++++++++++++++++++++++++++++++++++------
> xen/include/asm-x86/msr-index.h | 1 +
> xen/include/asm-x86/psr.h | 2 +
> 3 files changed, 125 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 4166a1c..10776d2 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -27,13 +27,16 @@
> * - CMT Cache Monitoring Technology
> * - COS/CLOS Class of Service. Also mean COS registers.
> * - COS_MAX Max number of COS for the feature (minus 1)
> + * - MBA Memory Bandwidth Allocation
> * - MSRs Machine Specific Registers
> * - PSR Intel Platform Shared Resource
> + * - THRTL_MAX Max throttle value (delay value) of MBA
> */
>
> #define PSR_CMT (1u << 0)
> #define PSR_CAT (1u << 1)
> #define PSR_CDP (1u << 2)
> +#define PSR_MBA (1u << 3)
>
> #define CAT_CBM_LEN_MASK 0x1f
> #define CAT_COS_MAX_MASK 0xffff
> @@ -60,10 +63,14 @@
> */
> #define MAX_COS_NUM 2
>
> +#define MBA_LINEAR_MASK (1u << 2)
> +#define MBA_THRTL_MAX_MASK 0xfff
> +
> enum psr_feat_type {
> FEAT_TYPE_L3_CAT,
> FEAT_TYPE_L3_CDP,
> FEAT_TYPE_L2_CAT,
> + FEAT_TYPE_MBA,
> FEAT_TYPE_NUM,
> FEAT_TYPE_UNKNOWN,
> };
> @@ -71,7 +78,6 @@ enum psr_feat_type {
> /*
> * This structure represents one feature.
> * cos_max - The max COS registers number got through CPUID.
> - * cbm_len - The length of CBM got through CPUID.
> * cos_reg_val - Array to store the values of COS registers. One entry stores
> * the value of one COS register.
> * For L3 CAT and L2 CAT, one entry corresponds to one COS_ID.
> @@ -80,9 +86,23 @@ enum psr_feat_type {
> * cos_reg_val[1] (Code).
> */
> struct feat_node {
> - /* cos_max and cbm_len are common values for all features so far. */
> + /* cos_max is common values for all features so far. */
...common among all features...
> unsigned int cos_max;
> - unsigned int cbm_len;
> +
> + /* Feature specific HW info. */
> + union {
> + struct {
> + /* The length of CBM got through CPUID. */
> + unsigned int cbm_len;
> + } cat;
> +
> + struct {
> + /* The max throttling value got through CPUID. */
> + unsigned int thrtl_max;
> + bool linear;
> + } mba;
> + };
> +
> uint32_t cos_reg_val[MAX_COS_REG_CNT];
> };
>
> @@ -161,6 +181,7 @@ static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
> */
> static struct feat_node *feat_l3;
> static struct feat_node *feat_l2_cat;
> +static struct feat_node *feat_mba;
>
> /* Common functions */
> #define cat_default_val(len) (0xffffffff >> (32 - (len)))
> @@ -272,7 +293,7 @@ static bool psr_check_cbm(unsigned int cbm_len, unsigned
> long cbm)
> return true;
> }
>
> -/* CAT common functions implementation. */
> +/* Implementation of allocation features' functions. */
> static int cat_init_feature(const struct cpuid_leaf *regs,
> struct feat_node *feat,
> struct psr_socket_info *info,
> @@ -288,8 +309,8 @@ static int cat_init_feature(const struct cpuid_leaf *regs,
> if ( !regs->a || !regs->d )
> return -ENOENT;
>
> - feat->cbm_len = (regs->a & CAT_CBM_LEN_MASK) + 1;
> feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
> + feat->cat.cbm_len = (regs->a & CAT_CBM_LEN_MASK) + 1;
>
> switch ( type )
> {
> @@ -299,12 +320,12 @@ static int cat_init_feature(const struct cpuid_leaf
> *regs,
> return -ENOENT;
>
> /* We reserve cos=0 as default cbm (all bits within cbm_len are 1).
> */
> - feat->cos_reg_val[0] = cat_default_val(feat->cbm_len);
> + feat->cos_reg_val[0] = cat_default_val(feat->cat.cbm_len);
>
> wrmsrl((type == FEAT_TYPE_L3_CAT ?
> MSR_IA32_PSR_L3_MASK(0) :
> MSR_IA32_PSR_L2_MASK(0)),
> - cat_default_val(feat->cbm_len));
> + cat_default_val(feat->cat.cbm_len));
>
> break;
>
> @@ -319,11 +340,13 @@ static int cat_init_feature(const struct cpuid_leaf
> *regs,
> 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);
> + get_cdp_code(feat, 0) = cat_default_val(feat->cat.cbm_len);
> + get_cdp_data(feat, 0) = cat_default_val(feat->cat.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));
> + wrmsrl(MSR_IA32_PSR_L3_MASK(0),
> + cat_default_val(feat->cat.cbm_len));
> + wrmsrl(MSR_IA32_PSR_L3_MASK(1),
> + cat_default_val(feat->cat.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));
> @@ -343,7 +366,50 @@ static int cat_init_feature(const struct cpuid_leaf
> *regs,
>
> printk(XENLOG_INFO "%s: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> cat_feat_name[type], cpu_to_socket(smp_processor_id()),
> - feat->cos_max, feat->cbm_len);
> + feat->cos_max, feat->cat.cbm_len);
> +
> + return 0;
> +}
> +
> +static int mba_init_feature(const struct cpuid_leaf *regs,
> + struct feat_node *feat,
> + struct psr_socket_info *info,
> + enum psr_feat_type type)
> +{
> + /* No valid value so do not enable feature. */
> + if ( !regs->a || !regs->d )
> + return -ENOENT;
> +
> + if ( type != FEAT_TYPE_MBA )
> + return -ENOENT;
You can join the two checks above in a single if.
> +
> + feat->cos_max = min(opt_cos_max, regs->d & CAT_COS_MAX_MASK);
> + if ( feat->cos_max < 1 )
> + return -ENOENT;
> +
> + feat->mba.thrtl_max = (regs->a & MBA_THRTL_MAX_MASK) + 1;
> +
> + if ( regs->c & MBA_LINEAR_MASK )
> + {
> + feat->mba.linear = true;
> +
> + if ( feat->mba.thrtl_max >= 100 )
> + return -ENOENT;
> + }
> +
> + /* We reserve cos=0 as default thrtl (0) which means no delay. */
> + feat->cos_reg_val[0] = 0;
AFAICT feat is allocated using xzalloc, so this will already be 0.
> + wrmsrl(MSR_IA32_PSR_MBA_MASK(0), 0);
> +
> + /* Add this feature into array. */
> + info->features[type] = feat;
> +
> + if ( !opt_cpu_info )
> + return 0;
> +
> + printk(XENLOG_INFO "MBA: enabled on socket %u, cos_max:%u, thrtl_max:%u,
> linear:%u.\n",
> + cpu_to_socket(smp_processor_id()),
> + feat->cos_max, feat->mba.thrtl_max, feat->mba.linear);
>
> return 0;
> }
> @@ -355,7 +421,7 @@ static bool cat_get_feat_info(const struct feat_node
> *feat,
> 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_CBM_LEN] = feat->cat.cbm_len;
> data[PSR_INFO_IDX_CAT_FLAG] = 0;
>
> return true;
> @@ -421,6 +487,26 @@ static const struct feat_props l2_cat_props = {
> .write_msr = l2_cat_write_msr,
> };
>
> +/* MBA props */
> +static bool mba_get_feat_info(const struct feat_node *feat,
> + uint32_t data[], unsigned int array_len)
> +{
> + return false;
> +}
> +
> +static void mba_write_msr(unsigned int cos, uint32_t val,
> + enum psr_type type)
> +{
> +}
> +
> +static const struct feat_props mba_props = {
> + .cos_num = 1,
> + .type[0] = PSR_TYPE_MBA_THRTL,
> + .alt_type = PSR_TYPE_UNKNOWN,
> + .get_feat_info = mba_get_feat_info,
> + .write_msr = mba_write_msr,
> +};
> +
> static void __init parse_psr_bool(char *s, char *value, char *feature,
> unsigned int mask)
> {
> @@ -456,6 +542,7 @@ static void __init parse_psr_param(char *s)
> parse_psr_bool(s, val_str, "cmt", PSR_CMT);
> parse_psr_bool(s, val_str, "cat", PSR_CAT);
> parse_psr_bool(s, val_str, "cdp", PSR_CDP);
> + parse_psr_bool(s, val_str, "mba", PSR_MBA);
>
> if ( val_str && !strcmp(s, "rmid_max") )
> opt_rmid_max = simple_strtoul(val_str, NULL, 0);
> @@ -862,7 +949,7 @@ static int insert_val_into_array(uint32_t val[],
> if ( array_len < props->cos_num )
> return -ENOSPC;
>
> - if ( !psr_check_cbm(feat->cbm_len, new_val) )
> + if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
> return -EINVAL;
>
> /*
> @@ -1380,6 +1467,10 @@ static int psr_cpu_prepare(void)
> (feat_l2_cat = xzalloc(struct feat_node)) == NULL )
> return -ENOMEM;
>
> + if ( feat_mba == NULL &&
> + (feat_mba = xzalloc(struct feat_node)) == NULL )
> + return -ENOMEM;
> +
> return 0;
> }
>
> @@ -1389,6 +1480,7 @@ static void psr_cpu_init(void)
> unsigned int socket, cpu = smp_processor_id();
> struct feat_node *feat;
> struct cpuid_leaf regs;
> + uint32_t reg_b;
Not sure of the benefit between using regs.b or reg_b (it's only 1
char shorter).
>
> if ( !psr_alloc_feat_enabled() || !boot_cpu_has(X86_FEATURE_PQE) )
> goto assoc_init;
> @@ -1407,7 +1499,8 @@ static void psr_cpu_init(void)
> spin_lock_init(&info->ref_lock);
>
> cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, ®s);
> - if ( regs.b & PSR_RESOURCE_TYPE_L3 )
> + reg_b = regs.b;
> + if ( reg_b & PSR_RESOURCE_TYPE_L3 )
> {
> cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 1, ®s);
>
> @@ -1428,8 +1521,7 @@ static void psr_cpu_init(void)
> }
> }
>
> - cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 0, ®s);
> - if ( regs.b & PSR_RESOURCE_TYPE_L2 )
> + if ( reg_b & PSR_RESOURCE_TYPE_L2 )
> {
> cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 2, ®s);
>
> @@ -1441,6 +1533,18 @@ static void psr_cpu_init(void)
> feat_l2_cat = feat;
> }
>
> + if ( reg_b & PSR_RESOURCE_TYPE_MBA )
> + {
> + cpuid_count_leaf(PSR_CPUID_LEVEL_CAT, 3, ®s);
> +
> + feat = feat_mba;
> + feat_mba = NULL;
> + if ( !mba_init_feature(®s, feat, info, FEAT_TYPE_MBA) )
Seems kind of pointless that mba_init_feature returns an error code
when it's ignored by it's callers. You could switch it to bool if you
are going to use it like that.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |