All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs
@ 2015-03-13 10:13 Chao Peng
  2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 UTC (permalink / raw)
  To: xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2, dgdegra

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].

[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 (6):
  x86: detect and initialize Intel CAT feature
  x86: add support for COS/CBM manangement
  X86: improve psr scheduling code
  x86: add scheduling support for Intel CAT
  xsm: add CAT related xsm policies
  tools: add tools support for Intel CAT

 docs/misc/xen-command-line.markdown          |  15 +-
 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                          |  18 ++
 tools/libxl/libxl_psr.c                      | 106 ++++++-
 tools/libxl/libxl_types.idl                  |   4 +
 tools/libxl/xl.h                             |   4 +
 tools/libxl/xl_cmdimpl.c                     | 225 +++++++++++++-
 tools/libxl/xl_cmdtable.c                    |  13 +
 xen/arch/x86/domain.c                        |  13 +-
 xen/arch/x86/domctl.c                        |  18 ++
 xen/arch/x86/psr.c                           | 436 +++++++++++++++++++++++++--
 xen/arch/x86/sysctl.c                        |  20 ++
 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                    |  11 +-
 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 +
 23 files changed, 977 insertions(+), 48 deletions(-)

-- 
1.9.1

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

* [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  2015-03-13 13:40   ` Konrad Rzeszutek Wilk
  2015-03-16 13:47   ` Jan Beulich
  2015-03-13 10:13 ` [PATCH 2/6] x86: add support for COS/CBM manangement Chao Peng
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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>
---
 docs/misc/xen-command-line.markdown |  15 +++-
 xen/arch/x86/psr.c                  | 151 +++++++++++++++++++++++++++++++++---
 xen/include/asm-x86/cpufeature.h    |   1 +
 3 files changed, 155 insertions(+), 12 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 63871cb..a4b36e6 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1090,9 +1090,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> | socket_num:<interger> )`
 
-> Default: `psr=cmt:0,rmid_max:255`
+> Default: `psr=cmt:0,rmid_max:255,cat:0,socket_num:0 (Detect at boot time)`
 
 Platform Shared Resource(PSR) Services.  Intel Haswell and later server
 platforms offer information about the sharing of resources.
@@ -1102,6 +1102,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
@@ -1112,6 +1117,12 @@ 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.
+  * `socket_num` indicates socket number. Detecte automatically at boot time
+    if not specified(0). Useful for CPU hot-plug case.
+
 ### reboot
 > `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
 
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 2ef83df..df557e8 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -19,6 +19,14 @@
 #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;
@@ -26,8 +34,11 @@ struct psr_assoc {
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
-static bool_t __initdata opt_psr;
+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 opt_socket_num;
 static uint64_t rmid_mask;
 static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
 
@@ -58,15 +69,31 @@ static void __init parse_psr_param(char *s)
                                     val_str);
             }
         }
+        else if ( !strcmp(s, "cat") )
+        {
+            if ( !val_str )
+                opt_psr |= PSR_CAT;
+            else
+            {
+                int val_int = parse_bool(val_str);
+                if ( val_int == 1 )
+                    opt_psr |= PSR_CAT;
+                else if ( val_int != 0 )
+                    printk("PSR: unknown cat value: %s - CAT disabled!\n",
+                                    val_str);
+            }
+        }
         else if ( val_str && !strcmp(s, "rmid_max") )
             opt_rmid_max = simple_strtoul(val_str, NULL, 0);
+        else if ( val_str && !strcmp(s, "socket_num") )
+            opt_socket_num = simple_strtoul(val_str, NULL, 0);
 
         s = ss + 1;
     } while ( ss );
 }
 custom_param("psr", parse_psr_param);
 
-static void __init init_psr_cmt(unsigned int rmid_max)
+static void __init psr_cmt_init(unsigned int rmid_max)
 {
     unsigned int eax, ebx, ecx, edx;
     unsigned int rmid;
@@ -115,14 +142,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)
 {
@@ -189,6 +208,118 @@ void psr_assoc_rmid(unsigned int rmid)
     }
 }
 
+static void do_cat_cpu_init(void* data)
+{
+    unsigned int eax, ebx, ecx, edx;
+    struct psr_cat_socket_info *info;
+
+    cpuid_count(0x10, 0, &eax, &ebx, &ecx, &edx);
+    if ( ebx & PSR_RESOURCE_TYPE_L3 )
+    {
+        info = data;
+
+        cpuid_count(0x10, 1, &eax, &ebx, &ecx, &edx);
+        info->cbm_len = (eax & 0x1f) + 1;
+        info->cos_max = (edx & 0xffff);
+
+        info->enabled = 1;
+        printk(XENLOG_INFO "CAT: enabled on socket %d, cos_max:%d, cbm_len:%d\n",
+               (int)(info - cat_socket_info), info->cos_max, info->cbm_len);
+    }
+}
+
+static void cat_cpu_init(unsigned int cpu)
+{
+    struct psr_cat_socket_info *info;
+    unsigned int socket;
+    const struct cpuinfo_x86 *c;
+
+    socket = cpu_to_socket(cpu);
+    if ( socket >= opt_socket_num )
+    {
+        printk(XENLOG_WARNING "CAT: disabled on socket %d because socket_num is %d\n",
+               socket, opt_socket_num);
+        return;
+    }
+
+    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;
+
+    if ( cpu == smp_processor_id() )
+        do_cat_cpu_init(info);
+    else
+        on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, info, 0);
+}
+
+static int cpu_callback(
+    struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+    unsigned int cpu = (unsigned long)hcpu;
+
+    switch ( action )
+    {
+    case CPU_ONLINE:
+        (void)cat_cpu_init(cpu);
+        break;
+    default:
+        break;
+    }
+
+    return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_nfb = {
+    .notifier_call = cpu_callback
+};
+
+static unsigned int get_max_socket(void)
+{
+    unsigned int cpu, max_apicid = boot_cpu_physical_apicid;
+
+    for_each_present_cpu(cpu)
+        if (max_apicid < x86_cpu_to_apicid[cpu])
+            max_apicid = x86_cpu_to_apicid[cpu];
+
+    return apicid_to_socket(max_apicid);
+}
+
+static void __init psr_cat_init(void)
+{
+    if ( opt_socket_num == 0 )
+        opt_socket_num = get_max_socket() + 1;
+    else if ( opt_socket_num > NR_CPUS )
+         printk(XENLOG_WARNING "socket_num > NR_CPUS(%d), set to NR_CPUS.\n",
+                NR_CPUS);
+
+    BUG_ON(opt_socket_num == 0);
+
+    cat_socket_info = xzalloc_array(struct psr_cat_socket_info, opt_socket_num);
+    if ( !cat_socket_info )
+        return;
+
+    cat_cpu_init(smp_processor_id());
+    register_cpu_notifier(&cpu_nfb);
+}
+
+static int __init psr_presmp_init(void)
+{
+    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
+        psr_cmt_init(opt_rmid_max);
+
+    if ( opt_psr & PSR_CAT )
+        psr_cat_init();
+
+    return 0;
+}
+presmp_initcall(psr_presmp_init);
+
 /*
  * Local variables:
  * mode: C
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 */
-- 
1.9.1

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

* [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
  2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  2015-03-13 13:53   ` Konrad Rzeszutek Wilk
  2015-03-16 17:10   ` Jan Beulich
  2015-03-13 10:13 ` [PATCH 3/6] X86: improve psr scheduling code Chao Peng
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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>
---
 xen/arch/x86/domain.c           |   6 +-
 xen/arch/x86/domctl.c           |  18 ++++
 xen/arch/x86/psr.c              | 211 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/sysctl.c           |  20 ++++
 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, 295 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 21f0766..e3b0724 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -615,6 +615,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);
@@ -633,6 +636,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;
 }
 
@@ -656,7 +660,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 a1c5db0..a3864f3 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1324,6 +1324,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 = -ENOSYS;
+            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 df557e8..4325eaf 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -21,11 +21,17 @@
 #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;
 };
 
 struct psr_assoc {
@@ -208,10 +214,204 @@ void psr_assoc_rmid(unsigned int rmid)
     }
 }
 
+static int get_cat_socket_info(unsigned int socket,
+                               struct psr_cat_socket_info **info)
+{
+    if ( !cat_socket_info )
+        return -ENODEV;
+
+    if ( socket >= opt_socket_num )
+        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 ( find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
+        return 0;
+
+    return 1;
+}
+
+static unsigned int get_socket_cpu(unsigned int socket)
+{
+    unsigned int cpu;
+
+    for_each_online_cpu ( cpu )
+       if ( cpu_data[cpu].phys_proc_id == socket )
+           return cpu;
+    return nr_cpu_ids;
+}
+
+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 void 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 = cpumask_check(get_socket_cpu(socket));
+        on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
+    }
+}
+
+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 no found, then keep this 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 refer 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 )
+    {
+        find->cbm = cbm;
+        write_l3_cbm(socket, cos, cbm);
+    }
+    find->ref++;
+    map[old_cos].ref--;
+    d->arch.psr_cos_ids[socket] = cos;
+    return 0;
+}
+
+static void psr_free_cos(struct domain *d)
+{
+    unsigned int socket;
+    unsigned int cos;
+    struct psr_cat_cbm *map;
+
+    if( !d->arch.psr_cos_ids )
+        return;
+
+    for ( socket = 0; socket < opt_socket_num; socket++)
+    {
+        cos = d->arch.psr_cos_ids[socket];
+        if ( cos == 0 )
+            continue;
+
+        map = cat_socket_info[socket].cos_cbm_map;
+        if ( map )
+            map[cos].ref--;
+    }
+
+    xfree(d->arch.psr_cos_ids);
+}
+
+int psr_domain_init(struct domain *d)
+{
+    if ( cat_socket_info )
+    {
+        d->arch.psr_cos_ids = xzalloc_array(unsigned int, opt_socket_num);
+        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 do_cat_cpu_init(void* data)
 {
     unsigned int eax, ebx, ecx, edx;
     struct psr_cat_socket_info *info;
+    unsigned int cos;
 
     cpuid_count(0x10, 0, &eax, &ebx, &ecx, &edx);
     if ( ebx & PSR_RESOURCE_TYPE_L3 )
@@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
         info->cbm_len = (eax & 0x1f) + 1;
         info->cos_max = (edx & 0xffff);
 
+        info->cos_cbm_map = xmalloc_array(struct psr_cat_cbm,
+                                          info->cos_max + 1UL);
+        if ( !info->cos_cbm_map)
+            return;
+
+        for ( cos = 0; cos <= info->cos_max; cos++ )
+            info->cos_cbm_map[cos].ref = 0;
+
+        /* 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_INFO "CAT: enabled on socket %d, cos_max:%d, cbm_len:%d\n",
                (int)(info - cat_socket_info), info->cos_max, info->cbm_len);
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 611a291..096dc31 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -171,6 +171,26 @@ 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 )
+                break;
+
+            if ( __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 c6076e9..999a4c6 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -48,6 +48,14 @@ int psr_alloc_rmid(struct domain *d);
 void psr_free_rmid(struct domain *d);
 void psr_assoc_rmid(unsigned int rmid);
 
+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..7c7baed 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 or cpu 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..9d8ed10 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 or cpu 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 3/6] X86: improve psr scheduling code
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
  2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
  2015-03-13 10:13 ` [PATCH 2/6] x86: add support for COS/CBM manangement Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  2015-03-16 16:53   ` Jan Beulich
  2015-03-13 10:13 ` [PATCH 4/6] x86: add scheduling support for Intel CAT Chao Peng
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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 so just switch
it as it does.

Introduce a generic routine for this so that other psr features can fit.

Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
 xen/arch/x86/domain.c     | 7 ++-----
 xen/arch/x86/psr.c        | 6 ++++++
 xen/include/asm-x86/psr.h | 3 ++-
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index e3b0724..c9be83b 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1447,8 +1447,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);
     }
 
@@ -1473,11 +1471,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 4325eaf..8c96c1b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -407,6 +407,12 @@ void psr_domain_free(struct domain *d)
     psr_free_cos(d);
 }
 
+void psr_ctxt_switch_to(struct domain *d)
+{
+    if ( psr_cmt_enabled() )
+        psr_assoc_rmid(d->arch.psr_rmid);
+}
+
 static void do_cat_cpu_init(void* data)
 {
     unsigned int eax, ebx, ecx, edx;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 999a4c6..212ba30 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -46,7 +46,6 @@ 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);
 
 int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
                         uint32_t *cos_max);
@@ -56,6 +55,8 @@ 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);
 
+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 4/6] x86: add scheduling support for Intel CAT
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (2 preceding siblings ...)
  2015-03-13 10:13 ` [PATCH 3/6] X86: improve psr scheduling code Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  2015-03-17  9:19   ` Jan Beulich
  2015-03-13 10:13 ` [PATCH 5/6] xsm: add CAT related xsm policies Chao Peng
  2015-03-13 10:13 ` [PATCH 6/6] tools: add tools support for Intel CAT Chao Peng
  5 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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. Both CMT and CAT
writes IA32_PQR_ASSOC, so integrate them into one routine.

For performance reason, IA32_PQR_ASSOC is updated lazily based on the change
of RMID and COS. 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>
---
 xen/arch/x86/psr.c | 90 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 68 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 8c96c1b..4e50106 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;
     bool_t initialized;
+    unsigned int socket;
+    uint64_t cos_mask;
 };
 
 struct psr_cmt *__read_mostly psr_cmt;
@@ -193,27 +195,6 @@ void psr_free_rmid(struct domain *d)
     d->arch.psr_rmid = 0;
 }
 
-void psr_assoc_rmid(unsigned int rmid)
-{
-    uint64_t val;
-    uint64_t new_val;
-    struct psr_assoc *psra = &this_cpu(psr_assoc);
-
-    if ( !psra->initialized )
-    {
-        rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
-        psra->initialized = 1;
-    }
-    val = psra->val;
-
-    new_val = (val & ~rmid_mask) | (rmid & rmid_mask);
-    if ( val != new_val )
-    {
-        wrmsrl(MSR_IA32_PSR_ASSOC, new_val);
-        psra->val = new_val;
-    }
-}
-
 static int get_cat_socket_info(unsigned int socket,
                                struct psr_cat_socket_info **info)
 {
@@ -407,10 +388,75 @@ void psr_domain_free(struct domain *d)
     psr_free_cos(d);
 }
 
+static void psr_assoc_init(struct psr_assoc *psra)
+{
+    unsigned int socket;
+    struct psr_cat_socket_info *info;
+
+    socket = cpu_to_socket(smp_processor_id());
+    psra->socket = socket;
+    if ( cat_socket_info && socket < opt_socket_num )
+    {
+        info = cat_socket_info + socket;
+        if ( info->enabled )
+            psra->cos_mask = (~(~0ull << (get_count_order(info->cos_max)
+                             + 32))) & (~0ull << 32);
+    }
+
+    if ( psr_cmt_enabled() || psra->cos_mask )
+        rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
+
+    psra->initialized = 1;
+}
+
+static inline void psr_assoc_reg_read(struct psr_assoc *psra, uint64_t *reg)
+{
+    if ( !psra->initialized )
+        psr_assoc_init(psra);
+
+    *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;
+    }
+}
+
+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;
+    struct psr_assoc *psra = &this_cpu(psr_assoc);
+
+    psr_assoc_reg_read(psra, &reg);
+
     if ( psr_cmt_enabled() )
-        psr_assoc_rmid(d->arch.psr_rmid);
+        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);
 }
 
 static void do_cat_cpu_init(void* data)
-- 
1.9.1

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

* [PATCH 5/6] xsm: add CAT related xsm policies
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (3 preceding siblings ...)
  2015-03-13 10:13 ` [PATCH 4/6] x86: add scheduling support for Intel CAT Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  2015-03-13 16:45   ` Daniel De Graaf
  2015-03-13 10:13 ` [PATCH 6/6] tools: add tools support for Intel CAT Chao Peng
  5 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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>
---
 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 65094bb..12a3c61 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 6/6] tools: add tools support for Intel CAT
  2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
                   ` (4 preceding siblings ...)
  2015-03-13 10:13 ` [PATCH 5/6] xsm: add CAT related xsm policies Chao Peng
@ 2015-03-13 10:13 ` Chao Peng
  5 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-13 10:13 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>
---
 tools/libxc/include/xenctrl.h |  15 +++
 tools/libxc/xc_psr.c          |  74 ++++++++++++++
 tools/libxl/libxl.h           |  18 ++++
 tools/libxl/libxl_psr.c       | 106 ++++++++++++++++++--
 tools/libxl/libxl_types.idl   |   4 +
 tools/libxl/xl.h              |   4 +
 tools/libxl/xl_cmdimpl.c      | 225 ++++++++++++++++++++++++++++++++++++++++--
 tools/libxl/xl_cmdtable.c     |  13 +++
 8 files changed, 444 insertions(+), 15 deletions(-)

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 6bbc52d..dd2c28f 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;
@@ -1501,6 +1508,17 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
                              uint64_t *tsc_r);
 #endif
 
+#ifdef LIBXL_HAVE_PSR_CAT
+int libxl_psr_cat_set_domain_data(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_psr_cat_type type, uint32_t target,
+                                  uint64_t data);
+int libxl_psr_cat_get_domain_data(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_psr_cat_type type, uint32_t target,
+                                  uint64_t *data_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..645f9f1 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -19,7 +19,7 @@
 
 #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;
 
@@ -27,6 +27,28 @@ static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
     case ENOSYS:
         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 +61,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 +289,56 @@ out:
     return rc;
 }
 
+int libxl_psr_cat_set_domain_data(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_psr_cat_type type, uint32_t target,
+                                  uint64_t data)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = xc_psr_cat_set_domain_data(ctx->xch, domid, type, target, data);
+    if (rc < 0) {
+        libxl__psr_cat_log_err_msg(gc, errno);
+        rc = ERROR_FAIL;
+    }
+
+    GC_FREE;
+    return rc;
+}
+
+int libxl_psr_cat_get_domain_data(libxl_ctx *ctx, uint32_t domid,
+                                  libxl_psr_cat_type type, uint32_t target,
+                                  uint64_t *data_r)
+{
+    GC_INIT(ctx);
+    int rc;
+
+    rc = xc_psr_cat_get_domain_data(ctx->xch, domid, type, target, data_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..228f37c 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -699,3 +699,7 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
     (2, "TOTAL_MEM_COUNT"),
     (3, "LOCAL_MEM_COUNT"),
     ])
+
+libxl_psr_cat_type = Enumeration("psr_cat_type", [
+    (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 5c40e84..fb0a9a4 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -85,6 +85,7 @@ libxl_ctx *ctx;
 
 xlchild children[child_max];
 
+#define ALL_SOCKETS ~0
 #define INVALID_DOMID ~0
 static const char *common_domname;
 static int fd_lock = -1;
@@ -7824,6 +7825,23 @@ out:
     return ret;
 }
 
+static uint32_t get_phy_socket_num(void)
+{
+    int rc;
+    uint32_t nr_sockets;
+    libxl_physinfo info;
+    libxl_physinfo_init(&info);
+    rc = libxl_get_physinfo(ctx, &info);
+    if (rc < 0) {
+        libxl_physinfo_dispose(&info);
+        return 0;
+    }
+    nr_sockets = info.nr_cpus / info.threads_per_core / info.cores_per_socket;
+    libxl_physinfo_dispose(&info);
+    return nr_sockets;
+
+}
+
 #ifdef LIBXL_HAVE_PSR_CMT
 
 #define MBM_SAMPLE_RETRY_MAX 4
@@ -7913,7 +7931,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)) {
@@ -7927,15 +7944,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 = get_phy_socket_num();
+    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) {
@@ -8057,6 +8070,202 @@ int main_psr_cmt_show(int argc, char **argv)
 }
 #endif
 
+#ifdef LIBXL_HAVE_PSR_CAT
+static int psr_cat_l3_cbm_set(uint32_t domid, uint32_t socket, uint64_t cbm)
+{
+    int rc;
+    uint32_t i, nr_sockets;
+
+    if (socket != ALL_SOCKETS) {
+        return libxl_psr_cat_set_domain_data(ctx, domid,
+                                             LIBXL_PSR_CAT_TYPE_L3_CBM,
+                                             socket, cbm);
+    } else {
+        nr_sockets = get_phy_socket_num();
+        if (nr_sockets == 0) {
+            fprintf(stderr, "Failed getting physinfo\n");
+            return -1;
+        }
+        for (i = 0; i < nr_sockets; i++) {
+            rc = libxl_psr_cat_set_domain_data(ctx, domid,
+                                               LIBXL_PSR_CAT_TYPE_L3_CBM,
+                                               i, cbm);
+            if (rc < 0) {
+                fprintf(stderr, "Failed to set l3 cbm for socket:%d\n", i);
+            return -1;
+            }
+        }
+    }
+    return 0;
+}
+
+struct psr_cat_l3_info
+{
+    uint32_t cos_max;
+    uint32_t cbm_len;
+};
+
+static void psr_cat_print_domain_info(libxl_dominfo *dominfo,
+                                      struct psr_cat_l3_info *l3_info,
+                                      uint32_t nr_sockets)
+{
+    char *domain_name;
+    uint32_t socketid;
+    uint64_t cbm;
+
+    domain_name = libxl_domid_to_name(ctx, dominfo->domid);
+    printf("%-40s %5d", domain_name, dominfo->domid);
+    free(domain_name);
+
+    for (socketid = 0; socketid < nr_sockets; socketid++) {
+        if (l3_info[socketid].cbm_len > 0 &&
+            !libxl_psr_cat_get_domain_data(ctx, dominfo->domid,
+                                           LIBXL_PSR_CAT_TYPE_L3_CBM,
+                                           socketid, &cbm) )
+            printf("%#16"PRIx64, cbm);
+    }
+
+    printf("\n");
+}
+
+static int psr_cat_show(uint32_t domid)
+{
+    uint32_t i, socketid, nr_sockets;
+    int rc, nr_domains;
+    uint32_t l3_cache_size;
+    struct psr_cat_l3_info *l3_info;
+
+    nr_sockets = get_phy_socket_num();
+    if (nr_sockets == 0) {
+        fprintf(stderr, "Failed getting physinfo\n");
+        return -1;
+    }
+
+    /* Header */
+    printf("%-40s %5s", "Name", "ID");
+    for (socketid = 0; socketid < nr_sockets; socketid++)
+        printf("%14s %d", "Socket", socketid);
+    printf("\n");
+
+    /* Total L3 cache size */
+    printf("%-46s", "Total L3 Cache Size");
+    for (socketid = 0; socketid < nr_sockets; socketid++) {
+        rc = libxl_psr_cmt_get_l3_cache_size(ctx, socketid, &l3_cache_size);
+        if (rc < 0) {
+            fprintf(stderr, "Failed to get system l3 cache size for socket:%d\n",
+                            socketid);
+            return -1;
+        }
+        printf("%13u KB", l3_cache_size);
+    }
+    printf("\n");
+
+    /* Max COS and CBM length */
+    l3_info = malloc(sizeof(l3_info) * nr_sockets);
+    //if (!l3_info)
+    for (socketid = 0; socketid < nr_sockets; socketid++) {
+        rc = libxl_psr_cat_get_l3_info(ctx, socketid,
+                                       &l3_info[socketid].cos_max,
+                                       &l3_info[socketid].cbm_len);
+        if (rc < 0) {
+            fprintf(stderr, "Failed to get system l3 info for socket:%d\n",
+                            socketid);
+            rc = -1;
+            goto out;
+        }
+    }
+    printf("%-46s", "Max COS");
+    for (socketid = 0; socketid < nr_sockets; socketid++) {
+        printf("%16u", l3_info[socketid].cos_max);
+    }
+    printf("\n");
+    printf("%-46s", "Default CBM");
+    for (socketid = 0; socketid < nr_sockets; socketid++) {
+        printf("%#16"PRIx64, (1ul << l3_info[socketid].cbm_len) - 1);
+    }
+    printf("\n");
+
+    /* Each domain */
+    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);
+            rc = -1;
+            goto out;
+        }
+        psr_cat_print_domain_info(&dominfo, l3_info, nr_sockets);
+    }
+    else
+    {
+        libxl_dominfo *list;
+        if (!(list = libxl_list_domain(ctx, &nr_domains))) {
+            fprintf(stderr, "Failed to get domain info for domain list.\n");
+            rc = -1;
+            goto out;
+        }
+        for (i = 0; i < nr_domains; i++)
+            psr_cat_print_domain_info(list + i, l3_info, nr_sockets);
+        libxl_dominfo_list_free(list, nr_domains);
+    }
+    rc = 0;
+out:
+    if (l3_info)
+        free(l3_info);
+    return rc;
+}
+
+int main_psr_cat_cbm_set(int argc, char **argv)
+{
+    uint32_t domid;
+    uint32_t socket = ALL_SOCKETS;
+    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':
+        socket = 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 psr_cat_l3_cbm_set(domid, socket, 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..ffaf4ed 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -542,6 +542,19 @@ 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, all sockets when not"
+      "specified\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 1/6] x86: detect and initialize Intel CAT feature
  2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-03-13 13:40   ` Konrad Rzeszutek Wilk
  2015-03-13 13:43     ` Konrad Rzeszutek Wilk
  2015-03-17  8:11     ` Chao Peng
  2015-03-16 13:47   ` Jan Beulich
  1 sibling, 2 replies; 26+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-13 13:40 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 Fri, Mar 13, 2015 at 06:13:20PM +0800, 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>
> ---
>  docs/misc/xen-command-line.markdown |  15 +++-
>  xen/arch/x86/psr.c                  | 151 +++++++++++++++++++++++++++++++++---
>  xen/include/asm-x86/cpufeature.h    |   1 +
>  3 files changed, 155 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 63871cb..a4b36e6 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1090,9 +1090,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> | socket_num:<interger> )`

That should also list the 'cat:<boolean>'
>  
> -> Default: `psr=cmt:0,rmid_max:255`
> +> Default: `psr=cmt:0,rmid_max:255,cat:0,socket_num:0 (Detect at boot time)`
>  
>  Platform Shared Resource(PSR) Services.  Intel Haswell and later server
>  platforms offer information about the sharing of resources.
> @@ -1102,6 +1102,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
> @@ -1112,6 +1117,12 @@ 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.
> +  * `socket_num` indicates socket number. Detecte automatically at boot time

s/Detecte/Detect/

Well, it is not exactly socket number. It is up-to socket number. Meaning
if you give it '3' it will be socket 0, 1, 2 and 3.

> +    if not specified(0). Useful for CPU hot-plug case.
> +
>  ### reboot
>  > `= t[riple] | k[bd] | a[cpi] | p[ci] | n[o] [, [w]arm | [c]old]`
>  
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 2ef83df..df557e8 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -19,6 +19,14 @@
>  #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;
> @@ -26,8 +34,11 @@ struct psr_assoc {
>  };
>  
>  struct psr_cmt *__read_mostly psr_cmt;
> -static bool_t __initdata opt_psr;
> +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 opt_socket_num;
>  static uint64_t rmid_mask;
>  static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
>  
> @@ -58,15 +69,31 @@ static void __init parse_psr_param(char *s)
>                                      val_str);
>              }
>          }
> +        else if ( !strcmp(s, "cat") )
> +        {
> +            if ( !val_str )
> +                opt_psr |= PSR_CAT;
> +            else
> +            {
> +                int val_int = parse_bool(val_str);
> +                if ( val_int == 1 )
> +                    opt_psr |= PSR_CAT;
> +                else if ( val_int != 0 )
> +                    printk("PSR: unknown cat value: %s - CAT disabled!\n",
> +                                    val_str);
> +            }
> +        }

This looks exactly like the PSR_CMT parsing. Why not make it a function
and just pass in the s, val_str, "cat", PSR_CAT to it so it can
do this determination?

>          else if ( val_str && !strcmp(s, "rmid_max") )
>              opt_rmid_max = simple_strtoul(val_str, NULL, 0);
> +        else if ( val_str && !strcmp(s, "socket_num") )
> +            opt_socket_num = simple_strtoul(val_str, NULL, 0);
>  
>          s = ss + 1;
>      } while ( ss );
>  }
>  custom_param("psr", parse_psr_param);
>  
> -static void __init init_psr_cmt(unsigned int rmid_max)
> +static void __init psr_cmt_init(unsigned int rmid_max)
>  {
>      unsigned int eax, ebx, ecx, edx;
>      unsigned int rmid;
> @@ -115,14 +142,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)
>  {
> @@ -189,6 +208,118 @@ void psr_assoc_rmid(unsigned int rmid)
>      }
>  }
>  
> +static void do_cat_cpu_init(void* data)
> +{
> +    unsigned int eax, ebx, ecx, edx;
> +    struct psr_cat_socket_info *info;
> +
> +    cpuid_count(0x10, 0, &eax, &ebx, &ecx, &edx);
> +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
> +    {
> +        info = data;
> +
> +        cpuid_count(0x10, 1, &eax, &ebx, &ecx, &edx);
> +        info->cbm_len = (eax & 0x1f) + 1;
> +        info->cos_max = (edx & 0xffff);
> +
> +        info->enabled = 1;
> +        printk(XENLOG_INFO "CAT: enabled on socket %d, cos_max:%d, cbm_len:%d\n",

Why not make those DEBUG?

> +               (int)(info - cat_socket_info), info->cos_max, info->cbm_len);
> +    }
> +}
> +
> +static void cat_cpu_init(unsigned int cpu)
> +{
> +    struct psr_cat_socket_info *info;
> +    unsigned int socket;
> +    const struct cpuinfo_x86 *c;
> +
> +    socket = cpu_to_socket(cpu);
> +    if ( socket >= opt_socket_num )
> +    {
> +        printk(XENLOG_WARNING "CAT: disabled on socket %d because socket_num is %d\n",
> +               socket, opt_socket_num);
> +        return;
> +    }
> +
> +    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;
> +
> +    if ( cpu == smp_processor_id() )
> +        do_cat_cpu_init(info);
> +    else
> +        on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, info, 0);
> +}
> +
> +static int cpu_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +
> +    switch ( action )
> +    {
> +    case CPU_ONLINE:
> +        (void)cat_cpu_init(cpu);
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block cpu_nfb = {
> +    .notifier_call = cpu_callback
> +};
> +
> +static unsigned int get_max_socket(void)
> +{
> +    unsigned int cpu, max_apicid = boot_cpu_physical_apicid;
> +
> +    for_each_present_cpu(cpu)
> +        if (max_apicid < x86_cpu_to_apicid[cpu])
> +            max_apicid = x86_cpu_to_apicid[cpu];
> +
> +    return apicid_to_socket(max_apicid);
> +}
> +
> +static void __init psr_cat_init(void)
> +{
> +    if ( opt_socket_num == 0 )
> +        opt_socket_num = get_max_socket() + 1;
> +    else if ( opt_socket_num > NR_CPUS )
> +         printk(XENLOG_WARNING "socket_num > NR_CPUS(%d), set to NR_CPUS.\n",
> +                NR_CPUS);

Well, not really. You did not set it to NR_CPUS.

> +
> +    BUG_ON(opt_socket_num == 0);
> +
> +    cat_socket_info = xzalloc_array(struct psr_cat_socket_info, opt_socket_num);
> +    if ( !cat_socket_info )
> +        return;
> +
> +    cat_cpu_init(smp_processor_id());

Do 'if (!cat_cpu_init(..)).`'

as the CPU might not support this.

At which point you should also free the cat_socket_info and
not register the cpu notifier.

> +    register_cpu_notifier(&cpu_nfb);
> +}
> +
> +static int __init psr_presmp_init(void)
> +{
> +    if ( (opt_psr & PSR_CMT) && opt_rmid_max )
> +        psr_cmt_init(opt_rmid_max);
> +
> +    if ( opt_psr & PSR_CAT )
> +        psr_cat_init();
> +
> +    return 0;
> +}
> +presmp_initcall(psr_presmp_init);
> +
>  /*
>   * Local variables:
>   * mode: C
> 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 */
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> 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 1/6] x86: detect and initialize Intel CAT feature
  2015-03-13 13:40   ` Konrad Rzeszutek Wilk
@ 2015-03-13 13:43     ` Konrad Rzeszutek Wilk
  2015-03-17  8:11     ` Chao Peng
  1 sibling, 0 replies; 26+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-13 13:43 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

> > +    cat_cpu_init(smp_processor_id());
> 
> Do 'if (!cat_cpu_init(..)).`'
> 
> as the CPU might not support this.
> 
> At which point you should also free the cat_socket_info and

