From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v5 3/3] tools & docs: add tools and docs support for Intel CDP Date: Tue, 29 Sep 2015 14:47:39 +0100 Message-ID: <1443534459.16718.79.camel@citrix.com> References: <1443428969-4695-1-git-send-email-he.chen@linux.intel.com> <1443428969-4695-4-git-send-email-he.chen@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZgvFy-0004Jp-2R for xen-devel@lists.xenproject.org; Tue, 29 Sep 2015 13:47:46 +0000 In-Reply-To: <1443428969-4695-4-git-send-email-he.chen@linux.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: He Chen , xen-devel@lists.xenproject.org Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, jbeulich@suse.com, chao.p.peng@linux.intel.com, keir@xen.org List-Id: xen-devel@lists.xenproject.org On Mon, 2015-09-28 at 16:29 +0800, He Chen wrote: > This is the xl/xc changes to support Intel Code/Data Prioritization. > CAT xl commands to set/get CBMs are extended to support CDP. > Add new CDP options with CAT commands in xl interface man page. > Add description of CDP in xl-psr.markdown. > > Signed-off-by: He Chen > --- > Changes in v5: > * merge tools and docs patches > * replace EINVAL with ENXIO in libxl__psr_cat_log_err_msg > * revise options parsing in psr-cat-cbm-set and invalidate passing -c > and -d at the same time > * refine CDP status output codes in psr_cat_hwinfo > * adjust CBM output format in command xl psr-cat-show > * docs revision > > Example of new output format for command xl psr-cat-show: > > CAT-only: > > Socket ID : 0 > L3 Cache : 56320KB > Default CBM : 0xfffff > ID NAME CBM > 0 Domain-0 0xfffff > 1 centos.hvm 0xfffff > > CDP enabled: > > Socket ID : 0 > L3 Cache : 56320KB > Default CBM : 0xfffff > ID NAME CBM > 0 Domain-0 code: 0xfffff data: 0xfffff > 1 centos.hvm code: 0xfffff data: 0xfffff The CBM column header seems a bit out of place. Perhaps have separate headings, e.g. "CBM (code)" and "CBD (data)"? diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown > index 3545912..0527211 100644 > --- a/docs/misc/xl-psr.markdown > +++ b/docs/misc/xl-psr.markdown > @@ -14,7 +14,7 @@ tracks cache utilization of memory accesses according > to the RMID and reports > monitored data via a counter register. > > For more detailed information please refer to Intel SDM chapter > -"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology". > +"Platform Shared Resource Monitoring: Cache Monitoring Technology". I think these will clash with Cheng Pao's PSR updates, which have already been applied. > > +Code/Data Prioritization (CDP) Technology is an extension of CAT, which is Other bits of documentation added here seem to suggest it is actually an extension of CBM, is that right? > diff --git a/tools/libxc/include/xenctrl.h > b/tools/libxc/include/xenctrl.h > index 2000f12..8f4acec 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -2794,7 +2794,9 @@ enum xc_psr_cmt_type { > typedef enum xc_psr_cmt_type xc_psr_cmt_type; > > enum xc_psr_cat_type { > - XC_PSR_CAT_L3_CBM = 1, > + XC_PSR_CAT_L3_CBM = 1, > + XC_PSR_CAT_L3_CODE = 2, > + XC_PSR_CAT_L3_DATA = 3, If CDP is an extension of CBM, then would CBM_CODE + CBM_DATA be better names (more important in the libxl API than here) > + if (opt_data && opt_code) { > + fprintf(stderr, "Invalid to pass both -c and -d at the same time\n"); To be correct this would need to say "It is invalid..." but a more natural sounding (to me) way to express this would be "Cannot handle -c and -d at the same time" I think.