Re: [Xen-devel] [PATCH v3 1/4] x86: Support enable CDP by boot parameter and add get CDP status

[Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>]

On Mon, Sep 14, 2015 at 11:27:04AM +0800, He Chen wrote:
> @@ -1165,9 +1165,9 @@ This option can be specified more than once (up to 8 
> times at present).
>  > `= <integer>`
>  
>  ### psr (Intel)
> -> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> | 
> cos_max:<integer> )`
> +> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> | 
> cos_max:<integer> | cdp:<boolean> )`
>  
> -> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255`
> +> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255,cdp:0`
>  
>  Platform Shared Resource(PSR) Services.  Intel Haswell and later server
>  platforms offer information about the sharing of resources.
> @@ -1197,6 +1197,10 @@ The following resources are available:
>    the cache allocation.
>    * `cat` instructs Xen to enable/disable Cache Allocation Technology.
>    * `cos_max` indicates the max value for COS ID.
> +* Code and Data Prioritization Technology (Broadwell and later). Information
> +  regarding the code cache and the data cache allocation. CDP is based on 
> CAT.
> +  * `cdp` instructs Xen to enable/disable Code and Data Prioritization.

It's better to have some description for cos_max here (e.g. if the
meaning is the same for CAT and CDP).

>  
>          set_bit(socket, cat_socket_enable);
> -        printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, 
> cbm_len:%u\n",
> -               socket, info->cos_max, info->cbm_len);

Then there will be no output in CAT-only mode. Probably not good just to
remove it. I guess you can add cdp information to it and then move it to
the end of the function.

> +
> +        if ( (ecx & PSR_CAT_CDP_CAPABILITY) && (opt_psr & PSR_CDP) )
> +        {
> +            if ( test_bit(socket, cdp_socket_enable) )
> +                return;
> +
> +            rdmsrl(MSR_IA32_PSR_L3_QOS_CFG, val);
> +            wrmsrl(MSR_IA32_PSR_L3_QOS_CFG, val | 1 << 
> PSR_L3_QOS_CDP_ENABLE_BIT);
> +
> +            info->cos_to_cbm[0].u.cdp.code = (1ull << info->cbm_len) - 1;
> +            info->cos_to_cbm[0].u.cdp.data = (1ull << info->cbm_len) - 1;
> +
> +            /* We only write mask1 since mask0 is always all ones by default 
> */

Missing '.'

> +            wrmsrl(MSR_IA32_PSR_L3_MASK(1), (1ull << info->cbm_len) - 1);
> +
> +            /* Cut half of cos_max when CDP enabled */

Ditto.

> +            info->cos_max = info->cos_max / 2;
> +
> +            set_bit(socket, cdp_socket_enable);
> +            printk(XENLOG_INFO "CDP: enabled on socket %u, cos_max:%u, 
> cbm_len:%u\n",
> +                   socket, info->cos_max, info->cbm_len);
> +        }
>      }
>  }
>  
> @@ -508,6 +557,8 @@ static void cat_cpu_fini(unsigned int cpu)
>      {
>          struct psr_cat_socket_info *info = cat_socket_info + socket;
>  
> +        clear_bit(socket, cdp_socket_enable);
> +

It's better to move this below, together with that of cat_socket_enable.

>          if ( info->cos_to_cbm )
>          {
>              xfree(info->cos_to_cbm);
> @@ -523,6 +574,8 @@ static void __init psr_cat_free(void)
>      cat_socket_enable = NULL;
>      xfree(cat_socket_info);
>      cat_socket_info = NULL;
> +    xfree(cdp_socket_enable);
> +    cdp_socket_enable = NULL;
>  }
>  

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