All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs
@ 2015-03-26 12:38 Chao Peng
  2015-03-26 12:38 ` [PATCH v3 1/8] x86: clean up psr boot parameter parsing Chao Peng
                   ` (7 more replies)
  0 siblings, 8 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Changes in v3:
* Address comments from Jan and Ian(Detail in patch).
* Add xl sample output in cover letter.
Changes in v2:
* Address comments from Konrad and Jan(Detail in patch):
* Make all cat unrelated changes into the preparation patches. 

This patch serial enable the new Cache Allocation Technology (CAT) feature
found in Intel Broadwell and later server platform. On XEN implementation,
CAT is used to control cache allocation on VM basis.

Detail hardware spec can be found in section 17.15 of the Intel SDM [1].
The design for XEN can be found at [2].

xl usage example:
To set CBM for a Domain:
$ xl psr-cat-cbm-set 0 0xff

To get system CAT information and domain CBM setting:
$ xl psr-cat-show
SocketID:	0
L3 Cache:	12288KB
Maxmium COS:	15
CBM length:	12
Default CBM:	0xfff
   ID                     NAME             CBM
    0                 Domain-0            0xff

[1] Intel SDM (http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf)
[2] CAT design for XEN( http://lists.xen.org/archives/html/xen-devel/2014-12/msg01382.html)

Chao Peng (8):
  x86: clean up psr boot parameter parsing
  x86: improve psr scheduling code
  x86: detect and initialize Intel CAT feature
  x86: add support for COS/CBM manangement
  x86: add scheduling support for Intel CAT
  xsm: add CAT related xsm policies
  tools/libxl: introduce libxl_count_physical_sockets
  tools: add tools support for Intel CAT

 docs/man/xl.pod.1                            |  31 ++
 docs/misc/xen-command-line.markdown          |  13 +-
 tools/flask/policy/policy/modules/xen/xen.if |   2 +-
 tools/flask/policy/policy/modules/xen/xen.te |   4 +-
 tools/libxc/include/xenctrl.h                |  15 +
 tools/libxc/xc_psr.c                         |  74 +++++
 tools/libxl/libxl.h                          |  20 ++
 tools/libxl/libxl_psr.c                      | 126 +++++++-
 tools/libxl/libxl_types.idl                  |   5 +
 tools/libxl/libxl_utils.c                    |  17 +
 tools/libxl/libxl_utils.h                    |   2 +
 tools/libxl/xl.h                             |   4 +
 tools/libxl/xl_cmdimpl.c                     | 157 ++++++++-
 tools/libxl/xl_cmdtable.c                    |  12 +
 xen/arch/x86/domain.c                        |  13 +-
 xen/arch/x86/domctl.c                        |  18 ++
 xen/arch/x86/psr.c                           | 465 +++++++++++++++++++++++++--
 xen/arch/x86/sysctl.c                        |  18 ++
 xen/include/asm-x86/cpufeature.h             |   1 +
 xen/include/asm-x86/domain.h                 |   5 +-
 xen/include/asm-x86/msr-index.h              |   1 +
 xen/include/asm-x86/psr.h                    |  14 +-
 xen/include/public/domctl.h                  |  12 +
 xen/include/public/sysctl.h                  |  16 +
 xen/xsm/flask/hooks.c                        |   6 +
 xen/xsm/flask/policy/access_vectors          |   6 +
 26 files changed, 995 insertions(+), 62 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3 1/8] x86: clean up psr boot parameter parsing
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-26 20:42   ` Andrew Cooper
  2015-03-26 12:38 ` [PATCH v3 2/8] x86: improve psr scheduling code Chao Peng
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Change type of opt_psr from bool to int so more psr features can fit.

Introduce a new routine to parse bool parameter so that both cmt and
future psr features like cat can use it.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v3:
* Set "off" value explicity if requested.
---
 xen/arch/x86/psr.c | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 2ef83df..cfa534b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -26,11 +26,31 @@ struct psr_assoc {
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
-static bool_t __initdata opt_psr;
+static unsigned int __initdata opt_psr;
 static unsigned int __initdata opt_rmid_max = 255;
 static uint64_t rmid_mask;
 static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
 
+static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)
+{
+    if ( !strcmp(s, feature) )
+    {
+        if ( !value )
+            opt_psr |= bit;
+        else
+        {
+            int val_int = parse_bool(value);
+
+            if ( val_int == 0 )
+                opt_psr &= ~bit;
+            else if ( val_int == 1 )
+                opt_psr |= bit;
+            else
+                printk("PSR: unknown %s value: %s\n", feature, value);
+        }
+    }
+}
+
 static void __init parse_psr_param(char *s)
 {
     char *ss, *val_str;
@@ -44,21 +64,9 @@ static void __init parse_psr_param(char *s)
         if ( val_str )
             *val_str++ = '\0';
 
-        if ( !strcmp(s, "cmt") )
-        {
-            if ( !val_str )
-                opt_psr |= PSR_CMT;
-            else
-            {
-                int val_int = parse_bool(val_str);
-                if ( val_int == 1 )
-                    opt_psr |= PSR_CMT;
-                else if ( val_int != 0 )
-                    printk("PSR: unknown cmt value: %s - CMT disabled!\n",
-                                    val_str);
-            }
-        }
-        else if ( val_str && !strcmp(s, "rmid_max") )
+        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
+
+        if ( val_str && !strcmp(s, "rmid_max") )
             opt_rmid_max = simple_strtoul(val_str, NULL, 0);
 
         s = ss + 1;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 2/8] x86: improve psr scheduling code
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
  2015-03-26 12:38 ` [PATCH v3 1/8] x86: clean up psr boot parameter parsing Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-27 18:15   ` Andrew Cooper
  2015-03-26 12:38 ` [PATCH v3 3/8] x86: detect and initialize Intel CAT feature Chao Peng
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Switching RMID from previous vcpu to next vcpu only needs to write
MSR_IA32_PSR_ASSOC once. Write it with the value of next vcpu is enough,
no need to write '0' first. Idle domain has RMID set to 0 and because MSR
is already updated lazily, so just switch it as it does.

Also move the initialization of per-CPU variable which used for lazy
update from context switch to CPU starting.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v2:
* Move initialization for psr_assoc from context switch to CPU_STARTING.
---
 xen/arch/x86/domain.c     |  7 ++--
 xen/arch/x86/psr.c        | 89 ++++++++++++++++++++++++++++++++++++-----------
 xen/include/asm-x86/psr.h |  3 +-
 3 files changed, 73 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 04c1898..695a2eb 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1444,8 +1444,6 @@ static void __context_switch(void)
     {
         memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES);
         vcpu_save_fpu(p);
-        if ( psr_cmt_enabled() )
-            psr_assoc_rmid(0);
         p->arch.ctxt_switch_from(p);
     }
 
@@ -1470,11 +1468,10 @@ static void __context_switch(void)
         }
         vcpu_restore_fpu_eager(n);
         n->arch.ctxt_switch_to(n);
-
-        if ( psr_cmt_enabled() && n->domain->arch.psr_rmid > 0 )
-            psr_assoc_rmid(n->domain->arch.psr_rmid);
     }
 
+    psr_ctxt_switch_to(n->domain);
+
     gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
                                   per_cpu(compat_gdt_table, cpu);
     if ( need_full_gdt(n) )
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index cfa534b..b3b540c 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -22,7 +22,6 @@
 
 struct psr_assoc {
     uint64_t val;
-    bool_t initialized;
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
@@ -123,14 +122,6 @@ static void __init init_psr_cmt(unsigned int rmid_max)
     printk(XENLOG_INFO "Cache Monitoring Technology enabled\n");
 }
 
-static int __init init_psr(void)
-{
-    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
-        init_psr_cmt(opt_rmid_max);
-    return 0;
-}
-__initcall(init_psr);
-
 /* Called with domain lock held, no psr specific lock needed */
 int psr_alloc_rmid(struct domain *d)
 {
@@ -176,26 +167,84 @@ void psr_free_rmid(struct domain *d)
     d->arch.psr_rmid = 0;
 }
 
-void psr_assoc_rmid(unsigned int rmid)
+static inline void psr_assoc_init(void)
 {
-    uint64_t val;
-    uint64_t new_val;
     struct psr_assoc *psra = &this_cpu(psr_assoc);
 
-    if ( !psra->initialized )
-    {
+    if ( psr_cmt_enabled() )
         rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
-        psra->initialized = 1;
+}
+
+static inline void psr_assoc_reg_read(struct psr_assoc *psra, uint64_t *reg)
+{
+    *reg = psra->val;
+}
+
+static inline void psr_assoc_reg_write(struct psr_assoc *psra, uint64_t reg)
+{
+    if ( reg != psra->val )
+    {
+        wrmsrl(MSR_IA32_PSR_ASSOC, reg);
+        psra->val = reg;
     }
-    val = psra->val;
+}
+
+static inline void psr_assoc_rmid(uint64_t *reg, unsigned int rmid)
+{
+    *reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
+}
+
+void psr_ctxt_switch_to(struct domain *d)
+{
+    uint64_t reg;
+    struct psr_assoc *psra = &this_cpu(psr_assoc);
+
+    psr_assoc_reg_read(psra, &reg);
 
-    new_val = (val & ~rmid_mask) | (rmid & rmid_mask);
-    if ( val != new_val )
+    if ( psr_cmt_enabled() )
+        psr_assoc_rmid(&reg, d->arch.psr_rmid);
+
+    psr_assoc_reg_write(psra, reg);
+}
+
+static void psr_cpu_init(unsigned int cpu)
+{
+    psr_assoc_init();
+}
+
+static int cpu_callback(
+    struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+    unsigned int cpu = (unsigned long)hcpu;
+
+    switch ( action )
+    {
+    case CPU_STARTING:
+        psr_cpu_init(cpu);
+        break;
+    }
+
+    return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_nfb = {
+    .notifier_call = cpu_callback
+};
+
+static int __init psr_presmp_init(void)
+{
+    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
+        init_psr_cmt(opt_rmid_max);
+
+    if (  psr_cmt_enabled() )
     {
-        wrmsrl(MSR_IA32_PSR_ASSOC, new_val);
-        psra->val = new_val;
+        psr_cpu_init(smp_processor_id());
+        register_cpu_notifier(&cpu_nfb);
     }
+
+    return 0;
 }
+presmp_initcall(psr_presmp_init);
 
 /*
  * Local variables:
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index c6076e9..585350c 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -46,7 +46,8 @@ static inline bool_t psr_cmt_enabled(void)
 
 int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
-void psr_assoc_rmid(unsigned int rmid);
+
+void psr_ctxt_switch_to(struct domain *d);
 
 #endif /* __ASM_PSR_H__ */
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 3/8] x86: detect and initialize Intel CAT feature
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
  2015-03-26 12:38 ` [PATCH v3 1/8] x86: clean up psr boot parameter parsing Chao Peng
  2015-03-26 12:38 ` [PATCH v3 2/8] x86: improve psr scheduling code Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-27 18:31   ` Andrew Cooper
  2015-03-26 12:38 ` [PATCH v3 4/8] x86: add support for COS/CBM manangement Chao Peng
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Detect Intel Cache Allocation Technology(CAT) feature and store the
cpuid information for later use. Currently only L3 cache allocation is
supported. The L3 CAT features may vary among sockets so per-socket
feature information is stored. The initialization can happen either at
boot time or when CPU(s) is hot plugged after booting.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v3:
* Remove num_sockets boot option instead calculate it at boot time.
* Name hardcoded CAT cpuid leaf as PSR_CPUID_LEVEL_CAT.
Changes in v2:
* socket_num => num_sockets and fix several documentaion issues.
* refactor boot line parameters parsing into standlone patch.
* set opt_num_sockets = NR_CPUS when opt_num_sockets > NR_CPUS.
* replace CPU_ONLINE with CPU_STARTING and integrate that into scheduling
  improvement patch.
* reimplement get_max_socket() with cpu_to_socket();
* cbm is still uint64 as there is a path forward for supporting long masks.
---
 docs/misc/xen-command-line.markdown | 13 ++++++-
 xen/arch/x86/psr.c                  | 75 ++++++++++++++++++++++++++++++++++---
 xen/include/asm-x86/cpufeature.h    |  1 +
 xen/include/asm-x86/psr.h           |  3 ++
 4 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 1dda1f0..9ad8801 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1122,9 +1122,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> )`
+> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> )`
 
-> Default: `psr=cmt:0,rmid_max:255`
+> Default: `psr=cmt:0,rmid_max:255,cat:0`
 
 Platform Shared Resource(PSR) Services.  Intel Haswell and later server
 platforms offer information about the sharing of resources.
@@ -1134,6 +1134,11 @@ Monitoring ID(RMID) is used to bind the domain to corresponding shared
 resource.  RMID is a hardware-provided layer of abstraction between software
 and logical processors.
 
+To use the PSR cache allocation service for a certain domain, a capacity
+bitmasks(CBM) is used to bind the domain to corresponding shared resource.
+CBM represents cache capacity and indicates the degree of overlap and isolation
+between domains.
+
 The following resources are available:
 
 * Cache Monitoring Technology (Haswell and later).  Information regarding the
@@ -1144,6 +1149,10 @@ The following resources are available:
   total/local memory bandwidth. Follow the same options with Cache Monitoring
   Technology.
 
+* Cache Alllocation Technology (Broadwell and later).  Information regarding
+  the cache allocation.
+  * `cat` instructs Xen to enable/disable Cache Allocation Technology.
+
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | [c]old]`
 
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index b3b540c..5087a75 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -19,17 +19,36 @@
 #include <asm/psr.h>
 
 #define PSR_CMT        (1<<0)
+#define PSR_CAT        (1<<1)
+
+struct psr_cat_socket_info {
+    bool_t initialized;
+    bool_t enabled;
+    unsigned int cbm_len;
+    unsigned int cos_max;
+};
 
 struct psr_assoc {
     uint64_t val;
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
+static struct psr_cat_socket_info *__read_mostly cat_socket_info;
+
 static unsigned int __initdata opt_psr;
 static unsigned int __initdata opt_rmid_max = 255;
+static unsigned int __read_mostly nr_sockets;
 static uint64_t rmid_mask;
 static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
 
+static unsigned int get_socket_count(void)
+{
+    unsigned int cpus_per_socket = boot_cpu_data.x86_max_cores *
+                                   boot_cpu_data.x86_num_siblings;
+
+    return DIV_ROUND_UP(nr_cpu_ids, cpus_per_socket);
+}
+
 static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)
 {
     if ( !strcmp(s, feature) )
@@ -64,6 +83,7 @@ static void __init parse_psr_param(char *s)
             *val_str++ = '\0';
 
         parse_psr_bool(s, val_str, "cmt", PSR_CMT);
+        parse_psr_bool(s, val_str, "cat", PSR_CAT);
 
         if ( val_str && !strcmp(s, "rmid_max") )
             opt_rmid_max = simple_strtoul(val_str, NULL, 0);
@@ -207,8 +227,50 @@ void psr_ctxt_switch_to(struct domain *d)
     psr_assoc_reg_write(psra, reg);
 }
 
+static void cat_cpu_init(unsigned int cpu)
+{
+    unsigned int eax, ebx, ecx, edx;
+    struct psr_cat_socket_info *info;
+    unsigned int socket;
+    const struct cpuinfo_x86 *c;
+
+    socket = cpu_to_socket(cpu);
+    ASSERT(socket < nr_sockets);
+
+    info = cat_socket_info + socket;
+
+    /* Avoid initializing more than one times for the same socket. */
+    if ( test_and_set_bool(info->initialized) )
+        return;
+
+    c = cpu_data + cpu;
+    if ( !cpu_has(c, X86_FEATURE_CAT) )
+        return;
+
+    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
+    if ( ebx & PSR_RESOURCE_TYPE_L3 )
+    {
+        cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
+        info->cbm_len = (eax & 0x1f) + 1;
+        info->cos_max = (edx & 0xffff);
+
+        info->enabled = 1;
+        printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+               socket, info->cos_max, info->cbm_len);
+    }
+}
+
+static void __init init_psr_cat(void)
+{
+    nr_sockets = get_socket_count();
+    cat_socket_info = xzalloc_array(struct psr_cat_socket_info, nr_sockets);
+}
+
 static void psr_cpu_init(unsigned int cpu)
 {
+    if ( cat_socket_info )
+        cat_cpu_init(cpu);
+
     psr_assoc_init();
 }
 
@@ -236,14 +298,17 @@ static int __init psr_presmp_init(void)
     if ( (opt_psr & PSR_CMT) && opt_rmid_max )
         init_psr_cmt(opt_rmid_max);
 
-    if (  psr_cmt_enabled() )
-    {
-        psr_cpu_init(smp_processor_id());
-        register_cpu_notifier(&cpu_nfb);
-    }
+    if ( opt_psr & PSR_CAT )
+        init_psr_cat();
+
+    psr_cpu_init(smp_processor_id());
+
+    if ( psr_cmt_enabled() || cat_socket_info )
+          register_cpu_notifier(&cpu_nfb);
 
     return 0;
 }
+
 presmp_initcall(psr_presmp_init);
 
 /*
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 7963a3a..8c0f0a6 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -149,6 +149,7 @@
 #define X86_FEATURE_CMT 	(7*32+12) /* Cache Monitoring Technology */
 #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
 #define X86_FEATURE_MPX		(7*32+14) /* Memory Protection Extensions */
+#define X86_FEATURE_CAT 	(7*32+15) /* Cache Allocation Technology */
 #define X86_FEATURE_RDSEED	(7*32+18) /* RDSEED instruction */
 #define X86_FEATURE_ADX		(7*32+19) /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 585350c..3bc5496 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -18,6 +18,9 @@
 
 #include <xen/types.h>
 
+/* CAT cpuid level */
+#define PSR_CPUID_LEVEL_CAT   0x10
+
 /* Resource Type Enumeration */
 #define PSR_RESOURCE_TYPE_L3            0x2
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 4/8] x86: add support for COS/CBM manangement
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (2 preceding siblings ...)
  2015-03-26 12:38 ` [PATCH v3 3/8] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-27 20:16   ` Andrew Cooper
  2015-03-26 12:38 ` [PATCH v3 5/8] x86: add scheduling support for Intel CAT Chao Peng
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

CAT introduces a mechanism for software to enable cache allocation based
on application priority or Class of Service(COS). Each COS can be
configured using capacity bitmasks(CBM) to represent cache capacity
and indicate the degree of overlap and isolation between COSs.

In XEN implementation, the cache allocation granularity is domain. All
VCPUs of a domain have the same COS, and therefore, correspond to the
same CBM. COS is maintained in hypervisor only while CBM is exposed to
user space directly to allow getting/setting domain's cache capacity.

Both CBM/COS may be socket-different for the same domain.

General Cache Allocation Technology(CAT) information such as maximum COS
and CBM length are exposed to user space by a SYSCTRL hypercall, to help
to construct the CBM from user side.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v3:
* Maintain socket_cpu_mask to make get_socket_cpu cheaper.
Changes in v2:
* set d->arch.psr_cos_ids when free it.
* check zero_bit < cbm_len when use it.
* change cpumask_check assertion to return error code.
---
 xen/arch/x86/domain.c           |   6 +-
 xen/arch/x86/domctl.c           |  18 +++
 xen/arch/x86/psr.c              | 243 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/sysctl.c           |  18 +++
 xen/include/asm-x86/domain.h    |   5 +-
 xen/include/asm-x86/msr-index.h |   1 +
 xen/include/asm-x86/psr.h       |   8 ++
 xen/include/public/domctl.h     |  12 ++
 xen/include/public/sysctl.h     |  16 +++
 9 files changed, 325 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 695a2eb..129d42f 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -616,6 +616,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
         /* 64-bit PV guest by default. */
         d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
 
+    if ( (rc = psr_domain_init(d)) != 0 )
+        goto fail;
+
     /* initialize default tsc behavior in case tools don't */
     tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
     spin_lock_init(&d->arch.vtsc_lock);
@@ -634,6 +637,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
     free_perdomain_mappings(d);
     if ( is_pv_domain(d) )
         free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
+    psr_domain_free(d);
     return rc;
 }
 
@@ -657,7 +661,7 @@ void arch_domain_destroy(struct domain *d)
     free_xenheap_page(d->shared_info);
     cleanup_domain_irq_mapping(d);
 
-    psr_free_rmid(d);
+    psr_domain_free(d);
 }
 
 void arch_domain_shutdown(struct domain *d)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index d4f6ccf..89a6b33 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1326,6 +1326,24 @@ long arch_do_domctl(
         }
         break;
 
+    case XEN_DOMCTL_psr_cat_op:
+        switch ( domctl->u.psr_cat_op.cmd )
+        {
+        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
+            ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 domctl->u.psr_cat_op.data);
+            break;
+        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
+            ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+                                 &domctl->u.psr_cat_op.data);
+            copyback = 1;
+            break;
+        default:
+            ret = -EOPNOTSUPP;
+            break;
+        }
+        break;
+
     default:
         ret = iommu_do_domctl(domctl, d, u_domctl);
         break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 5087a75..cb8e3bc 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -21,11 +21,18 @@
 #define PSR_CMT        (1<<0)
 #define PSR_CAT        (1<<1)
 
+struct psr_cat_cbm {
+    unsigned int ref;
+    uint64_t cbm;
+};
+
 struct psr_cat_socket_info {
     bool_t initialized;
     bool_t enabled;
     unsigned int cbm_len;
     unsigned int cos_max;
+    struct psr_cat_cbm *cos_cbm_map;
+    cpumask_t *socket_cpu_mask;
 };
 
 struct psr_assoc {
@@ -49,6 +56,19 @@ static unsigned int get_socket_count(void)
     return DIV_ROUND_UP(nr_cpu_ids, cpus_per_socket);
 }
 
+static unsigned int get_socket_cpu(unsigned int socket)
+{
+    cpumask_t *cpu_mask;
+
+    if ( socket < nr_sockets )
+    {
+        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
+        ASSERT(cpu_mask != NULL);
+        return cpumask_any(cpu_mask);
+    }
+    return nr_cpu_ids;
+}
+
 static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)
 {
     if ( !strcmp(s, feature) )
@@ -227,6 +247,198 @@ void psr_ctxt_switch_to(struct domain *d)
     psr_assoc_reg_write(psra, reg);
 }
 
+static int get_cat_socket_info(unsigned int socket,
+                               struct psr_cat_socket_info **info)
+{
+    if ( !cat_socket_info )
+        return -ENODEV;
+
+    if ( socket >= nr_sockets )
+        return -EBADSLT;
+
+    if ( !cat_socket_info[socket].enabled )
+        return -ENOENT;
+
+    *info = cat_socket_info + socket;
+    return 0;
+}
+
+int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
+                        uint32_t *cos_max)
+{
+    struct psr_cat_socket_info *info;
+    int ret = get_cat_socket_info(socket, &info);
+
+    if ( ret )
+        return ret;
+
+    *cbm_len = info->cbm_len;
+    *cos_max = info->cos_max;
+
+    return 0;
+}
+
+int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
+{
+    unsigned int cos;
+    struct psr_cat_socket_info *info;
+    int ret = get_cat_socket_info(socket, &info);
+
+    if ( ret )
+        return ret;
+
+    cos = d->arch.psr_cos_ids[socket];
+    *cbm = info->cos_cbm_map[cos].cbm;
+    return 0;
+}
+
+static bool_t psr_check_cbm(unsigned int cbm_len, uint64_t cbm)
+{
+    unsigned int first_bit, zero_bit;
+
+    /* Set bits should only in the range of [0, cbm_len). */
+    if ( cbm & (~0ull << cbm_len) )
+        return 0;
+
+    /* At least two contiguous bits need to be set. */
+    if ( hweight_long(cbm) < 2 )
+        return 0;
+
+    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 0;
+
+    return 1;
+}
+
+struct cos_cbm_info
+{
+    unsigned int cos;
+    uint64_t cbm;
+};
+
+static void do_write_l3_cbm(void *data)
+{
+    struct cos_cbm_info *info = data;
+    wrmsrl(MSR_IA32_L3_MASK(info->cos), info->cbm);
+}
+
+static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
+{
+    struct cos_cbm_info info = {.cos = cos, .cbm = cbm };
+
+    if ( socket == cpu_to_socket(smp_processor_id()) )
+        do_write_l3_cbm(&info);
+    else
+    {
+        unsigned int cpu = get_socket_cpu(socket);
+
+        if ( cpu >= nr_cpu_ids )
+            return -EBADSLT;
+        on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
+    }
+
+    return 0;
+}
+
+int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
+{
+    unsigned int old_cos, cos;
+    struct psr_cat_cbm *map, *find;
+    struct psr_cat_socket_info *info;
+    int ret = get_cat_socket_info(socket, &info);
+
+    if ( ret )
+        return ret;
+
+    if ( !psr_check_cbm(info->cbm_len, cbm) )
+        return -EINVAL;
+
+    old_cos = d->arch.psr_cos_ids[socket];
+    map = info->cos_cbm_map;
+    find = NULL;
+
+    for ( cos = 0; cos <= info->cos_max; cos++ )
+    {
+        /* If still not found, then keep unused one. */
+        if ( !find && cos != 0 && map[cos].ref == 0 )
+            find = map + cos;
+        else if ( map[cos].cbm == cbm )
+        {
+            if ( unlikely(cos == old_cos) )
+                return -EEXIST;
+            find = map + cos;
+            break;
+        }
+    }
+
+    /* If old cos is referred only by the domain, then use it. */
+    if ( !find && map[old_cos].ref == 1 )
+        find = map + old_cos;
+
+    if ( !find )
+        return -EUSERS;
+
+    cos = find - map;
+    if ( find->cbm != cbm )
+    {
+        ret = write_l3_cbm(socket, cos, cbm);
+        if ( ret )
+            return ret;
+        find->cbm = cbm;
+    }
+    find->ref++;
+    map[old_cos].ref--;
+    d->arch.psr_cos_ids[socket] = cos;
+    return 0;
+}
+
+/* Called with domain lock held, no psr specific lock needed */
+static void psr_free_cos(struct domain *d)
+{
+    unsigned int socket;
+    unsigned int cos;
+
+    if( !d->arch.psr_cos_ids )
+        return;
+
+    for ( socket = 0; socket < nr_sockets; socket++ )
+    {
+        if ( !cat_socket_info[socket].enabled )
+            continue;
+
+        if ( (cos = d->arch.psr_cos_ids[socket]) == 0 )
+            continue;
+
+        cat_socket_info[socket].cos_cbm_map[cos].ref--;
+    }
+
+    xfree(d->arch.psr_cos_ids);
+    d->arch.psr_cos_ids = NULL;
+}
+
+int psr_domain_init(struct domain *d)
+{
+    if ( cat_socket_info )
+    {
+        d->arch.psr_cos_ids = xzalloc_array(unsigned int, nr_sockets);
+        if ( !d->arch.psr_cos_ids )
+            return -ENOMEM;
+    }
+
+    return 0;
+}
+
+void psr_domain_free(struct domain *d)
+{
+    psr_free_rmid(d);
+    psr_free_cos(d);
+}
+
 static void cat_cpu_init(unsigned int cpu)
 {
     unsigned int eax, ebx, ecx, edx;
@@ -238,6 +450,8 @@ static void cat_cpu_init(unsigned int cpu)
     ASSERT(socket < nr_sockets);
 
     info = cat_socket_info + socket;
+    if ( info->socket_cpu_mask == NULL )
+        info->socket_cpu_mask = per_cpu(cpu_core_mask, cpu);
 
     /* Avoid initializing more than one times for the same socket. */
     if ( test_and_set_bool(info->initialized) )
@@ -254,6 +468,14 @@ static void cat_cpu_init(unsigned int cpu)
         info->cbm_len = (eax & 0x1f) + 1;
         info->cos_max = (edx & 0xffff);
 
+        info->cos_cbm_map = xzalloc_array(struct psr_cat_cbm,
+                                          info->cos_max + 1UL);
+        if ( !info->cos_cbm_map )
+            return;
+
+        /* cos=0 is reserved as default cbm(all ones). */
+        info->cos_cbm_map[0].cbm = (1ull << info->cbm_len) - 1;
+
         info->enabled = 1;
         printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
                socket, info->cos_max, info->cbm_len);
@@ -274,6 +496,24 @@ static void psr_cpu_init(unsigned int cpu)
     psr_assoc_init();
 }
 
+static void psr_cpu_fini(unsigned int cpu)
+{
+    unsigned int socket, next;
+    cpumask_t *cpu_mask;
+
+    if ( cat_socket_info )
+    {
+        socket = cpu_to_socket(cpu);
+        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
+
+        if ( (next = cpumask_cycle(cpu, cpu_mask)) == cpu )
+            cat_socket_info[socket].socket_cpu_mask = NULL;
+        else
+            cat_socket_info[socket].socket_cpu_mask =
+                                    per_cpu(cpu_core_mask, next);
+    }
+}
+
 static int cpu_callback(
     struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
@@ -284,6 +524,9 @@ static int cpu_callback(
     case CPU_STARTING:
         psr_cpu_init(cpu);
         break;
+    case CPU_DYING:
+        psr_cpu_fini(cpu);
+        break;
     }
 
     return NOTIFY_DONE;
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 611a291..75366fe 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -171,6 +171,24 @@ long arch_do_sysctl(
 
         break;
 
+    case XEN_SYSCTL_psr_cat_op:
+        switch ( sysctl->u.psr_cat_op.cmd )
+        {
+        case XEN_SYSCTL_PSR_CAT_get_l3_info:
+            ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
+                                      &sysctl->u.psr_cat_op.u.l3_info.cbm_len,
+                                      &sysctl->u.psr_cat_op.u.l3_info.cos_max);
+
+            if ( !ret && __copy_to_guest(u_sysctl, sysctl, 1) )
+                ret = -EFAULT;
+
+            break;
+        default:
+            ret = -ENOSYS;
+            break;
+        }
+        break;
+
     default:
         ret = -ENOSYS;
         break;
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9cdffa8..9c4d0e6 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -333,7 +333,10 @@ struct arch_domain
     struct e820entry *e820;
     unsigned int nr_e820;
 
-    unsigned int psr_rmid; /* RMID assigned to the domain for CMT */
+    /* RMID assigned to the domain for CMT */
+    unsigned int psr_rmid;
+    /* COS assigned to the domain for each socket */
+    unsigned int *psr_cos_ids;
 
     /* Shared page for notifying that explicit PIRQ EOI is required. */
     unsigned long *pirq_eoi_map;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 83f2f70..b96f0f6 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -327,6 +327,7 @@
 #define MSR_IA32_CMT_EVTSEL		0x00000c8d
 #define MSR_IA32_CMT_CTR		0x00000c8e
 #define MSR_IA32_PSR_ASSOC		0x00000c8f
+#define MSR_IA32_L3_MASK(n)		(0x00000c90 + (n))
 
 /* Intel Model 6 */
 #define MSR_P6_PERFCTR(n)		(0x000000c1 + (n))
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 3bc5496..fb474bb 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -52,6 +52,14 @@ void psr_free_rmid(struct domain *d);
 
 void psr_ctxt_switch_to(struct domain *d);
 
+int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
+                        uint32_t *cos_max);
+int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
+int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
+
+int psr_domain_init(struct domain *d);
+void psr_domain_free(struct domain *d);
+
 #endif /* __ASM_PSR_H__ */
 
 /*
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ca0e51e..9f04836 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1005,6 +1005,16 @@ struct xen_domctl_psr_cmt_op {
 typedef struct xen_domctl_psr_cmt_op xen_domctl_psr_cmt_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
 
+struct xen_domctl_psr_cat_op {
+#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM     0
+#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM     1
+    uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
+    uint32_t target;    /* IN: socket to be operated on */
+    uint64_t data;      /* IN/OUT */
+};
+typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -1080,6 +1090,7 @@ struct xen_domctl {
 #define XEN_DOMCTL_setvnumainfo                  74
 #define XEN_DOMCTL_psr_cmt_op                    75
 #define XEN_DOMCTL_arm_configure_domain          76
+#define XEN_DOMCTL_psr_cat_op                    77
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -1145,6 +1156,7 @@ struct xen_domctl {
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
         struct xen_domctl_vnuma             vnuma;
         struct xen_domctl_psr_cmt_op        psr_cmt_op;
+        struct xen_domctl_psr_cat_op        psr_cat_op;
         uint8_t                             pad[128];
     } u;
 };
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 8552dc6..91d90b8 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -656,6 +656,20 @@ struct xen_sysctl_psr_cmt_op {
 typedef struct xen_sysctl_psr_cmt_op xen_sysctl_psr_cmt_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cmt_op_t);
 
+#define XEN_SYSCTL_PSR_CAT_get_l3_info               0
+struct xen_sysctl_psr_cat_op {
+    uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CAT_* */
+    uint32_t target;    /* IN: socket to be operated on */
+    union {
+        struct {
+            uint32_t cbm_len;   /* OUT: CBM length */
+            uint32_t cos_max;   /* OUT: Maximum COS */
+        } l3_info;
+    } u;
+};
+typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
+
 struct xen_sysctl {
     uint32_t cmd;
 #define XEN_SYSCTL_readconsole                    1
@@ -678,6 +692,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_scheduler_op                  19
 #define XEN_SYSCTL_coverage_op                   20
 #define XEN_SYSCTL_psr_cmt_op                    21
+#define XEN_SYSCTL_psr_cat_op                    22
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -700,6 +715,7 @@ struct xen_sysctl {
         struct xen_sysctl_scheduler_op      scheduler_op;
         struct xen_sysctl_coverage_op       coverage_op;
         struct xen_sysctl_psr_cmt_op        psr_cmt_op;
+        struct xen_sysctl_psr_cat_op        psr_cat_op;
         uint8_t                             pad[128];
     } u;
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 5/8] x86: add scheduling support for Intel CAT
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (3 preceding siblings ...)
  2015-03-26 12:38 ` [PATCH v3 4/8] x86: add support for COS/CBM manangement Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-26 12:38 ` [PATCH v3 6/8] xsm: add CAT related xsm policies Chao Peng
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

On context switch, write the the domain's Class of Service(COS) to MSR
IA32_PQR_ASSOC, to notify hardware to use the new COS.

For performance reason, the socket number and COS mask for current cpu
is also cached in the local per-CPU variable.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v2:
* merge common scheduling changes into scheduling improvement patch.
* use readable expr for psra->cos_mask.
---
 xen/arch/x86/psr.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index cb8e3bc..0cfbc1f 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -37,6 +37,8 @@ struct psr_cat_socket_info {
 
 struct psr_assoc {
     uint64_t val;
+    unsigned int socket;
+    uint64_t cos_mask;
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
@@ -209,9 +211,22 @@ void psr_free_rmid(struct domain *d)
 
 static inline void psr_assoc_init(void)
 {
+    unsigned int socket;
+    struct psr_cat_socket_info *info;
     struct psr_assoc *psra = &this_cpu(psr_assoc);
 
-    if ( psr_cmt_enabled() )
+    if ( cat_socket_info )
+    {
+        socket = cpu_to_socket(smp_processor_id());
+        psra->socket = socket;
+
+        info = cat_socket_info + socket;
+        if ( info->enabled )
+            psra->cos_mask = ((1ull << get_count_order(info->cos_max)) - 1)
+                             << 32;
+    }
+
+    if ( psr_cmt_enabled() || psra->cos_mask )
         rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
 }
 
@@ -234,6 +249,12 @@ static inline void psr_assoc_rmid(uint64_t *reg, unsigned int rmid)
     *reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
 }
 
+static inline void psr_assoc_cos(uint64_t *reg, unsigned int cos,
+                                 uint64_t cos_mask)
+{
+    *reg = (*reg & ~cos_mask) | (((uint64_t)cos << 32) & cos_mask);
+}
+
 void psr_ctxt_switch_to(struct domain *d)
 {
     uint64_t reg;
@@ -244,6 +265,15 @@ void psr_ctxt_switch_to(struct domain *d)
     if ( psr_cmt_enabled() )
         psr_assoc_rmid(&reg, d->arch.psr_rmid);
 
+    if ( psra->cos_mask )
+    {
+        if ( d->arch.psr_cos_ids )
+            psr_assoc_cos(&reg, d->arch.psr_cos_ids[psra->socket],
+                          psra->cos_mask);
+        else
+            psr_assoc_cos(&reg, 0, psra->cos_mask);
+    }
+
     psr_assoc_reg_write(psra, reg);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 6/8] xsm: add CAT related xsm policies
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (4 preceding siblings ...)
  2015-03-26 12:38 ` [PATCH v3 5/8] x86: add scheduling support for Intel CAT Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-26 12:38 ` [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets Chao Peng
  2015-03-26 12:38 ` [PATCH v3 8/8] tools: add tools support for Intel CAT Chao Peng
  7 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Add xsm policies for Cache Allocation Technology(CAT) related hypercalls
to restrict the functions visibility to control domain only.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by:  Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 tools/flask/policy/policy/modules/xen/xen.if | 2 +-
 tools/flask/policy/policy/modules/xen/xen.te | 4 +++-
 xen/xsm/flask/hooks.c                        | 6 ++++++
 xen/xsm/flask/policy/access_vectors          | 6 ++++++
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if
index 2d32e1c..8bb081a 100644
--- a/tools/flask/policy/policy/modules/xen/xen.if
+++ b/tools/flask/policy/policy/modules/xen/xen.if
@@ -51,7 +51,7 @@ define(`create_domain_common', `
 			getaffinity setaffinity setvcpuextstate };
 	allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
 			set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
-			psr_cmt_op configure_domain };
+			psr_cmt_op configure_domain psr_cat_op };
 	allow $1 $2:security check_context;
 	allow $1 $2:shadow enable;
 	allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp };
diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index c0128aa..d431aaf 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -67,6 +67,7 @@ allow dom0_t xen_t:xen {
 allow dom0_t xen_t:xen2 {
     resource_op
     psr_cmt_op
+    psr_cat_op
 };
 allow dom0_t xen_t:mmu memorymap;
 
@@ -80,7 +81,8 @@ allow dom0_t dom0_t:domain {
 	getpodtarget setpodtarget set_misc_info set_virq_handler
 };
 allow dom0_t dom0_t:domain2 {
-	set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op
+	set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
+	get_vnumainfo psr_cmt_op psr_cat_op
 };
 allow dom0_t dom0_t:resource { add remove };
 
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 05dafed..8964321 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -729,6 +729,9 @@ static int flask_domctl(struct domain *d, int cmd)
     case XEN_DOMCTL_psr_cmt_op:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP);
 
+    case XEN_DOMCTL_psr_cat_op:
+        return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CAT_OP);
+
     case XEN_DOMCTL_arm_configure_domain:
         return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__CONFIGURE_DOMAIN);
 
@@ -790,6 +793,9 @@ static int flask_sysctl(int cmd)
     case XEN_SYSCTL_psr_cmt_op:
         return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
                                     XEN2__PSR_CMT_OP, NULL);
+    case XEN_SYSCTL_psr_cat_op:
+        return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+                                    XEN2__PSR_CAT_OP, NULL);
 
     default:
         printk("flask_sysctl: Unknown op %d\n", cmd);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 8f44b9d..8cc1ef3 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -84,6 +84,9 @@ class xen2
     resource_op
 # XEN_SYSCTL_psr_cmt_op
     psr_cmt_op
+# XEN_SYSCTL_psr_cat_op
+    psr_cat_op
+
 }
 
 # Classes domain and domain2 consist of operations that a domain performs on
@@ -221,6 +224,9 @@ class domain2
     psr_cmt_op
 # XEN_DOMCTL_configure_domain
     configure_domain
+# XEN_DOMCTL_psr_cat_op
+    psr_cat_op
+
 }
 
 # Similar to class domain, but primarily contains domctls related to HVM domains
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (5 preceding siblings ...)
  2015-03-26 12:38 ` [PATCH v3 6/8] xsm: add CAT related xsm policies Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-30 14:51   ` Wei Liu
  2015-03-26 12:38 ` [PATCH v3 8/8] tools: add tools support for Intel CAT Chao Peng
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

Introduce a util function libxl_count_physical_sockets() to get physical
socket count. Replaced CMT code with the new function in xl.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 tools/libxl/libxl_utils.c | 17 +++++++++++++++++
 tools/libxl/libxl_utils.h |  2 ++
 tools/libxl/xl_cmdimpl.c  | 11 +++--------
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..6dbc611 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -853,6 +853,23 @@ int libxl_get_online_cpus(libxl_ctx *ctx)
     return online_cpus < 0 ? ERROR_FAIL : online_cpus;
 }
 
+uint32_t libxl_count_physical_sockets(libxl_ctx *ctx)
+{
+    int rc;
+    libxl_physinfo info;
+    uint32_t nr_sockets = 0;
+
+    libxl_physinfo_init(&info);
+
+    rc = libxl_get_physinfo(ctx, &info);
+    if (!rc)
+        nr_sockets = info.nr_cpus / info.threads_per_core
+                                  / info.cores_per_socket;
+
+    libxl_physinfo_dispose(&info);
+    return nr_sockets;
+}
+
 int libxl_get_max_nodes(libxl_ctx *ctx)
 {
     int max_nodes = xc_get_max_nodes(ctx->xch);
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..f664de1 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -154,6 +154,8 @@ int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
 
 void libxl_string_copy(libxl_ctx *ctx, char **dst, char **src);
 
+uint32_t libxl_count_physical_sockets(libxl_ctx *ctx);
+
 #endif
 
 /*
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 04faf98..f26a02d 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7899,7 +7899,6 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
 {
     uint32_t i, socketid, nr_sockets, total_rmid;
     uint32_t l3_cache_size;
-    libxl_physinfo info;
     int rc, nr_domains;
 
     if (!libxl_psr_cmt_enabled(ctx)) {
@@ -7913,15 +7912,11 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
         return -1;
     }
 
-    libxl_physinfo_init(&info);
-    rc = libxl_get_physinfo(ctx, &info);
-    if (rc < 0) {
-        fprintf(stderr, "Failed getting physinfo, rc: %d\n", rc);
-        libxl_physinfo_dispose(&info);
+    nr_sockets = libxl_count_physical_sockets(ctx);
+    if (nr_sockets == 0) {
+        fprintf(stderr, "Failed getting physinfo\n");
         return -1;
     }
-    nr_sockets = info.nr_cpus / info.threads_per_core / info.cores_per_socket;
-    libxl_physinfo_dispose(&info);
 
     rc = libxl_psr_cmt_get_total_rmid(ctx, &total_rmid);
     if (rc < 0) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (6 preceding siblings ...)
  2015-03-26 12:38 ` [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets Chao Peng
@ 2015-03-26 12:38 ` Chao Peng
  2015-03-31 16:28   ` Ian Campbell
  7 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-26 12:38 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

This is the xc/xl changes to support Intel Cache Allocation
Technology(CAT). Two commands are introduced:
- xl psr-cat-cbm-set [-s socket] <domain> <cbm>
  Set cache capacity bitmasks(CBM) for a domain.
- xl psr-cat-show <domain>
  Show Cache Allocation Technology information.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v3:
* Add manpage.
* libxl_psr_cat_set/get_domain_data => libxl_psr_cat_set/get_cbm.
* Move libxl_count_physical_sockets into seperate patch.
* Support LIBXL_PSR_TARGET_ALL for libxl_psr_cat_set_cbm.
* Clean up the print codes.
---
 docs/man/xl.pod.1             |  31 +++++++++
 tools/libxc/include/xenctrl.h |  15 +++++
 tools/libxc/xc_psr.c          |  74 +++++++++++++++++++++
 tools/libxl/libxl.h           |  20 ++++++
 tools/libxl/libxl_psr.c       | 126 ++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_types.idl   |   5 ++
 tools/libxl/xl.h              |   4 ++
 tools/libxl/xl_cmdimpl.c      | 146 ++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/xl_cmdtable.c     |  12 ++++
 9 files changed, 426 insertions(+), 7 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index b016272..99979a5 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1492,6 +1492,37 @@ monitor types are:
 
 =back
 
+=head1 CACHE ALLOCATION TECHNOLOGY
+
+Intel Broadwell and later server platforms offer capabilities to configure and
+make use of the Cache Allocation Technology (CAT) mechanisms, which enable more
+cache resources (i.e. L3 cache) to be made available for high priority
+applications. In Xen implementation, CAT is used to control cache allocation
+on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
+(CBM) for the domain.
+
+=over 4
+
+=item B<psr-cat-cbm-set> [I<OPTIONS>] [I<domain-id>]
+
+Set cache capacity bitmasks(CBM) for a domain.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-s SOCKET>, B<--socket=SOCKET>
+
+Specify the socket to process, otherwise all sockets are processed.
+
+=back
+
+=item B<psr-cat-show> [I<domain-id>]
+
+Show CAT settings for a certain domain or all domains.
+
+=back
+
 =head1 TO BE DOCUMENTED
 
 We need better documentation for:
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index df18292..1373a46 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2692,6 +2692,12 @@ enum xc_psr_cmt_type {
     XC_PSR_CMT_LOCAL_MEM_COUNT,
 };
 typedef enum xc_psr_cmt_type xc_psr_cmt_type;
+
+enum xc_psr_cat_type {
+    XC_PSR_CAT_L3_CBM = 1,
+};
+typedef enum xc_psr_cat_type xc_psr_cat_type;
+
 int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
 int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
@@ -2706,6 +2712,15 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
                         uint32_t psr_cmt_type, uint64_t *monitor_data,
                         uint64_t *tsc);
 int xc_psr_cmt_enabled(xc_interface *xch);
+
+int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
+                               xc_psr_cat_type type, uint32_t target,
+                               uint64_t data);
+int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
+                               xc_psr_cat_type type, uint32_t target,
+                               uint64_t *data);
+int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
+                           uint32_t *cos_max, uint32_t *cbm_len);
 #endif
 
 #endif /* XENCTRL_H */
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index e367a80..6c670d5 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -248,6 +248,80 @@ int xc_psr_cmt_enabled(xc_interface *xch)
 
     return 0;
 }
+int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
+                               xc_psr_cat_type type, uint32_t target,
+                               uint64_t data)
+{
+    DECLARE_DOMCTL;
+    uint32_t cmd;
+
+    switch ( type )
+    {
+    case XC_PSR_CAT_L3_CBM:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
+        break;
+    default:
+        return -1;
+    }
+
+    domctl.cmd = XEN_DOMCTL_psr_cat_op;
+    domctl.domain = (domid_t)domid;
+    domctl.u.psr_cat_op.cmd = cmd;
+    domctl.u.psr_cat_op.target = target;
+    domctl.u.psr_cat_op.data = data;
+
+    return do_domctl(xch, &domctl);
+}
+
+int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
+                               xc_psr_cat_type type, uint32_t target,
+                               uint64_t *data)
+{
+    int rc;
+    DECLARE_DOMCTL;
+    uint32_t cmd;
+
+    switch ( type )
+    {
+    case XC_PSR_CAT_L3_CBM:
+        cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
+        break;
+    default:
+        return -1;
+    }
+
+    domctl.cmd = XEN_DOMCTL_psr_cat_op;
+    domctl.domain = (domid_t)domid;
+    domctl.u.psr_cat_op.cmd = cmd;
+    domctl.u.psr_cat_op.target = target;
+
+    rc = do_domctl(xch, &domctl);
+
+    if ( !rc )
+        *data = domctl.u.psr_cat_op.data;
+
+    return rc;
+}
+
+int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
+                           uint32_t *cos_max, uint32_t *cbm_len)
+{
+    int rc;
+    DECLARE_SYSCTL;
+
+    sysctl.cmd = XEN_SYSCTL_psr_cat_op;
+    sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
+    sysctl.u.psr_cat_op.target = socket;
+
+    rc = xc_sysctl(xch, &sysctl);
+    if ( !rc )
+    {
+        *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
+        *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
+    }
+
+    return rc;
+}
 
 /*
  * Local variables:
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 5eec092..57afddf 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -718,6 +718,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
  * If this is defined, the Memory Bandwidth Monitoring feature is supported.
  */
 #define LIBXL_HAVE_PSR_MBM 1
+
+/*
+ * LIBXL_HAVE_PSR_CAT
+ *
+ * If this is defined, the Cache Allocation Technology feature is supported.
+ */
+#define LIBXL_HAVE_PSR_CAT 1
 #endif
 
 typedef char **libxl_string_list;
@@ -1513,6 +1520,19 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint64_t *tsc_r);
 #endif
 
+#ifdef LIBXL_HAVE_PSR_CAT
+
+#define LIBXL_PSR_TARGET_ALL (~0U)
+int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
+                          libxl_psr_cat_type type, uint32_t target,
+                          uint64_t cbm);
+int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
+                          libxl_psr_cat_type type, uint32_t target,
+                          uint64_t *cbm_r);
+int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
+                              uint32_t *cos_max_r, uint32_t *cbm_len_r);
+#endif
+
 /* misc */
 
 /* Each of these sets or clears the flag according to whether the
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3e1c792..037d536 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -19,14 +19,37 @@
 
 #define IA32_QM_CTR_ERROR_MASK         (0x3ul << 62)
 
-static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
+static void libxl__psr_log_err_msg(libxl__gc *gc, int err)
 {
     char *msg;
 
     switch (err) {
     case ENOSYS:
+    case EOPNOTSUPP:
         msg = "unsupported operation";
         break;
+    case ESRCH:
+        msg = "invalid domain ID";
+        break;
+    case EBADSLT:
+        msg = "socket is not supported";
+        break;
+    case EFAULT:
+        msg = "failed to exchange data with Xen";
+        break;
+    default:
+        msg = "unknown error";
+        break;
+    }
+
+    LOGE(ERROR, "%s", msg);
+}
+
+static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
+{
+    char *msg;
+
+    switch (err) {
     case ENODEV:
         msg = "CMT is not supported in this system";
         break;
@@ -39,15 +62,35 @@ static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
     case EUSERS:
         msg = "no free RMID available";
         break;
-    case ESRCH:
-        msg = "invalid domain ID";
+    default:
+        libxl__psr_log_err_msg(gc, err);
+        return;
+    }
+
+    LOGE(ERROR, "%s", msg);
+}
+
+static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
+{
+    char *msg;
+
+    switch (err) {
+    case ENODEV:
+        msg = "CAT is not supported in this system";
         break;
-    case EFAULT:
-        msg = "failed to exchange data with Xen";
+    case ENOENT:
+        msg = "CAT is not enabled on the socket";
         break;
-    default:
-        msg = "unknown error";
+    case EUSERS:
+        msg = "no free COS available";
+        break;
+    case EEXIST:
+        msg = "The same CBM is already set to this domain";
         break;
+
+    default:
+        libxl__psr_log_err_msg(gc, err);
+        return;
     }
 
     LOGE(ERROR, "%s", msg);
@@ -247,6 +290,75 @@ out:
     return rc;
 }
 
+int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
+                          libxl_psr_cat_type type, uint32_t target,
+                          uint64_t cbm)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    uint32_t i, nr_sockets;
+
+    if (target != LIBXL_PSR_TARGET_ALL) {
+        rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, target, cbm);
+        if (rc < 0) {
+            libxl__psr_cat_log_err_msg(gc, errno);
+            rc = ERROR_FAIL;
+        }
+    } else {
+        nr_sockets = libxl_count_physical_sockets(ctx);
+        if (nr_sockets == 0) {
+            LOGE(ERROR, "failed to get system socket count");
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        for (i = 0; i < nr_sockets; i++) {
+            rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, i, cbm);
+            if (rc < 0) {
+                libxl__psr_cat_log_err_msg(gc, errno);
+                rc = ERROR_FAIL;
+            }
+        }
+    }
+
+out:
+    GC_FREE;
+    return rc;
+}
+
+int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
+                          libxl_psr_cat_type type, uint32_t target,
+                          uint64_t *cbm_r)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = xc_psr_cat_get_domain_data(ctx->xch, domid, type, target, cbm_r);
+    if (rc < 0) {
+        libxl__psr_cat_log_err_msg(gc, errno);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
+}
+
+int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
+                              uint32_t *cos_max_r, uint32_t *cbm_len_r)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = xc_psr_cat_get_l3_info(ctx->xch, socket, cos_max_r, cbm_len_r);
+    if (rc < 0) {
+        libxl__psr_cat_log_err_msg(gc, errno);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 47af340..b1ea963 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -699,3 +699,8 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
     (2, "TOTAL_MEM_COUNT"),
     (3, "LOCAL_MEM_COUNT"),
     ])
+
+libxl_psr_cat_type = Enumeration("psr_cat_type", [
+    (0, "UNKNOWN"),
+    (1, "L3_CBM"),
+    ])
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5bc138c..85fa997 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -117,6 +117,10 @@ int main_psr_cmt_attach(int argc, char **argv);
 int main_psr_cmt_detach(int argc, char **argv);
 int main_psr_cmt_show(int argc, char **argv);
 #endif
+#ifdef LIBXL_HAVE_PSR_CAT
+int main_psr_cat_cbm_set(int argc, char **argv);
+int main_psr_cat_show(int argc, char **argv);
+#endif
 
 void help(const char *command);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index f26a02d..ff963df 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8038,6 +8038,152 @@ int main_psr_cmt_show(int argc, char **argv)
 }
 #endif
 
+#ifdef LIBXL_HAVE_PSR_CAT
+static void psr_cat_print_one_domain_cbm(libxl_dominfo *dominfo,
+                                         uint32_t socket)
+{
+    char *domain_name;
+    uint64_t cbm;
+
+    domain_name = libxl_domid_to_name(ctx, dominfo->domid);
+    printf("%5d%25s", dominfo->domid, domain_name);
+    free(domain_name);
+
+    if (!libxl_psr_cat_get_cbm(ctx, dominfo->domid,
+                               LIBXL_PSR_CAT_TYPE_L3_CBM, socket, &cbm))
+         printf("%#16"PRIx64, cbm);
+
+    printf("\n");
+}
+
+static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socket)
+{
+    int i, nr_domains;
+
+    if (domid != INVALID_DOMID) {
+        libxl_dominfo dominfo;
+
+        if (libxl_domain_info(ctx, &dominfo, domid)) {
+            fprintf(stderr, "Failed to get domain info for %d\n", domid);
+            return -1;
+        }
+        psr_cat_print_one_domain_cbm(&dominfo, socket);
+    } else {
+        libxl_dominfo *list;
+
+        if (!(list = libxl_list_domain(ctx, &nr_domains))) {
+            fprintf(stderr, "Failed to get domain info for domain list.\n");
+            return -1;
+        }
+        for (i = 0; i < nr_domains; i++)
+            psr_cat_print_one_domain_cbm(list + i, socket);
+        libxl_dominfo_list_free(list, nr_domains);
+    }
+
+    return 0;
+}
+
+static int psr_cat_print_socket(uint32_t domid, uint32_t socket)
+{
+    uint32_t l3_cache_size, cos_max, cbm_len;
+    int rc;
+
+    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
+    if (rc) {
+        fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
+        return -1;
+    }
+
+    rc = libxl_psr_cat_get_l3_info(ctx, socket, &cos_max, &cbm_len);
+    if (rc) {
+        fprintf(stderr, "Failed to get cat info for socket:%d\n", socket);
+        return -1;
+    }
+
+    /* Header */
+    printf("SocketID:\t%u\n", socket);
+    printf("L3 Cache:\t%uKB\n", l3_cache_size);
+    printf("Maximum COS:\t%u\n", cos_max);
+    printf("CBM length:\t%u\n", cbm_len);
+    printf("Default CBM:\t%#"PRIx64"\n", (1ul << cbm_len) - 1);
+    printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+
+    return psr_cat_print_domain_cbm(domid, socket);
+}
+
+static int psr_cat_show(uint32_t domid)
+{
+    uint32_t socket, nr_sockets;
+    int rc;
+
+    nr_sockets = libxl_count_physical_sockets(ctx);
+    if (nr_sockets == 0) {
+        fprintf(stderr, "Failed getting physinfo\n");
+        return -1;
+    }
+
+    for (socket = 0; socket < nr_sockets; socket++) {
+        rc = psr_cat_print_socket(domid, socket);
+        if (rc)
+            return rc;
+    }
+
+    return 0;
+}
+
+int main_psr_cat_cbm_set(int argc, char **argv)
+{
+    uint32_t domid;
+    uint32_t target = LIBXL_PSR_TARGET_ALL;
+    libxl_psr_cat_type type = LIBXL_PSR_CAT_TYPE_L3_CBM;
+    uint64_t cbm;
+    char *ptr;
+    int opt = 0;
+
+    static struct option opts[] = {
+        {"socket", 0, 0, 's'},
+        {0, 0, 0, 0}
+    };
+
+    SWITCH_FOREACH_OPT(opt, "s", opts, "psr-cat-cbm-set", 1) {
+    case 's':
+        target = strtol(optarg, NULL, 10);
+        break;
+    }
+
+    domid = find_domain(argv[optind]);
+    ptr = argv[optind + 1];
+    if (strlen(ptr) > 2 && ptr[0] == '0' && ptr[1] == 'x')
+        cbm = strtoll(ptr, NULL , 16);
+    else
+        cbm = strtoll(ptr, NULL , 10);
+
+    return libxl_psr_cat_set_cbm(ctx, domid, type, target, cbm);
+}
+
+int main_psr_cat_show(int argc, char **argv)
+{
+    int opt;
+    uint32_t domid;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "psr-cat-show", 0) {
+        /* No options */
+    }
+
+    if (optind >= argc)
+        domid = INVALID_DOMID;
+    else if (optind == argc - 1)
+        domid = find_domain(argv[optind]);
+    else {
+        help("psr-cat-show");
+        return 2;
+    }
+
+    return psr_cat_show(domid);
+}
+
+#endif
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 22ab63b..c83a60b 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -542,6 +542,18 @@ struct cmd_spec cmd_table[] = {
       "\"total_mem_bandwidth\":     Show total memory bandwidth(KB/s)\n"
       "\"local_mem_bandwidth\":     Show local memory bandwidth(KB/s)\n",
     },
+    { "psr-cat-cbm-set",
+      &main_psr_cat_cbm_set, 0, 1,
+      "Set cache capacity bitmasks(CBM) for a domain",
+      "-s <socket>       Specify the socket to process, otherwise all sockets are processed\n"
+      "<Domain> <CBM>",
+    },
+    { "psr-cat-show",
+      &main_psr_cat_show, 0, 1,
+      "Show Cache Allocation Technology information",
+      "<Domain>",
+    },
+
 #endif
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 1/8] x86: clean up psr boot parameter parsing
  2015-03-26 12:38 ` [PATCH v3 1/8] x86: clean up psr boot parameter parsing Chao Peng
@ 2015-03-26 20:42   ` Andrew Cooper
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Cooper @ 2015-03-26 20:42 UTC (permalink / raw)
  To: Chao Peng, xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, Ian.Jackson, will.auld,
	JBeulich, wei.liu2, dgdegra

On 26/03/15 12:38, Chao Peng wrote:
> Change type of opt_psr from bool to int so more psr features can fit.
>
> Introduce a new routine to parse bool parameter so that both cmt and
> future psr features like cat can use it.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> Changes in v3:
> * Set "off" value explicity if requested.
> ---
>   xen/arch/x86/psr.c | 40 ++++++++++++++++++++++++----------------
>   1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 2ef83df..cfa534b 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -26,11 +26,31 @@ struct psr_assoc {
>   };
>   
>   struct psr_cmt *__read_mostly psr_cmt;
> -static bool_t __initdata opt_psr;
> +static unsigned int __initdata opt_psr;
>   static unsigned int __initdata opt_rmid_max = 255;
>   static uint64_t rmid_mask;
>   static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
>   
> +static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)

* should be on the right hand side of the space for all char * 
parmaters.  Each string should also be const, and bit should be unsigned 
to match opt_psr.  Futhermore, it should probably be named mask, or you 
work with (1U << bit).

> +{
> +    if ( !strcmp(s, feature) )
> +    {
> +        if ( !value )
> +            opt_psr |= bit;
> +        else
> +        {
> +            int val_int = parse_bool(value);
> +
> +            if ( val_int == 0 )
> +                opt_psr &= ~bit;
> +            else if ( val_int == 1 )
> +                opt_psr |= bit;
> +            else
> +                printk("PSR: unknown %s value: %s\n", feature, value);

This all runs before the console has been set up.  These printk()s go 
nowhere useful.  I would just discard them.

~Andrew

> +        }
> +    }
> +}
> +
>   static void __init parse_psr_param(char *s)
>   {
>       char *ss, *val_str;
> @@ -44,21 +64,9 @@ static void __init parse_psr_param(char *s)
>           if ( val_str )
>               *val_str++ = '\0';
>   
> -        if ( !strcmp(s, "cmt") )
> -        {
> -            if ( !val_str )
> -                opt_psr |= PSR_CMT;
> -            else
> -            {
> -                int val_int = parse_bool(val_str);
> -                if ( val_int == 1 )
> -                    opt_psr |= PSR_CMT;
> -                else if ( val_int != 0 )
> -                    printk("PSR: unknown cmt value: %s - CMT disabled!\n",
> -                                    val_str);
> -            }
> -        }
> -        else if ( val_str && !strcmp(s, "rmid_max") )
> +        parse_psr_bool(s, val_str, "cmt", PSR_CMT);
> +
> +        if ( val_str && !strcmp(s, "rmid_max") )
>               opt_rmid_max = simple_strtoul(val_str, NULL, 0);
>   
>           s = ss + 1;

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 2/8] x86: improve psr scheduling code
  2015-03-26 12:38 ` [PATCH v3 2/8] x86: improve psr scheduling code Chao Peng
@ 2015-03-27 18:15   ` Andrew Cooper
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Cooper @ 2015-03-27 18:15 UTC (permalink / raw)
  To: Chao Peng, xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, Ian.Jackson, will.auld,
	JBeulich, wei.liu2, dgdegra

On 26/03/15 12:38, Chao Peng wrote:
> Switching RMID from previous vcpu to next vcpu only needs to write
> MSR_IA32_PSR_ASSOC once. Write it with the value of next vcpu is enough,
> no need to write '0' first. Idle domain has RMID set to 0 and because MSR
> is already updated lazily, so just switch it as it does.
>
> Also move the initialization of per-CPU variable which used for lazy
> update from context switch to CPU starting.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> Changes in v2:
> * Move initialization for psr_assoc from context switch to CPU_STARTING.
> ---
>  xen/arch/x86/domain.c     |  7 ++--
>  xen/arch/x86/psr.c        | 89 ++++++++++++++++++++++++++++++++++++-----------
>  xen/include/asm-x86/psr.h |  3 +-
>  3 files changed, 73 insertions(+), 26 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 04c1898..695a2eb 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1444,8 +1444,6 @@ static void __context_switch(void)
>      {
>          memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES);
>          vcpu_save_fpu(p);
> -        if ( psr_cmt_enabled() )
> -            psr_assoc_rmid(0);
>          p->arch.ctxt_switch_from(p);
>      }
>  
> @@ -1470,11 +1468,10 @@ static void __context_switch(void)
>          }
>          vcpu_restore_fpu_eager(n);
>          n->arch.ctxt_switch_to(n);
> -
> -        if ( psr_cmt_enabled() && n->domain->arch.psr_rmid > 0 )
> -            psr_assoc_rmid(n->domain->arch.psr_rmid);
>      }
>  
> +    psr_ctxt_switch_to(n->domain);
> +
>      gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
>                                    per_cpu(compat_gdt_table, cpu);
>      if ( need_full_gdt(n) )
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index cfa534b..b3b540c 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -22,7 +22,6 @@
>  
>  struct psr_assoc {
>      uint64_t val;
> -    bool_t initialized;
>  };
>  
>  struct psr_cmt *__read_mostly psr_cmt;
> @@ -123,14 +122,6 @@ static void __init init_psr_cmt(unsigned int rmid_max)
>      printk(XENLOG_INFO "Cache Monitoring Technology enabled\n");
>  }
>  
> -static int __init init_psr(void)
> -{
> -    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
> -        init_psr_cmt(opt_rmid_max);
> -    return 0;
> -}
> -__initcall(init_psr);
> -
>  /* Called with domain lock held, no psr specific lock needed */
>  int psr_alloc_rmid(struct domain *d)
>  {
> @@ -176,26 +167,84 @@ void psr_free_rmid(struct domain *d)
>      d->arch.psr_rmid = 0;
>  }
>  
> -void psr_assoc_rmid(unsigned int rmid)
> +static inline void psr_assoc_init(void)
>  {
> -    uint64_t val;
> -    uint64_t new_val;
>      struct psr_assoc *psra = &this_cpu(psr_assoc);
>  
> -    if ( !psra->initialized )
> -    {
> +    if ( psr_cmt_enabled() )
>          rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
> -        psra->initialized = 1;
> +}
> +
> +static inline void psr_assoc_reg_read(struct psr_assoc *psra, uint64_t *reg)
> +{
> +    *reg = psra->val;
> +}
> +
> +static inline void psr_assoc_reg_write(struct psr_assoc *psra, uint64_t reg)
> +{
> +    if ( reg != psra->val )
> +    {
> +        wrmsrl(MSR_IA32_PSR_ASSOC, reg);
> +        psra->val = reg;
>      }
> -    val = psra->val;
> +}

Throughout the series, these assoc_reg_read/write() only appear to be
used once, in psr_ctxt_switch_to().

I would fold them in manually.  It doesn't make psr_ctxt_switch_to() any
more complicated.

> +
> +static inline void psr_assoc_rmid(uint64_t *reg, unsigned int rmid)
> +{
> +    *reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
> +}
> +
> +void psr_ctxt_switch_to(struct domain *d)
> +{
> +    uint64_t reg;
> +    struct psr_assoc *psra = &this_cpu(psr_assoc);
> +
> +    psr_assoc_reg_read(psra, &reg);
>  
> -    new_val = (val & ~rmid_mask) | (rmid & rmid_mask);
> -    if ( val != new_val )
> +    if ( psr_cmt_enabled() )
> +        psr_assoc_rmid(&reg, d->arch.psr_rmid);
> +
> +    psr_assoc_reg_write(psra, reg);
> +}
> +
> +static void psr_cpu_init(unsigned int cpu)
> +{
> +    psr_assoc_init();
> +}
> +
> +static int cpu_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +
> +    switch ( action )
> +    {
> +    case CPU_STARTING:
> +        psr_cpu_init(cpu);
> +        break;
> +    }
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_nfb = {
> +    .notifier_call = cpu_callback
> +};
> +
> +static int __init psr_presmp_init(void)
> +{
> +    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
> +        init_psr_cmt(opt_rmid_max);
> +
> +    if (  psr_cmt_enabled() )

Double space.

~Andrew

>      {
> -        wrmsrl(MSR_IA32_PSR_ASSOC, new_val);
> -        psra->val = new_val;
> +        psr_cpu_init(smp_processor_id());
> +        register_cpu_notifier(&cpu_nfb);
>      }
> +
> +    return 0;
>  }
> +presmp_initcall(psr_presmp_init);
>  
>  /*
>   * Local variables:
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index c6076e9..585350c 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -46,7 +46,8 @@ static inline bool_t psr_cmt_enabled(void)
>  
>  int psr_alloc_rmid(struct domain *d);
>  void psr_free_rmid(struct domain *d);
> -void psr_assoc_rmid(unsigned int rmid);
> +
> +void psr_ctxt_switch_to(struct domain *d);
>  
>  #endif /* __ASM_PSR_H__ */
>  

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/8] x86: detect and initialize Intel CAT feature
  2015-03-26 12:38 ` [PATCH v3 3/8] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-03-27 18:31   ` Andrew Cooper
  2015-04-13 10:51     ` Jan Beulich
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Cooper @ 2015-03-27 18:31 UTC (permalink / raw)
  To: Chao Peng, xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, Ian.Jackson, will.auld,
	JBeulich, wei.liu2, dgdegra

On 26/03/15 12:38, Chao Peng wrote:
> Detect Intel Cache Allocation Technology(CAT) feature and store the
> cpuid information for later use. Currently only L3 cache allocation is
> supported. The L3 CAT features may vary among sockets so per-socket
> feature information is stored. The initialization can happen either at
> boot time or when CPU(s) is hot plugged after booting.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> Changes in v3:
> * Remove num_sockets boot option instead calculate it at boot time.
> * Name hardcoded CAT cpuid leaf as PSR_CPUID_LEVEL_CAT.
> Changes in v2:
> * socket_num => num_sockets and fix several documentaion issues.
> * refactor boot line parameters parsing into standlone patch.
> * set opt_num_sockets = NR_CPUS when opt_num_sockets > NR_CPUS.
> * replace CPU_ONLINE with CPU_STARTING and integrate that into scheduling
>   improvement patch.
> * reimplement get_max_socket() with cpu_to_socket();
> * cbm is still uint64 as there is a path forward for supporting long masks.
> ---
>  docs/misc/xen-command-line.markdown | 13 ++++++-
>  xen/arch/x86/psr.c                  | 75 ++++++++++++++++++++++++++++++++++---
>  xen/include/asm-x86/cpufeature.h    |  1 +
>  xen/include/asm-x86/psr.h           |  3 ++
>  4 files changed, 85 insertions(+), 7 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 1dda1f0..9ad8801 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1122,9 +1122,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> )`
> +> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> )`
>  
> -> Default: `psr=cmt:0,rmid_max:255`
> +> Default: `psr=cmt:0,rmid_max:255,cat:0`
>  
>  Platform Shared Resource(PSR) Services.  Intel Haswell and later server
>  platforms offer information about the sharing of resources.
> @@ -1134,6 +1134,11 @@ Monitoring ID(RMID) is used to bind the domain to corresponding shared
>  resource.  RMID is a hardware-provided layer of abstraction between software
>  and logical processors.
>  
> +To use the PSR cache allocation service for a certain domain, a capacity
> +bitmasks(CBM) is used to bind the domain to corresponding shared resource.
> +CBM represents cache capacity and indicates the degree of overlap and isolation
> +between domains.
> +
>  The following resources are available:
>  
>  * Cache Monitoring Technology (Haswell and later).  Information regarding the
> @@ -1144,6 +1149,10 @@ The following resources are available:
>    total/local memory bandwidth. Follow the same options with Cache Monitoring
>    Technology.
>  
> +* Cache Alllocation Technology (Broadwell and later).  Information regarding
> +  the cache allocation.
> +  * `cat` instructs Xen to enable/disable Cache Allocation Technology.
> +
>  ### reboot
>  > `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | [c]old]`
>  
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index b3b540c..5087a75 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -19,17 +19,36 @@
>  #include <asm/psr.h>
>  
>  #define PSR_CMT        (1<<0)
> +#define PSR_CAT        (1<<1)
> +
> +struct psr_cat_socket_info {
> +    bool_t initialized;
> +    bool_t enabled;
> +    unsigned int cbm_len;
> +    unsigned int cos_max;
> +};
>  
>  struct psr_assoc {
>      uint64_t val;
>  };
>  
>  struct psr_cmt *__read_mostly psr_cmt;
> +static struct psr_cat_socket_info *__read_mostly cat_socket_info;
> +
>  static unsigned int __initdata opt_psr;
>  static unsigned int __initdata opt_rmid_max = 255;
> +static unsigned int __read_mostly nr_sockets;
>  static uint64_t rmid_mask;
>  static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
>  
> +static unsigned int get_socket_count(void)
> +{
> +    unsigned int cpus_per_socket = boot_cpu_data.x86_max_cores *
> +                                   boot_cpu_data.x86_num_siblings;
> +
> +    return DIV_ROUND_UP(nr_cpu_ids, cpus_per_socket);
> +}
> +
>  static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)
>  {
>      if ( !strcmp(s, feature) )
> @@ -64,6 +83,7 @@ static void __init parse_psr_param(char *s)
>              *val_str++ = '\0';
>  
>          parse_psr_bool(s, val_str, "cmt", PSR_CMT);
> +        parse_psr_bool(s, val_str, "cat", PSR_CAT);
>  
>          if ( val_str && !strcmp(s, "rmid_max") )
>              opt_rmid_max = simple_strtoul(val_str, NULL, 0);
> @@ -207,8 +227,50 @@ void psr_ctxt_switch_to(struct domain *d)
>      psr_assoc_reg_write(psra, reg);
>  }
>  
> +static void cat_cpu_init(unsigned int cpu)
> +{
> +    unsigned int eax, ebx, ecx, edx;
> +    struct psr_cat_socket_info *info;
> +    unsigned int socket;
> +    const struct cpuinfo_x86 *c;
> +
> +    socket = cpu_to_socket(cpu);
> +    ASSERT(socket < nr_sockets);
> +
> +    info = cat_socket_info + socket;
> +
> +    /* Avoid initializing more than one times for the same socket. */
> +    if ( test_and_set_bool(info->initialized) )
> +        return;
> +
> +    c = cpu_data + cpu;
> +    if ( !cpu_has(c, X86_FEATURE_CAT) )
> +        return;

This check should be before the test_and_set_bool().

> +
> +    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
> +    {
> +        cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> +        info->cbm_len = (eax & 0x1f) + 1;
> +        info->cos_max = (edx & 0xffff);
> +
> +        info->enabled = 1;
> +        printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> +               socket, info->cos_max, info->cbm_len);

I would bump this to INFO, at least for socket 0.  Similar to CMT, a
user which has gone to the effort of enabling CAT will want some
indication in the boot log without also having to bump the logging level.

> +    }
> +}
> +
> +static void __init init_psr_cat(void)
> +{
> +    nr_sockets = get_socket_count();
> +    cat_socket_info = xzalloc_array(struct psr_cat_socket_info, nr_sockets);
> +}
> +
>  static void psr_cpu_init(unsigned int cpu)
>  {
> +    if ( cat_socket_info )
> +        cat_cpu_init(cpu);
> +
>      psr_assoc_init();
>  }
>  
> @@ -236,14 +298,17 @@ static int __init psr_presmp_init(void)
>      if ( (opt_psr & PSR_CMT) && opt_rmid_max )
>          init_psr_cmt(opt_rmid_max);
>  
> -    if (  psr_cmt_enabled() )
> -    {
> -        psr_cpu_init(smp_processor_id());
> -        register_cpu_notifier(&cpu_nfb);
> -    }
> +    if ( opt_psr & PSR_CAT )
> +        init_psr_cat();
> +
> +    psr_cpu_init(smp_processor_id());

You can be certain that smp_processor_id() is 0 in a presmp_init() callback.

> +
> +    if ( psr_cmt_enabled() || cat_socket_info )
> +          register_cpu_notifier(&cpu_nfb);
>  
>      return 0;
>  }
> +

Please drop this newline. (or of you wish to keep it, fold it into patch 2)

~Andrew

>  presmp_initcall(psr_presmp_init);
>  
>  /*
> diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
> index 7963a3a..8c0f0a6 100644
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -149,6 +149,7 @@
>  #define X86_FEATURE_CMT 	(7*32+12) /* Cache Monitoring Technology */
>  #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
>  #define X86_FEATURE_MPX		(7*32+14) /* Memory Protection Extensions */
> +#define X86_FEATURE_CAT 	(7*32+15) /* Cache Allocation Technology */
>  #define X86_FEATURE_RDSEED	(7*32+18) /* RDSEED instruction */
>  #define X86_FEATURE_ADX		(7*32+19) /* ADCX, ADOX instructions */
>  #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index 585350c..3bc5496 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -18,6 +18,9 @@
>  
>  #include <xen/types.h>
>  
> +/* CAT cpuid level */
> +#define PSR_CPUID_LEVEL_CAT   0x10
> +
>  /* Resource Type Enumeration */
>  #define PSR_RESOURCE_TYPE_L3            0x2
>  

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 4/8] x86: add support for COS/CBM manangement
  2015-03-26 12:38 ` [PATCH v3 4/8] x86: add support for COS/CBM manangement Chao Peng
@ 2015-03-27 20:16   ` Andrew Cooper
  2015-03-31  8:40     ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Andrew Cooper @ 2015-03-27 20:16 UTC (permalink / raw)
  To: Chao Peng, xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, Ian.Jackson, will.auld,
	JBeulich, wei.liu2, dgdegra

On 26/03/15 12:38, Chao Peng wrote:
> CAT introduces a mechanism for software to enable cache allocation based
> on application priority or Class of Service(COS). Each COS can be
> configured using capacity bitmasks(CBM) to represent cache capacity
> and indicate the degree of overlap and isolation between COSs.
>
> In XEN implementation, the cache allocation granularity is domain. All

"Xen"

Also, I suspect you mean "In Xen's implementation, the cache allocation
granularity is per domain."

> VCPUs of a domain have the same COS, and therefore, correspond to the
> same CBM. COS is maintained in hypervisor only while CBM is exposed to
> user space directly to allow getting/setting domain's cache capacity.
>
> Both CBM/COS may be socket-different for the same domain.
>
> General Cache Allocation Technology(CAT) information such as maximum COS
> and CBM length are exposed to user space by a SYSCTRL hypercall, to help

SYSCTL hypercall

> to construct the CBM from user side.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> Changes in v3:
> * Maintain socket_cpu_mask to make get_socket_cpu cheaper.
> Changes in v2:
> * set d->arch.psr_cos_ids when free it.
> * check zero_bit < cbm_len when use it.
> * change cpumask_check assertion to return error code.
> ---
>  xen/arch/x86/domain.c           |   6 +-
>  xen/arch/x86/domctl.c           |  18 +++
>  xen/arch/x86/psr.c              | 243 ++++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/sysctl.c           |  18 +++
>  xen/include/asm-x86/domain.h    |   5 +-
>  xen/include/asm-x86/msr-index.h |   1 +
>  xen/include/asm-x86/psr.h       |   8 ++
>  xen/include/public/domctl.h     |  12 ++
>  xen/include/public/sysctl.h     |  16 +++
>  9 files changed, 325 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 695a2eb..129d42f 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -616,6 +616,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>          /* 64-bit PV guest by default. */
>          d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
>  
> +    if ( (rc = psr_domain_init(d)) != 0 )
> +        goto fail;
> +
>      /* initialize default tsc behavior in case tools don't */
>      tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
>      spin_lock_init(&d->arch.vtsc_lock);
> @@ -634,6 +637,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>      free_perdomain_mappings(d);
>      if ( is_pv_domain(d) )
>          free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> +    psr_domain_free(d);
>      return rc;
>  }
>  
> @@ -657,7 +661,7 @@ void arch_domain_destroy(struct domain *d)
>      free_xenheap_page(d->shared_info);
>      cleanup_domain_irq_mapping(d);
>  
> -    psr_free_rmid(d);
> +    psr_domain_free(d);
>  }
>  
>  void arch_domain_shutdown(struct domain *d)
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index d4f6ccf..89a6b33 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1326,6 +1326,24 @@ long arch_do_domctl(
>          }
>          break;
>  
> +    case XEN_DOMCTL_psr_cat_op:
> +        switch ( domctl->u.psr_cat_op.cmd )
> +        {
> +        case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
> +            ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
> +                                 domctl->u.psr_cat_op.data);
> +            break;
> +        case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
> +            ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
> +                                 &domctl->u.psr_cat_op.data);
> +            copyback = 1;
> +            break;
> +        default:
> +            ret = -EOPNOTSUPP;
> +            break;
> +        }
> +        break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, d, u_domctl);
>          break;
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 5087a75..cb8e3bc 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -21,11 +21,18 @@
>  #define PSR_CMT        (1<<0)
>  #define PSR_CAT        (1<<1)
>  
> +struct psr_cat_cbm {
> +    unsigned int ref;
> +    uint64_t cbm;
> +};
> +
>  struct psr_cat_socket_info {
>      bool_t initialized;
>      bool_t enabled;
>      unsigned int cbm_len;
>      unsigned int cos_max;
> +    struct psr_cat_cbm *cos_cbm_map;
> +    cpumask_t *socket_cpu_mask;
>  };
>  
>  struct psr_assoc {
> @@ -49,6 +56,19 @@ static unsigned int get_socket_count(void)
>      return DIV_ROUND_UP(nr_cpu_ids, cpus_per_socket);
>  }
>  
> +static unsigned int get_socket_cpu(unsigned int socket)
> +{
> +    cpumask_t *cpu_mask;

This declaration can move inside the if.

> +
> +    if ( socket < nr_sockets )
> +    {
> +        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
> +        ASSERT(cpu_mask != NULL);
> +        return cpumask_any(cpu_mask);
> +    }
> +    return nr_cpu_ids;
> +}
> +
>  static void __init parse_psr_bool(char* s, char* value, char* feature, int bit)
>  {
>      if ( !strcmp(s, feature) )
> @@ -227,6 +247,198 @@ void psr_ctxt_switch_to(struct domain *d)
>      psr_assoc_reg_write(psra, reg);
>  }
>  
> +static int get_cat_socket_info(unsigned int socket,
> +                               struct psr_cat_socket_info **info)
> +{
> +    if ( !cat_socket_info )
> +        return -ENODEV;
> +
> +    if ( socket >= nr_sockets )
> +        return -EBADSLT;
> +
> +    if ( !cat_socket_info[socket].enabled )
> +        return -ENOENT;
> +
> +    *info = cat_socket_info + socket;
> +    return 0;
> +}
> +
> +int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
> +                        uint32_t *cos_max)
> +{
> +    struct psr_cat_socket_info *info;
> +    int ret = get_cat_socket_info(socket, &info);
> +
> +    if ( ret )
> +        return ret;
> +
> +    *cbm_len = info->cbm_len;
> +    *cos_max = info->cos_max;
> +
> +    return 0;
> +}
> +
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
> +{
> +    unsigned int cos;
> +    struct psr_cat_socket_info *info;
> +    int ret = get_cat_socket_info(socket, &info);
> +
> +    if ( ret )
> +        return ret;
> +
> +    cos = d->arch.psr_cos_ids[socket];
> +    *cbm = info->cos_cbm_map[cos].cbm;
> +    return 0;
> +}
> +
> +static bool_t psr_check_cbm(unsigned int cbm_len, uint64_t cbm)
> +{
> +    unsigned int first_bit, zero_bit;
> +
> +    /* Set bits should only in the range of [0, cbm_len). */
> +    if ( cbm & (~0ull << cbm_len) )
> +        return 0;
> +
> +    /* At least two contiguous bits need to be set. */

Is this true? The manual indicates otherwise, so long as you only have a
single group of set bits.

> +    if ( hweight_long(cbm) < 2 )
> +        return 0;
> +
> +    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 0;
> +
> +    return 1;
> +}
> +
> +struct cos_cbm_info
> +{
> +    unsigned int cos;
> +    uint64_t cbm;
> +};
> +
> +static void do_write_l3_cbm(void *data)
> +{
> +    struct cos_cbm_info *info = data;
> +    wrmsrl(MSR_IA32_L3_MASK(info->cos), info->cbm);
> +}
> +
> +static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
> +{
> +    struct cos_cbm_info info = {.cos = cos, .cbm = cbm };

{ .cos =

> +
> +    if ( socket == cpu_to_socket(smp_processor_id()) )
> +        do_write_l3_cbm(&info);
> +    else
> +    {
> +        unsigned int cpu = get_socket_cpu(socket);
> +
> +        if ( cpu >= nr_cpu_ids )
> +            return -EBADSLT;
> +        on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
> +    }
> +
> +    return 0;
> +}
> +
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
> +{
> +    unsigned int old_cos, cos;
> +    struct psr_cat_cbm *map, *find;
> +    struct psr_cat_socket_info *info;
> +    int ret = get_cat_socket_info(socket, &info);
> +
> +    if ( ret )
> +        return ret;
> +
> +    if ( !psr_check_cbm(info->cbm_len, cbm) )
> +        return -EINVAL;
> +
> +    old_cos = d->arch.psr_cos_ids[socket];
> +    map = info->cos_cbm_map;
> +    find = NULL;
> +
> +    for ( cos = 0; cos <= info->cos_max; cos++ )
> +    {
> +        /* If still not found, then keep unused one. */
> +        if ( !find && cos != 0 && map[cos].ref == 0 )
> +            find = map + cos;
> +        else if ( map[cos].cbm == cbm )
> +        {
> +            if ( unlikely(cos == old_cos) )
> +                return -EEXIST;
> +            find = map + cos;
> +            break;
> +        }
> +    }
> +
> +    /* If old cos is referred only by the domain, then use it. */
> +    if ( !find && map[old_cos].ref == 1 )
> +        find = map + old_cos;
> +
> +    if ( !find )
> +        return -EUSERS;
> +
> +    cos = find - map;
> +    if ( find->cbm != cbm )
> +    {
> +        ret = write_l3_cbm(socket, cos, cbm);
> +        if ( ret )
> +            return ret;
> +        find->cbm = cbm;
> +    }
> +    find->ref++;
> +    map[old_cos].ref--;
> +    d->arch.psr_cos_ids[socket] = cos;
> +    return 0;
> +}
> +
> +/* Called with domain lock held, no psr specific lock needed */
> +static void psr_free_cos(struct domain *d)
> +{
> +    unsigned int socket;
> +    unsigned int cos;
> +
> +    if( !d->arch.psr_cos_ids )
> +        return;
> +
> +    for ( socket = 0; socket < nr_sockets; socket++ )
> +    {
> +        if ( !cat_socket_info[socket].enabled )
> +            continue;
> +
> +        if ( (cos = d->arch.psr_cos_ids[socket]) == 0 )
> +            continue;
> +
> +        cat_socket_info[socket].cos_cbm_map[cos].ref--;
> +    }
> +
> +    xfree(d->arch.psr_cos_ids);
> +    d->arch.psr_cos_ids = NULL;
> +}
> +
> +int psr_domain_init(struct domain *d)
> +{
> +    if ( cat_socket_info )
> +    {
> +        d->arch.psr_cos_ids = xzalloc_array(unsigned int, nr_sockets);
> +        if ( !d->arch.psr_cos_ids )
> +            return -ENOMEM;
> +    }
> +
> +    return 0;
> +}
> +
> +void psr_domain_free(struct domain *d)
> +{
> +    psr_free_rmid(d);
> +    psr_free_cos(d);
> +}
> +
>  static void cat_cpu_init(unsigned int cpu)
>  {
>      unsigned int eax, ebx, ecx, edx;
> @@ -238,6 +450,8 @@ static void cat_cpu_init(unsigned int cpu)
>      ASSERT(socket < nr_sockets);
>  
>      info = cat_socket_info + socket;
> +    if ( info->socket_cpu_mask == NULL )
> +        info->socket_cpu_mask = per_cpu(cpu_core_mask, cpu);

Surely this wants to be skipped if info is already initialised?

>  
>      /* Avoid initializing more than one times for the same socket. */
>      if ( test_and_set_bool(info->initialized) )
> @@ -254,6 +468,14 @@ static void cat_cpu_init(unsigned int cpu)
>          info->cbm_len = (eax & 0x1f) + 1;
>          info->cos_max = (edx & 0xffff);
>  
> +        info->cos_cbm_map = xzalloc_array(struct psr_cat_cbm,
> +                                          info->cos_max + 1UL);
> +        if ( !info->cos_cbm_map )
> +            return;

This indicates that cat_cpu_init() needs to be able to signal ENOMEM.

> +
> +        /* cos=0 is reserved as default cbm(all ones). */
> +        info->cos_cbm_map[0].cbm = (1ull << info->cbm_len) - 1;
> +
>          info->enabled = 1;
>          printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>                 socket, info->cos_max, info->cbm_len);
> @@ -274,6 +496,24 @@ static void psr_cpu_init(unsigned int cpu)
>      psr_assoc_init();
>  }
>  
> +static void psr_cpu_fini(unsigned int cpu)
> +{
> +    unsigned int socket, next;
> +    cpumask_t *cpu_mask;
> +
> +    if ( cat_socket_info )
> +    {
> +        socket = cpu_to_socket(cpu);
> +        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
> +
> +        if ( (next = cpumask_cycle(cpu, cpu_mask)) == cpu )
> +            cat_socket_info[socket].socket_cpu_mask = NULL;
> +        else
> +            cat_socket_info[socket].socket_cpu_mask =
> +                                    per_cpu(cpu_core_mask, next);
> +    }
> +}
> +
>  static int cpu_callback(
>      struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
> @@ -284,6 +524,9 @@ static int cpu_callback(
>      case CPU_STARTING:
>          psr_cpu_init(cpu);
>          break;
> +    case CPU_DYING:
> +        psr_cpu_fini(cpu);
> +        break;
>      }
>  
>      return NOTIFY_DONE;
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 611a291..75366fe 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -171,6 +171,24 @@ long arch_do_sysctl(
>  
>          break;
>  
> +    case XEN_SYSCTL_psr_cat_op:
> +        switch ( sysctl->u.psr_cat_op.cmd )
> +        {
> +        case XEN_SYSCTL_PSR_CAT_get_l3_info:
> +            ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
> +                                      &sysctl->u.psr_cat_op.u.l3_info.cbm_len,
> +                                      &sysctl->u.psr_cat_op.u.l3_info.cos_max);
> +
> +            if ( !ret && __copy_to_guest(u_sysctl, sysctl, 1) )
> +                ret = -EFAULT;
> +
> +            break;
> +        default:
> +            ret = -ENOSYS;

ENOTSUPP

> +            break;
> +        }
> +        break;
> +
>      default:
>          ret = -ENOSYS;
>          break;
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 9cdffa8..9c4d0e6 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -333,7 +333,10 @@ struct arch_domain
>      struct e820entry *e820;
>      unsigned int nr_e820;
>  
> -    unsigned int psr_rmid; /* RMID assigned to the domain for CMT */
> +    /* RMID assigned to the domain for CMT */
> +    unsigned int psr_rmid;
> +    /* COS assigned to the domain for each socket */
> +    unsigned int *psr_cos_ids;
>  
>      /* Shared page for notifying that explicit PIRQ EOI is required. */
>      unsigned long *pirq_eoi_map;
> diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
> index 83f2f70..b96f0f6 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -327,6 +327,7 @@
>  #define MSR_IA32_CMT_EVTSEL		0x00000c8d
>  #define MSR_IA32_CMT_CTR		0x00000c8e
>  #define MSR_IA32_PSR_ASSOC		0x00000c8f
> +#define MSR_IA32_L3_MASK(n)		(0x00000c90 + (n))

PSR_L3_MASK perhaps ?

>  
>  /* Intel Model 6 */
>  #define MSR_P6_PERFCTR(n)		(0x000000c1 + (n))
> diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
> index 3bc5496..fb474bb 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -52,6 +52,14 @@ void psr_free_rmid(struct domain *d);
>  
>  void psr_ctxt_switch_to(struct domain *d);
>  
> +int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
> +                        uint32_t *cos_max);
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
> +int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
> +
> +int psr_domain_init(struct domain *d);
> +void psr_domain_free(struct domain *d);
> +
>  #endif /* __ASM_PSR_H__ */
>  
>  /*
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index ca0e51e..9f04836 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1005,6 +1005,16 @@ struct xen_domctl_psr_cmt_op {
>  typedef struct xen_domctl_psr_cmt_op xen_domctl_psr_cmt_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
>  
> +struct xen_domctl_psr_cat_op {
> +#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM     0
> +#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM     1
> +    uint32_t cmd;       /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
> +    uint32_t target;    /* IN: socket to be operated on */
> +    uint64_t data;      /* IN/OUT */
> +};
> +typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> @@ -1080,6 +1090,7 @@ struct xen_domctl {
>  #define XEN_DOMCTL_setvnumainfo                  74
>  #define XEN_DOMCTL_psr_cmt_op                    75
>  #define XEN_DOMCTL_arm_configure_domain          76
> +#define XEN_DOMCTL_psr_cat_op                    77
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1145,6 +1156,7 @@ struct xen_domctl {
>          struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
>          struct xen_domctl_vnuma             vnuma;
>          struct xen_domctl_psr_cmt_op        psr_cmt_op;
> +        struct xen_domctl_psr_cat_op        psr_cat_op;
>          uint8_t                             pad[128];
>      } u;
>  };
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 8552dc6..91d90b8 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -656,6 +656,20 @@ struct xen_sysctl_psr_cmt_op {
>  typedef struct xen_sysctl_psr_cmt_op xen_sysctl_psr_cmt_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cmt_op_t);
>  
> +#define XEN_SYSCTL_PSR_CAT_get_l3_info               0
> +struct xen_sysctl_psr_cat_op {
> +    uint32_t cmd;       /* IN: XEN_SYSCTL_PSR_CAT_* */
> +    uint32_t target;    /* IN: socket to be operated on */
> +    union {
> +        struct {
> +            uint32_t cbm_len;   /* OUT: CBM length */
> +            uint32_t cos_max;   /* OUT: Maximum COS */
> +        } l3_info;
> +    } u;
> +};
> +typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
> +
>  struct xen_sysctl {
>      uint32_t cmd;
>  #define XEN_SYSCTL_readconsole                    1
> @@ -678,6 +692,7 @@ struct xen_sysctl {
>  #define XEN_SYSCTL_scheduler_op                  19
>  #define XEN_SYSCTL_coverage_op                   20
>  #define XEN_SYSCTL_psr_cmt_op                    21
> +#define XEN_SYSCTL_psr_cat_op                    22
>      uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
>      union {
>          struct xen_sysctl_readconsole       readconsole;
> @@ -700,6 +715,7 @@ struct xen_sysctl {
>          struct xen_sysctl_scheduler_op      scheduler_op;
>          struct xen_sysctl_coverage_op       coverage_op;
>          struct xen_sysctl_psr_cmt_op        psr_cmt_op;
> +        struct xen_sysctl_psr_cat_op        psr_cat_op;
>          uint8_t                             pad[128];
>      } u;
>  };

Overall, this patch has a lot of moving parts in it.  I have not spotted
any major problems, but I also don't feel confident that I understand
all of what is going on.

It would certainly be easier to review if you split the patch into at
least 3; the core infrastructure, the domctl and the sysctl bits.  Even
then, there appear to be several different bits of core changes going
on, with some per-socket infrastructure and per-domain infrastructure.

The code itself appears to attempt to deal with sockets having a
different quantity of COS entries, but how does it resolve having a
different number of entries in the COS bitmaps?  This would appear to
mean that a domain given a certain COS would exhibit different behaviour
depending on which socket it happened to be scheduled on.

~Andrew

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets
  2015-03-26 12:38 ` [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets Chao Peng
@ 2015-03-30 14:51   ` Wei Liu
  2015-03-31  8:51     ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Wei Liu @ 2015-03-30 14:51 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

On Thu, Mar 26, 2015 at 08:38:24PM +0800, Chao Peng wrote:
> Introduce a util function libxl_count_physical_sockets() to get physical
> socket count. Replaced CMT code with the new function in xl.
> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
>  tools/libxl/libxl_utils.c | 17 +++++++++++++++++
>  tools/libxl/libxl_utils.h |  2 ++
>  tools/libxl/xl_cmdimpl.c  | 11 +++--------
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index 9053b27..6dbc611 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -853,6 +853,23 @@ int libxl_get_online_cpus(libxl_ctx *ctx)
>      return online_cpus < 0 ? ERROR_FAIL : online_cpus;
>  }
>  
> +uint32_t libxl_count_physical_sockets(libxl_ctx *ctx)
> +{
> +    int rc;
> +    libxl_physinfo info;
> +    uint32_t nr_sockets = 0;
> +
> +    libxl_physinfo_init(&info);
> +
> +    rc = libxl_get_physinfo(ctx, &info);
> +    if (!rc)
> +        nr_sockets = info.nr_cpus / info.threads_per_core
> +                                  / info.cores_per_socket;
> +
> +    libxl_physinfo_dispose(&info);
> +    return nr_sockets;
> +}
> +

This function looks x86 centric. If I'm right then it should be
surrounded by #ifdef.

I think Ian suggested you make this an internal function not a public
API. But if I misunderstand Ian's intention, please make this function
return libxl error code. The socket number can be returned from an out
parameter. I.e.

   int libxl_count_physical_sockets(libxl_ctx *ctx, uint32_t *nr_sockets);

Wei.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 4/8] x86: add support for COS/CBM manangement
  2015-03-27 20:16   ` Andrew Cooper
@ 2015-03-31  8:40     ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-31  8:40 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: keir, Ian.Campbell, stefano.stabellini, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Fri, Mar 27, 2015 at 08:16:10PM +0000, Andrew Cooper wrote:
> On 26/03/15 12:38, Chao Peng wrote:
> > @@ -238,6 +450,8 @@ static void cat_cpu_init(unsigned int cpu)
> >      ASSERT(socket < nr_sockets);
> >  
> >      info = cat_socket_info + socket;
> > +    if ( info->socket_cpu_mask == NULL )
> > +        info->socket_cpu_mask = per_cpu(cpu_core_mask, cpu);
> 
> Surely this wants to be skipped if info is already initialised?

The mask will be set to NULL in psr_cpu_fini() when the last CPU of the
socket is hot un-plugged. If any CPU of the socket is plugged in again
then the mask needs to be initialized but not skip.
> 
> >  
> >      /* Avoid initializing more than one times for the same socket. */
> >      if ( test_and_set_bool(info->initialized) )
> > @@ -254,6 +468,14 @@ static void cat_cpu_init(unsigned int cpu)
> >          info->cbm_len = (eax & 0x1f) + 1;
> >          info->cos_max = (edx & 0xffff);
> >  
> > +        info->cos_cbm_map = xzalloc_array(struct psr_cat_cbm,
> > +                                          info->cos_max + 1UL);
> > +        if ( !info->cos_cbm_map )
> > +            return;
> 
> This indicates that cat_cpu_init() needs to be able to signal ENOMEM.

The current behavior for ENOMEM case is: CAT is not enabled on the
socket. Both cat_cpu_init() and the caller will not actually make use
of this error code. 
> 
> > +
> > +        /* cos=0 is reserved as default cbm(all ones). */
> > +        info->cos_cbm_map[0].cbm = (1ull << info->cbm_len) - 1;
> > +
> >          info->enabled = 1;
> >          printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
> >                 socket, info->cos_max, info->cbm_len);
> > @@ -274,6 +496,24 @@ static void psr_cpu_init(unsigned int cpu)
> >      psr_assoc_init();
> >  }
> >  
> > +static void psr_cpu_fini(unsigned int cpu)
> > +{
> > +    unsigned int socket, next;
> > +    cpumask_t *cpu_mask;
> > +
> > +    if ( cat_socket_info )
> > +    {
> > +        socket = cpu_to_socket(cpu);
> > +        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
> > +
> > +        if ( (next = cpumask_cycle(cpu, cpu_mask)) == cpu )
> > +            cat_socket_info[socket].socket_cpu_mask = NULL;
> > +        else
> > +            cat_socket_info[socket].socket_cpu_mask =
> > +                                    per_cpu(cpu_core_mask, next);
> > +    }
> > +}
> > +
> Overall, this patch has a lot of moving parts in it.  I have not spotted
> any major problems, but I also don't feel confident that I understand
> all of what is going on.
> 
> It would certainly be easier to review if you split the patch into at
> least 3; the core infrastructure, the domctl and the sysctl bits.  Even
> then, there appear to be several different bits of core changes going
> on, with some per-socket infrastructure and per-domain infrastructure.

OK, I will split it.

> 
> The code itself appears to attempt to deal with sockets having a
> different quantity of COS entries, but how does it resolve having a
> different number of entries in the COS bitmaps?  This would appear to
> mean that a domain given a certain COS would exhibit different behaviour
> depending on which socket it happened to be scheduled on.

Hypervisor maintains per-socket COS bitmask for each domain as well. So
a domain actually has COS bitmask for each socket but not only one COS
bitmask. xen_domctl_psr_cat_op interface allows toolstack to set/get
each COS bitmask on socket basis.

Chao

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets
  2015-03-30 14:51   ` Wei Liu
@ 2015-03-31  8:51     ` Chao Peng
  2015-03-31 16:11       ` Ian Campbell
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-31  8:51 UTC (permalink / raw)
  To: Wei Liu
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, dgdegra

On Mon, Mar 30, 2015 at 03:51:01PM +0100, Wei Liu wrote:
> On Thu, Mar 26, 2015 at 08:38:24PM +0800, Chao Peng wrote:
> > Introduce a util function libxl_count_physical_sockets() to get physical
> > socket count. Replaced CMT code with the new function in xl.
> > 
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > ---
> >  tools/libxl/libxl_utils.c | 17 +++++++++++++++++
> >  tools/libxl/libxl_utils.h |  2 ++
> >  tools/libxl/xl_cmdimpl.c  | 11 +++--------
> >  3 files changed, 22 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> > index 9053b27..6dbc611 100644
> > --- a/tools/libxl/libxl_utils.c
> > +++ b/tools/libxl/libxl_utils.c
> > @@ -853,6 +853,23 @@ int libxl_get_online_cpus(libxl_ctx *ctx)
> >      return online_cpus < 0 ? ERROR_FAIL : online_cpus;
> >  }
> >  
> > +uint32_t libxl_count_physical_sockets(libxl_ctx *ctx)
> > +{
> > +    int rc;
> > +    libxl_physinfo info;
> > +    uint32_t nr_sockets = 0;
> > +
> > +    libxl_physinfo_init(&info);
> > +
> > +    rc = libxl_get_physinfo(ctx, &info);
> > +    if (!rc)
> > +        nr_sockets = info.nr_cpus / info.threads_per_core
> > +                                  / info.cores_per_socket;
> > +
> > +    libxl_physinfo_dispose(&info);
> > +    return nr_sockets;
> > +}
> > +
> 
> This function looks x86 centric. If I'm right then it should be
> surrounded by #ifdef.
> 
> I think Ian suggested you make this an internal function not a public
> API. But if I misunderstand Ian's intention, please make this function
> return libxl error code. The socket number can be returned from an out
> parameter. I.e.
> 
>    int libxl_count_physical_sockets(libxl_ctx *ctx, uint32_t *nr_sockets);
> 
Making it public is the need of supporting LIBXL_PSR_TARGET_ALL in
libxl_psr_cat_set_cbm(), which is suggested by Ian as well. In such
case, both xl/libxl need to get socket_count. Introduce it in libxl so
that both can make use of it.

Except this, I would like to adopt your suggestions(#ifdef to x86 only
and return error code).

Chao

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets
  2015-03-31  8:51     ` Chao Peng
@ 2015-03-31 16:11       ` Ian Campbell
  0 siblings, 0 replies; 26+ messages in thread
From: Ian Campbell @ 2015-03-31 16:11 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, Wei Liu, dgdegra

On Tue, 2015-03-31 at 16:51 +0800, Chao Peng wrote:
> > I think Ian suggested you make this an internal function not a public
> > API. But if I misunderstand Ian's intention, please make this function
> > return libxl error code. The socket number can be returned from an out
> > parameter. I.e.
> > 
> >    int libxl_count_physical_sockets(libxl_ctx *ctx, uint32_t *nr_sockets);
> > 
> Making it public is the need of supporting LIBXL_PSR_TARGET_ALL in
> libxl_psr_cat_set_cbm(), which is suggested by Ian as well. In such
> case, both xl/libxl need to get socket_count. Introduce it in libxl so
> that both can make use of it.

My expectation was that adding LIBXL_PSR_TARGET_ALL would remove the
need for xl to get the socket count, I expected that passing
LIBXL_PSR_TARGET_ALL would cause libxl to do all of that internally.
I've not looked at the next patch to see why that isn't the case though.

Ian.

> 
> Except this, I would like to adopt your suggestions(#ifdef to x86 only
> and return error code).
> 
> Chao

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-03-26 12:38 ` [PATCH v3 8/8] tools: add tools support for Intel CAT Chao Peng
@ 2015-03-31 16:28   ` Ian Campbell
  2015-04-01  7:55     ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2015-03-31 16:28 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Thu, 2015-03-26 at 20:38 +0800, Chao Peng wrote:
> This is the xc/xl changes to support Intel Cache Allocation
> Technology(CAT). Two commands are introduced:
> - xl psr-cat-cbm-set [-s socket] <domain> <cbm>
>   Set cache capacity bitmasks(CBM) for a domain.
> - xl psr-cat-show <domain>
>   Show Cache Allocation Technology information.

Please could you show an example of the output from this one.

> 
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> Changes in v3:
> * Add manpage.
> * libxl_psr_cat_set/get_domain_data => libxl_psr_cat_set/get_cbm.
> * Move libxl_count_physical_sockets into seperate patch.
> * Support LIBXL_PSR_TARGET_ALL for libxl_psr_cat_set_cbm.
> * Clean up the print codes.
> ---
>  docs/man/xl.pod.1             |  31 +++++++++
>  tools/libxc/include/xenctrl.h |  15 +++++
>  tools/libxc/xc_psr.c          |  74 +++++++++++++++++++++
>  tools/libxl/libxl.h           |  20 ++++++
>  tools/libxl/libxl_psr.c       | 126 ++++++++++++++++++++++++++++++++++--
>  tools/libxl/libxl_types.idl   |   5 ++
>  tools/libxl/xl.h              |   4 ++
>  tools/libxl/xl_cmdimpl.c      | 146 ++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/xl_cmdtable.c     |  12 ++++
>  9 files changed, 426 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index b016272..99979a5 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1492,6 +1492,37 @@ monitor types are:
>  
>  =back
>  
> +=head1 CACHE ALLOCATION TECHNOLOGY
> +
> +Intel Broadwell and later server platforms offer capabilities to configure and
> +make use of the Cache Allocation Technology (CAT) mechanisms, which enable more
> +cache resources (i.e. L3 cache) to be made available for high priority
> +applications. In Xen implementation, CAT is used to control cache allocation
> +on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
> +(CBM) for the domain.
> +
> +=over 4
> +
> +=item B<psr-cat-cbm-set> [I<OPTIONS>] [I<domain-id>]
> +
> +Set cache capacity bitmasks(CBM) for a domain.

What is the syntax of these bitmaps, and where do I pass them?

I think there is also a bunch of terminology (CBM, COS) which need
explaining, otherwise no one will know how to use it. Perhaps that
belongs in a separate document or the wiki though?

> diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
> index e367a80..6c670d5 100644
> --- a/tools/libxc/xc_psr.c
> +++ b/tools/libxc/xc_psr.c
> @@ -248,6 +248,80 @@ int xc_psr_cmt_enabled(xc_interface *xch)
>  
>      return 0;
>  }
> +int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
> +                               xc_psr_cat_type type, uint32_t target,
> +                               uint64_t data)
> +{
> +    DECLARE_DOMCTL;
> +    uint32_t cmd;
> +
> +    switch ( type )
> +    {
> +    case XC_PSR_CAT_L3_CBM:
> +        cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
> +        break;
> +    default:
> +        return -1;

You should also set errno to an appropriate value, since calling code
will try and use it to log with.

There were several instances of this I think.

> +    }


> @@ -1513,6 +1520,19 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
>                               uint64_t *tsc_r);
>  #endif
>  
> +#ifdef LIBXL_HAVE_PSR_CAT
> +
> +#define LIBXL_PSR_TARGET_ALL (~0U)
> +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> +                          libxl_psr_cat_type type, uint32_t target,
> +                          uint64_t cbm);
> +int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> +                          libxl_psr_cat_type type, uint32_t target,
> +                          uint64_t *cbm_r);

What are the units of the various cbm*

If they are now more precisely typed (i.e. not the opaque data from last
time) then is the type parameter still needed?

> +int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
> +                              uint32_t *cos_max_r, uint32_t *cbm_len_r);

Is there going to be any user documentation regarding what cos and cbm
are and how to interpret them and set them?

> @@ -247,6 +290,75 @@ out:
>      return rc;
>  }
>  
> +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> +                          libxl_psr_cat_type type, uint32_t target,
> +                          uint64_t cbm)
> +{
> +    GC_INIT(ctx);
> +    int rc;
> +
> +    uint32_t i, nr_sockets;
> +
> +    if (target != LIBXL_PSR_TARGET_ALL) {
> +        rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, target, cbm);
> +        if (rc < 0) {
> +            libxl__psr_cat_log_err_msg(gc, errno);
> +            rc = ERROR_FAIL;
> +        }
> +    } else {
> +        nr_sockets = libxl_count_physical_sockets(ctx);
> +        if (nr_sockets == 0) {
> +            LOGE(ERROR, "failed to get system socket count");
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        for (i = 0; i < nr_sockets; i++) {
> +            rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, i, cbm);
> +            if (rc < 0) {
> +                libxl__psr_cat_log_err_msg(gc, errno);
> +                rc = ERROR_FAIL;

You neither error out nor latch this value, so you only return the
status of the final socket.

I think you probably want to exit on first error. And depending on the
semantics of this stuff you may also need to unwind any work already
done.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index f26a02d..ff963df 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8038,6 +8038,152 @@ int main_psr_cmt_show(int argc, char **argv)
>  }
>  #endif
>  
> +#ifdef LIBXL_HAVE_PSR_CAT
> +static void psr_cat_print_one_domain_cbm(libxl_dominfo *dominfo,
> +                                         uint32_t socket)
> +{
> +    char *domain_name;
> +    uint64_t cbm;
> +
> +    domain_name = libxl_domid_to_name(ctx, dominfo->domid);

AFAICT you use dominfo here for domifo->domid, but you looked it up with
libxl_domain_info(,... domid) so you already had that in your hand.

IOW I think you can just pass the domid to this function, and omit the
call to libxl_domain_info in the callers, or use list->domid in the case
you have that in hand.

> +    printf("%5d%25s", dominfo->domid, domain_name);
> +    free(domain_name);
> +
> +    if (!libxl_psr_cat_get_cbm(ctx, dominfo->domid,
> +                               LIBXL_PSR_CAT_TYPE_L3_CBM, socket, &cbm))
> +         printf("%#16"PRIx64, cbm);
> +
> +    printf("\n");
> +}
> +
> +static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socket)
> +{
> +    int i, nr_domains;
> +
> +    if (domid != INVALID_DOMID) {
> +        libxl_dominfo dominfo;
> +
> +        if (libxl_domain_info(ctx, &dominfo, domid)) {
> +            fprintf(stderr, "Failed to get domain info for %d\n", domid);
> +            return -1;
> +        }
> +        psr_cat_print_one_domain_cbm(&dominfo, socket);
> +    } else {
> +        libxl_dominfo *list;
> +
> +        if (!(list = libxl_list_domain(ctx, &nr_domains))) {
> +            fprintf(stderr, "Failed to get domain info for domain list.\n");

"...for cbm something something."

> +            return -1;
> +        }
> +        for (i = 0; i < nr_domains; i++)
> +            psr_cat_print_one_domain_cbm(list + i, socket);
> +        libxl_dominfo_list_free(list, nr_domains);
> +    }
> +
> +    return 0;
> +}
> +
> +static int psr_cat_print_socket(uint32_t domid, uint32_t socket)
> +{
> +    uint32_t l3_cache_size, cos_max, cbm_len;
> +    int rc;
> +
> +    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
> +    if (rc) {
> +        fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
> +        return -1;
> +    }
> +
> +    rc = libxl_psr_cat_get_l3_info(ctx, socket, &cos_max, &cbm_len);

Would an interface which returns a list with information for all sockets
not be more convenient?

> +    if (rc) {
> +        fprintf(stderr, "Failed to get cat info for socket:%d\n", socket);
> +        return -1;
> +    }
> +
> +    /* Header */
> +    printf("SocketID:\t%u\n", socket);
> +    printf("L3 Cache:\t%uKB\n", l3_cache_size);
> +    printf("Maximum COS:\t%u\n", cos_max);
> +    printf("CBM length:\t%u\n", cbm_len);
> +    printf("Default CBM:\t%#"PRIx64"\n", (1ul << cbm_len) - 1);
> +    printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
> +
> +    return psr_cat_print_domain_cbm(domid, socket);
> +}
> +
> +static int psr_cat_show(uint32_t domid)
> +{
> +    uint32_t socket, nr_sockets;
> +    int rc;
> +
> +    nr_sockets = libxl_count_physical_sockets(ctx);
> +    if (nr_sockets == 0) {
> +        fprintf(stderr, "Failed getting physinfo\n");

That wasn't a physinfo call.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-03-31 16:28   ` Ian Campbell
@ 2015-04-01  7:55     ` Chao Peng
  2015-04-01  8:41       ` Ian Campbell
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-04-01  7:55 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Tue, Mar 31, 2015 at 05:28:54PM +0100, Ian Campbell wrote:
> On Thu, 2015-03-26 at 20:38 +0800, Chao Peng wrote:
> > This is the xc/xl changes to support Intel Cache Allocation
> > Technology(CAT). Two commands are introduced:
> > - xl psr-cat-cbm-set [-s socket] <domain> <cbm>
> >   Set cache capacity bitmasks(CBM) for a domain.
> > - xl psr-cat-show <domain>
> >   Show Cache Allocation Technology information.
> 
> Please could you show an example of the output from this one.

I did put the sample output in the cover letter, but sounds like here is
the right place.

> 
> > 
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > ---
> > +
> > +=item B<psr-cat-cbm-set> [I<OPTIONS>] [I<domain-id>]
> > +
> > +Set cache capacity bitmasks(CBM) for a domain.
> 
> What is the syntax of these bitmaps, and where do I pass them?

[I<cbm>] is missing here.

> 
> I think there is also a bunch of terminology (CBM, COS) which need
> explaining, otherwise no one will know how to use it. Perhaps that
> belongs in a separate document or the wiki though?

Sounds it's worthy to create docs/misc/xl-psr.markdown.

> > +
> > +#define LIBXL_PSR_TARGET_ALL (~0U)
> > +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > +                          libxl_psr_cat_type type, uint32_t target,
> > +                          uint64_t cbm);
> > +int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> > +                          libxl_psr_cat_type type, uint32_t target,
> > +                          uint64_t *cbm_r);
> 
> What are the units of the various cbm*
> 
> If they are now more precisely typed (i.e. not the opaque data from last
> time) then is the type parameter still needed?

'type' is still used because in the future the possible value can be
L3_CODE_CBM/L3_DATA_CBM or even L2_CBM. We want to keep it common.

> 
> > +int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
> > +                              uint32_t *cos_max_r, uint32_t *cbm_len_r);
> 
> Is there going to be any user documentation regarding what cos and cbm
> are and how to interpret them and set them?

So I'd like to create docs/misc/xl-psr.markdown to answer all these
questions.

> 
> > @@ -247,6 +290,75 @@ out:
> >      return rc;
> >  }
> >  
> > +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > +                          libxl_psr_cat_type type, uint32_t target,
> > +                          uint64_t cbm)
> > +{
> > +    GC_INIT(ctx);
> > +    int rc;
> > +
> > +    uint32_t i, nr_sockets;
> > +
> > +    if (target != LIBXL_PSR_TARGET_ALL) {
> > +        rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, target, cbm);
> > +        if (rc < 0) {
> > +            libxl__psr_cat_log_err_msg(gc, errno);
> > +            rc = ERROR_FAIL;
> > +        }
> > +    } else {
> > +        nr_sockets = libxl_count_physical_sockets(ctx);
> > +        if (nr_sockets == 0) {
> > +            LOGE(ERROR, "failed to get system socket count");
> > +            rc = ERROR_FAIL;
> > +            goto out;
> > +        }
> > +        for (i = 0; i < nr_sockets; i++) {
> > +            rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, i, cbm);
> > +            if (rc < 0) {
> > +                libxl__psr_cat_log_err_msg(gc, errno);
> > +                rc = ERROR_FAIL;
> 
> You neither error out nor latch this value, so you only return the
> status of the final socket.
> 
> I think you probably want to exit on first error. And depending on the
> semantics of this stuff you may also need to unwind any work already
> done.

That's right, exit on first error is my initial purpose. 

> 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index f26a02d..ff963df 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -8038,6 +8038,152 @@ int main_psr_cmt_show(int argc, char **argv)
> >  }
> >  #endif
> >  
> > +#ifdef LIBXL_HAVE_PSR_CAT
> > +static void psr_cat_print_one_domain_cbm(libxl_dominfo *dominfo,
> > +                                         uint32_t socket)
> > +{
> > +    char *domain_name;
> > +    uint64_t cbm;
> > +
> > +    domain_name = libxl_domid_to_name(ctx, dominfo->domid);
> 
> AFAICT you use dominfo here for domifo->domid, but you looked it up with
> libxl_domain_info(,... domid) so you already had that in your hand.
> 
> IOW I think you can just pass the domid to this function, and omit the
> call to libxl_domain_info in the callers, or use list->domid in the case
> you have that in hand.

Agreed.

> 
> > +    printf("%5d%25s", dominfo->domid, domain_name);
> > +    free(domain_name);
> > +
> > +    if (!libxl_psr_cat_get_cbm(ctx, dominfo->domid,
> > +                               LIBXL_PSR_CAT_TYPE_L3_CBM, socket, &cbm))
> > +         printf("%#16"PRIx64, cbm);
> > +
> > +    printf("\n");
> > +}
> > +
> > +static int psr_cat_print_socket(uint32_t domid, uint32_t socket)
> > +{
> > +    uint32_t l3_cache_size, cos_max, cbm_len;
> > +    int rc;
> > +
> > +    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
> > +    if (rc) {
> > +        fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
> > +        return -1;
> > +    }
> > +
> > +    rc = libxl_psr_cat_get_l3_info(ctx, socket, &cos_max, &cbm_len);
> 
> Would an interface which returns a list with information for all sockets
> not be more convenient?

If this one returns all sockets but not a specified socket data (which I agreed)
and not consider legacy cmt code, then I think I can make
libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.

Chao

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-04-01  7:55     ` Chao Peng
@ 2015-04-01  8:41       ` Ian Campbell
  2015-04-01  9:06         ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2015-04-01  8:41 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Wed, 2015-04-01 at 15:55 +0800, Chao Peng wrote:
> On Tue, Mar 31, 2015 at 05:28:54PM +0100, Ian Campbell wrote:
> > On Thu, 2015-03-26 at 20:38 +0800, Chao Peng wrote:
> > > This is the xc/xl changes to support Intel Cache Allocation
> > > Technology(CAT). Two commands are introduced:
> > > - xl psr-cat-cbm-set [-s socket] <domain> <cbm>
> > >   Set cache capacity bitmasks(CBM) for a domain.
> > > - xl psr-cat-show <domain>
> > >   Show Cache Allocation Technology information.
> > 
> > Please could you show an example of the output from this one.
> 
> I did put the sample output in the cover letter, but sounds like here is
> the right place.

Sorry, I didn't think to look there at the time.

> > 
> > > 
> > > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > > ---
> > > +
> > > +=item B<psr-cat-cbm-set> [I<OPTIONS>] [I<domain-id>]
> > > +
> > > +Set cache capacity bitmasks(CBM) for a domain.
> > 
> > What is the syntax of these bitmaps, and where do I pass them?
> 
> [I<cbm>] is missing here.
> 
> > 
> > I think there is also a bunch of terminology (CBM, COS) which need
> > explaining, otherwise no one will know how to use it. Perhaps that
> > belongs in a separate document or the wiki though?
> 
> Sounds it's worthy to create docs/misc/xl-psr.markdown.

I think the syntax of I<cbm> needs to be in this document, for sure, but
I suspect that this stuff is complex enough that a more complete
overview in a separate document might be useful too (possibly it should
cover other related things, like the existing stuff like CMT and MBM
too, assuming they are related enough?

> 
> > > +
> > > +#define LIBXL_PSR_TARGET_ALL (~0U)
> > > +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > > +                          libxl_psr_cat_type type, uint32_t target,
> > > +                          uint64_t cbm);
> > > +int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> > > +                          libxl_psr_cat_type type, uint32_t target,
> > > +                          uint64_t *cbm_r);
> > 
> > What are the units of the various cbm*

?

> > 
> > If they are now more precisely typed (i.e. not the opaque data from last
> > time) then is the type parameter still needed?
> 
> 'type' is still used because in the future the possible value can be
> L3_CODE_CBM/L3_DATA_CBM or even L2_CBM. We want to keep it common.

OK. And all future "cat_type" values will all have similar
types/semantics?

> > > +int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
> > > +                              uint32_t *cos_max_r, uint32_t *cbm_len_r);
> > 
> > Is there going to be any user documentation regarding what cos and cbm
> > are and how to interpret them and set them?
> 
> So I'd like to create docs/misc/xl-psr.markdown to answer all these
> questions.

OK.


> > 
> > > +    printf("%5d%25s", dominfo->domid, domain_name);
> > > +    free(domain_name);
> > > +
> > > +    if (!libxl_psr_cat_get_cbm(ctx, dominfo->domid,
> > > +                               LIBXL_PSR_CAT_TYPE_L3_CBM, socket, &cbm))
> > > +         printf("%#16"PRIx64, cbm);
> > > +
> > > +    printf("\n");
> > > +}
> > > +
> > > +static int psr_cat_print_socket(uint32_t domid, uint32_t socket)
> > > +{
> > > +    uint32_t l3_cache_size, cos_max, cbm_len;
> > > +    int rc;
> > > +
> > > +    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
> > > +    if (rc) {
> > > +        fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
> > > +        return -1;
> > > +    }
> > > +
> > > +    rc = libxl_psr_cat_get_l3_info(ctx, socket, &cos_max, &cbm_len);
> > 
> > Would an interface which returns a list with information for all sockets
> > not be more convenient?
> 
> If this one returns all sockets but not a specified socket data (which I agreed)
> and not consider legacy cmt code, then I think I can make
> libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.

What is the legacy cmt code? But otherwise I agree, yes.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-04-01  8:41       ` Ian Campbell
@ 2015-04-01  9:06         ` Chao Peng
  2015-04-01  9:23           ` Ian Campbell
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-04-01  9:06 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Wed, Apr 01, 2015 at 09:41:06AM +0100, Ian Campbell wrote:
> On Wed, 2015-04-01 at 15:55 +0800, Chao Peng wrote:
> > On Tue, Mar 31, 2015 at 05:28:54PM +0100, Ian Campbell wrote:
> > > On Thu, 2015-03-26 at 20:38 +0800, Chao Peng wrote:
> > > > This is the xc/xl changes to support Intel Cache Allocation
> > > > Technology(CAT). Two commands are introduced:
> > > > - xl psr-cat-cbm-set [-s socket] <domain> <cbm>
> > > >   Set cache capacity bitmasks(CBM) for a domain.
> > > > - xl psr-cat-show <domain>
> > > >   Show Cache Allocation Technology information.
> > > 
> > > Please could you show an example of the output from this one.
> > 
> > I did put the sample output in the cover letter, but sounds like here is
> > the right place.
> 
> Sorry, I didn't think to look there at the time.
> 
> > > 
> > > > 
> > > > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > > > ---
> > > > +
> > > > +=item B<psr-cat-cbm-set> [I<OPTIONS>] [I<domain-id>]
> > > > +
> > > > +Set cache capacity bitmasks(CBM) for a domain.
> > > 
> > > What is the syntax of these bitmaps, and where do I pass them?
> > 
> > [I<cbm>] is missing here.
> > 
> > > 
> > > I think there is also a bunch of terminology (CBM, COS) which need
> > > explaining, otherwise no one will know how to use it. Perhaps that
> > > belongs in a separate document or the wiki though?
> > 
> > Sounds it's worthy to create docs/misc/xl-psr.markdown.
> 
> I think the syntax of I<cbm> needs to be in this document, for sure, but
> I suspect that this stuff is complex enough that a more complete
> overview in a separate document might be useful too (possibly it should
> cover other related things, like the existing stuff like CMT and MBM
> too, assuming they are related enough?

Yes, they are also in the plan.

> 
> > 
> > > > +
> > > > +#define LIBXL_PSR_TARGET_ALL (~0U)
> > > > +int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
> > > > +                          libxl_psr_cat_type type, uint32_t target,
> > > > +                          uint64_t cbm);
> > > > +int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
> > > > +                          libxl_psr_cat_type type, uint32_t target,
> > > > +                          uint64_t *cbm_r);
> > > 
> > > What are the units of the various cbm*
> 
> ?
> 
> > > 
> > > If they are now more precisely typed (i.e. not the opaque data from last
> > > time) then is the type parameter still needed?
> > 
> > 'type' is still used because in the future the possible value can be
> > L3_CODE_CBM/L3_DATA_CBM or even L2_CBM. We want to keep it common.
> 
> OK. And all future "cat_type" values will all have similar
> types/semantics?

Yes, they will. 

> 
> > > > +int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
> > > > +                              uint32_t *cos_max_r, uint32_t *cbm_len_r);
> > > 
> > > Is there going to be any user documentation regarding what cos and cbm
> > > are and how to interpret them and set them?
> > 
> > So I'd like to create docs/misc/xl-psr.markdown to answer all these
> > questions.
> 
> OK.
> 
> 
> > > 
> > > > +    printf("%5d%25s", dominfo->domid, domain_name);
> > > > +    free(domain_name);
> > > > +
> > > > +    if (!libxl_psr_cat_get_cbm(ctx, dominfo->domid,
> > > > +                               LIBXL_PSR_CAT_TYPE_L3_CBM, socket, &cbm))
> > > > +         printf("%#16"PRIx64, cbm);
> > > > +
> > > > +    printf("\n");
> > > > +}
> > > > +
> > > > +static int psr_cat_print_socket(uint32_t domid, uint32_t socket)
> > > > +{
> > > > +    uint32_t l3_cache_size, cos_max, cbm_len;
> > > > +    int rc;
> > > > +
> > > > +    rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
> > > > +    if (rc) {
> > > > +        fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
> > > > +        return -1;
> > > > +    }
> > > > +
> > > > +    rc = libxl_psr_cat_get_l3_info(ctx, socket, &cos_max, &cbm_len);
> > > 
> > > Would an interface which returns a list with information for all sockets
> > > not be more convenient?
> > 
> > If this one returns all sockets but not a specified socket data (which I agreed)
> > and not consider legacy cmt code, then I think I can make
> > libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.
> 
> What is the legacy cmt code? But otherwise I agree, yes.

In libxl/xl_cmdimpl.c, psr_cmt_show also calculates the socket count
itself. If we want to refactor it with new libxl_count_physical_sockets
then libxl_count_physical_sockets should be public, otherwise it can be
private to libxl_psr.c only. From my side, both directions sound OK.

Chao

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-04-01  9:06         ` Chao Peng
@ 2015-04-01  9:23           ` Ian Campbell
  2015-04-02  3:15             ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Ian Campbell @ 2015-04-01  9:23 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Wed, 2015-04-01 at 17:06 +0800, Chao Peng wrote:
> > > If this one returns all sockets but not a specified socket data (which I agreed)
> > > and not consider legacy cmt code, then I think I can make
> > > libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.
> > 
> > What is the legacy cmt code? But otherwise I agree, yes.
> 
> In libxl/xl_cmdimpl.c, psr_cmt_show also calculates the socket count
> itself. If we want to refactor it with new libxl_count_physical_sockets
> then libxl_count_physical_sockets should be public, otherwise it can be
> private to libxl_psr.c only. From my side, both directions sound OK.

Ah, so we would want a "return a list of all sockets" variant of
libxl_psr_cmt_get_cache_occupancy too? I think that's fine, we need to
keep the old interface but we could easily add a new one, e.g.
libxl_psr_cmt_get_all_cache_occupancy (insert the word "sockets" if you
like).

So both interfaces would be something like:
   int libxl_psr_....(ctx, domid, TYPE **list_r, int *nr);

And on success *list_r points to a newly allocated array and *nr is the
number of elements in that array. TYPE depends on which op it is, so for
cache_occupancy it seems a uint32_t.

Is the socket address space always contiguous and starting at zero? If
not then the array might need to contain (socket,TYPE) structs.

Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-04-01  9:23           ` Ian Campbell
@ 2015-04-02  3:15             ` Chao Peng
  2015-04-07  8:57               ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-04-02  3:15 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Wed, Apr 01, 2015 at 10:23:01AM +0100, Ian Campbell wrote:
> On Wed, 2015-04-01 at 17:06 +0800, Chao Peng wrote:
> > > > If this one returns all sockets but not a specified socket data (which I agreed)
> > > > and not consider legacy cmt code, then I think I can make
> > > > libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.
> > > 
> > > What is the legacy cmt code? But otherwise I agree, yes.
> > 
> > In libxl/xl_cmdimpl.c, psr_cmt_show also calculates the socket count
> > itself. If we want to refactor it with new libxl_count_physical_sockets
> > then libxl_count_physical_sockets should be public, otherwise it can be
> > private to libxl_psr.c only. From my side, both directions sound OK.
> 
> Ah, so we would want a "return a list of all sockets" variant of
> libxl_psr_cmt_get_cache_occupancy too? I think that's fine, we need to
> keep the old interface but we could easily add a new one, e.g.
> libxl_psr_cmt_get_all_cache_occupancy (insert the word "sockets" if you
> like).

The libxl_psr_cmt_get_cache_occupancy is actually not used in xl anymore
and should be considered as obsolete. Substitute is
libxl_psr_cmt_get_sample which is the common API for both cache
occupancy and memory bandwidth(For the latter the routine will be called
twice). So if we really want to do everything batch in libxl then
perhaps we should do that for libxl_psr_cmt_get_sample.

The other candidate is libxl_psr_cmt_get_l3_cache_size, which can also
do batch in libxl. 

While there is also drawback: If both
libxl_psr_cmt_get_sample/libxl_psr_cmt_get_l3_cache_size do their
batches in libxl, then libxl_count_physical_sockets will be called twice
in libxl which may bring performance drop.

Another solution is only do batch for libxl_psr_cmt_get_l3_cache_size.
It returns all sockets's cache size and meantime the socket number. Once
we get socket number, other functions(Both libxl_psr_cmt_get_sample for
CMT/MBM and libxl_psr_cat_get_l3_info/libxl_psr_cat_get_cbm for CAT)
can be called socket by socket in xl.

Chao
> 
> So both interfaces would be something like:
>    int libxl_psr_....(ctx, domid, TYPE **list_r, int *nr);
> 
> And on success *list_r points to a newly allocated array and *nr is the
> number of elements in that array. TYPE depends on which op it is, so for
> cache_occupancy it seems a uint32_t.
> 
> Is the socket address space always contiguous and starting at zero? If
> not then the array might need to contain (socket,TYPE) structs.
> 
> Ian.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 8/8] tools: add tools support for Intel CAT
  2015-04-02  3:15             ` Chao Peng
@ 2015-04-07  8:57               ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-04-07  8:57 UTC (permalink / raw)
  To: Ian Campbell
  Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
	will.auld, JBeulich, wei.liu2, dgdegra

On Thu, Apr 02, 2015 at 11:15:53AM +0800, Chao Peng wrote:
> On Wed, Apr 01, 2015 at 10:23:01AM +0100, Ian Campbell wrote:
> > On Wed, 2015-04-01 at 17:06 +0800, Chao Peng wrote:
> > > > > If this one returns all sockets but not a specified socket data (which I agreed)
> > > > > and not consider legacy cmt code, then I think I can make
> > > > > libxl_count_physical_sockets() private and move it to libxl/libxl_psr.c.
> > > > 
> > > > What is the legacy cmt code? But otherwise I agree, yes.
> > > 
> > > In libxl/xl_cmdimpl.c, psr_cmt_show also calculates the socket count
> > > itself. If we want to refactor it with new libxl_count_physical_sockets
> > > then libxl_count_physical_sockets should be public, otherwise it can be
> > > private to libxl_psr.c only. From my side, both directions sound OK.
> > 
> > Ah, so we would want a "return a list of all sockets" variant of
> > libxl_psr_cmt_get_cache_occupancy too? I think that's fine, we need to
> > keep the old interface but we could easily add a new one, e.g.
> > libxl_psr_cmt_get_all_cache_occupancy (insert the word "sockets" if you
> > like).
> 
> The libxl_psr_cmt_get_cache_occupancy is actually not used in xl anymore
> and should be considered as obsolete. Substitute is
> libxl_psr_cmt_get_sample which is the common API for both cache
> occupancy and memory bandwidth(For the latter the routine will be called
> twice). So if we really want to do everything batch in libxl then
> perhaps we should do that for libxl_psr_cmt_get_sample.
> 
> The other candidate is libxl_psr_cmt_get_l3_cache_size, which can also
> do batch in libxl. 
> 
> While there is also drawback: If both
> libxl_psr_cmt_get_sample/libxl_psr_cmt_get_l3_cache_size do their
> batches in libxl, then libxl_count_physical_sockets will be called twice
> in libxl which may bring performance drop.
> 
> Another solution is only do batch for libxl_psr_cmt_get_l3_cache_size.
> It returns all sockets's cache size and meantime the socket number. Once
> we get socket number, other functions(Both libxl_psr_cmt_get_sample for
> CMT/MBM and libxl_psr_cat_get_l3_info/libxl_psr_cat_get_cbm for CAT)
> can be called socket by socket in xl.

Ian, does this solution sounds OK to you?
Chao
> > 
> > So both interfaces would be something like:
> >    int libxl_psr_....(ctx, domid, TYPE **list_r, int *nr);
> > 
> > And on success *list_r points to a newly allocated array and *nr is the
> > number of elements in that array. TYPE depends on which op it is, so for
> > cache_occupancy it seems a uint32_t.
> > 
> > Is the socket address space always contiguous and starting at zero? If
> > not then the array might need to contain (socket,TYPE) structs.
> > 
> > Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/8] x86: detect and initialize Intel CAT feature
  2015-03-27 18:31   ` Andrew Cooper
@ 2015-04-13 10:51     ` Jan Beulich
  2015-04-13 10:58       ` Andrew Cooper
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-04-13 10:51 UTC (permalink / raw)
  To: Andrew Cooper, Chao Peng, xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, keir, dgdegra

>>> On 27.03.15 at 19:31, <andrew.cooper3@citrix.com> wrote:
> On 26/03/15 12:38, Chao Peng wrote:
>> +    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
>> +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
>> +    {
>> +        cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
>> +        info->cbm_len = (eax & 0x1f) + 1;
>> +        info->cos_max = (edx & 0xffff);
>> +
>> +        info->enabled = 1;
>> +        printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>> +               socket, info->cos_max, info->cbm_len);
> 
> I would bump this to INFO, at least for socket 0.  Similar to CMT, a
> user which has gone to the effort of enabling CAT will want some
> indication in the boot log without also having to bump the logging level.

Which INFO wouldn't be sufficient for (at least not with the xen.org
defaults, not sure about XenServer's).

Jan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3 3/8] x86: detect and initialize Intel CAT feature
  2015-04-13 10:51     ` Jan Beulich
@ 2015-04-13 10:58       ` Andrew Cooper
  0 siblings, 0 replies; 26+ messages in thread
From: Andrew Cooper @ 2015-04-13 10:58 UTC (permalink / raw)
  To: Jan Beulich, Chao Peng, xen-devel
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, Ian.Jackson,
	will.auld, keir, dgdegra

On 13/04/15 11:51, Jan Beulich wrote:
>>>> On 27.03.15 at 19:31, <andrew.cooper3@citrix.com> wrote:
>> On 26/03/15 12:38, Chao Peng wrote:
>>> +    cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
>>> +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
>>> +    {
>>> +        cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
>>> +        info->cbm_len = (eax & 0x1f) + 1;
>>> +        info->cos_max = (edx & 0xffff);
>>> +
>>> +        info->enabled = 1;
>>> +        printk(XENLOG_DEBUG "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
>>> +               socket, info->cos_max, info->cbm_len);
>> I would bump this to INFO, at least for socket 0.  Similar to CMT, a
>> user which has gone to the effort of enabling CAT will want some
>> indication in the boot log without also having to bump the logging level.
> Which INFO wouldn't be sufficient for (at least not with the xen.org
> defaults, not sure about XenServer's).

Huh - you are right.  XenServer doesn't patch this, but I spend most of
my time using debug builds so I tend not to notice.

~Andrew

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-04-13 10:58 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-26 12:38 [PATCH v3 0/8] enable Cache Allocation Technology (CAT) for VMs Chao Peng
2015-03-26 12:38 ` [PATCH v3 1/8] x86: clean up psr boot parameter parsing Chao Peng
2015-03-26 20:42   ` Andrew Cooper
2015-03-26 12:38 ` [PATCH v3 2/8] x86: improve psr scheduling code Chao Peng
2015-03-27 18:15   ` Andrew Cooper
2015-03-26 12:38 ` [PATCH v3 3/8] x86: detect and initialize Intel CAT feature Chao Peng
2015-03-27 18:31   ` Andrew Cooper
2015-04-13 10:51     ` Jan Beulich
2015-04-13 10:58       ` Andrew Cooper
2015-03-26 12:38 ` [PATCH v3 4/8] x86: add support for COS/CBM manangement Chao Peng
2015-03-27 20:16   ` Andrew Cooper
2015-03-31  8:40     ` Chao Peng
2015-03-26 12:38 ` [PATCH v3 5/8] x86: add scheduling support for Intel CAT Chao Peng
2015-03-26 12:38 ` [PATCH v3 6/8] xsm: add CAT related xsm policies Chao Peng
2015-03-26 12:38 ` [PATCH v3 7/8] tools/libxl: introduce libxl_count_physical_sockets Chao Peng
2015-03-30 14:51   ` Wei Liu
2015-03-31  8:51     ` Chao Peng
2015-03-31 16:11       ` Ian Campbell
2015-03-26 12:38 ` [PATCH v3 8/8] tools: add tools support for Intel CAT Chao Peng
2015-03-31 16:28   ` Ian Campbell
2015-04-01  7:55     ` Chao Peng
2015-04-01  8:41       ` Ian Campbell
2015-04-01  9:06         ` Chao Peng
2015-04-01  9:23           ` Ian Campbell
2015-04-02  3:15             ` Chao Peng
2015-04-07  8:57               ` Chao Peng

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.