Re: [PATCH v1 04/13] x86: implement data structure and CPU init flow for MBA

[Wei Liu <wei.liu2@citrix.com>]

On Wed, Aug 09, 2017 at 03:41:43PM +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 cos
> registers 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 COS 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.
> 
> This patch also implements init flow for MBA and register stub
> callback functions.
> 
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> ---
> v1:
>     - rebase codes onto L2 CAT v15.
>     - use '(1u << X)'.
>       (suggested by Wei Liu)
>     - 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              | 144 ++++++++++++++++++++++++++++++++++------
>  xen/include/asm-x86/msr-index.h |   1 +
>  xen/include/asm-x86/psr.h       |   2 +
>  3 files changed, 126 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 5ec00a9..d94a5b1 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        (1<<0)
> -#define PSR_CAT        (1<<1)
> -#define PSR_CDP        (1<<2)
> +#define PSR_CMT        (1u << 0)
> +#define PSR_CAT        (1u << 1)
> +#define PSR_CDP        (1u << 2)
> +#define PSR_MBA        (1u << 3)

I would split this part out to a separate patch so that it can be
applied immediately.

>  
>  #define CAT_CBM_LEN_MASK 0x1f
>  #define CAT_COS_MAX_MASK 0xffff
> @@ -60,10 +63,14 @@
>   */
>  #define MAX_COS_NUM 2
>  
> +#define MBA_LINEAR         (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. */
>      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_info;
> +
> +        struct {
> +            /* The max throttling value got through CPUID. */
> +            unsigned int thrtl_max;
> +            unsigned int linear;
> +        } mba_info;
> +    };
> +

I suggest you add a tag to specify which struct is in effect in the
union and ASSERT accordingly in their respective type specific
functions.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel