All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Chao Peng <chao.p.peng@linux.intel.com>
To: He Chen <he.chen@linux.intel.com>
Cc: wei.liu2@citrix.com, ian.campbell@citrix.com,
	stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com,
	ian.jackson@eu.citrix.com, jbeulich@suse.com,
	xen-devel@lists.xenproject.org, keir@xen.org
Subject: Re: [PATCH v3 1/4] x86: Support enable CDP by boot parameter and add get CDP status
Date: Mon, 14 Sep 2015 16:39:08 +0800	[thread overview]
Message-ID: <20150914083907.GA4475@pengc-linux.bj.intel.com> (raw)
In-Reply-To: <1442201227-8610-2-git-send-email-he.chen@linux.intel.com>

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;
>  }
>  

  reply	other threads:[~2015-09-14  8:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-14  3:27 [PATCH v3 0/4] Intel Code and Data Prioritization (CDP) feature enabling He Chen
2015-09-14  3:27 ` [PATCH v3 1/4] x86: Support enable CDP by boot parameter and add get CDP status He Chen
2015-09-14  8:39   ` Chao Peng [this message]
2015-09-14 14:12   ` Jan Beulich
2015-09-14  3:27 ` [PATCH v3 2/4] x86: add domctl cmd to set/get CDP code/data CBM He Chen
2015-09-14  9:04   ` Chao Peng
2015-09-14 14:28   ` Jan Beulich
2015-09-14  3:27 ` [PATCH v3 3/4] tools: add tools support for Intel CDP He Chen
2015-09-14  9:11   ` Chao Peng
2015-09-14  3:27 ` [PATCH v3 4/4] docs: add document to introduce CDP command He Chen
2015-09-14  9:35   ` Chao Peng
2015-09-14  9:48 ` [PATCH v3 0/4] Intel Code and Data Prioritization (CDP) feature enabling Chao Peng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150914083907.GA4475@pengc-linux.bj.intel.com \
    --to=chao.p.peng@linux.intel.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=he.chen@linux.intel.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=keir@xen.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.