And also set cat_socket_info = NULL.
> not register the cpu notifier.
> 
> > +    register_cpu_notifier(&cpu_nfb);

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-13 10:13 ` [PATCH 2/6] x86: add support for COS/CBM manangement Chao Peng
@ 2015-03-13 13:53   ` Konrad Rzeszutek Wilk
  2015-03-17  8:57     ` Chao Peng
  2015-03-16 17:10   ` Jan Beulich
  1 sibling, 1 reply; 26+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-13 13:53 UTC (permalink / raw)
  To: Chao Peng
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

> +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) */

Missing '.'.

> +    if ( cbm & (~0ull << cbm_len) )
> +        return 0;
> +
> +    /* At least two contiguous bits need to be set */

Missing '.'

> +    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 */

Missing '.'

> +    if ( find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
> +        return 0;
> +
> +    return 1;
> +}
> +
> +static unsigned int get_socket_cpu(unsigned int socket)
> +{
> +    unsigned int cpu;
> +
> +    for_each_online_cpu ( cpu )
> +       if ( cpu_data[cpu].phys_proc_id == socket )
> +           return cpu;
> +    return nr_cpu_ids;
> +}
> +
> +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 void 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 = cpumask_check(get_socket_cpu(socket));
> +        on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
> +    }
> +}
> +
> +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 no found, then keep this unused one */

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 refer only by the domain, then use it */

s/refer/referred/

Missing '.'

> +    if ( !find && map[old_cos].ref == 1 )
> +        find = map + old_cos;
> +
> +    if ( !find )
> +        return -EUSERS;
> +
> +    cos = find - map;
> +    if ( find->cbm != cbm )
> +    {
> +        find->cbm = cbm;
> +        write_l3_cbm(socket, cos, cbm);
> +    }
> +    find->ref++;
> +    map[old_cos].ref--;
> +    d->arch.psr_cos_ids[socket] = cos;
> +    return 0;
> +}
> +
> +static void psr_free_cos(struct domain *d)
> +{
> +    unsigned int socket;
> +    unsigned int cos;
> +    struct psr_cat_cbm *map;
> +
> +    if( !d->arch.psr_cos_ids )
> +        return;
> +
> +    for ( socket = 0; socket < opt_socket_num; socket++)
> +    {
> +        cos = d->arch.psr_cos_ids[socket];
> +        if ( cos == 0 )
> +            continue;
> +
> +        map = cat_socket_info[socket].cos_cbm_map;
> +        if ( map )
> +            map[cos].ref--;
> +    }
> +
> +    xfree(d->arch.psr_cos_ids);

And d->arch.psr_cos_ids = NULL

(however this depends on who calls psr_domain_init, which
is unclear to me).

> +}
> +
> +int psr_domain_init(struct domain *d)
> +{
> +    if ( cat_socket_info )
> +    {
> +        d->arch.psr_cos_ids = xzalloc_array(unsigned int, opt_socket_num);
> +        if ( !d->arch.psr_cos_ids )
> +            return -ENOMEM;
> +    }
> +
> +    return 0;
> +}
> +
> +void psr_domain_free(struct domain *d)

So who calls this?

> +{
> +    psr_free_rmid(d);
> +    psr_free_cos(d);
> +}
> +
>  static void do_cat_cpu_init(void* data)
>  {
>      unsigned int eax, ebx, ecx, edx;
>      struct psr_cat_socket_info *info;
> +    unsigned int cos;
>  
>      cpuid_count(0x10, 0, &eax, &ebx, &ecx, &edx);
>      if ( ebx & PSR_RESOURCE_TYPE_L3 )
> @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
>          info->cbm_len = (eax & 0x1f) + 1;
>          info->cos_max = (edx & 0xffff);
>  
> +        info->cos_cbm_map = xmalloc_array(struct psr_cat_cbm,
> +                                          info->cos_max + 1UL);
> +        if ( !info->cos_cbm_map)

The ')' should be moved by one space.

> +            return;
> +
> +        for ( cos = 0; cos <= info->cos_max; cos++ )
> +            info->cos_cbm_map[cos].ref = 0;
> +
> +        /* cos=0 is reserved as default cbm(all ones) */

Missing '.'

> +        info->cos_cbm_map[0].cbm =  (1ull << info->cbm_len) - 1;
> +
>          info->enabled = 1;
>          printk(XENLOG_INFO "CAT: enabled on socket %d, cos_max:%d, cbm_len:%d\n",
>                 (int)(info - cat_socket_info), info->cos_max, info->cbm_len);
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 611a291..096dc31 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -171,6 +171,26 @@ 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 )
> +                break;
> +
> +            if ( __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 c6076e9..999a4c6 100644
> --- a/xen/include/asm-x86/psr.h
> +++ b/xen/include/asm-x86/psr.h
> @@ -48,6 +48,14 @@ int psr_alloc_rmid(struct domain *d);
>  void psr_free_rmid(struct domain *d);
>  void psr_assoc_rmid(unsigned int rmid);
>  
> +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..7c7baed 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 or cpu 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..9d8ed10 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 or cpu 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
> 
> 
> _______________________________________________
> 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 5/6] xsm: add CAT related xsm policies
  2015-03-13 10:13 ` [PATCH 5/6] xsm: add CAT related xsm policies Chao Peng
@ 2015-03-13 16:45   ` Daniel De Graaf
  0 siblings, 0 replies; 26+ messages in thread
From: Daniel De Graaf @ 2015-03-13 16:45 UTC (permalink / raw)
  To: Chao Peng, xen-devel
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, will.auld, JBeulich, wei.liu2

On 03/13/2015 06:13 AM, Chao Peng wrote:
> 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>

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
  2015-03-13 13:40   ` Konrad Rzeszutek Wilk
@ 2015-03-16 13:47   ` Jan Beulich
  2015-03-17  8:48     ` Chao Peng
  1 sibling, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-03-16 13:47 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> @@ -1112,6 +1117,12 @@ 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.
> +  * `socket_num` indicates socket number. Detecte automatically at boot time
> +    if not specified(0). Useful for CPU hot-plug case.

While saying something, at least for me what is being said doesn't at
all make clear what "socket number" here is: The number of a specific
socket? The number of sockets? Yet something else? Without knowing
what it means by _just_ reading this description, people won't know
what to use the command line option for.

> @@ -58,15 +69,31 @@ static void __init parse_psr_param(char *s)
>                                      val_str);
>              }
>          }
> +        else if ( !strcmp(s, "cat") )
> +        {
> +            if ( !val_str )
> +                opt_psr |= PSR_CAT;
> +            else
> +            {
> +                int val_int = parse_bool(val_str);
> +                if ( val_int == 1 )

Blank line between declarations and statements please.

> -static void __init init_psr_cmt(unsigned int rmid_max)
> +static void __init psr_cmt_init(unsigned int rmid_max)

Is this renaming really an integral part of this patch?

> +static void do_cat_cpu_init(void* data)

* and blank are reversed here.

> +{
> +    unsigned int eax, ebx, ecx, edx;
> +    struct psr_cat_socket_info *info;
> +
> +    cpuid_count(0x10, 0, &eax, &ebx, &ecx, &edx);
> +    if ( ebx & PSR_RESOURCE_TYPE_L3 )
> +    {
> +        info = data;
> +
> +        cpuid_count(0x10, 1, &eax, &ebx, &ecx, &edx);
> +        info->cbm_len = (eax & 0x1f) + 1;
> +        info->cos_max = (edx & 0xffff);
> +
> +        info->enabled = 1;
> +        printk(XENLOG_INFO "CAT: enabled on socket %d, cos_max:%d, cbm_len:%d\n",
> +               (int)(info - cat_socket_info), info->cos_max, info->cbm_len);

Bogus cast. Also the format specifier to output "unsigned int" is %u.

> +static void cat_cpu_init(unsigned int cpu)
> +{
> +    struct psr_cat_socket_info *info;
> +    unsigned int socket;
> +    const struct cpuinfo_x86 *c;
> +
> +    socket = cpu_to_socket(cpu);
> +    if ( socket >= opt_socket_num )
> +    {
> +        printk(XENLOG_WARNING "CAT: disabled on socket %d because socket_num is %d\n",
> +               socket, opt_socket_num);
> +        return;
> +    }
> +
> +    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;
> +
> +    if ( cpu == smp_processor_id() )
> +        do_cat_cpu_init(info);
> +    else
> +        on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, info, 0);

Hmm, using an IPI here seems odd. Is there a reason to can't hook
this onto CPU_STARTING instead of CPU_ONLINE?

> +static int cpu_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +
> +    switch ( action )
> +    {
> +    case CPU_ONLINE:
> +        (void)cat_cpu_init(cpu);

Bogus cast.

> +        break;
> +    default:
> +        break;

Pointless default case.

> +static unsigned int get_max_socket(void)
> +{
> +    unsigned int cpu, max_apicid = boot_cpu_physical_apicid;
> +
> +    for_each_present_cpu(cpu)
> +        if (max_apicid < x86_cpu_to_apicid[cpu])

Coding style.

> +            max_apicid = x86_cpu_to_apicid[cpu];
> +
> +    return apicid_to_socket(max_apicid);

Since when is the socket with the highest numbered APIC ID
the highest numbered socket? I think the whole function needs
to act on socket numbers only.

> +static void __init psr_cat_init(void)
> +{
> +    if ( opt_socket_num == 0 )
> +        opt_socket_num = get_max_socket() + 1;
> +    else if ( opt_socket_num > NR_CPUS )
> +         printk(XENLOG_WARNING "socket_num > NR_CPUS(%d), set to NR_CPUS.\n",
> +                NR_CPUS);
> +
> +    BUG_ON(opt_socket_num == 0);

Is that really a useful check? It triggering would imply
get_max_sockets() returned -1...

Jan

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

* Re: [PATCH 3/6] X86: improve psr scheduling code
  2015-03-13 10:13 ` [PATCH 3/6] X86: improve psr scheduling code Chao Peng
@ 2015-03-16 16:53   ` Jan Beulich
  2015-03-17  9:12     ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-03-16 16:53 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> @@ -1473,11 +1471,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);

But you now potentially do the MSR write despite it already being
the needed value (e.g. when switching between an idle vCPU and
another one also having RMID zero).

Jan

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-13 10:13 ` [PATCH 2/6] x86: add support for COS/CBM manangement Chao Peng
  2015-03-13 13:53   ` Konrad Rzeszutek Wilk
@ 2015-03-16 17:10   ` Jan Beulich
  2015-03-17  9:11     ` Chao Peng
  1 sibling, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-03-16 17:10 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1324,6 +1324,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 = -ENOSYS;
> +            break;

Please leave -ENOSYS to unimplemented hypercalls. -EOPNOTSUPP
or -EINVAL seem to be better candidates here.

> +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 ( find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )

You can't reliably invoke this without having checked that
zero_bit is less than cbm_len (undefined behavior may result).

> +static void 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 = cpumask_check(get_socket_cpu(socket));

Isn't this going to trigger an assertion when the socket count got
specified on the command line?

> +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 no found, then keep this 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;
> +        }
> +    }

Please add a comment explaining that this is implicitly serialized
via the domctl lock.

> +static void psr_free_cos(struct domain *d)
> +{
> +    unsigned int socket;
> +    unsigned int cos;
> +    struct psr_cat_cbm *map;
> +
> +    if( !d->arch.psr_cos_ids )
> +        return;

Considering this check ...

> +    for ( socket = 0; socket < opt_socket_num; socket++)
> +    {
> +        cos = d->arch.psr_cos_ids[socket];
> +        if ( cos == 0 )
> +            continue;
> +
> +        map = cat_socket_info[socket].cos_cbm_map;
> +        if ( map )
> +            map[cos].ref--;
> +    }
> +
> +    xfree(d->arch.psr_cos_ids);

... I think you want to clear the pointer here.

> @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
>          info->cbm_len = (eax & 0x1f) + 1;

This means cbm_len <= 32. Why is cos_cbm_map[].cbm then a
uint64_t?

Jan

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-13 13:40   ` Konrad Rzeszutek Wilk
  2015-03-13 13:43     ` Konrad Rzeszutek Wilk
@ 2015-03-17  8:11     ` Chao Peng
  2015-03-17 13:00       ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-17  8:11 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

On Fri, Mar 13, 2015 at 09:40:13AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 13, 2015 at 06:13:20PM +0800, 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>
> > ---
> >  docs/misc/xen-command-line.markdown |  15 +++-
> >  xen/arch/x86/psr.c                  | 151 +++++++++++++++++++++++++++++++++---
> >  xen/include/asm-x86/cpufeature.h    |   1 +
> >  3 files changed, 155 insertions(+), 12 deletions(-)
> > 
> > +    cat_cpu_init(smp_processor_id());
> 
> Do 'if (!cat_cpu_init(..)).`'
> 
> as the CPU might not support this.
> 
> At which point you should also free the cat_socket_info and
> not register the cpu notifier.

Even the booting CPU does not support this, other CPUs may still support
this. Generally the feature is a per-socket feature. So break here is
not the intention.

Except this, all other comments will be addressed by the next version.
Thanks for your time.

Chao
> 
> > +    register_cpu_notifier(&cpu_nfb);
> > +}
> > +

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-16 13:47   ` Jan Beulich
@ 2015-03-17  8:48     ` Chao Peng
  2015-03-17  9:01       ` Jan Beulich
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-17  8:48 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

On Mon, Mar 16, 2015 at 01:47:06PM +0000, Jan Beulich wrote:
> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> > @@ -1112,6 +1117,12 @@ 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.
> > +  * `socket_num` indicates socket number. Detecte automatically at boot time
> > +    if not specified(0). Useful for CPU hot-plug case.
> 
> While saying something, at least for me what is being said doesn't at
> all make clear what "socket number" here is: The number of a specific
> socket? The number of sockets? Yet something else? Without knowing
> what it means by _just_ reading this description, people won't know
> what to use the command line option for.

I agree, the name is a little confusing. Or perhaps: max_socket? E.g.

`max_socket` indicates the maximum number of available sockets for CAT
feature detection. All the sockets up to max_socket will be checked for
CAT feature. The value is normally detected at boot time automatically
if not specified(0). While the value may need to be specified manually
for CPU hot-plug scenario in which case the maximum socket number detected
at boot time may be not correct.

> 
> > -static void __init init_psr_cmt(unsigned int rmid_max)
> > +static void __init psr_cmt_init(unsigned int rmid_max)
> 
> Is this renaming really an integral part of this patch?

OK, I will move it to separate one or just keep it unchanged.

> 
> > +        on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, info, 0);
> 
> Hmm, using an IPI here seems odd. Is there a reason to can't hook
> this onto CPU_STARTING instead of CPU_ONLINE?

Looks like CPU_STARTING is what I need, IPI then is unnecessary.

> 
> > +static unsigned int get_max_socket(void)
> > +{
> > +    unsigned int cpu, max_apicid = boot_cpu_physical_apicid;
> > +
> > +    for_each_present_cpu(cpu)
> > +        if (max_apicid < x86_cpu_to_apicid[cpu])
> 
> Coding style.
> 
> > +            max_apicid = x86_cpu_to_apicid[cpu];
> > +
> > +    return apicid_to_socket(max_apicid);
> 
> Since when is the socket with the highest numbered APIC ID
> the highest numbered socket? I think the whole function needs
> to act on socket numbers only.

Then perhaps looping cpus and checking against cpu_to_socket(cpu)
directly make sense.

Thanks Jan.
Chao

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-13 13:53   ` Konrad Rzeszutek Wilk
@ 2015-03-17  8:57     ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-17  8:57 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

On Fri, Mar 13, 2015 at 09:53:37AM -0400, Konrad Rzeszutek Wilk wrote:
> > +    xfree(d->arch.psr_cos_ids);
> 
> And d->arch.psr_cos_ids = NULL
> 
> (however this depends on who calls psr_domain_init, which
> is unclear to me).
> 
> > +}
> > +
> > +int psr_domain_init(struct domain *d)
> > +{
> > +    if ( cat_socket_info )
> > +    {
> > +        d->arch.psr_cos_ids = xzalloc_array(unsigned int, opt_socket_num);
> > +        if ( !d->arch.psr_cos_ids )
> > +            return -ENOMEM;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +void psr_domain_free(struct domain *d)
> 
> So who calls this?

This function is called by arch_domain_destroy() or the failure path of
arch_domain_create(). In both cases the domain structure will be freed
later so setting NULL is pointless.

Thanks,
Chao
> 
> > +{
> > +    psr_free_rmid(d);
> > +    psr_free_cos(d);
> > +}
> > +

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-17  8:48     ` Chao Peng
@ 2015-03-17  9:01       ` Jan Beulich
  0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2015-03-17  9:01 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 17.03.15 at 09:48, <chao.p.peng@linux.intel.com> wrote:
> On Mon, Mar 16, 2015 at 01:47:06PM +0000, Jan Beulich wrote:
>> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
>> > @@ -1112,6 +1117,12 @@ 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.
>> > +  * `socket_num` indicates socket number. Detecte automatically at boot time
>> > +    if not specified(0). Useful for CPU hot-plug case.
>> 
>> While saying something, at least for me what is being said doesn't at
>> all make clear what "socket number" here is: The number of a specific
>> socket? The number of sockets? Yet something else? Without knowing
>> what it means by _just_ reading this description, people won't know
>> what to use the command line option for.
> 
> I agree, the name is a little confusing. Or perhaps: max_socket? E.g.
> 
> `max_socket` indicates the maximum number of available sockets for CAT
> feature detection. All the sockets up to max_socket will be checked for
> CAT feature. The value is normally detected at boot time automatically
> if not specified(0). While the value may need to be specified manually
> for CPU hot-plug scenario in which case the maximum socket number detected
> at boot time may be not correct.

Thanks, yes - max_socket or (perhaps more intuitive to users)
socket_count or num_sockets.

Jan

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-16 17:10   ` Jan Beulich
@ 2015-03-17  9:11     ` Chao Peng
  2015-03-17  9:25       ` Jan Beulich
  0 siblings, 1 reply; 26+ messages in thread
From: Chao Peng @ 2015-03-17  9:11 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

On Mon, Mar 16, 2015 at 05:10:32PM +0000, Jan Beulich wrote:
> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> > +    else
> > +    {
> > +        unsigned int cpu = cpumask_check(get_socket_cpu(socket));
> 
> Isn't this going to trigger an assertion when the socket count got
> specified on the command line?

Yes, the assertion is needed in tools side anyway. Here the check seems
unnecessary.

> 
> > +    if( !d->arch.psr_cos_ids )
> > +        return;
> 
> Considering this check ...
> 
> > +    for ( socket = 0; socket < opt_socket_num; socket++)
> > +    {
> > +        cos = d->arch.psr_cos_ids[socket];
> > +        if ( cos == 0 )
> > +            continue;
> > +
> > +        map = cat_socket_info[socket].cos_cbm_map;
> > +        if ( map )
> > +            map[cos].ref--;
> > +    }
> > +
> > +    xfree(d->arch.psr_cos_ids);
> 
> ... I think you want to clear the pointer here.

This function is called by arch_domain_destroy() or the failure path of
arch_domain_create(). In both cases the domain structure will be freed
later so setting NULL is pointless.

> 
> > @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
> >          info->cbm_len = (eax & 0x1f) + 1;
> 
> This means cbm_len <= 32. Why is cos_cbm_map[].cbm then a
> uint64_t?

Currently the cbm_len is EAX[4:0], 64 bits cbm here is for future
possible enhancement.

Chao

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

* Re: [PATCH 3/6] X86: improve psr scheduling code
  2015-03-16 16:53   ` Jan Beulich
@ 2015-03-17  9:12     ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-17  9:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

On Mon, Mar 16, 2015 at 04:53:35PM +0000, Jan Beulich wrote:
> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> > @@ -1473,11 +1471,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);
> 
> But you now potentially do the MSR write despite it already being
> the needed value (e.g. when switching between an idle vCPU and
> another one also having RMID zero).

Not really. The MSR is already updated lazily in psr_assoc_rmid().

Chao

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

* Re: [PATCH 4/6] x86: add scheduling support for Intel CAT
  2015-03-13 10:13 ` [PATCH 4/6] x86: add scheduling support for Intel CAT Chao Peng
@ 2015-03-17  9:19   ` Jan Beulich
  2015-03-17  9:33     ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-03-17  9:19 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> @@ -407,10 +388,75 @@ void psr_domain_free(struct domain *d)
>      psr_free_cos(d);
>  }
>  
> +static void psr_assoc_init(struct psr_assoc *psra)
> +{
> +    unsigned int socket;
> +    struct psr_cat_socket_info *info;
> +
> +    socket = cpu_to_socket(smp_processor_id());
> +    psra->socket = socket;
> +    if ( cat_socket_info && socket < opt_socket_num )
> +    {
> +        info = cat_socket_info + socket;
> +        if ( info->enabled )
> +            psra->cos_mask = (~(~0ull << (get_count_order(info->cos_max)
> +                             + 32))) & (~0ull << 32);

((1ull << get_count_order()) - 1) << 32

seems better readable to me.

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

This ultimately leading to psr_assoc_init() makes it really undesirable
here imo: Initialization work shouldn't be done during a context
switch, even is this a one time thing on each CPU.

Jan

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-17  9:11     ` Chao Peng
@ 2015-03-17  9:25       ` Jan Beulich
  2015-03-17 10:06         ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2015-03-17  9:25 UTC (permalink / raw)
  To: Chao Peng
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

>>> On 17.03.15 at 10:11, <chao.p.peng@linux.intel.com> wrote:
> On Mon, Mar 16, 2015 at 05:10:32PM +0000, Jan Beulich wrote:
>> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
>> > +    else
>> > +    {
>> > +        unsigned int cpu = cpumask_check(get_socket_cpu(socket));
>> 
>> Isn't this going to trigger an assertion when the socket count got
>> specified on the command line?
> 
> Yes, the assertion is needed in tools side anyway. Here the check seems
> unnecessary.

No - if the get_socket_cpu() may return nr_cpu_ids, you need a
check. Just not in the form of an assertion.

>> > +    if( !d->arch.psr_cos_ids )
>> > +        return;
>> 
>> Considering this check ...
>> 
>> > +    for ( socket = 0; socket < opt_socket_num; socket++)
>> > +    {
>> > +        cos = d->arch.psr_cos_ids[socket];
>> > +        if ( cos == 0 )
>> > +            continue;
>> > +
>> > +        map = cat_socket_info[socket].cos_cbm_map;
>> > +        if ( map )
>> > +            map[cos].ref--;
>> > +    }
>> > +
>> > +    xfree(d->arch.psr_cos_ids);
>> 
>> ... I think you want to clear the pointer here.
> 
> This function is called by arch_domain_destroy() or the failure path of
> arch_domain_create(). In both cases the domain structure will be freed
> later so setting NULL is pointless.

But this is not visible from this function. The alternative to not
clearing the pointer here would be to do the checks of the
pointer in the callers, which then would make clear whether this
is really only on error and domain destruction paths (and that
hence the pointer can't be double freed).

>> > @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
>> >          info->cbm_len = (eax & 0x1f) + 1;
>> 
>> This means cbm_len <= 32. Why is cos_cbm_map[].cbm then a
>> uint64_t?
> 
> Currently the cbm_len is EAX[4:0], 64 bits cbm here is for future
> possible enhancement.

Unless the specification explicitly says that the field width in EAX
may be extended in the future, please omit such preparations
for possible future extensions - it is simply going to confuse
readers (as it did for me already) trying to understand why the
type of the field is what it is.

Jan

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

* Re: [PATCH 4/6] x86: add scheduling support for Intel CAT
  2015-03-17  9:19   ` Jan Beulich
@ 2015-03-17  9:33     ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-17  9:33 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

On Tue, Mar 17, 2015 at 09:19:42AM +0000, Jan Beulich wrote:
> > +            psra->cos_mask = (~(~0ull << (get_count_order(info->cos_max)
> > +                             + 32))) & (~0ull << 32);
> 
> ((1ull << get_count_order()) - 1) << 32
> 
> seems better readable to me.

Indeed :)

> 
> >  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);
> 
> This ultimately leading to psr_assoc_init() makes it really undesirable
> here imo: Initialization work shouldn't be done during a context
> switch, even is this a one time thing on each CPU.

This is what we did in CMT patch. But now I think psr_assoc_init()
can be called from cat_cpu_init() other than here.

Chao

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

* Re: [PATCH 2/6] x86: add support for COS/CBM manangement
  2015-03-17  9:25       ` Jan Beulich
@ 2015-03-17 10:06         ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-17 10:06 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, keir, dgdegra

On Tue, Mar 17, 2015 at 09:25:48AM +0000, Jan Beulich wrote:
> >>> On 17.03.15 at 10:11, <chao.p.peng@linux.intel.com> wrote:
> > On Mon, Mar 16, 2015 at 05:10:32PM +0000, Jan Beulich wrote:
> >> >>> On 13.03.15 at 11:13, <chao.p.peng@linux.intel.com> wrote:
> >> > +    else
> >> > +    {
> >> > +        unsigned int cpu = cpumask_check(get_socket_cpu(socket));
> >> 
> >> Isn't this going to trigger an assertion when the socket count got
> >> specified on the command line?
> > 
> > Yes, the assertion is needed in tools side anyway. Here the check seems
> > unnecessary.
> 
> No - if the get_socket_cpu() may return nr_cpu_ids, you need a
> check. Just not in the form of an assertion.

Right, error code can be returned.

> 
> >> > +    if( !d->arch.psr_cos_ids )
> >> > +        return;
> >> 
> >> Considering this check ...
> >> 
> >> > +    for ( socket = 0; socket < opt_socket_num; socket++)
> >> > +    {
> >> > +        cos = d->arch.psr_cos_ids[socket];
> >> > +        if ( cos == 0 )
> >> > +            continue;
> >> > +
> >> > +        map = cat_socket_info[socket].cos_cbm_map;
> >> > +        if ( map )
> >> > +            map[cos].ref--;
> >> > +    }
> >> > +
> >> > +    xfree(d->arch.psr_cos_ids);
> >> 
> >> ... I think you want to clear the pointer here.
> > 
> > This function is called by arch_domain_destroy() or the failure path of
> > arch_domain_create(). In both cases the domain structure will be freed
> > later so setting NULL is pointless.
> 
> But this is not visible from this function. The alternative to not
> clearing the pointer here would be to do the checks of the
> pointer in the callers, which then would make clear whether this
> is really only on error and domain destruction paths (and that
> hence the pointer can't be double freed).

Then I'd like to clear the pointer here, so that even in the future the
code will not surprise somebody.

> 
> >> > @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
> >> >          info->cbm_len = (eax & 0x1f) + 1;
> >> 
> >> This means cbm_len <= 32. Why is cos_cbm_map[].cbm then a
> >> uint64_t?
> > 
> > Currently the cbm_len is EAX[4:0], 64 bits cbm here is for future
> > possible enhancement.
> 
> Unless the specification explicitly says that the field width in EAX
> may be extended in the future, please omit such preparations
> for possible future extensions - it is simply going to confuse
> readers (as it did for me already) trying to understand why the
> type of the field is what it is.

I will try to understand this internally, if 32 bit is enough, then it
will be used.

Thanks,
Chao

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-17  8:11     ` Chao Peng
@ 2015-03-17 13:00       ` Konrad Rzeszutek Wilk
  2015-03-18  8:31         ` Chao Peng
  0 siblings, 1 reply; 26+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-03-17 13:00 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 Tue, Mar 17, 2015 at 04:11:33PM +0800, Chao Peng wrote:
> On Fri, Mar 13, 2015 at 09:40:13AM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 13, 2015 at 06:13:20PM +0800, 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>
> > > ---
> > >  docs/misc/xen-command-line.markdown |  15 +++-
> > >  xen/arch/x86/psr.c                  | 151 +++++++++++++++++++++++++++++++++---
> > >  xen/include/asm-x86/cpufeature.h    |   1 +
> > >  3 files changed, 155 insertions(+), 12 deletions(-)
> > > 
> > > +    cat_cpu_init(smp_processor_id());
> > 
> > Do 'if (!cat_cpu_init(..)).`'
> > 
> > as the CPU might not support this.
> > 
> > At which point you should also free the cat_socket_info and
> > not register the cpu notifier.
> 
> Even the booting CPU does not support this, other CPUs may still support
> this. Generally the feature is a per-socket feature. So break here is
> not the intention.

Oooh, and you did mention that in the git commit description and I dived
right in the code - without looking there - sorry for that noise!

Thought I am curious - what if all the sockets don't support and the
user does try enable it on the command line (user error)? Shouldn't we
then figure out that all of the CPUs don't support and xfree
cat_socket_info and not register the CPU notifier?
> 
> Except this, all other comments will be addressed by the next version.

thank you!
> Thanks for your time.
> 
> Chao
> > 
> > > +    register_cpu_notifier(&cpu_nfb);
> > > +}
> > > +

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

* Re: [PATCH 1/6] x86: detect and initialize Intel CAT feature
  2015-03-17 13:00       ` Konrad Rzeszutek Wilk
@ 2015-03-18  8:31         ` Chao Peng
  0 siblings, 0 replies; 26+ messages in thread
From: Chao Peng @ 2015-03-18  8:31 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
	Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra

On Tue, Mar 17, 2015 at 09:00:27AM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 17, 2015 at 04:11:33PM +0800, Chao Peng wrote:
> > On Fri, Mar 13, 2015 at 09:40:13AM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Fri, Mar 13, 2015 at 06:13:20PM +0800, 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>
> > > > ---
> > > >  docs/misc/xen-command-line.markdown |  15 +++-
> > > >  xen/arch/x86/psr.c                  | 151 +++++++++++++++++++++++++++++++++---
> > > >  xen/include/asm-x86/cpufeature.h    |   1 +
> > > >  3 files changed, 155 insertions(+), 12 deletions(-)
> > > > 
> > > > +    cat_cpu_init(smp_processor_id());
> > > 
> > > Do 'if (!cat_cpu_init(..)).`'
> > > 
> > > as the CPU might not support this.
> > > 
> > > At which point you should also free the cat_socket_info and
> > > not register the cpu notifier.
> > 
> > Even the booting CPU does not support this, other CPUs may still support
> > this. Generally the feature is a per-socket feature. So break here is
> > not the intention.
> 
> Oooh, and you did mention that in the git commit description and I dived
> right in the code - without looking there - sorry for that noise!
> 
> Thought I am curious - what if all the sockets don't support and the
> user does try enable it on the command line (user error)? Shouldn't we
> then figure out that all of the CPUs don't support and xfree
> cat_socket_info and not register the CPU notifier?

Ideally, we should. Tricky thing is to support cpu hot plug, which
happens not at but after the booting time.

Chao
> > 
> > Except this, all other comments will be addressed by the next version.
> 
> thank you!
> > Thanks for your time.
> > 
> > Chao
> > > 
> > > > +    register_cpu_notifier(&cpu_nfb);
> > > > +}
> > > > +
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

end of thread, other threads:[~2015-03-18  8:31 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 10:13 [PATCH 0/6] enable Cache Allocation Technology (CAT) for VMs Chao Peng
2015-03-13 10:13 ` [PATCH 1/6] x86: detect and initialize Intel CAT feature Chao Peng
2015-03-13 13:40   ` Konrad Rzeszutek Wilk
2015-03-13 13:43     ` Konrad Rzeszutek Wilk
2015-03-17  8:11     ` Chao Peng
2015-03-17 13:00       ` Konrad Rzeszutek Wilk
2015-03-18  8:31         ` Chao Peng
2015-03-16 13:47   ` Jan Beulich
2015-03-17  8:48     ` Chao Peng
2015-03-17  9:01       ` Jan Beulich
2015-03-13 10:13 ` [PATCH 2/6] x86: add support for COS/CBM manangement Chao Peng
2015-03-13 13:53   ` Konrad Rzeszutek Wilk
2015-03-17  8:57     ` Chao Peng
2015-03-16 17:10   ` Jan Beulich
2015-03-17  9:11     ` Chao Peng
2015-03-17  9:25       ` Jan Beulich
2015-03-17 10:06         ` Chao Peng
2015-03-13 10:13 ` [PATCH 3/6] X86: improve psr scheduling code Chao Peng
2015-03-16 16:53   ` Jan Beulich
2015-03-17  9:12     ` Chao Peng
2015-03-13 10:13 ` [PATCH 4/6] x86: add scheduling support for Intel CAT Chao Peng
2015-03-17  9:19   ` Jan Beulich
2015-03-17  9:33     ` Chao Peng
2015-03-13 10:13 ` [PATCH 5/6] xsm: add CAT related xsm policies Chao Peng
2015-03-13 16:45   ` Daniel De Graaf
2015-03-13 10:13 ` [PATCH 6/6] tools: add tools support for Intel CAT 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.