|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 07/15] x86: implement set value flow for MBA
On Tue, Sep 05, 2017 at 05:32:29PM +0800, Yi Sun wrote:
> This patch implements set value flow for MBA including its callback
> function and domctl interface.
>
> It also changes the memebers in 'cos_write_info' to transfer the
> feature array, feature properties array and value array. Then, we
> can write all features values on the cos id into MSRs.
>
> Because multiple features may co-exist, we need handle all features to write
> values of them into a COS register with new COS ID. E.g:
> 1. L3 CAT and MBA co-exist.
> 2. Dom1 and Dom2 share a same COS ID (2). The L3 CAT CBM of Dom1 is 0x1ff,
> the MBA Thrtle of Dom1 is 0xa.
> 3. User wants to change MBA Thrtl of Dom1 to be 0x14. Because COS ID 2 is
> used by Dom2 too, we have to pick a new COS ID 3. The values of Dom1 on
> COS ID 3 are all default values as below:
> ---------
> | COS 3 |
> ---------
> L3 CAT | 0x7ff |
> ---------
> MBA | 0x0 |
> ---------
> 4. After setting, the L3 CAT CBM value of Dom1 should be kept and the new MBA
> Thrtl is set. So, the values on COS ID 3 should be below.
> ---------
> | COS 3 |
> ---------
> L3 CAT | 0x1ff |
> ---------
> MBA | 0x14 |
> ---------
>
> So, we should write all features values into their MSRs. That requires the
> feature array, feature properties array and value array are input.
^ as
>
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
> ---
> v3:
> - modify commit message to make it clear.
> (suggested by Roger Pau Monné)
> - modify functionality of 'check_val' to make it simple to only check
> value.
> Change the last parameter type from 'unsigned long *' to 'unsigned
> long'.
> (suggested by Roger Pau Monné)
> - call rdmsrl to get value just written into MSR for MBA. Because HW can
> automatically change input value to what it wants.
> (suggested by Roger Pau Monné)
> - change type of 'write_msr' to 'uint32_t' to return the value actually
> written into MSR. Then, change 'do_write_psr_msrs' to set the returned
> value into 'cos_reg_val[]'
> - move the declaration of 'j' into loop in 'do_write_psr_msrs'.
> (suggested by Roger Pau Monné)
> - change 'mba_info' to 'mba'.
> (suggested by Roger Pau Monné)
> - change 'cat_info' to 'cat'.
> (suggested by Roger Pau Monné)
> - rename 'psr_cat/PSR_CAT' to 'psr_alloc/PSR_ALLOC' and remove 'op/OP'
> from name.
> (suggested by Roger Pau Monné)
> - change 'PSR_VAL_TYPE_MBA' to 'PSR_TYPE_MBA_THRTL'.
> (suggested by Roger Pau Monné)
> v2:
> - remove linear mode 'thrtl_max' check in 'mba_check_thrtl' because it has
> been checked in 'mba_init_feature'.
> (suggested by Chao Peng)
> - for non-linear mode, check if '*thrtl' is not 0 in 'mba_check_thrtl'. If
> it is 0, we do not need to change it.
> (suggested by Chao Peng)
> - move comments to explain changes of 'cos_write_info' from psr.c to
> commit
> message.
> (suggested by Chao Peng)
> ---
> xen/arch/x86/domctl.c | 6 ++
> xen/arch/x86/psr.c | 146
> +++++++++++++++++++++++++++-----------------
> xen/include/public/domctl.h | 1 +
> 3 files changed, 96 insertions(+), 57 deletions(-)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 7902af7..8550d06 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1468,6 +1468,12 @@ long arch_do_domctl(
> PSR_TYPE_L2_CBM);
> break;
>
> + case XEN_DOMCTL_PSR_ALLOC_SET_MBA_THRTL:
> + ret = psr_set_val(d, domctl->u.psr_alloc.target,
> + domctl->u.psr_alloc.data,
> + PSR_TYPE_MBA_THRTL);
> + break;
> +
> case XEN_DOMCTL_PSR_ALLOC_GET_L3_CBM:
> ret = psr_get_val(d, domctl->u.psr_alloc.target,
> &val32, PSR_TYPE_L3_CBM);
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 0486d2d..d633194 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -137,7 +137,10 @@ static const struct feat_props {
> uint32_t data[], unsigned int array_len);
>
> /* write_msr is used to write out feature MSR register. */
> - void (*write_msr)(unsigned int cos, uint32_t val, enum psr_type type);
> + uint32_t (*write_msr)(unsigned int cos, uint32_t val, enum psr_type
> type);
> +
> + /* check_val is used to check if input val fulfills SDM requirement. */
> + bool (*check_val)(const struct feat_node *feat, unsigned long val);
> } *feat_props[FEAT_TYPE_NUM];
>
> /*
> @@ -274,29 +277,6 @@ static enum psr_feat_type psr_type_to_feat_type(enum
> psr_type type)
> return feat_type;
> }
>
> -static bool psr_check_cbm(unsigned int cbm_len, unsigned long cbm)
> -{
> - unsigned int first_bit, zero_bit;
> -
> - /* Set bits should only in the range of [0, cbm_len]. */
> - if ( cbm & (~0ul << cbm_len) )
> - return false;
> -
> - /* At least one bit need to be set. */
> - if ( cbm == 0 )
> - return false;
> -
> - first_bit = find_first_bit(&cbm, cbm_len);
> - zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
> -
> - /* Set bits should be contiguous. */
> - if ( zero_bit < cbm_len &&
> - find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
> - return false;
> -
> - return true;
> -}
> -
> /* Implementation of allocation features' functions. */
> static int cat_init_feature(const struct cpuid_leaf *regs,
> struct feat_node *feat,
> @@ -431,11 +411,37 @@ static bool cat_get_feat_info(const struct feat_node
> *feat,
> return true;
> }
>
> +static bool cat_check_cbm(const struct feat_node *feat, unsigned long cbm)
> +{
> + unsigned int first_bit, zero_bit;
> + unsigned int cbm_len = feat->cat.cbm_len;
> +
> + /* Set bits should only in the range of [0, cbm_len]. */
> + if ( cbm & (~0ul << cbm_len) )
> + return false;
> +
> + /* At least one bit need to be set. */
> + if ( cbm == 0 )
> + return false;
You can join both checks into a single if.
> +
> + first_bit = find_first_bit(&cbm, cbm_len);
> + zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
> +
> + /* Set bits should be contiguous. */
> + if ( zero_bit < cbm_len &&
> + find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
> + return false;
> +
> + return true;
> +}
> +
> /* L3 CAT props */
> -static void l3_cat_write_msr(unsigned int cos, uint32_t val,
> - enum psr_type type)
> +static uint32_t l3_cat_write_msr(unsigned int cos, uint32_t val,
> + enum psr_type type)
> {
> wrmsrl(MSR_IA32_PSR_L3_MASK(cos), val);
> +
> + return val;
> }
>
> static const struct feat_props l3_cat_props = {
> @@ -444,6 +450,7 @@ static const struct feat_props l3_cat_props = {
> .alt_type = PSR_TYPE_UNKNOWN,
> .get_feat_info = cat_get_feat_info,
> .write_msr = l3_cat_write_msr,
> + .check_val = cat_check_cbm,
> };
>
> /* L3 CDP props */
> @@ -458,13 +465,15 @@ static bool l3_cdp_get_feat_info(const struct feat_node
> *feat,
> return true;
> }
>
> -static void l3_cdp_write_msr(unsigned int cos, uint32_t val,
> - enum psr_type type)
> +static uint32_t l3_cdp_write_msr(unsigned int cos, uint32_t val,
> + enum psr_type type)
> {
> wrmsrl(((type == PSR_TYPE_L3_DATA) ?
> MSR_IA32_PSR_L3_MASK_DATA(cos) :
> MSR_IA32_PSR_L3_MASK_CODE(cos)),
> val);
> +
> + return val;
> }
>
> static const struct feat_props l3_cdp_props = {
> @@ -474,13 +483,16 @@ static const struct feat_props l3_cdp_props = {
> .alt_type = PSR_TYPE_L3_CBM,
> .get_feat_info = l3_cdp_get_feat_info,
> .write_msr = l3_cdp_write_msr,
> + .check_val = cat_check_cbm,
> };
>
> /* L2 CAT props */
> -static void l2_cat_write_msr(unsigned int cos, uint32_t val,
> - enum psr_type type)
> +static uint32_t l2_cat_write_msr(unsigned int cos, uint32_t val,
> + enum psr_type type)
> {
> wrmsrl(MSR_IA32_PSR_L2_MASK(cos), val);
> +
> + return val;
> }
>
> static const struct feat_props l2_cat_props = {
> @@ -489,6 +501,7 @@ static const struct feat_props l2_cat_props = {
> .alt_type = PSR_TYPE_UNKNOWN,
> .get_feat_info = cat_get_feat_info,
> .write_msr = l2_cat_write_msr,
> + .check_val = cat_check_cbm,
> };
>
> /* MBA props */
> @@ -509,9 +522,23 @@ static bool mba_get_feat_info(const struct feat_node
> *feat,
> return true;
> }
>
> -static void mba_write_msr(unsigned int cos, uint32_t val,
> - enum psr_type type)
> +static uint32_t mba_write_msr(unsigned int cos, uint32_t val,
> + enum psr_type type)
> {
> + wrmsrl(MSR_IA32_PSR_MBA_MASK(cos), val);
> +
> + /* Read actual value set by hardware. */
> + rdmsrl(MSR_IA32_PSR_MBA_MASK(cos), val);
> +
> + return val;
> +}
> +
> +static bool mba_check_thrtl(const struct feat_node *feat, unsigned long
> thrtl)
> +{
> + if ( thrtl > feat->mba.thrtl_max )
> + return false;
> +
> + return true;
> }
>
> static const struct feat_props mba_props = {
> @@ -520,6 +547,7 @@ static const struct feat_props mba_props = {
> .alt_type = PSR_TYPE_UNKNOWN,
> .get_feat_info = mba_get_feat_info,
> .write_msr = mba_write_msr,
> + .check_val = mba_check_thrtl,
> };
>
> static void __init parse_psr_bool(char *s, char *value, char *feature,
> @@ -964,7 +992,7 @@ static int insert_val_into_array(uint32_t val[],
> if ( array_len < props->cos_num )
> return -ENOSPC;
>
> - if ( !psr_check_cbm(feat->cat.cbm_len, new_val) )
> + if ( !props->check_val(feat, new_val) )
> return -EINVAL;
>
> /*
> @@ -1196,25 +1224,40 @@ static unsigned int get_socket_cpu(unsigned int
> socket)
> struct cos_write_info
> {
> unsigned int cos;
> - struct feat_node *feature;
> + struct feat_node **features;
> const uint32_t *val;
> - const struct feat_props *props;
> + unsigned int array_len;
> + const struct feat_props **props;
> };
>
> static void do_write_psr_msrs(void *data)
Why does this function take a 'void *data' instead of 'const struct
cos_write_info *info'?
> {
> const struct cos_write_info *info = data;
> - struct feat_node *feat = info->feature;
> - const struct feat_props *props = info->props;
> - unsigned int i, cos = info->cos, cos_num = props->cos_num;
> + unsigned int i, index = 0, array_len = info->array_len, cos = info->cos;
> + const uint32_t *val_array = info->val;
>
> - for ( i = 0; i < cos_num; i++ )
> + for ( i = 0; i < ARRAY_SIZE(feat_props); i++ )
> {
> - if ( feat->cos_reg_val[cos * cos_num + i] != info->val[i] )
> + struct feat_node *feat = info->features[i];
> + const struct feat_props *props = info->props[i];
> + unsigned int cos_num, j;
> +
> + if ( !feat || !props )
> + continue;
> +
> + cos_num = props->cos_num;
> + if ( array_len < cos_num )
Not sure you need array_len, couldn't you use:
if ( index + cos_num >= info->array_len )
return;
?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |