Xen devel - Nov 2013 - [PATCH v2 0/8] enable Cache QoS Monitoring (CQM) feature

From: Dongxiao Xu <dongxiao.xu@intel.com>

Changes from v1:
 - Address comments from Andrew Cooper, including:
   * Change function names, e.g., alloc_cqm_rmid(), system_supports_cqm(), etc.
   * Change some structure element order to save packing cost.
   * Correct some function''s return value.
   * Some programming styles change.
   * ...

Future generations of Intel Xeon processor may offer monitoring capability in
each logical processor to measure specific quality-of-service metric,
for example, the Cache QoS Monitoring to get L3 cache occupancy.
Detailed information please refer to Intel SDM chapter 17.14.

Cache QoS Monitoring provides a layer of abstraction between applications and
logical processors through the use of Resource Monitoring IDs (RMIDs).
In Xen design, each guest in the system can be assigned an RMID independently,
while RMID=0 is reserved for monitoring domains that doesn''t enable CQM
service.
When any of the domain''s vcpu is scheduled on a logical processor, the
domain''s
RMID will be activated by programming the value into one specific MSR, and when
the vcpu is scheduled out, a RMID=0 will be programmed into that MSR.
The Cache QoS Hardware tracks cache utilization of memory accesses according to
the RMIDs and reports monitored data via a counter register. With this solution,
we can get the knowledge how much L3 cache is used by a certain guest.

To attach CQM service to a certain guest, two approaches are provided:
1) Create the guest with "pqos_cqm=1" set in configuration file.
2) Use "xl pqos-attach cqm domid" for a running guest.

To detached CQM service from a guest, users can:
1) Use "xl pqos-detach cqm domid" for a running guest.
2) Also destroying a guest will detach the CQM service.

To get the L3 cache usage, users can use the command of:
$ xl pqos-list cqm (domid)

The below data is just an example showing how the CQM related data is exposed to
end user.

[root@localhost]# xl pqos-list cqm
RMID count    56        RMID available    53
Name               ID  SocketID        L3C_Usage       SocketID        L3C_Usage
Domain-0            0         0         20127744              1         25231360
ExampleHVMDomain    1         0          3211264              1         10551296

Dongxiao Xu (8):
  x86: detect and initialize Cache QoS Monitoring feature
  x86: handle CQM resource when creating/destroying guests
  tools: handle CQM resource when creating/destroying guests
  x86: dynamically attach/detach CQM service for a guest
  tools: dynamically attach/detach CQM service for a guest
  x86: get per domain CQM information
  tools: get per domain CQM information
  x86: enable CQM monitoring for each domain RMID

 tools/libxc/xc_domain.c          |   51 +++++++++
 tools/libxc/xenctrl.h            |   14 +++
 tools/libxl/Makefile             |    3 +-
 tools/libxl/libxl.h              |    8 ++
 tools/libxl/libxl_create.c       |    3 +
 tools/libxl/libxl_pqos.c         |  102 +++++++++++++++++
 tools/libxl/libxl_types.idl      |    1 +
 tools/libxl/xl.h                 |    3 +
 tools/libxl/xl_cmdimpl.c         |  129 ++++++++++++++++++++++
 tools/libxl/xl_cmdtable.c        |   15 +++
 xen/arch/x86/Makefile            |    1 +
 xen/arch/x86/cpu/intel.c         |    6 +
 xen/arch/x86/domain.c            |   14 +++
 xen/arch/x86/domctl.c            |   40 +++++++
 xen/arch/x86/pqos.c              |  224 ++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/setup.c             |    3 +
 xen/arch/x86/sysctl.c            |   64 +++++++++++
 xen/common/domctl.c              |    5 +-
 xen/include/asm-x86/cpufeature.h |    1 +
 xen/include/asm-x86/domain.h     |    2 +
 xen/include/asm-x86/msr-index.h  |    5 +
 xen/include/asm-x86/pqos.h       |   56 ++++++++++
 xen/include/public/domctl.h      |   31 ++++++
 xen/include/public/sysctl.h      |   15 +++
 xen/include/xen/sched.h          |    3 +
 25 files changed, 797 insertions(+), 2 deletions(-)
 create mode 100644 tools/libxl/libxl_pqos.c
 create mode 100644 xen/arch/x86/pqos.c
 create mode 100644 xen/include/asm-x86/pqos.h

-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Detect platform QoS feature status and enumerate the resource types,
one of which is to monitor the L3 cache occupancy.

Also introduce a Xen grub command line parameter to control the
QoS feature status globally.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 xen/arch/x86/Makefile            |    1 +
 xen/arch/x86/cpu/intel.c         |    6 +++
 xen/arch/x86/pqos.c              |   89 ++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/setup.c             |    3 ++
 xen/include/asm-x86/cpufeature.h |    1 +
 xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
 6 files changed, 137 insertions(+)
 create mode 100644 xen/arch/x86/pqos.c
 create mode 100644 xen/include/asm-x86/pqos.h

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d502bdf..54962e0 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -58,6 +58,7 @@ obj-y += crash.o
 obj-y += tboot.o
 obj-y += hpet.o
 obj-y += xstate.o
+obj-y += pqos.o
 
 obj-$(crash_debug) += gdbstub.o
 
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 27fe762..f0d83ea 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -230,6 +230,12 @@ static void __devinit init_intel(struct cpuinfo_x86 *c)
 	     ( c->cpuid_level >= 0x00000006 ) &&
 	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
 		set_bit(X86_FEATURE_ARAT, c->x86_capability);
+
+	/* Check platform QoS monitoring capability */
+	if ((c->cpuid_level >= 0x00000007) &&
+	    (cpuid_ebx(0x00000007) & (1u<<12)))
+		set_bit(X86_FEATURE_QOSM, c->x86_capability);
+
 }
 
 static struct cpu_dev intel_cpu_dev __cpuinitdata = {
diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
new file mode 100644
index 0000000..e6ab416
--- /dev/null
+++ b/xen/arch/x86/pqos.c
@@ -0,0 +1,89 @@
+/*
+ * pqos.c: Platform QoS related service for guest.
+ *
+ * Copyright (c) 2013, Intel Corporation
+ * Author: Jiongxi Li  <jiongxi.li@intel.com>
+ * Author: Dongxiao Xu <dongxiao.xu@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+#include <asm/processor.h>
+#include <xen/init.h>
+#include <asm/pqos.h>
+
+static bool_t pqos_enabled = 1;
+boolean_param("pqos", pqos_enabled);
+
+unsigned int cqm_res_count = 0;
+unsigned int cqm_upscaling_factor = 0;
+bool_t cqm_enabled = 0;
+struct cqm_res_struct *cqm_res_array = NULL;
+
+static void __init init_cqm(void)
+{
+    unsigned int eax, edx;
+    unsigned int max_cqm_rmid;
+
+    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor, &max_cqm_rmid,
&edx);
+    if ( !(edx & QOS_MONITOR_EVTID_L3) )
+        return;
+
+    cqm_res_count = max_cqm_rmid + 1;
+
+    cqm_res_array = xzalloc_array(struct cqm_res_struct, cqm_res_count);
+    if ( !cqm_res_array )
+    {
+        cqm_res_count = 0;
+        return;
+    }
+
+    cqm_enabled = 1;
+
+    /* Reserve RMID 0 for all domains not being monitored */
+    cqm_res_array[0].inuse = 1;
+
+    printk(XENLOG_INFO "Cache QoS Monitoring Enabled.\n");
+}
+
+static void __init init_qos_monitor(void)
+{
+    unsigned int qm_features;
+    unsigned int eax, ebx, ecx;
+
+    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
+        return;
+
+    cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
+
+    if ( qm_features & QOS_MONITOR_TYPE_L3 )
+        init_cqm();
+}
+
+void __init init_platform_qos(void)
+{
+    if ( !pqos_enabled )
+        return;
+
+    init_qos_monitor();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5bf4ee0..95418e4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -48,6 +48,7 @@
 #include <asm/setup.h>
 #include <xen/cpu.h>
 #include <asm/nmi.h>
+#include <asm/pqos.h>
 
 /* opt_nosmp: If true, secondary processors are ignored. */
 static bool_t __initdata opt_nosmp;
@@ -1402,6 +1403,8 @@ void __init __start_xen(unsigned long mbi_p)
 
     domain_unpause_by_systemcontroller(dom0);
 
+    init_platform_qos();
+
     reset_stack_and_jump(init_done);
 }
 
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 1cfaf94..ca59668 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -147,6 +147,7 @@
 #define X86_FEATURE_ERMS	(7*32+ 9) /* Enhanced REP MOVSB/STOSB */
 #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context ID */
 #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional Memory */
+#define X86_FEATURE_QOSM	(7*32+12) /* Platform QoS monitoring capability */
 #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
 
diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
new file mode 100644
index 0000000..934d68a
--- /dev/null
+++ b/xen/include/asm-x86/pqos.h
@@ -0,0 +1,37 @@
+/*
+ * pqos.h: Platform QoS related service for guest.
+ *
+ * Copyright (c) 2013, Intel Corporation
+ * Author: Jiongxi Li  <jiongxi.li@intel.com>
+ * Author: Dongxiao Xu <dongxiao.xu@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ */
+#ifndef ASM_PQOS_H
+#define ASM_PQOS_H
+
+/* QoS Resource Type Enumeration */
+#define QOS_MONITOR_TYPE_L3            0x2
+
+/* QoS Monitoring Event ID */
+#define QOS_MONITOR_EVTID_L3           0x1
+
+struct cqm_res_struct {
+    domid_t  domain_id;
+    bool_t   inuse;
+};
+
+void init_platform_qos(void);
+
+#endif
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Allocate an RMID for a guest when it is created. This per-guest
RMID will be used to monitor Cache QoS related data. The RMID will
be relinquished when guest is destroyed.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 xen/arch/x86/domain.c        |    9 +++++++
 xen/arch/x86/pqos.c          |   59 ++++++++++++++++++++++++++++++++++++++++++
 xen/common/domctl.c          |    5 +++-
 xen/include/asm-x86/domain.h |    2 ++
 xen/include/asm-x86/pqos.h   |    4 +++
 xen/include/public/domctl.h  |    3 +++
 xen/include/xen/sched.h      |    3 +++
 7 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a3868f9..9725649 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -60,6 +60,7 @@
 #include <xen/numa.h>
 #include <xen/iommu.h>
 #include <compat/vcpu.h>
+#include <asm/pqos.h>
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
 DEFINE_PER_CPU(unsigned long, cr4);
@@ -579,6 +580,11 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
     tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
     spin_lock_init(&d->arch.vtsc_lock);
 
+    /* Allocate CQM RMID for guest */
+    d->arch.pqos_cqm_rmid = 0;
+    if ( system_supports_cqm() && !!(domcr_flags & DOMCRF_pqos_cqm)
)
+        alloc_cqm_rmid(d);
+
     return 0;
 
  fail:
@@ -612,6 +618,9 @@ void arch_domain_destroy(struct domain *d)
 
     free_xenheap_page(d->shared_info);
     cleanup_domain_irq_mapping(d);
+
+    if ( system_supports_cqm() )
+        free_cqm_rmid(d);
 }
 
 unsigned long pv_guest_cr4_fixup(const struct vcpu *v, unsigned long guest_cr4)
diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
index e6ab416..e294799 100644
--- a/xen/arch/x86/pqos.c
+++ b/xen/arch/x86/pqos.c
@@ -20,6 +20,8 @@
  */
 #include <asm/processor.h>
 #include <xen/init.h>
+#include <xen/spinlock.h>
+#include <xen/sched.h>
 #include <asm/pqos.h>
 
 static bool_t pqos_enabled = 1;
@@ -29,6 +31,7 @@ unsigned int cqm_res_count = 0;
 unsigned int cqm_upscaling_factor = 0;
 bool_t cqm_enabled = 0;
 struct cqm_res_struct *cqm_res_array = NULL;
+static DEFINE_SPINLOCK(cqm_lock);
 
 static void __init init_cqm(void)
 {
@@ -78,6 +81,62 @@ void __init init_platform_qos(void)
     init_qos_monitor();
 }
 
+bool_t system_supports_cqm(void)
+{
+    return cqm_enabled;
+}
+
+int alloc_cqm_rmid(struct domain *d)
+{
+    int rmid, rc = 0;
+    unsigned long flags;
+
+    ASSERT(system_supports_cqm());
+
+    spin_lock_irqsave(&cqm_lock, flags);
+    /* RMID=0 is reserved, enumerate from 1 */
+    for ( rmid = 1; rmid < cqm_res_count; rmid++ )
+    {
+        if ( cqm_res_array[rmid].inuse)
+            continue;
+
+        cqm_res_array[rmid].inuse = 1;
+        cqm_res_array[rmid].domain_id = d->domain_id;
+        break;
+    }
+    spin_unlock_irqrestore(&cqm_lock, flags);
+
+    /* No CQM RMID available, assign RMID=0 by default */
+    if ( rmid == cqm_res_count )
+    {
+        rmid = 0;
+        rc = -1;
+    }
+
+    d->arch.pqos_cqm_rmid = rmid;
+
+    return rc;
+}
+
+void free_cqm_rmid(struct domain *d)
+{
+    int rmid = d->arch.pqos_cqm_rmid;
+    unsigned long flags;
+
+    ASSERT(system_supports_cqm());
+
+    spin_lock_irqsave(&cqm_lock, flags);
+    /* We do not free system reserved "RMID=0" */
+    if ( rmid > 0 )
+    {
+        cqm_res_array[rmid].inuse = 0;
+        cqm_res_array[rmid].domain_id = 0;
+    }
+    spin_unlock_irqrestore(&cqm_lock, flags);
+
+    d->arch.pqos_cqm_rmid = 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 904d27b..1c2e320 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -425,7 +425,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
                | XEN_DOMCTL_CDF_pvh_guest
                | XEN_DOMCTL_CDF_hap
                | XEN_DOMCTL_CDF_s3_integrity
-               | XEN_DOMCTL_CDF_oos_off)) )
+               | XEN_DOMCTL_CDF_oos_off
+               | XEN_DOMCTL_CDF_pqos_cqm)) )
             break;
 
         dom = op->domain;
@@ -467,6 +468,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
             domcr_flags |= DOMCRF_s3_integrity;
         if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off )
             domcr_flags |= DOMCRF_oos_off;
+        if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_pqos_cqm )
+            domcr_flags |= DOMCRF_pqos_cqm;
 
         d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref);
         if ( IS_ERR(d) )
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 9d39061..b0479aa 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -313,6 +313,8 @@ struct arch_domain
     spinlock_t e820_lock;
     struct e820entry *e820;
     unsigned int nr_e820;
+
+    int pqos_cqm_rmid;       /* CQM RMID assigned to the domain */
 } __cacheline_aligned;
 
 #define has_arch_pdevs(d)    (!list_empty(&(d)->arch.pdev_list))
diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
index 934d68a..88de139 100644
--- a/xen/include/asm-x86/pqos.h
+++ b/xen/include/asm-x86/pqos.h
@@ -34,4 +34,8 @@ struct cqm_res_struct {
 
 void init_platform_qos(void);
 
+bool_t system_supports_cqm(void);
+int alloc_cqm_rmid(struct domain *);
+void free_cqm_rmid(struct domain *);
+
 #endif
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 01a3652..47a850a 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -62,6 +62,9 @@ struct xen_domctl_createdomain {
  /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
 #define _XEN_DOMCTL_CDF_pvh_guest     4
 #define XEN_DOMCTL_CDF_pvh_guest      (1U<<_XEN_DOMCTL_CDF_pvh_guest)
+ /* Enable pqos-cqm? */
+#define _XEN_DOMCTL_CDF_pqos_cqm      5
+#define XEN_DOMCTL_CDF_pqos_cqm       (1U<<_XEN_DOMCTL_CDF_pqos_cqm)
     uint32_t flags;
 };
 typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index cbdf377..3a42656 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -507,6 +507,9 @@ struct domain *domain_create(
  /* DOMCRF_pvh: Create PV domain in HVM container. */
 #define _DOMCRF_pvh             5
 #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
+ /* DOMCRF_pqos_cqm: Create a domain with CQM support */
+#define _DOMCRF_pqos_cqm        6
+#define DOMCRF_pqos_cqm         (1U<<_DOMCRF_pqos_cqm)
 
 /*
  * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

If a guest is created with parameter "pqos_cqm" set to 1, an RMID
will be allocated to the guest. This RMID will be relinquished when
guest is destroyed.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 tools/libxl/libxl_create.c  |    3 +++
 tools/libxl/libxl_types.idl |    1 +
 tools/libxl/xl_cmdimpl.c    |    2 ++
 3 files changed, 6 insertions(+)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 5e9cdcc..454c69d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -40,6 +40,8 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
 
     libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
 
+    libxl_defbool_setdefault(&c_info->pqos_cqm, false);
+
     return 0;
 }
 
@@ -454,6 +456,7 @@ int libxl__domain_make(libxl__gc *gc,
libxl_domain_create_info *info,
         }
         flags |= XEN_DOMCTL_CDF_hap;
     }
+    flags |= libxl_defbool_val(info->pqos_cqm) ? XEN_DOMCTL_CDF_pqos_cqm :
0;
     *domid = -1;
 
     /* Ultimately, handle is an array of 16 uint8_t, same as uuid */
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index de5bac3..22688d8 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -275,6 +275,7 @@ libxl_domain_create_info =
Struct("domain_create_info",[
     ("poolid",       uint32),
     ("run_hotplug_scripts",libxl_defbool),
     ("pvh",          libxl_defbool),
+    ("pqos_cqm",     libxl_defbool),
     ], dir=DIR_IN)
 
 libxl_domain_restore_params = Struct("domain_restore_params", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 8690ec7..84a604f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -670,6 +670,8 @@ static void parse_config_data(const char *config_source,
         exit(1);
     }
 
+    xlu_cfg_get_defbool(config, "pqos_cqm", &c_info->pqos_cqm,
0);
+
     libxl_domain_build_info_init_type(b_info, c_info->type);
     if (blkdev_start)
         b_info->blkdev_start = strdup(blkdev_start);
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Add hypervisor side support for dynamically attach and detach CQM
services for a certain guest.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 xen/arch/x86/domctl.c       |   40 ++++++++++++++++++++++++++++++++++++++++
 xen/include/public/domctl.h |   14 ++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index f7e4586..5ef21f9 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -35,6 +35,7 @@
 #include <asm/mem_sharing.h>
 #include <asm/xstate.h>
 #include <asm/debugger.h>
+#include <asm/pqos.h>
 
 static int gdbsx_guest_mem_io(
     domid_t domid, struct xen_domctl_gdbsx_memio *iop)
@@ -1223,6 +1224,45 @@ long arch_do_domctl(
     }
     break;
 
+    case XEN_DOMCTL_attach_pqos:
+    {
+        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
+        {
+            if ( !system_supports_cqm() )
+                ret = -ENODEV;
+            else if ( d->arch.pqos_cqm_rmid > 0 )
+                ret = -EINVAL;
+            else
+            {
+                ret = alloc_cqm_rmid(d);
+                if ( ret < 0 )
+                    ret = -EBUSY;
+            }
+        }
+        else
+            ret = -EINVAL;
+    }
+    break;
+
+    case XEN_DOMCTL_detach_pqos:
+    {
+        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
+        {
+            if ( !system_supports_cqm() )
+                ret = -ENODEV;
+            else if ( d->arch.pqos_cqm_rmid > 0 )
+            {
+                free_cqm_rmid(d);
+                ret = 0;
+            }
+            else
+                ret = -EINVAL;
+        }
+        else
+            ret = -EINVAL;
+    }
+    break;
+
     default:
         ret = iommu_do_domctl(domctl, d, u_domctl);
         break;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 47a850a..f5d7062 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -872,6 +872,17 @@ struct xen_domctl_set_max_evtchn {
 typedef struct xen_domctl_set_max_evtchn xen_domctl_set_max_evtchn_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_max_evtchn_t);
 
+/* XEN_DOMCTL_attach_pqos */
+/* XEN_DOMCTL_detach_pqos */
+struct xen_domctl_qos_resource {
+ /* Attach or detach flag for cqm */
+#define _XEN_DOMCTL_ADF_pqos_cqm      0
+#define XEN_DOMCTL_ADF_pqos_cqm       (1U<<_XEN_DOMCTL_ADF_pqos_cqm)
+    uint32_t flags;
+};
+typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
@@ -941,6 +952,8 @@ struct xen_domctl {
 #define XEN_DOMCTL_setnodeaffinity               68
 #define XEN_DOMCTL_getnodeaffinity               69
 #define XEN_DOMCTL_set_max_evtchn                70
+#define XEN_DOMCTL_attach_pqos                   71
+#define XEN_DOMCTL_detach_pqos                   72
 #define XEN_DOMCTL_gdbsx_guestmemio            1000
 #define XEN_DOMCTL_gdbsx_pausevcpu             1001
 #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
@@ -1001,6 +1014,7 @@ struct xen_domctl {
         struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
         struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
         struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
+        struct xen_domctl_qos_resource      qos_res;
         uint8_t                             pad[128];
     } u;
 };
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Add two new xl instruction for attaching and detaching CQM services
for a running guest.

Above "qos_res" is the monitoring resource type, and current supported
type is "cqm".

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 tools/libxc/xc_domain.c   |   18 ++++++++++
 tools/libxc/xenctrl.h     |    2 ++
 tools/libxl/Makefile      |    3 +-
 tools/libxl/libxl.h       |    3 ++
 tools/libxl/libxl_pqos.c  |   88 +++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/xl.h          |    2 ++
 tools/libxl/xl_cmdimpl.c  |   36 +++++++++++++++++++
 tools/libxl/xl_cmdtable.c |   10 ++++++
 8 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_pqos.c

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 1ccafc5..85c2d4d 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1776,6 +1776,24 @@ int xc_domain_set_max_evtchn(xc_interface *xch, uint32_t
domid,
     return do_domctl(xch, &domctl);
 }
 
+int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags)
+{
+    DECLARE_DOMCTL;
+    domctl.cmd = XEN_DOMCTL_attach_pqos;
+    domctl.domain = (domid_t)domid;
+    domctl.u.qos_res.flags = flags;
+    return do_domctl(xch, &domctl);
+}
+
+int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags)
+{
+    DECLARE_DOMCTL;
+    domctl.cmd = XEN_DOMCTL_detach_pqos;
+    domctl.domain = (domid_t)domid;
+    domctl.u.qos_res.flags = flags;
+    return do_domctl(xch, &domctl);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index 4ac6b8a..a57f147 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2395,4 +2395,6 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
  */
 int xc_kexec_unload(xc_interface *xch, int type);
 
+int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags);
+int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags);
 #endif /* XENCTRL_H */
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index cf214bb..35f0b97 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -74,7 +74,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o
libxl_pci.o \
 			libxl_internal.o libxl_utils.o libxl_uuid.o \
 			libxl_json.o libxl_aoutils.o libxl_numa.o \
 			libxl_save_callout.o _libxl_save_msgs_callout.o \
-			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+			libxl_qmp.o libxl_event.o libxl_fork.o libxl_pqos.o \
+			$(LIBXL_OBJS-y)
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
 
 $(LIBXL_OBJS): CFLAGS += $(CFLAGS_LIBXL) -include $(XEN_ROOT)/tools/config.h
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index c7dceda..a9a506f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1051,6 +1051,9 @@ int libxl_flask_getenforce(libxl_ctx *ctx);
 int libxl_flask_setenforce(libxl_ctx *ctx, int mode);
 int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
 
+int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
+int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
+
 /* misc */
 
 /* Each of these sets or clears the flag according to whether the
diff --git a/tools/libxl/libxl_pqos.c b/tools/libxl/libxl_pqos.c
new file mode 100644
index 0000000..e92b94b
--- /dev/null
+++ b/tools/libxl/libxl_pqos.c
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2013      Intel Corporation
+ * Author Jiongxi Li <jiongxi.li@intel.com>
+ * Author Dongxiao Xu <dongxiao.xu@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxl_internal.h"
+
+int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
+{
+    int rc;
+    int flags = 0;
+
+    if (!strcmp(qosres, "cqm"))
+        flags |= XEN_DOMCTL_ADF_pqos_cqm;
+    else {
+        rc = -EINVAL;
+        fprintf(stderr, "Invalid QoS resource type! "
+                        "Supported types are: \"cqm\".\n");
+        return rc;
+    }
+
+    rc = xc_domain_pqos_attach(ctx->xch, domid, flags);
+    if (rc < 0) {
+        fprintf(stderr, "Failed to attach CQM service to domain %d.
", domid);
+        if (errno == ENODEV)
+            fprintf(stderr, "CQM is not supported in this
system.\n");
+        else if (errno == EBUSY)
+            fprintf(stderr, "There is no free CQM RMID
available.\n");
+        else if (errno == EINVAL)
+            fprintf(stderr, "Domain %d is already attached with CQM
service.\n", domid);
+        else if (errno == ESRCH)
+            fprintf(stderr, "Is Domain %d a valid domain?\n", domid);
+        else
+            fprintf(stderr, "errno: %d\n", errno);
+    }
+
+    return rc;
+}
+
+int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
+{
+    int rc;
+    int flags = 0;
+
+    if (!strcmp(qosres, "cqm"))
+        flags |= XEN_DOMCTL_ADF_pqos_cqm;
+    else {
+        rc = -EINVAL;
+        fprintf(stderr, "Invalid QoS resource type! "
+                        "Supported types are: \"cqm\".\n");
+        return rc;
+    }
+
+    rc = xc_domain_pqos_detach(ctx->xch, domid, flags);
+    if (rc < 0) {
+        fprintf(stderr, "Failed to detach CQM service to domain %d.
", domid);
+        if (errno == ENODEV)
+            fprintf(stderr, "CQM is not supported in this
system.\n");
+        else if (errno == EINVAL)
+            fprintf(stderr, "Domain %d is not attached with CQM
service.\n", domid);
+        else if (errno == ESRCH)
+            fprintf(stderr, "Is Domain %d a valid domain?\n", domid);
+        else
+            fprintf(stderr, "errno: %d\n", errno);
+    }
+
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index e005c39..78738b9 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -105,6 +105,8 @@ int main_getenforce(int argc, char **argv);
 int main_setenforce(int argc, char **argv);
 int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
+int main_pqosattach(int argc, char **argv);
+int main_pqosdetach(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 84a604f..1a12e8f 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7195,6 +7195,42 @@ int main_remus(int argc, char **argv)
     return -ERROR_FAIL;
 }
 
+int main_pqosattach(int argc, char **argv)
+{
+    uint32_t domid;
+    int opt, rc;
+    const char *qosres = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-attach", 2) {
+        /* No options */
+    }
+
+    qosres = argv[optind];
+    domid = find_domain(argv[optind + 1]);
+
+    rc = libxl_pqos_attach(ctx, domid, qosres);
+
+    return rc;
+}
+
+int main_pqosdetach(int argc, char **argv)
+{
+    uint32_t domid;
+    int opt, rc;
+    const char *qosres = NULL;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-detach", 2) {
+        /* No options */
+    }
+
+    qosres = argv[optind];
+    domid = find_domain(argv[optind + 1]);
+
+    rc = libxl_pqos_detach(ctx, domid, qosres);
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 326a660..02a2572 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -488,6 +488,16 @@ struct cmd_spec cmd_table[] = {
       "                        of the domain."
 
     },
+    { "pqos-attach",
+      &main_pqosattach, 0, 1,
+      "Allocate and map qos resource",
+      "<Resource> <Domain>",
+    },
+    { "pqos-detach",
+      &main_pqosdetach, 0, 1,
+      "Reliquish qos resource",
+      "<Resource> <Domain>",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Retrive CQM information for certain domain, which reflects the L3 cache
occupancy for a socket.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 xen/arch/x86/pqos.c             |   57 ++++++++++++++++++++++++++++++++++
 xen/arch/x86/sysctl.c           |   64 +++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/msr-index.h |    4 +++
 xen/include/asm-x86/pqos.h      |   14 +++++++++
 xen/include/public/domctl.h     |   14 +++++++++
 xen/include/public/sysctl.h     |   15 +++++++++
 6 files changed, 168 insertions(+)

diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
index e294799..d95784c 100644
--- a/xen/arch/x86/pqos.c
+++ b/xen/arch/x86/pqos.c
@@ -19,14 +19,30 @@
  * Place - Suite 330, Boston, MA 02111-1307 USA.
  */
 #include <asm/processor.h>
+#include <asm/msr.h>
 #include <xen/init.h>
 #include <xen/spinlock.h>
 #include <xen/sched.h>
+#include <public/domctl.h>
 #include <asm/pqos.h>
 
 static bool_t pqos_enabled = 1;
 boolean_param("pqos", pqos_enabled);
 
+static void read_qm_data(void *arg)
+{
+    struct qm_element *qm_element = arg;
+
+    wrmsr(MSR_IA32_QOSEVTSEL, qm_element->evtid, qm_element->rmid);
+    rdmsrl(MSR_IA32_QMC, qm_element->qm_data);
+}
+
+static void get_generic_qm_info(struct qm_element *qm_element)
+{
+    unsigned int cpu = qm_element->cpu;
+    on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
+}
+
 unsigned int cqm_res_count = 0;
 unsigned int cqm_upscaling_factor = 0;
 bool_t cqm_enabled = 0;
@@ -86,6 +102,23 @@ bool_t system_supports_cqm(void)
     return cqm_enabled;
 }
 
+unsigned int get_cqm_count(void)
+{
+    return cqm_res_count;
+}
+
+unsigned int get_cqm_avail(void)
+{
+    unsigned int cqm_avail = 0;
+    int i;
+
+    for ( i = 0; i < cqm_res_count; i++ )
+        if ( !cqm_res_array[i].inuse )
+            cqm_avail++;
+
+    return cqm_avail;
+}
+
 int alloc_cqm_rmid(struct domain *d)
 {
     int rmid, rc = 0;
@@ -137,6 +170,30 @@ void free_cqm_rmid(struct domain *d)
     d->arch.pqos_cqm_rmid = 0;
 }
 
+void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
+                  struct xen_domctl_getdomcqminfo *info)
+{
+    struct qm_element element;
+    unsigned int cpu, i;
+
+    for_each_cpu ( cpu, &cpu_cqmdata_map )
+    {
+        element.cpu = cpu;
+        element.rmid = rmid;
+        element.evtid = QOS_MONITOR_EVTID_L3;
+
+        get_generic_qm_info(&element);
+
+        i = cpu_to_socket(cpu);
+        info->socket_cqmdata[i].valid +            (element.qm_data &
IA32_QM_CTR_ERROR_MASK) ? 0 : 1;
+        if ( info->socket_cqmdata[i].valid )
+            info->socket_cqmdata[i].l3c_occupancy = element.qm_data *
cqm_upscaling_factor;
+        else
+            info->socket_cqmdata[i].l3c_occupancy = 0;
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 15d4b91..d631769 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -28,6 +28,7 @@
 #include <xen/nodemask.h>
 #include <xen/cpu.h>
 #include <xsm/xsm.h>
+#include <asm/pqos.h>
 
 #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
 
@@ -101,6 +102,69 @@ long arch_do_sysctl(
     }
     break;
 
+    case XEN_SYSCTL_getdomcqminfolist:
+    {
+        struct domain *d;
+        struct xen_domctl_getdomcqminfo info;
+        uint32_t resource_count;
+        uint32_t resource_avail;
+        uint32_t num_domains = 0;
+        cpumask_t cpu_cqmdata_map;
+        DECLARE_BITMAP(sockets, QOS_MAX_SOCKETS);
+        unsigned int cpu;
+
+        if ( !system_supports_cqm() )
+        {
+            ret = -ENODEV;
+            break;
+        }
+
+        resource_count = get_cqm_count();
+        resource_avail = get_cqm_avail();
+
+        cpumask_clear(&cpu_cqmdata_map);
+        bitmap_zero(sockets, QOS_MAX_SOCKETS);
+        for_each_online_cpu(cpu)
+        {
+            int i = cpu_to_socket(cpu);
+            if ( test_and_set_bit(i, sockets) )
+                continue;
+            cpumask_set_cpu(cpu, &cpu_cqmdata_map);
+        }
+
+        rcu_read_lock(&domlist_read_lock);
+        for_each_domain ( d )
+        {
+            if ( d->domain_id <
sysctl->u.getdomaininfolist.first_domain )
+                continue;
+            if ( num_domains == sysctl->u.getdomaininfolist.max_domains )
+                break;
+            if ( d->arch.pqos_cqm_rmid <= 0 )
+                continue;
+            memset(&info, 0, sizeof(struct xen_domctl_getdomcqminfo));
+            info.domain = d->domain_id;
+            get_cqm_info(d->arch.pqos_cqm_rmid, cpu_cqmdata_map, &info);
+
+            if ( copy_to_guest_offset(sysctl->u.getdomcqminfolist.buffer,
+                                      num_domains, &info, 1) )
+            {
+                ret = -EFAULT;
+                break;
+            }
+
+            num_domains++;
+        }
+        rcu_read_unlock(&domlist_read_lock);
+
+        sysctl->u.getdomcqminfolist.num_domains = num_domains;
+        sysctl->u.getdomcqminfolist.resource_count = resource_count;
+        sysctl->u.getdomcqminfolist.resource_avail = resource_avail;
+
+        if ( copy_to_guest(u_sysctl, sysctl, 1) )
+            ret = -EFAULT;
+    }
+    break;
+
     default:
         ret = -ENOSYS;
         break;
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index e597a28..46ef165 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -488,4 +488,8 @@
 /* Geode defined MSRs */
 #define MSR_GEODE_BUSCONT_CONF0		0x00001900
 
+/* Platform QoS register */
+#define MSR_IA32_QOSEVTSEL             0x00000c8d
+#define MSR_IA32_QMC                   0x00000c8e
+
 #endif /* __ASM_MSR_INDEX_H */
diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
index 88de139..5c86c5d 100644
--- a/xen/include/asm-x86/pqos.h
+++ b/xen/include/asm-x86/pqos.h
@@ -27,15 +27,29 @@
 /* QoS Monitoring Event ID */
 #define QOS_MONITOR_EVTID_L3           0x1
 
+/* IA32_QM_CTR */
+#define IA32_QM_CTR_ERROR_MASK         (0x3ul << 62)
+
 struct cqm_res_struct {
     domid_t  domain_id;
     bool_t   inuse;
 };
 
+struct qm_element {
+    uint64_t  qm_data;
+    uint32_t  cpu;
+    uint32_t  rmid;
+    uint8_t   evtid;
+};
+
 void init_platform_qos(void);
 
 bool_t system_supports_cqm(void);
 int alloc_cqm_rmid(struct domain *);
 void free_cqm_rmid(struct domain *);
+unsigned int get_cqm_count(void);
+unsigned int get_cqm_avail(void);
+void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
+                  struct xen_domctl_getdomcqminfo *info);
 
 #endif
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index f5d7062..fe8b37f 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -883,6 +883,20 @@ struct xen_domctl_qos_resource {
 typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
 DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
 
+struct xen_socket_cqmdata {
+    uint64_t l3c_occupancy;
+    uint8_t  valid;
+};
+
+struct xen_domctl_getdomcqminfo {
+    /* OUT variables. */
+    domid_t  domain;
+#define QOS_MAX_SOCKETS    128
+    struct xen_socket_cqmdata socket_cqmdata[QOS_MAX_SOCKETS];
+};
+typedef struct xen_domctl_getdomcqminfo xen_domctl_getdomcqminfo_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_getdomcqminfo_t);
+
 struct xen_domctl {
     uint32_t cmd;
 #define XEN_DOMCTL_createdomain                   1
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 8437d31..0def306 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -632,6 +632,19 @@ struct xen_sysctl_coverage_op {
 typedef struct xen_sysctl_coverage_op xen_sysctl_coverage_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
 
+/* XEN_SYSCTL_getdomcqminfolist */
+struct xen_sysctl_getdomcqminfolist {
+    /* IN variables. */
+    domid_t               first_domain;
+    uint32_t              max_domains;
+    XEN_GUEST_HANDLE_64(xen_domctl_getdomcqminfo_t) buffer;
+    /* OUT variables. */
+    uint32_t              num_domains;
+    uint32_t              resource_count;
+    uint32_t              resource_avail;
+};
+typedef struct xen_sysctl_getdomcqminfolist xen_sysctl_getdomcqminfolist_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_getdomcqminfolist_t);
 
 struct xen_sysctl {
     uint32_t cmd;
@@ -654,6 +667,7 @@ struct xen_sysctl {
 #define XEN_SYSCTL_cpupool_op                    18
 #define XEN_SYSCTL_scheduler_op                  19
 #define XEN_SYSCTL_coverage_op                   20
+#define XEN_SYSCTL_getdomcqminfolist             21
     uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
     union {
         struct xen_sysctl_readconsole       readconsole;
@@ -675,6 +689,7 @@ struct xen_sysctl {
         struct xen_sysctl_cpupool_op        cpupool_op;
         struct xen_sysctl_scheduler_op      scheduler_op;
         struct xen_sysctl_coverage_op       coverage_op;
+        struct xen_sysctl_getdomcqminfolist getdomcqminfolist;
         uint8_t                             pad[128];
     } u;
 };
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

Retrive CQM information for certain domain, which reflects the L3 cache
occupancy for a socket.

The xl command is "xl pqos-list cqm (domid)".

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 tools/libxc/xc_domain.c   |   33 ++++++++++++++++
 tools/libxc/xenctrl.h     |   12 ++++++
 tools/libxl/libxl.h       |    5 +++
 tools/libxl/libxl_pqos.c  |   14 +++++++
 tools/libxl/xl.h          |    1 +
 tools/libxl/xl_cmdimpl.c  |   91 +++++++++++++++++++++++++++++++++++++++++++++
 tools/libxl/xl_cmdtable.c |    5 +++
 7 files changed, 161 insertions(+)

diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 85c2d4d..96a1d50 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -1794,6 +1794,39 @@ int xc_domain_pqos_detach(xc_interface *xch, uint32_t
domid, int flags)
     return do_domctl(xch, &domctl);
 }
 
+int xc_domain_getcqminfolist(xc_interface *xch,
+                             uint32_t first_domain,
+                             unsigned int max_domains,
+                             sysctl_cqminfo_t *info)
+{
+    int ret = 0;
+    xen_domctl_getdomcqminfo_t *d_info = info->dom_cqminfo;
+    DECLARE_SYSCTL;
+
+    DECLARE_HYPERCALL_BOUNCE(d_info, max_domains*sizeof(*d_info),
XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+    if ( xc_hypercall_bounce_pre(xch, d_info) )
+        return -1;
+
+    sysctl.cmd = XEN_SYSCTL_getdomcqminfolist;
+    sysctl.u.getdomcqminfolist.first_domain = first_domain;
+    sysctl.u.getdomcqminfolist.max_domains  = max_domains;
+    set_xen_guest_handle(sysctl.u.getdomcqminfolist.buffer, d_info);
+
+    if ( xc_sysctl(xch, &sysctl) < 0 )
+        ret = -1;
+    else
+    {
+        ret = sysctl.u.getdomcqminfolist.num_domains;
+        info->resource_count = sysctl.u.getdomcqminfolist.resource_count;
+        info->resource_avail = sysctl.u.getdomcqminfolist.resource_avail;
+    }
+
+    xc_hypercall_bounce_post(xch, d_info);
+
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
index a57f147..96a057f 100644
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -2395,6 +2395,18 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
  */
 int xc_kexec_unload(xc_interface *xch, int type);
 
+struct xen_sysctl_getcqminfo
+{
+    uint32_t resource_count;
+    uint32_t resource_avail;
+    xen_domctl_getdomcqminfo_t *dom_cqminfo;
+};
+typedef struct xen_sysctl_getcqminfo sysctl_cqminfo_t;
+
 int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags);
 int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags);
+int xc_domain_getcqminfolist(xc_interface *xch,
+                             uint32_t first_domain,
+                             unsigned int max_domains,
+                             sysctl_cqminfo_t *info);
 #endif /* XENCTRL_H */
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index a9a506f..530363c 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -285,6 +285,7 @@
 
 #include <libxl_uuid.h>
 #include <_libxl_list.h>
+#include <xenctrl.h>
 
 /* API compatibility. */
 #ifdef LIBXL_API_VERSION
@@ -1053,6 +1054,10 @@ int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy,
uint32_t size);
 
 int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
 int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres);
+int libxl_list_domain_cqm(libxl_ctx *ctx,
+                          uint32_t first_domain,
+                          unsigned int max_domains,
+                          sysctl_cqminfo_t *info);
 
 /* misc */
 
diff --git a/tools/libxl/libxl_pqos.c b/tools/libxl/libxl_pqos.c
index e92b94b..e5fa592 100644
--- a/tools/libxl/libxl_pqos.c
+++ b/tools/libxl/libxl_pqos.c
@@ -79,6 +79,20 @@ int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const
char * qosres)
     return rc;
 }
 
+int libxl_list_domain_cqm(libxl_ctx *ctx,
+                          uint32_t first_domain,
+                          unsigned int max_domains,
+                          sysctl_cqminfo_t *info)
+{
+    int ret;
+
+    ret = xc_domain_getcqminfolist(ctx->xch, first_domain, max_domains,
info);
+    if (ret < 0)
+        return -EINVAL;
+
+    return ret;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 78738b9..994d3be 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -107,6 +107,7 @@ int main_loadpolicy(int argc, char **argv);
 int main_remus(int argc, char **argv);
 int main_pqosattach(int argc, char **argv);
 int main_pqosdetach(int argc, char **argv);
+int main_pqoslist(int argc, char **argv);
 
 void help(const char *command);
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 1a12e8f..52c1321 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -7231,6 +7231,97 @@ int main_pqosdetach(int argc, char **argv)
     return rc;
 }
 
+static void print_cqm_info(const sysctl_cqminfo_t *info, int nb_domain)
+{
+    int i, j;
+    int socket_num = 0;
+    xen_domctl_getdomcqminfo_t *dom_cqminfo;
+
+    printf("RMID count %5d\tRMID available %5d\n",
info->resource_count, info->resource_avail);
+    if (info->resource_count == 0)
+        printf("System doesn''t supoort CQM\n");
+    else if (info->resource_count - info->resource_avail == 1)
+        printf("No RMID is assigned to domains\n");
+    else if (!nb_domain)
+        printf("No RMID is assgined to your monitoring domains\n");
+    else {
+        printf("Name                                        ID");
+        do {
+            printf("\tSocketID\tL3C_Usage");
+            socket_num++;
+        } while (info->dom_cqminfo->socket_cqmdata[socket_num].valid);
+        printf("\n");
+        for (i = 0; i < nb_domain; i++) {
+            char *domname;
+            dom_cqminfo = info->dom_cqminfo + i;
+            domname = libxl_domid_to_name(ctx, dom_cqminfo->domain);
+            printf("%-40s %5d",
+                    domname,
+                    dom_cqminfo->domain);
+            free(domname);
+            for(j = 0; j < socket_num; j++)
+                printf("%10u %16lu     ", j,
dom_cqminfo->socket_cqmdata[j].l3c_occupancy);
+            printf("\n");
+        }
+    }
+}
+int main_pqoslist(int argc, char **argv)
+{
+    int opt;
+    const char *qosres = NULL;
+    uint32_t first_domain;
+    unsigned int max_domains;
+    int num_domains;
+    sysctl_cqminfo_t info;
+
+    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-list", 1) {
+        /* No options */
+    }
+
+    qosres = argv[optind];
+
+    if (!strcmp(qosres, "cqm")) {
+        if (optind + 1 >= argc) {
+            first_domain = 0;
+            max_domains = 1024;
+        } else if (optind + 1 == argc - 1) {
+            first_domain = find_domain(argv[optind + 1]);
+            max_domains = 1;
+        } else {
+            help("pqos-list");
+            return 2;
+        }
+
+        info.dom_cqminfo = calloc(max_domains,
+                                  sizeof(xen_domctl_getdomcqminfo_t));
+        if (!info.dom_cqminfo) {
+            fprintf(stderr, "allocating domain cqminfo failed.\n");
+            return ERROR_FAIL;
+        }
+
+        num_domains = libxl_list_domain_cqm(ctx, first_domain,
+                                            max_domains, &info);
+
+        if (num_domains < 0) {
+            fprintf(stderr, "Failed to get domain CQM info, "
+                            "check whether CQM feature is
supported.\n");
+            if (info.dom_cqminfo)
+                free(info.dom_cqminfo);
+            return 1;
+        }
+        print_cqm_info(&info, num_domains);
+
+        if (info.dom_cqminfo)
+            free(info.dom_cqminfo);
+    } else {
+        fprintf(stderr, "qos resource type supported is: cqm\n");
+        help("pqos-list");
+        return 2;
+    }
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 02a2572..6ced416 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -498,6 +498,11 @@ struct cmd_spec cmd_table[] = {
       "Reliquish qos resource",
       "<Resource> <Domain>",
     },
+    { "pqos-list",
+      &main_pqoslist, 0, 0,
+      "List qos information about all/some domains",
+      "<Resource> [Domain]",
+    },
 };
 
 int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
-- 
1.7.9.5

From: Dongxiao Xu <dongxiao.xu@intel.com>

If the CQM service is attached to a domain, its related RMID will be set
to hardware for monitoring when the domain''s vcpu is scheduled in. When
the domain''s vcpu is scheduled out, RMID 0 (system reserved) will be
set
for monitoring.

Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
---
 xen/arch/x86/domain.c           |    5 +++++
 xen/arch/x86/pqos.c             |   21 ++++++++++++++++++++-
 xen/include/asm-x86/msr-index.h |    1 +
 xen/include/asm-x86/pqos.h      |    1 +
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 9725649..1eda0ab 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1372,6 +1372,8 @@ static void __context_switch(void)
     {
         memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES);
         vcpu_save_fpu(p);
+        if ( system_supports_cqm() )
+            cqm_assoc_rmid(0);
         p->arch.ctxt_switch_from(p);
     }
 
@@ -1396,6 +1398,9 @@ static void __context_switch(void)
         }
         vcpu_restore_fpu_eager(n);
         n->arch.ctxt_switch_to(n);
+
+        if ( system_supports_cqm() &&
n->domain->arch.pqos_cqm_rmid > 0 )
+            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);
     }
 
     gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
index d95784c..0cec172 100644
--- a/xen/arch/x86/pqos.c
+++ b/xen/arch/x86/pqos.c
@@ -27,6 +27,7 @@
 #include <asm/pqos.h>
 
 static bool_t pqos_enabled = 1;
+static unsigned int rmid_bitwidth;
 boolean_param("pqos", pqos_enabled);
 
 static void read_qm_data(void *arg)
@@ -43,6 +44,14 @@ static void get_generic_qm_info(struct qm_element
*qm_element)
     on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
 }
 
+void cqm_assoc_rmid(uint16_t rmid)
+{
+    uint64_t val;
+    uint64_t rmid_mask = ~(~0ull << rmid_bitwidth);
+    rdmsrl(MSR_IA32_PQR_ASSOC, val);
+    wrmsrl(MSR_IA32_PQR_ASSOC, (val & ~(rmid_mask)) | (rmid &
rmid_mask));
+}
+
 unsigned int cqm_res_count = 0;
 unsigned int cqm_upscaling_factor = 0;
 bool_t cqm_enabled = 0;
@@ -78,13 +87,23 @@ static void __init init_cqm(void)
 static void __init init_qos_monitor(void)
 {
     unsigned int qm_features;
-    unsigned int eax, ebx, ecx;
+    unsigned int eax, ebx, ecx, i = 0;
 
     if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
         return;
 
     cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
 
+    while (1)
+    {
+        if ( (1 << i) - 1 >= ebx )
+        {
+            rmid_bitwidth = i;
+            break;
+        }
+        i++;
+    }
+
     if ( qm_features & QOS_MONITOR_TYPE_L3 )
         init_cqm();
 }
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 46ef165..45f4918 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -491,5 +491,6 @@
 /* Platform QoS register */
 #define MSR_IA32_QOSEVTSEL             0x00000c8d
 #define MSR_IA32_QMC                   0x00000c8e
+#define MSR_IA32_PQR_ASSOC             0x00000c8f
 
 #endif /* __ASM_MSR_INDEX_H */
diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
index 5c86c5d..2bb983b 100644
--- a/xen/include/asm-x86/pqos.h
+++ b/xen/include/asm-x86/pqos.h
@@ -51,5 +51,6 @@ unsigned int get_cqm_count(void);
 unsigned int get_cqm_avail(void);
 void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
                   struct xen_domctl_getdomcqminfo *info);
+void cqm_assoc_rmid(uint16_t rmid);
 
 #endif
-- 
1.7.9.5

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> Detect platform QoS feature status and enumerate the resource types,
> one of which is to monitor the L3 cache occupancy.
>
> Also introduce a Xen grub command line parameter to control the
> QoS feature status globally.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/Makefile            |    1 +
>  xen/arch/x86/cpu/intel.c         |    6 +++
>  xen/arch/x86/pqos.c              |   89
++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/setup.c             |    3 ++
>  xen/include/asm-x86/cpufeature.h |    1 +
>  xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
>  6 files changed, 137 insertions(+)
>  create mode 100644 xen/arch/x86/pqos.c
>  create mode 100644 xen/include/asm-x86/pqos.h
>
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index d502bdf..54962e0 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -58,6 +58,7 @@ obj-y += crash.o
>  obj-y += tboot.o
>  obj-y += hpet.o
>  obj-y += xstate.o
> +obj-y += pqos.o
>  
>  obj-$(crash_debug) += gdbstub.o
>  
> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> index 27fe762..f0d83ea 100644
> --- a/xen/arch/x86/cpu/intel.c
> +++ b/xen/arch/x86/cpu/intel.c
> @@ -230,6 +230,12 @@ static void __devinit init_intel(struct cpuinfo_x86
*c)
>  	     ( c->cpuid_level >= 0x00000006 ) &&
>  	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
>  		set_bit(X86_FEATURE_ARAT, c->x86_capability);
> +
> +	/* Check platform QoS monitoring capability */
> +	if ((c->cpuid_level >= 0x00000007) &&
> +	    (cpuid_ebx(0x00000007) & (1u<<12)))
> +		set_bit(X86_FEATURE_QOSM, c->x86_capability);
> +
>  }
>  
>  static struct cpu_dev intel_cpu_dev __cpuinitdata = {
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> new file mode 100644
> index 0000000..e6ab416
> --- /dev/null
> +++ b/xen/arch/x86/pqos.c
> @@ -0,0 +1,89 @@
> +/*
> + * pqos.c: Platform QoS related service for guest.
> + *
> + * Copyright (c) 2013, Intel Corporation
> + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
with
> + * this program; if not, write to the Free Software Foundation, Inc., 59
Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +#include <asm/processor.h>
> +#include <xen/init.h>
> +#include <asm/pqos.h>
> +
> +static bool_t pqos_enabled = 1;
I cant see anywhere in the series where this is used outside of an init
function, in which case it should be __initdata.
> +boolean_param("pqos", pqos_enabled);
> +
> +unsigned int cqm_res_count = 0;
> +unsigned int cqm_upscaling_factor = 0;
> +bool_t cqm_enabled = 0;
> +struct cqm_res_struct *cqm_res_array = NULL;
> +
> +static void __init init_cqm(void)
> +{
> +    unsigned int eax, edx;
> +    unsigned int max_cqm_rmid;
> +
> +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor,
&max_cqm_rmid, &edx);
> +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> +        return;
> +
> +    cqm_res_count = max_cqm_rmid + 1;
> +
Range check on cqm_res_count ?  If max_cqm_rmid ends up as -1 from the
cpuid, we will allocate a 0 length array and crash later when reserving
RMID 0
> +    cqm_res_array = xzalloc_array(struct cqm_res_struct, cqm_res_count);
> +    if ( !cqm_res_array )
> +    {
> +        cqm_res_count = 0;
> +        return;
> +    }
> +
> +    cqm_enabled = 1;
> +
> +    /* Reserve RMID 0 for all domains not being monitored */
> +    cqm_res_array[0].inuse = 1;
> +
> +    printk(XENLOG_INFO "Cache QoS Monitoring Enabled.\n");
> +}
> +
> +static void __init init_qos_monitor(void)
> +{
> +    unsigned int qm_features;
> +    unsigned int eax, ebx, ecx;
> +
> +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
> +        return;
> +
> +    cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
> +
> +    if ( qm_features & QOS_MONITOR_TYPE_L3 )
> +        init_cqm();
> +}
> +
> +void __init init_platform_qos(void)
> +{
> +    if ( !pqos_enabled )
> +        return;
> +
> +    init_qos_monitor();
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 5bf4ee0..95418e4 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -48,6 +48,7 @@
>  #include <asm/setup.h>
>  #include <xen/cpu.h>
>  #include <asm/nmi.h>
> +#include <asm/pqos.h>
>  
>  /* opt_nosmp: If true, secondary processors are ignored. */
>  static bool_t __initdata opt_nosmp;
> @@ -1402,6 +1403,8 @@ void __init __start_xen(unsigned long mbi_p)
>  
>      domain_unpause_by_systemcontroller(dom0);
>  
> +    init_platform_qos();
> +
Does this want to be slightly earlier during startup? Does it make sense
to have dom0 monitored?  Is it possible to monitor Xen''s L3 usage as
well?
>      reset_stack_and_jump(init_done);
>  }
>  
> diff --git a/xen/include/asm-x86/cpufeature.h
b/xen/include/asm-x86/cpufeature.h
> index 1cfaf94..ca59668 100644
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -147,6 +147,7 @@
>  #define X86_FEATURE_ERMS	(7*32+ 9) /* Enhanced REP MOVSB/STOSB */
>  #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context ID */
>  #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional Memory */
> +#define X86_FEATURE_QOSM	(7*32+12) /* Platform QoS monitoring capability
*/
>  #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero */
>  #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
>  
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> new file mode 100644
> index 0000000..934d68a
> --- /dev/null
> +++ b/xen/include/asm-x86/pqos.h
It might make sense to split this file.  The QOS_* defines could be
argued as asm-x86, (perhaps asm-x86/qpos-defines.h)

However, the cqm_res_struct and function declaration are very certainly
not asm code, and would probably better live in arch/x86/qpos.h
> @@ -0,0 +1,37 @@
> +/*
> + * pqos.h: Platform QoS related service for guest.
> + *
> + * Copyright (c) 2013, Intel Corporation
> + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
with
> + * this program; if not, write to the Free Software Foundation, Inc., 59
Temple
> + * Place - Suite 330, Boston, MA 02111-1307 USA.
> + */
> +#ifndef ASM_PQOS_H
> +#define ASM_PQOS_H
> +
> +/* QoS Resource Type Enumeration */
> +#define QOS_MONITOR_TYPE_L3            0x2
> +
> +/* QoS Monitoring Event ID */
> +#define QOS_MONITOR_EVTID_L3           0x1
> +
> +struct cqm_res_struct {
This name is redundant with the extra "_struct" at the end. How about
cqm_res or cqm_resource ?
> +    domid_t  domain_id;
You will need to include one of the types headers.
> +    bool_t   inuse;
> +};
> +
> +void init_platform_qos(void);
> +
> +#endif

On 21/11/13 12:14, Andrew Cooper wrote:
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
>> From: Dongxiao Xu <dongxiao.xu@intel.com>
>>
>> Detect platform QoS feature status and enumerate the resource types,
>> one of which is to monitor the L3 cache occupancy.
>>
>> Also introduce a Xen grub command line parameter to control the
>> QoS feature status globally.
>>
>> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>> ---
>>  xen/arch/x86/Makefile            |    1 +
>>  xen/arch/x86/cpu/intel.c         |    6 +++
>>  xen/arch/x86/pqos.c              |   89
++++++++++++++++++++++++++++++++++++++
>>  xen/arch/x86/setup.c             |    3 ++
>>  xen/include/asm-x86/cpufeature.h |    1 +
>>  xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
>>  6 files changed, 137 insertions(+)
>>  create mode 100644 xen/arch/x86/pqos.c
>>  create mode 100644 xen/include/asm-x86/pqos.h
>>
>> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
>> index d502bdf..54962e0 100644
>> --- a/xen/arch/x86/Makefile
>> +++ b/xen/arch/x86/Makefile
>> @@ -58,6 +58,7 @@ obj-y += crash.o
>>  obj-y += tboot.o
>>  obj-y += hpet.o
>>  obj-y += xstate.o
>> +obj-y += pqos.o
>>  
>>  obj-$(crash_debug) += gdbstub.o
>>  
>> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
>> index 27fe762..f0d83ea 100644
>> --- a/xen/arch/x86/cpu/intel.c
>> +++ b/xen/arch/x86/cpu/intel.c
>> @@ -230,6 +230,12 @@ static void __devinit init_intel(struct
cpuinfo_x86 *c)
>>  	     ( c->cpuid_level >= 0x00000006 ) &&
>>  	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
>>  		set_bit(X86_FEATURE_ARAT, c->x86_capability);
>> +
>> +	/* Check platform QoS monitoring capability */
>> +	if ((c->cpuid_level >= 0x00000007) &&
>> +	    (cpuid_ebx(0x00000007) & (1u<<12)))
>> +		set_bit(X86_FEATURE_QOSM, c->x86_capability);
>> +
>>  }
>>  
>>  static struct cpu_dev intel_cpu_dev __cpuinitdata = {
>> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
>> new file mode 100644
>> index 0000000..e6ab416
>> --- /dev/null
>> +++ b/xen/arch/x86/pqos.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * pqos.c: Platform QoS related service for guest.
>> + *
>> + * Copyright (c) 2013, Intel Corporation
>> + * Author: Jiongxi Li  <jiongxi.li@intel.com>
>> + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but
WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
along with
>> + * this program; if not, write to the Free Software Foundation, Inc.,
59 Temple
>> + * Place - Suite 330, Boston, MA 02111-1307 USA.
>> + */
>> +#include <asm/processor.h>
>> +#include <xen/init.h>
>> +#include <asm/pqos.h>
>> +
>> +static bool_t pqos_enabled = 1;
> I cant see anywhere in the series where this is used outside of an init
> function, in which case it should be __initdata.
>
>> +boolean_param("pqos", pqos_enabled);
>> +
>> +unsigned int cqm_res_count = 0;
>> +unsigned int cqm_upscaling_factor = 0;
>> +bool_t cqm_enabled = 0;
>> +struct cqm_res_struct *cqm_res_array = NULL;
>> +
>> +static void __init init_cqm(void)
>> +{
>> +    unsigned int eax, edx;
>> +    unsigned int max_cqm_rmid;
>> +
>> +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor,
&max_cqm_rmid, &edx);
>> +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
>> +        return;
>> +
>> +    cqm_res_count = max_cqm_rmid + 1;
>> +
> Range check on cqm_res_count ?  If max_cqm_rmid ends up as -1 from the
> cpuid, we will allocate a 0 length array and crash later when reserving
> RMID 0
>
>> +    cqm_res_array = xzalloc_array(struct cqm_res_struct,
cqm_res_count);
>> +    if ( !cqm_res_array )
>> +    {
>> +        cqm_res_count = 0;
>> +        return;
>> +    }
>> +
>> +    cqm_enabled = 1;
>> +
>> +    /* Reserve RMID 0 for all domains not being monitored */
>> +    cqm_res_array[0].inuse = 1;
>> +
>> +    printk(XENLOG_INFO "Cache QoS Monitoring Enabled.\n");
>> +}
>> +
>> +static void __init init_qos_monitor(void)
>> +{
>> +    unsigned int qm_features;
>> +    unsigned int eax, ebx, ecx;
>> +
>> +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
>> +        return;
>> +
>> +    cpuid_count(0xf, 0, &eax, &ebx, &ecx,
&qm_features);
>> +
>> +    if ( qm_features & QOS_MONITOR_TYPE_L3 )
>> +        init_cqm();
>> +}
>> +
>> +void __init init_platform_qos(void)
>> +{
>> +    if ( !pqos_enabled )
>> +        return;
>> +
>> +    init_qos_monitor();
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * tab-width: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 5bf4ee0..95418e4 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -48,6 +48,7 @@
>>  #include <asm/setup.h>
>>  #include <xen/cpu.h>
>>  #include <asm/nmi.h>
>> +#include <asm/pqos.h>
>>  
>>  /* opt_nosmp: If true, secondary processors are ignored. */
>>  static bool_t __initdata opt_nosmp;
>> @@ -1402,6 +1403,8 @@ void __init __start_xen(unsigned long mbi_p)
>>  
>>      domain_unpause_by_systemcontroller(dom0);
>>  
>> +    init_platform_qos();
>> +
> Does this want to be slightly earlier during startup? Does it make sense
> to have dom0 monitored?  Is it possible to monitor Xen''s L3 usage
as well?
>
>>      reset_stack_and_jump(init_done);
>>  }
>>  
>> diff --git a/xen/include/asm-x86/cpufeature.h
b/xen/include/asm-x86/cpufeature.h
>> index 1cfaf94..ca59668 100644
>> --- a/xen/include/asm-x86/cpufeature.h
>> +++ b/xen/include/asm-x86/cpufeature.h
>> @@ -147,6 +147,7 @@
>>  #define X86_FEATURE_ERMS	(7*32+ 9) /* Enhanced REP MOVSB/STOSB */
>>  #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context ID
*/
>>  #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional Memory
*/
>> +#define X86_FEATURE_QOSM	(7*32+12) /* Platform QoS monitoring
capability */
>>  #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as zero
*/
>>  #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access
Prevention */
>>  
>> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
>> new file mode 100644
>> index 0000000..934d68a
>> --- /dev/null
>> +++ b/xen/include/asm-x86/pqos.h
> It might make sense to split this file.  The QOS_* defines could be
> argued as asm-x86, (perhaps asm-x86/qpos-defines.h)
>
> However, the cqm_res_struct and function declaration are very certainly
> not asm code, and would probably better live in arch/x86/qpos.h
Ignore this - I got my include hierarchy confused.  Sorry.

~Andrew
>
>> @@ -0,0 +1,37 @@
>> +/*
>> + * pqos.h: Platform QoS related service for guest.
>> + *
>> + * Copyright (c) 2013, Intel Corporation
>> + * Author: Jiongxi Li  <jiongxi.li@intel.com>
>> + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
>> + *
>> + * This program is free software; you can redistribute it and/or
modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope it will be useful, but
WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
License for
>> + * more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
along with
>> + * this program; if not, write to the Free Software Foundation, Inc.,
59 Temple
>> + * Place - Suite 330, Boston, MA 02111-1307 USA.
>> + */
>> +#ifndef ASM_PQOS_H
>> +#define ASM_PQOS_H
>> +
>> +/* QoS Resource Type Enumeration */
>> +#define QOS_MONITOR_TYPE_L3            0x2
>> +
>> +/* QoS Monitoring Event ID */
>> +#define QOS_MONITOR_EVTID_L3           0x1
>> +
>> +struct cqm_res_struct {
> This name is redundant with the extra "_struct" at the end. How
about
> cqm_res or cqm_resource ?
>
>> +    domid_t  domain_id;
> You will need to include one of the types headers.
>
>> +    bool_t   inuse;
>> +};
>> +
>> +void init_platform_qos(void);
>> +
>> +#endif
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> Allocate an RMID for a guest when it is created. This per-guest
> RMID will be used to monitor Cache QoS related data. The RMID will
> be relinquished when guest is destroyed.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/domain.c        |    9 +++++++
>  xen/arch/x86/pqos.c          |   59
++++++++++++++++++++++++++++++++++++++++++
>  xen/common/domctl.c          |    5 +++-
>  xen/include/asm-x86/domain.h |    2 ++
>  xen/include/asm-x86/pqos.h   |    4 +++
>  xen/include/public/domctl.h  |    3 +++
>  xen/include/xen/sched.h      |    3 +++
>  7 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index a3868f9..9725649 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -60,6 +60,7 @@
>  #include <xen/numa.h>
>  #include <xen/iommu.h>
>  #include <compat/vcpu.h>
> +#include <asm/pqos.h>
>  
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
>  DEFINE_PER_CPU(unsigned long, cr4);
> @@ -579,6 +580,11 @@ int arch_domain_create(struct domain *d, unsigned int
domcr_flags)
>      tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
>      spin_lock_init(&d->arch.vtsc_lock);
>  
> +    /* Allocate CQM RMID for guest */
> +    d->arch.pqos_cqm_rmid = 0;
> +    if ( system_supports_cqm() && !!(domcr_flags &
DOMCRF_pqos_cqm) )
The !! is redundant here as far as the logical test goes.
> +        alloc_cqm_rmid(d);
> +
>      return 0;
>  
>   fail:
> @@ -612,6 +618,9 @@ void arch_domain_destroy(struct domain *d)
>  
>      free_xenheap_page(d->shared_info);
>      cleanup_domain_irq_mapping(d);
> +
> +    if ( system_supports_cqm() )
You can remove this conditional if ... (See free_cqm_rmid())
> +        free_cqm_rmid(d);
>  }
>  
>  unsigned long pv_guest_cr4_fixup(const struct vcpu *v, unsigned long
guest_cr4)
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> index e6ab416..e294799 100644
> --- a/xen/arch/x86/pqos.c
> +++ b/xen/arch/x86/pqos.c
> @@ -20,6 +20,8 @@
>   */
>  #include <asm/processor.h>
>  #include <xen/init.h>
> +#include <xen/spinlock.h>
> +#include <xen/sched.h>
>  #include <asm/pqos.h>
>  
>  static bool_t pqos_enabled = 1;
> @@ -29,6 +31,7 @@ unsigned int cqm_res_count = 0;
>  unsigned int cqm_upscaling_factor = 0;
>  bool_t cqm_enabled = 0;
>  struct cqm_res_struct *cqm_res_array = NULL;
> +static DEFINE_SPINLOCK(cqm_lock);
>  
>  static void __init init_cqm(void)
>  {
> @@ -78,6 +81,62 @@ void __init init_platform_qos(void)
>      init_qos_monitor();
>  }
>  
> +bool_t system_supports_cqm(void)
> +{
> +    return cqm_enabled;
> +}
> +
> +int alloc_cqm_rmid(struct domain *d)
> +{
> +    int rmid, rc = 0;
> +    unsigned long flags;
> +
> +    ASSERT(system_supports_cqm());
> +
> +    spin_lock_irqsave(&cqm_lock, flags);
> +    /* RMID=0 is reserved, enumerate from 1 */
> +    for ( rmid = 1; rmid < cqm_res_count; rmid++ )
> +    {
> +        if ( cqm_res_array[rmid].inuse)
> +            continue;
> +
> +        cqm_res_array[rmid].inuse = 1;
> +        cqm_res_array[rmid].domain_id = d->domain_id;
> +        break;
> +    }
> +    spin_unlock_irqrestore(&cqm_lock, flags);
> +
> +    /* No CQM RMID available, assign RMID=0 by default */
> +    if ( rmid == cqm_res_count )
> +    {
> +        rmid = 0;
> +        rc = -1;
> +    }
> +
> +    d->arch.pqos_cqm_rmid = rmid;
> +
> +    return rc;
> +}
> +
> +void free_cqm_rmid(struct domain *d)
> +{
> +    int rmid = d->arch.pqos_cqm_rmid;
> +    unsigned long flags;
> +
> +    ASSERT(system_supports_cqm());
... you remove this assertion and have a return early if rmid is 0.
> +
> +    spin_lock_irqsave(&cqm_lock, flags);
> +    /* We do not free system reserved "RMID=0" */
> +    if ( rmid > 0 )
> +    {
> +        cqm_res_array[rmid].inuse = 0;
> +        cqm_res_array[rmid].domain_id = 0;
Would DOMID_INVALID be more appropriate here? 0 is valid domain
identifier.  It would also mean that you could remove the inuse flag
from the structure, and the structure itself degrades to an array of
domid_t''s

You can then further use cmpxchg() and avoid the spinlock.

I guess this all depends on whether you are expecting to add new
information into the structure or not.
> +    }
> +    spin_unlock_irqrestore(&cqm_lock, flags);
> +
> +    d->arch.pqos_cqm_rmid = 0;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 904d27b..1c2e320 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -425,7 +425,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
>                 | XEN_DOMCTL_CDF_pvh_guest
>                 | XEN_DOMCTL_CDF_hap
>                 | XEN_DOMCTL_CDF_s3_integrity
> -               | XEN_DOMCTL_CDF_oos_off)) )
> +               | XEN_DOMCTL_CDF_oos_off
> +               | XEN_DOMCTL_CDF_pqos_cqm)) )
>              break;
>  
>          dom = op->domain;
> @@ -467,6 +468,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
u_domctl)
>              domcr_flags |= DOMCRF_s3_integrity;
>          if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off )
>              domcr_flags |= DOMCRF_oos_off;
> +        if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_pqos_cqm )
> +            domcr_flags |= DOMCRF_pqos_cqm;
>  
>          d = domain_create(dom, domcr_flags,
op->u.createdomain.ssidref);
>          if ( IS_ERR(d) )
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 9d39061..b0479aa 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -313,6 +313,8 @@ struct arch_domain
>      spinlock_t e820_lock;
>      struct e820entry *e820;
>      unsigned int nr_e820;
> +
> +    int pqos_cqm_rmid;       /* CQM RMID assigned to the domain */
>  } __cacheline_aligned;
>  
>  #define has_arch_pdevs(d)    (!list_empty(&(d)->arch.pdev_list))
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> index 934d68a..88de139 100644
> --- a/xen/include/asm-x86/pqos.h
> +++ b/xen/include/asm-x86/pqos.h
> @@ -34,4 +34,8 @@ struct cqm_res_struct {
>  
>  void init_platform_qos(void);
>  
> +bool_t system_supports_cqm(void);
> +int alloc_cqm_rmid(struct domain *);
> +void free_cqm_rmid(struct domain *);
Please use a variable in the function declaration.  "d" would suffice.

~Andrew
> +
>  #endif
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 01a3652..47a850a 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -62,6 +62,9 @@ struct xen_domctl_createdomain {
>   /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
>  #define _XEN_DOMCTL_CDF_pvh_guest     4
>  #define XEN_DOMCTL_CDF_pvh_guest     
(1U<<_XEN_DOMCTL_CDF_pvh_guest)
> + /* Enable pqos-cqm? */
> +#define _XEN_DOMCTL_CDF_pqos_cqm      5
> +#define XEN_DOMCTL_CDF_pqos_cqm       (1U<<_XEN_DOMCTL_CDF_pqos_cqm)
>      uint32_t flags;
>  };
>  typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index cbdf377..3a42656 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -507,6 +507,9 @@ struct domain *domain_create(
>   /* DOMCRF_pvh: Create PV domain in HVM container. */
>  #define _DOMCRF_pvh             5
>  #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
> + /* DOMCRF_pqos_cqm: Create a domain with CQM support */
> +#define _DOMCRF_pqos_cqm        6
> +#define DOMCRF_pqos_cqm         (1U<<_DOMCRF_pqos_cqm)
>  
>  /*
>   * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> Add hypervisor side support for dynamically attach and detach CQM
> services for a certain guest.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/domctl.c       |   40
++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/domctl.h |   14 ++++++++++++++
>  2 files changed, 54 insertions(+)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index f7e4586..5ef21f9 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -35,6 +35,7 @@
>  #include <asm/mem_sharing.h>
>  #include <asm/xstate.h>
>  #include <asm/debugger.h>
> +#include <asm/pqos.h>
>  
>  static int gdbsx_guest_mem_io(
>      domid_t domid, struct xen_domctl_gdbsx_memio *iop)
> @@ -1223,6 +1224,45 @@ long arch_do_domctl(
>      }
>      break;
>  
> +    case XEN_DOMCTL_attach_pqos:
> +    {
> +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
> +        {
> +            if ( !system_supports_cqm() )
> +                ret = -ENODEV;
> +            else if ( d->arch.pqos_cqm_rmid > 0 )
> +                ret = -EINVAL;
EEXISTS perhaps?
> +            else
> +            {
> +                ret = alloc_cqm_rmid(d);
> +                if ( ret < 0 )
> +                    ret = -EBUSY;
On reflection, EUSERS is probably more accurate here.
> +            }
> +        }
> +        else
> +            ret = -EINVAL;
> +    }
> +    break;
> +
> +    case XEN_DOMCTL_detach_pqos:
> +    {
> +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
> +        {
> +            if ( !system_supports_cqm() )
> +                ret = -ENODEV;
> +            else if ( d->arch.pqos_cqm_rmid > 0 )
> +            {
> +                free_cqm_rmid(d);
> +                ret = 0;
> +            }
> +            else
> +                ret = -EINVAL;
> +        }
> +        else
> +            ret = -EINVAL;
> +    }
> +    break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, d, u_domctl);
>          break;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 47a850a..f5d7062 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -872,6 +872,17 @@ struct xen_domctl_set_max_evtchn {
>  typedef struct xen_domctl_set_max_evtchn xen_domctl_set_max_evtchn_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_max_evtchn_t);
>  
> +/* XEN_DOMCTL_attach_pqos */
> +/* XEN_DOMCTL_detach_pqos */
> +struct xen_domctl_qos_resource {
> + /* Attach or detach flag for cqm */
> +#define _XEN_DOMCTL_ADF_pqos_cqm      0
> +#define XEN_DOMCTL_ADF_pqos_cqm       (1U<<_XEN_DOMCTL_ADF_pqos_cqm)
Does the ADF help here?  My personal opinion is that it makes the
constants more confusing.

~Andrew
> +    uint32_t flags;
> +};
> +typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> @@ -941,6 +952,8 @@ struct xen_domctl {
>  #define XEN_DOMCTL_setnodeaffinity               68
>  #define XEN_DOMCTL_getnodeaffinity               69
>  #define XEN_DOMCTL_set_max_evtchn                70
> +#define XEN_DOMCTL_attach_pqos                   71
> +#define XEN_DOMCTL_detach_pqos                   72
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1001,6 +1014,7 @@ struct xen_domctl {
>          struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
>          struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
>          struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
> +        struct xen_domctl_qos_resource      qos_res;
>          uint8_t                             pad[128];
>      } u;
>  };

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> Retrive CQM information for certain domain, which reflects the L3 cache
> occupancy for a socket.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/pqos.c             |   57 ++++++++++++++++++++++++++++++++++
>  xen/arch/x86/sysctl.c           |   64
+++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-x86/msr-index.h |    4 +++
>  xen/include/asm-x86/pqos.h      |   14 +++++++++
>  xen/include/public/domctl.h     |   14 +++++++++
>  xen/include/public/sysctl.h     |   15 +++++++++
>  6 files changed, 168 insertions(+)
>
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> index e294799..d95784c 100644
> --- a/xen/arch/x86/pqos.c
> +++ b/xen/arch/x86/pqos.c
> @@ -19,14 +19,30 @@
>   * Place - Suite 330, Boston, MA 02111-1307 USA.
>   */
>  #include <asm/processor.h>
> +#include <asm/msr.h>
>  #include <xen/init.h>
>  #include <xen/spinlock.h>
>  #include <xen/sched.h>
> +#include <public/domctl.h>
>  #include <asm/pqos.h>
>  
>  static bool_t pqos_enabled = 1;
>  boolean_param("pqos", pqos_enabled);
>  
> +static void read_qm_data(void *arg)
> +{
> +    struct qm_element *qm_element = arg;
> +
> +    wrmsr(MSR_IA32_QOSEVTSEL, qm_element->evtid, qm_element->rmid);
> +    rdmsrl(MSR_IA32_QMC, qm_element->qm_data);
> +}
> +
> +static void get_generic_qm_info(struct qm_element *qm_element)
> +{
> +    unsigned int cpu = qm_element->cpu;
> +    on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
> +}
> +
>  unsigned int cqm_res_count = 0;
>  unsigned int cqm_upscaling_factor = 0;
>  bool_t cqm_enabled = 0;
> @@ -86,6 +102,23 @@ bool_t system_supports_cqm(void)
>      return cqm_enabled;
>  }
>  
> +unsigned int get_cqm_count(void)
> +{
> +    return cqm_res_count;
> +}
> +
> +unsigned int get_cqm_avail(void)
> +{
> +    unsigned int cqm_avail = 0;
> +    int i;
unsigned int please.  If cqm_res_count has its top bit set, the
following loop may never terminate.
> +
> +    for ( i = 0; i < cqm_res_count; i++ )
> +        if ( !cqm_res_array[i].inuse )
> +            cqm_avail++;
> +
> +    return cqm_avail;
> +}
> +
>  int alloc_cqm_rmid(struct domain *d)
>  {
>      int rmid, rc = 0;
> @@ -137,6 +170,30 @@ void free_cqm_rmid(struct domain *d)
>      d->arch.pqos_cqm_rmid = 0;
>  }
>  
> +void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
A cpumask_t is already quite large, and will get larger in the future. 
Pass by pointer please.
> +                  struct xen_domctl_getdomcqminfo *info)
> +{
> +    struct qm_element element;
> +    unsigned int cpu, i;
> +
> +    for_each_cpu ( cpu, &cpu_cqmdata_map )
> +    {
> +        element.cpu = cpu;
> +        element.rmid = rmid;
> +        element.evtid = QOS_MONITOR_EVTID_L3;
> +
> +        get_generic_qm_info(&element);
> +
> +        i = cpu_to_socket(cpu);
cpu_to_socket() can return BAD_APICID.
> +        info->socket_cqmdata[i].valid > +           
(element.qm_data & IA32_QM_CTR_ERROR_MASK) ? 0 : 1;
info->socket_cqmdata[i].valid = !(element.qm_data &
IA32_QM_CTR_ERROR_MASK);
> +        if ( info->socket_cqmdata[i].valid )
> +            info->socket_cqmdata[i].l3c_occupancy = element.qm_data *
cqm_upscaling_factor;
> +        else
> +            info->socket_cqmdata[i].l3c_occupancy = 0;
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 15d4b91..d631769 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -28,6 +28,7 @@
>  #include <xen/nodemask.h>
>  #include <xen/cpu.h>
>  #include <xsm/xsm.h>
> +#include <asm/pqos.h>
>  
>  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while (0)
>  
> @@ -101,6 +102,69 @@ long arch_do_sysctl(
>      }
>      break;
>  
> +    case XEN_SYSCTL_getdomcqminfolist:
> +    {
This whole hypercall makes me somewhat uneasy.
> +        struct domain *d;
> +        struct xen_domctl_getdomcqminfo info;
> +        uint32_t resource_count;
> +        uint32_t resource_avail;
> +        uint32_t num_domains = 0;
> +        cpumask_t cpu_cqmdata_map;
> +        DECLARE_BITMAP(sockets, QOS_MAX_SOCKETS);
> +        unsigned int cpu;
> +
> +        if ( !system_supports_cqm() )
> +        {
> +            ret = -ENODEV;
> +            break;
> +        }
> +
> +        resource_count = get_cqm_count();
> +        resource_avail = get_cqm_avail();
> +
> +        cpumask_clear(&cpu_cqmdata_map);
> +        bitmap_zero(sockets, QOS_MAX_SOCKETS);
> +        for_each_online_cpu(cpu)
> +        {
> +            int i = cpu_to_socket(cpu);
> +            if ( test_and_set_bit(i, sockets) )
> +                continue;
> +            cpumask_set_cpu(cpu, &cpu_cqmdata_map);
> +        }
What is this doing? It appears to be finding the first cpu on each socket.
> +
> +        rcu_read_lock(&domlist_read_lock);
> +        for_each_domain ( d )
> +        {
> +            if ( d->domain_id <
sysctl->u.getdomaininfolist.first_domain )
> +                continue;
> +            if ( num_domains == sysctl->u.getdomaininfolist.max_domains
)
> +                break;
> +            if ( d->arch.pqos_cqm_rmid <= 0 )
> +                continue;
Is there any case where pqos_cqm_rmid can be negative? alloc_cqm_rmid()
never assigns a negative number now in v2, in which case
d->arch.pqos_cqm_rmid can probably be unsigned (and related int
rmid''s
can be similarly promoted to unsigned)
> +            memset(&info, 0, sizeof(struct xen_domctl_getdomcqminfo));
> +            info.domain = d->domain_id;
> +            get_cqm_info(d->arch.pqos_cqm_rmid, cpu_cqmdata_map,
&info);
So for a domain the hypercallee is interested in, we get its rmid, and
ask get_cqm_info() to individually IPI each one cpu from a socket to
fill in the info field?

The IPIs are quite expensive, and this system will currently monopolise
the first cpu on each socket.
> +
> +            if (
copy_to_guest_offset(sysctl->u.getdomcqminfolist.buffer,
> +                                      num_domains, &info, 1) )
> +            {
> +                ret = -EFAULT;
> +                break;
> +            }
> +
> +            num_domains++;
So this loop is primarily bounded by the number of domains, where each
domain with a valid rmid will result in a spate of IPI?

This looks like it needs hypercall continuation logic.

Also, how well does this intersect with updating the rmid assignment?
> +        }
> +        rcu_read_unlock(&domlist_read_lock);
> +
> +        sysctl->u.getdomcqminfolist.num_domains = num_domains;
> +        sysctl->u.getdomcqminfolist.resource_count = resource_count;
> +        sysctl->u.getdomcqminfolist.resource_avail = resource_avail;
> +
> +        if ( copy_to_guest(u_sysctl, sysctl, 1) )
> +            ret = -EFAULT;
> +    }
> +    break;
break should be inside the brace.
> +
>      default:
>          ret = -ENOSYS;
>          break;
> diff --git a/xen/include/asm-x86/msr-index.h
b/xen/include/asm-x86/msr-index.h
> index e597a28..46ef165 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -488,4 +488,8 @@
>  /* Geode defined MSRs */
>  #define MSR_GEODE_BUSCONT_CONF0		0x00001900
>  
> +/* Platform QoS register */
> +#define MSR_IA32_QOSEVTSEL             0x00000c8d
> +#define MSR_IA32_QMC                   0x00000c8e
> +
>  #endif /* __ASM_MSR_INDEX_H */
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> index 88de139..5c86c5d 100644
> --- a/xen/include/asm-x86/pqos.h
> +++ b/xen/include/asm-x86/pqos.h
> @@ -27,15 +27,29 @@
>  /* QoS Monitoring Event ID */
>  #define QOS_MONITOR_EVTID_L3           0x1
>  
> +/* IA32_QM_CTR */
> +#define IA32_QM_CTR_ERROR_MASK         (0x3ul << 62)
> +
>  struct cqm_res_struct {
>      domid_t  domain_id;
>      bool_t   inuse;
>  };
>  
> +struct qm_element {
> +    uint64_t  qm_data;
> +    uint32_t  cpu;
> +    uint32_t  rmid;
> +    uint8_t   evtid;
> +};
> +
>  void init_platform_qos(void);
>  
>  bool_t system_supports_cqm(void);
>  int alloc_cqm_rmid(struct domain *);
>  void free_cqm_rmid(struct domain *);
> +unsigned int get_cqm_count(void);
> +unsigned int get_cqm_avail(void);
> +void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
> +                  struct xen_domctl_getdomcqminfo *info);
>  
>  #endif
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index f5d7062..fe8b37f 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -883,6 +883,20 @@ struct xen_domctl_qos_resource {
>  typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
>  
> +struct xen_socket_cqmdata {
> +    uint64_t l3c_occupancy;
> +    uint8_t  valid;
> +};
> +
> +struct xen_domctl_getdomcqminfo {
> +    /* OUT variables. */
> +    domid_t  domain;
> +#define QOS_MAX_SOCKETS    128
Baking this into the ABI seems short sighted, and in this specific case
looks to blow the 128 byte union size in a domctl structure.

The toolstack should be able to find the number of sockets on the
system, and provide a GUEST_HANDLE to an array of socket_cqmdata''s of
the appropriate length.
> +    struct xen_socket_cqmdata socket_cqmdata[QOS_MAX_SOCKETS];
> +};
> +typedef struct xen_domctl_getdomcqminfo xen_domctl_getdomcqminfo_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_getdomcqminfo_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 8437d31..0def306 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -632,6 +632,19 @@ struct xen_sysctl_coverage_op {
>  typedef struct xen_sysctl_coverage_op xen_sysctl_coverage_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
>  
> +/* XEN_SYSCTL_getdomcqminfolist */
> +struct xen_sysctl_getdomcqminfolist {
> +    /* IN variables. */
> +    domid_t               first_domain;
> +    uint32_t              max_domains;
> +    XEN_GUEST_HANDLE_64(xen_domctl_getdomcqminfo_t) buffer;
> +    /* OUT variables. */
> +    uint32_t              num_domains;
num_domains and max_domains can be folded together as both an in and an
out parameter.  Also, "max_domains" is confusingly close to
"max_domain"
at a glance.

~Andrew
> +    uint32_t              resource_count;
> +    uint32_t              resource_avail;
> +};
> +typedef struct xen_sysctl_getdomcqminfolist
xen_sysctl_getdomcqminfolist_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_getdomcqminfolist_t);
>  
>  struct xen_sysctl {
>      uint32_t cmd;
> @@ -654,6 +667,7 @@ struct xen_sysctl {
>  #define XEN_SYSCTL_cpupool_op                    18
>  #define XEN_SYSCTL_scheduler_op                  19
>  #define XEN_SYSCTL_coverage_op                   20
> +#define XEN_SYSCTL_getdomcqminfolist             21
>      uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
>      union {
>          struct xen_sysctl_readconsole       readconsole;
> @@ -675,6 +689,7 @@ struct xen_sysctl {
>          struct xen_sysctl_cpupool_op        cpupool_op;
>          struct xen_sysctl_scheduler_op      scheduler_op;
>          struct xen_sysctl_coverage_op       coverage_op;
> +        struct xen_sysctl_getdomcqminfolist getdomcqminfolist;
>          uint8_t                             pad[128];
>      } u;
>  };

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
>
> If the CQM service is attached to a domain, its related RMID will be set
> to hardware for monitoring when the domain''s vcpu is scheduled in.
When
> the domain''s vcpu is scheduled out, RMID 0 (system reserved) will
be set
> for monitoring.
>
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/domain.c           |    5 +++++
>  xen/arch/x86/pqos.c             |   21 ++++++++++++++++++++-
>  xen/include/asm-x86/msr-index.h |    1 +
>  xen/include/asm-x86/pqos.h      |    1 +
>  4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 9725649..1eda0ab 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1372,6 +1372,8 @@ static void __context_switch(void)
>      {
>          memcpy(&p->arch.user_regs, stack_regs,
CTXT_SWITCH_STACK_BYTES);
>          vcpu_save_fpu(p);
> +        if ( system_supports_cqm() )
> +            cqm_assoc_rmid(0);
So actions from the idle domain are accounted against the previously
scheduled vcpu?
>          p->arch.ctxt_switch_from(p);
>      }
>  
> @@ -1396,6 +1398,9 @@ static void __context_switch(void)
>          }
>          vcpu_restore_fpu_eager(n);
>          n->arch.ctxt_switch_to(n);
> +
> +        if ( system_supports_cqm() &&
n->domain->arch.pqos_cqm_rmid > 0 )
n->domain->arch.pqos_cqm_rmid can only be greater than 0 if the system
already supports cqm()
> +            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);
What happens to subsequent Xen accesses before returning to the guest?

What happens for Xen accesses in interrupt handlers?
>      }
>  
>      gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> index d95784c..0cec172 100644
> --- a/xen/arch/x86/pqos.c
> +++ b/xen/arch/x86/pqos.c
> @@ -27,6 +27,7 @@
>  #include <asm/pqos.h>
>  
>  static bool_t pqos_enabled = 1;
> +static unsigned int rmid_bitwidth;
>  boolean_param("pqos", pqos_enabled);
>  
>  static void read_qm_data(void *arg)
> @@ -43,6 +44,14 @@ static void get_generic_qm_info(struct qm_element
*qm_element)
>      on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
>  }
>  
> +void cqm_assoc_rmid(uint16_t rmid)
> +{
> +    uint64_t val;
> +    uint64_t rmid_mask = ~(~0ull << rmid_bitwidth);
> +    rdmsrl(MSR_IA32_PQR_ASSOC, val);
> +    wrmsrl(MSR_IA32_PQR_ASSOC, (val & ~(rmid_mask)) | (rmid &
rmid_mask));
> +}
> +
>  unsigned int cqm_res_count = 0;
>  unsigned int cqm_upscaling_factor = 0;
>  bool_t cqm_enabled = 0;
> @@ -78,13 +87,23 @@ static void __init init_cqm(void)
>  static void __init init_qos_monitor(void)
>  {
>      unsigned int qm_features;
> -    unsigned int eax, ebx, ecx;
> +    unsigned int eax, ebx, ecx, i = 0;
>  
>      if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
>          return;
>  
>      cpuid_count(0xf, 0, &eax, &ebx, &ecx, &qm_features);
>  
> +    while (1)
> +    {
> +        if ( (1 << i) - 1 >= ebx )
> +        {
> +            rmid_bitwidth = i;
> +            break;
> +        }
> +        i++;
> +    }
Is this some approximation a logarithm ?

Would it not be better to store rmid_mask (which would appear to be ebx)
rather than recalculate it from the constant rmid_bitwidth every time
cqm_assoc_rmid() is called ?
> +
>      if ( qm_features & QOS_MONITOR_TYPE_L3 )
>          init_cqm();
>  }
> diff --git a/xen/include/asm-x86/msr-index.h
b/xen/include/asm-x86/msr-index.h
> index 46ef165..45f4918 100644
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -491,5 +491,6 @@
>  /* Platform QoS register */
>  #define MSR_IA32_QOSEVTSEL             0x00000c8d
>  #define MSR_IA32_QMC                   0x00000c8e
> +#define MSR_IA32_PQR_ASSOC             0x00000c8f
>  
>  #endif /* __ASM_MSR_INDEX_H */
> diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> index 5c86c5d..2bb983b 100644
> --- a/xen/include/asm-x86/pqos.h
> +++ b/xen/include/asm-x86/pqos.h
> @@ -51,5 +51,6 @@ unsigned int get_cqm_count(void);
>  unsigned int get_cqm_avail(void);
>  void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
>                    struct xen_domctl_getdomcqminfo *info);
> +void cqm_assoc_rmid(uint16_t rmid);
rmid is inconsistently an int, u32 and u16 across this patch series. 
Can this be cleaned up.

~Andrew
>  
>  #endif

On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
You will need to CC the relevant maintainers.  This would be Jan and
Keir for the Hypervisor changes, and IanC, IanJ and Stefano for the
tools changes.

I would also suggest that you group the tools changes together at the
end of the series rather than having them interspersed.

~Andrew
>
> Changes from v1:
>  - Address comments from Andrew Cooper, including:
>    * Change function names, e.g., alloc_cqm_rmid(), system_supports_cqm(),
etc.
>    * Change some structure element order to save packing cost.
>    * Correct some function''s return value.
>    * Some programming styles change.
>    * ...
>
> Future generations of Intel Xeon processor may offer monitoring capability
in
> each logical processor to measure specific quality-of-service metric,
> for example, the Cache QoS Monitoring to get L3 cache occupancy.
> Detailed information please refer to Intel SDM chapter 17.14.
>
> Cache QoS Monitoring provides a layer of abstraction between applications
and
> logical processors through the use of Resource Monitoring IDs (RMIDs).
> In Xen design, each guest in the system can be assigned an RMID
independently,
> while RMID=0 is reserved for monitoring domains that doesn''t
enable CQM service.
> When any of the domain''s vcpu is scheduled on a logical processor,
the domain''s
> RMID will be activated by programming the value into one specific MSR, and
when
> the vcpu is scheduled out, a RMID=0 will be programmed into that MSR.
> The Cache QoS Hardware tracks cache utilization of memory accesses
according to
> the RMIDs and reports monitored data via a counter register. With this
solution,
> we can get the knowledge how much L3 cache is used by a certain guest.
>
> To attach CQM service to a certain guest, two approaches are provided:
> 1) Create the guest with "pqos_cqm=1" set in configuration file.
> 2) Use "xl pqos-attach cqm domid" for a running guest.
>
> To detached CQM service from a guest, users can:
> 1) Use "xl pqos-detach cqm domid" for a running guest.
> 2) Also destroying a guest will detach the CQM service.
>
> To get the L3 cache usage, users can use the command of:
> $ xl pqos-list cqm (domid)
>
> The below data is just an example showing how the CQM related data is
exposed to
> end user.
>
> [root@localhost]# xl pqos-list cqm
> RMID count    56        RMID available    53
> Name               ID  SocketID        L3C_Usage       SocketID       
L3C_Usage
> Domain-0            0         0         20127744              1        
25231360
> ExampleHVMDomain    1         0          3211264              1        
10551296
>
> Dongxiao Xu (8):
>   x86: detect and initialize Cache QoS Monitoring feature
>   x86: handle CQM resource when creating/destroying guests
>   tools: handle CQM resource when creating/destroying guests
>   x86: dynamically attach/detach CQM service for a guest
>   tools: dynamically attach/detach CQM service for a guest
>   x86: get per domain CQM information
>   tools: get per domain CQM information
>   x86: enable CQM monitoring for each domain RMID
>
>  tools/libxc/xc_domain.c          |   51 +++++++++
>  tools/libxc/xenctrl.h            |   14 +++
>  tools/libxl/Makefile             |    3 +-
>  tools/libxl/libxl.h              |    8 ++
>  tools/libxl/libxl_create.c       |    3 +
>  tools/libxl/libxl_pqos.c         |  102 +++++++++++++++++
>  tools/libxl/libxl_types.idl      |    1 +
>  tools/libxl/xl.h                 |    3 +
>  tools/libxl/xl_cmdimpl.c         |  129 ++++++++++++++++++++++
>  tools/libxl/xl_cmdtable.c        |   15 +++
>  xen/arch/x86/Makefile            |    1 +
>  xen/arch/x86/cpu/intel.c         |    6 +
>  xen/arch/x86/domain.c            |   14 +++
>  xen/arch/x86/domctl.c            |   40 +++++++
>  xen/arch/x86/pqos.c              |  224
++++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/setup.c             |    3 +
>  xen/arch/x86/sysctl.c            |   64 +++++++++++
>  xen/common/domctl.c              |    5 +-
>  xen/include/asm-x86/cpufeature.h |    1 +
>  xen/include/asm-x86/domain.h     |    2 +
>  xen/include/asm-x86/msr-index.h  |    5 +
>  xen/include/asm-x86/pqos.h       |   56 ++++++++++
>  xen/include/public/domctl.h      |   31 ++++++
>  xen/include/public/sysctl.h      |   15 +++
>  xen/include/xen/sched.h          |    3 +
>  25 files changed, 797 insertions(+), 2 deletions(-)
>  create mode 100644 tools/libxl/libxl_pqos.c
>  create mode 100644 xen/arch/x86/pqos.c
>  create mode 100644 xen/include/asm-x86/pqos.h
>

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 8:14 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 1/8] x86: detect and initialize Cache
QoS
> Monitoring feature
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> >
> > Detect platform QoS feature status and enumerate the resource types,
> > one of which is to monitor the L3 cache occupancy.
> >
> > Also introduce a Xen grub command line parameter to control the
> > QoS feature status globally.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > ---
> >  xen/arch/x86/Makefile            |    1 +
> >  xen/arch/x86/cpu/intel.c         |    6 +++
> >  xen/arch/x86/pqos.c              |   89
> ++++++++++++++++++++++++++++++++++++++
> >  xen/arch/x86/setup.c             |    3 ++
> >  xen/include/asm-x86/cpufeature.h |    1 +
> >  xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
> >  6 files changed, 137 insertions(+)
> >  create mode 100644 xen/arch/x86/pqos.c
> >  create mode 100644 xen/include/asm-x86/pqos.h
> >
> > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > index d502bdf..54962e0 100644
> > --- a/xen/arch/x86/Makefile
> > +++ b/xen/arch/x86/Makefile
> > @@ -58,6 +58,7 @@ obj-y += crash.o
> >  obj-y += tboot.o
> >  obj-y += hpet.o
> >  obj-y += xstate.o
> > +obj-y += pqos.o
> >
> >  obj-$(crash_debug) += gdbstub.o
> >
> > diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> > index 27fe762..f0d83ea 100644
> > --- a/xen/arch/x86/cpu/intel.c
> > +++ b/xen/arch/x86/cpu/intel.c
> > @@ -230,6 +230,12 @@ static void __devinit init_intel(struct
cpuinfo_x86 *c)
> >  	     ( c->cpuid_level >= 0x00000006 ) &&
> >  	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
> >  		set_bit(X86_FEATURE_ARAT, c->x86_capability);
> > +
> > +	/* Check platform QoS monitoring capability */
> > +	if ((c->cpuid_level >= 0x00000007) &&
> > +	    (cpuid_ebx(0x00000007) & (1u<<12)))
> > +		set_bit(X86_FEATURE_QOSM, c->x86_capability);
> > +
> >  }
> >
> >  static struct cpu_dev intel_cpu_dev __cpuinitdata = {
> > diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> > new file mode 100644
> > index 0000000..e6ab416
> > --- /dev/null
> > +++ b/xen/arch/x86/pqos.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * pqos.c: Platform QoS related service for guest.
> > + *
> > + * Copyright (c) 2013, Intel Corporation
> > + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> > + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
along
> with
> > + * this program; if not, write to the Free Software Foundation, Inc.,
59 Temple
> > + * Place - Suite 330, Boston, MA 02111-1307 USA.
> > + */
> > +#include <asm/processor.h>
> > +#include <xen/init.h>
> > +#include <asm/pqos.h>
> > +
> > +static bool_t pqos_enabled = 1;
> 
> I cant see anywhere in the series where this is used outside of an init
> function, in which case it should be __initdata.
OK.
> 
> > +boolean_param("pqos", pqos_enabled);
> > +
> > +unsigned int cqm_res_count = 0;
> > +unsigned int cqm_upscaling_factor = 0;
> > +bool_t cqm_enabled = 0;
> > +struct cqm_res_struct *cqm_res_array = NULL;
> > +
> > +static void __init init_cqm(void)
> > +{
> > +    unsigned int eax, edx;
> > +    unsigned int max_cqm_rmid;
> > +
> > +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor,
&max_cqm_rmid,
> &edx);
> > +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> > +        return;
> > +
> > +    cqm_res_count = max_cqm_rmid + 1;
> > +
> 
> Range check on cqm_res_count ?  If max_cqm_rmid ends up as -1 from the
> cpuid, we will allocate a 0 length array and crash later when reserving
> RMID 0
That''s a good point.
I will add a range check here.
> 
> > +    cqm_res_array = xzalloc_array(struct cqm_res_struct,
cqm_res_count);
> > +    if ( !cqm_res_array )
> > +    {
> > +        cqm_res_count = 0;
> > +        return;
> > +    }
> > +
> > +    cqm_enabled = 1;
> > +
> > +    /* Reserve RMID 0 for all domains not being monitored */
> > +    cqm_res_array[0].inuse = 1;
> > +
> > +    printk(XENLOG_INFO "Cache QoS Monitoring Enabled.\n");
> > +}
> > +
> > +static void __init init_qos_monitor(void)
> > +{
> > +    unsigned int qm_features;
> > +    unsigned int eax, ebx, ecx;
> > +
> > +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
> > +        return;
> > +
> > +    cpuid_count(0xf, 0, &eax, &ebx, &ecx,
&qm_features);
> > +
> > +    if ( qm_features & QOS_MONITOR_TYPE_L3 )
> > +        init_cqm();
> > +}
> > +
> > +void __init init_platform_qos(void)
> > +{
> > +    if ( !pqos_enabled )
> > +        return;
> > +
> > +    init_qos_monitor();
> > +}
> > +
> > +/*
> > + * Local variables:
> > + * mode: C
> > + * c-file-style: "BSD"
> > + * c-basic-offset: 4
> > + * tab-width: 4
> > + * indent-tabs-mode: nil
> > + * End:
> > + */
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 5bf4ee0..95418e4 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -48,6 +48,7 @@
> >  #include <asm/setup.h>
> >  #include <xen/cpu.h>
> >  #include <asm/nmi.h>
> > +#include <asm/pqos.h>
> >
> >  /* opt_nosmp: If true, secondary processors are ignored. */
> >  static bool_t __initdata opt_nosmp;
> > @@ -1402,6 +1403,8 @@ void __init __start_xen(unsigned long mbi_p)
> >
> >      domain_unpause_by_systemcontroller(dom0);
> >
> > +    init_platform_qos();
> > +
> 
> Does this want to be slightly earlier during startup? Does it make sense
> to have dom0 monitored?  Is it possible to monitor Xen''s L3 usage
as well?
In this design, we monitored per domain basis, including all the L3 cache usage
within both guest and hypervisor mode.
Dom0 can also be monitored by "xl pqos-attach cqm 0".

However for specific Xen''s L3 usage, it is not covered in this design,
since we can only write one RMID into the hardware at a time.
> 
> >      reset_stack_and_jump(init_done);
> >  }
> >
> > diff --git a/xen/include/asm-x86/cpufeature.h
> b/xen/include/asm-x86/cpufeature.h
> > index 1cfaf94..ca59668 100644
> > --- a/xen/include/asm-x86/cpufeature.h
> > +++ b/xen/include/asm-x86/cpufeature.h
> > @@ -147,6 +147,7 @@
> >  #define X86_FEATURE_ERMS	(7*32+ 9) /* Enhanced REP MOVSB/STOSB */
> >  #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process Context
> ID */
> >  #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional
> Memory */
> > +#define X86_FEATURE_QOSM	(7*32+12) /* Platform QoS monitoring
> capability */
> >  #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as
> zero */
> >  #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access
> Prevention */
> >
> > diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> > new file mode 100644
> > index 0000000..934d68a
> > --- /dev/null
> > +++ b/xen/include/asm-x86/pqos.h
> 
> It might make sense to split this file.  The QOS_* defines could be
> argued as asm-x86, (perhaps asm-x86/qpos-defines.h)
> 
> However, the cqm_res_struct and function declaration are very certainly
> not asm code, and would probably better live in arch/x86/qpos.h
> 
> > @@ -0,0 +1,37 @@
> > +/*
> > + * pqos.h: Platform QoS related service for guest.
> > + *
> > + * Copyright (c) 2013, Intel Corporation
> > + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> > + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
along
> with
> > + * this program; if not, write to the Free Software Foundation, Inc.,
59 Temple
> > + * Place - Suite 330, Boston, MA 02111-1307 USA.
> > + */
> > +#ifndef ASM_PQOS_H
> > +#define ASM_PQOS_H
> > +
> > +/* QoS Resource Type Enumeration */
> > +#define QOS_MONITOR_TYPE_L3            0x2
> > +
> > +/* QoS Monitoring Event ID */
> > +#define QOS_MONITOR_EVTID_L3           0x1
> > +
> > +struct cqm_res_struct {
> 
> This name is redundant with the extra "_struct" at the end. How
about
> cqm_res or cqm_resource ?
OK.
> 
> > +    domid_t  domain_id;
> 
> You will need to include one of the types headers.
Sorry maybe I didn''t quite understand your point here... But,
domid_t''s definition header file will be included in the
arch/x86/pqos.c file before pqos.h is included.
> 
> > +    bool_t   inuse;
> > +};
> > +
> > +void init_platform_qos(void);
> > +
> > +#endif

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 8:34 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 2/8] x86: handle CQM resource when
> creating/destroying guests
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> >
> > Allocate an RMID for a guest when it is created. This per-guest
> > RMID will be used to monitor Cache QoS related data. The RMID will
> > be relinquished when guest is destroyed.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > ---
> >  xen/arch/x86/domain.c        |    9 +++++++
> >  xen/arch/x86/pqos.c          |   59
> ++++++++++++++++++++++++++++++++++++++++++
> >  xen/common/domctl.c          |    5 +++-
> >  xen/include/asm-x86/domain.h |    2 ++
> >  xen/include/asm-x86/pqos.h   |    4 +++
> >  xen/include/public/domctl.h  |    3 +++
> >  xen/include/xen/sched.h      |    3 +++
> >  7 files changed, 84 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > index a3868f9..9725649 100644
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -60,6 +60,7 @@
> >  #include <xen/numa.h>
> >  #include <xen/iommu.h>
> >  #include <compat/vcpu.h>
> > +#include <asm/pqos.h>
> >
> >  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
> >  DEFINE_PER_CPU(unsigned long, cr4);
> > @@ -579,6 +580,11 @@ int arch_domain_create(struct domain *d, unsigned
int
> domcr_flags)
> >      tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
> >      spin_lock_init(&d->arch.vtsc_lock);
> >
> > +    /* Allocate CQM RMID for guest */
> > +    d->arch.pqos_cqm_rmid = 0;
> > +    if ( system_supports_cqm() && !!(domcr_flags &
DOMCRF_pqos_cqm) )
> 
> The !! is redundant here as far as the logical test goes.
OK, I will remove it.
> 
> > +        alloc_cqm_rmid(d);
> > +
> >      return 0;
> >
> >   fail:
> > @@ -612,6 +618,9 @@ void arch_domain_destroy(struct domain *d)
> >
> >      free_xenheap_page(d->shared_info);
> >      cleanup_domain_irq_mapping(d);
> > +
> > +    if ( system_supports_cqm() )
> 
> You can remove this conditional if ... (See free_cqm_rmid())
OK.
> 
> > +        free_cqm_rmid(d);
> >  }
> >
> >  unsigned long pv_guest_cr4_fixup(const struct vcpu *v, unsigned long
> guest_cr4)
> > diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> > index e6ab416..e294799 100644
> > --- a/xen/arch/x86/pqos.c
> > +++ b/xen/arch/x86/pqos.c
> > @@ -20,6 +20,8 @@
> >   */
> >  #include <asm/processor.h>
> >  #include <xen/init.h>
> > +#include <xen/spinlock.h>
> > +#include <xen/sched.h>
> >  #include <asm/pqos.h>
> >
> >  static bool_t pqos_enabled = 1;
> > @@ -29,6 +31,7 @@ unsigned int cqm_res_count = 0;
> >  unsigned int cqm_upscaling_factor = 0;
> >  bool_t cqm_enabled = 0;
> >  struct cqm_res_struct *cqm_res_array = NULL;
> > +static DEFINE_SPINLOCK(cqm_lock);
> >
> >  static void __init init_cqm(void)
> >  {
> > @@ -78,6 +81,62 @@ void __init init_platform_qos(void)
> >      init_qos_monitor();
> >  }
> >
> > +bool_t system_supports_cqm(void)
> > +{
> > +    return cqm_enabled;
> > +}
> > +
> > +int alloc_cqm_rmid(struct domain *d)
> > +{
> > +    int rmid, rc = 0;
> > +    unsigned long flags;
> > +
> > +    ASSERT(system_supports_cqm());
> > +
> > +    spin_lock_irqsave(&cqm_lock, flags);
> > +    /* RMID=0 is reserved, enumerate from 1 */
> > +    for ( rmid = 1; rmid < cqm_res_count; rmid++ )
> > +    {
> > +        if ( cqm_res_array[rmid].inuse)
> > +            continue;
> > +
> > +        cqm_res_array[rmid].inuse = 1;
> > +        cqm_res_array[rmid].domain_id = d->domain_id;
> > +        break;
> > +    }
> > +    spin_unlock_irqrestore(&cqm_lock, flags);
> > +
> > +    /* No CQM RMID available, assign RMID=0 by default */
> > +    if ( rmid == cqm_res_count )
> > +    {
> > +        rmid = 0;
> > +        rc = -1;
> > +    }
> > +
> > +    d->arch.pqos_cqm_rmid = rmid;
> > +
> > +    return rc;
> > +}
> > +
> > +void free_cqm_rmid(struct domain *d)
> > +{
> > +    int rmid = d->arch.pqos_cqm_rmid;
> > +    unsigned long flags;
> > +
> > +    ASSERT(system_supports_cqm());
> 
> ... you remove this assertion and have a return early if rmid is 0.
OK.
> 
> > +
> > +    spin_lock_irqsave(&cqm_lock, flags);
> > +    /* We do not free system reserved "RMID=0" */
> > +    if ( rmid > 0 )
> > +    {
> > +        cqm_res_array[rmid].inuse = 0;
> > +        cqm_res_array[rmid].domain_id = 0;
> 
> Would DOMID_INVALID be more appropriate here? 0 is valid domain
> identifier.  It would also mean that you could remove the inuse flag
> from the structure, and the structure itself degrades to an array of
> domid_t''s
> 
> You can then further use cmpxchg() and avoid the spinlock.
> 
> I guess this all depends on whether you are expecting to add new
> information into the structure or not.
Per my understanding, DOMID_xxx is somewhat related with memory management,
e.g., DOMID_INVALID is used to identify pages with unknown owner. Is it
appropriate to use it in CQM feature?

According to your proposal:
 - DOMID_INVALID is for RMIDs that are not allocated yet;
 - A valid domain number stands for the RMID is used for a certain domain;
 - Maybe DOMID_SELF or DOMID_XEN for the system reserved RMID=0?

Do you think it is OK if we introduce extra meanings (CQM specific) for those
macros?
> 
> > +    }
> > +    spin_unlock_irqrestore(&cqm_lock, flags);
> > +
> > +    d->arch.pqos_cqm_rmid = 0;
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> > diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> > index 904d27b..1c2e320 100644
> > --- a/xen/common/domctl.c
> > +++ b/xen/common/domctl.c
> > @@ -425,7 +425,8 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> >                 | XEN_DOMCTL_CDF_pvh_guest
> >                 | XEN_DOMCTL_CDF_hap
> >                 | XEN_DOMCTL_CDF_s3_integrity
> > -               | XEN_DOMCTL_CDF_oos_off)) )
> > +               | XEN_DOMCTL_CDF_oos_off
> > +               | XEN_DOMCTL_CDF_pqos_cqm)) )
> >              break;
> >
> >          dom = op->domain;
> > @@ -467,6 +468,8 @@ long
> do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> >              domcr_flags |= DOMCRF_s3_integrity;
> >          if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off
)
> >              domcr_flags |= DOMCRF_oos_off;
> > +        if ( op->u.createdomain.flags &
XEN_DOMCTL_CDF_pqos_cqm )
> > +            domcr_flags |= DOMCRF_pqos_cqm;
> >
> >          d = domain_create(dom, domcr_flags,
op->u.createdomain.ssidref);
> >          if ( IS_ERR(d) )
> > diff --git a/xen/include/asm-x86/domain.h
b/xen/include/asm-x86/domain.h
> > index 9d39061..b0479aa 100644
> > --- a/xen/include/asm-x86/domain.h
> > +++ b/xen/include/asm-x86/domain.h
> > @@ -313,6 +313,8 @@ struct arch_domain
> >      spinlock_t e820_lock;
> >      struct e820entry *e820;
> >      unsigned int nr_e820;
> > +
> > +    int pqos_cqm_rmid;       /* CQM RMID assigned to the domain */
> >  } __cacheline_aligned;
> >
> >  #define has_arch_pdevs(d)   
(!list_empty(&(d)->arch.pdev_list))
> > diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> > index 934d68a..88de139 100644
> > --- a/xen/include/asm-x86/pqos.h
> > +++ b/xen/include/asm-x86/pqos.h
> > @@ -34,4 +34,8 @@ struct cqm_res_struct {
> >
> >  void init_platform_qos(void);
> >
> > +bool_t system_supports_cqm(void);
> > +int alloc_cqm_rmid(struct domain *);
> > +void free_cqm_rmid(struct domain *);
> 
> Please use a variable in the function declaration.  "d" would
suffice.
OK.
> 
> ~Andrew
> 
> > +
> >  #endif
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 01a3652..47a850a 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -62,6 +62,9 @@ struct xen_domctl_createdomain {
> >   /* Is this a PVH guest (as opposed to an HVM or PV guest)? */
> >  #define _XEN_DOMCTL_CDF_pvh_guest     4
> >  #define XEN_DOMCTL_CDF_pvh_guest
> (1U<<_XEN_DOMCTL_CDF_pvh_guest)
> > + /* Enable pqos-cqm? */
> > +#define _XEN_DOMCTL_CDF_pqos_cqm      5
> > +#define XEN_DOMCTL_CDF_pqos_cqm
> (1U<<_XEN_DOMCTL_CDF_pqos_cqm)
> >      uint32_t flags;
> >  };
> >  typedef struct xen_domctl_createdomain xen_domctl_createdomain_t;
> > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> > index cbdf377..3a42656 100644
> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -507,6 +507,9 @@ struct domain *domain_create(
> >   /* DOMCRF_pvh: Create PV domain in HVM container. */
> >  #define _DOMCRF_pvh             5
> >  #define DOMCRF_pvh              (1U<<_DOMCRF_pvh)
> > + /* DOMCRF_pqos_cqm: Create a domain with CQM support */
> > +#define _DOMCRF_pqos_cqm        6
> > +#define DOMCRF_pqos_cqm         (1U<<_DOMCRF_pqos_cqm)
> >
> >  /*
> >   * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 8:50 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 4/8] x86: dynamically attach/detach CQM
> service for a guest
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> >
> > Add hypervisor side support for dynamically attach and detach CQM
> > services for a certain guest.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > ---
> >  xen/arch/x86/domctl.c       |   40
> ++++++++++++++++++++++++++++++++++++++++
> >  xen/include/public/domctl.h |   14 ++++++++++++++
> >  2 files changed, 54 insertions(+)
> >
> > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> > index f7e4586..5ef21f9 100644
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -35,6 +35,7 @@
> >  #include <asm/mem_sharing.h>
> >  #include <asm/xstate.h>
> >  #include <asm/debugger.h>
> > +#include <asm/pqos.h>
> >
> >  static int gdbsx_guest_mem_io(
> >      domid_t domid, struct xen_domctl_gdbsx_memio *iop)
> > @@ -1223,6 +1224,45 @@ long arch_do_domctl(
> >      }
> >      break;
> >
> > +    case XEN_DOMCTL_attach_pqos:
> > +    {
> > +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm
)
> > +        {
> > +            if ( !system_supports_cqm() )
> > +                ret = -ENODEV;
> > +            else if ( d->arch.pqos_cqm_rmid > 0 )
> > +                ret = -EINVAL;
> 
> EEXISTS perhaps?
EEXIST stands for "File exists" while EINVAL stands for "Invalid
argument ".
Here I used EINVAL because if the domain is already CQM enabled, then the
"domid" parameter in "xl pqos-attach cqm domid" should be an
invalid argument.
Do you think it is OK?
> 
> > +            else
> > +            {
> > +                ret = alloc_cqm_rmid(d);
> > +                if ( ret < 0 )
> > +                    ret = -EBUSY;
> 
> On reflection, EUSERS is probably more accurate here.
Agree.
> 
> > +            }
> > +        }
> > +        else
> > +            ret = -EINVAL;
> > +    }
> > +    break;
> > +
> > +    case XEN_DOMCTL_detach_pqos:
> > +    {
> > +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm
)
> > +        {
> > +            if ( !system_supports_cqm() )
> > +                ret = -ENODEV;
> > +            else if ( d->arch.pqos_cqm_rmid > 0 )
> > +            {
> > +                free_cqm_rmid(d);
> > +                ret = 0;
> > +            }
> > +            else
> > +                ret = -EINVAL;
> > +        }
> > +        else
> > +            ret = -EINVAL;
> > +    }
> > +    break;
> > +
> >      default:
> >          ret = iommu_do_domctl(domctl, d, u_domctl);
> >          break;
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 47a850a..f5d7062 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -872,6 +872,17 @@ struct xen_domctl_set_max_evtchn {
> >  typedef struct xen_domctl_set_max_evtchn
> xen_domctl_set_max_evtchn_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_max_evtchn_t);
> >
> > +/* XEN_DOMCTL_attach_pqos */
> > +/* XEN_DOMCTL_detach_pqos */
> > +struct xen_domctl_qos_resource {
> > + /* Attach or detach flag for cqm */
> > +#define _XEN_DOMCTL_ADF_pqos_cqm      0
> > +#define XEN_DOMCTL_ADF_pqos_cqm
> (1U<<_XEN_DOMCTL_ADF_pqos_cqm)
> 
> Does the ADF help here?  My personal opinion is that it makes the
> constants more confusing.
OK, will remove the "ADF" characters.

Thanks,
Dongxiao
> 
> ~Andrew
> 
> > +    uint32_t flags;
> > +};
> > +typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
> > +
> >  struct xen_domctl {
> >      uint32_t cmd;
> >  #define XEN_DOMCTL_createdomain                   1
> > @@ -941,6 +952,8 @@ struct xen_domctl {
> >  #define XEN_DOMCTL_setnodeaffinity               68
> >  #define XEN_DOMCTL_getnodeaffinity               69
> >  #define XEN_DOMCTL_set_max_evtchn                70
> > +#define XEN_DOMCTL_attach_pqos                   71
> > +#define XEN_DOMCTL_detach_pqos                   72
> >  #define XEN_DOMCTL_gdbsx_guestmemio            1000
> >  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
> >  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> > @@ -1001,6 +1014,7 @@ struct xen_domctl {
> >          struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
> >          struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
> >          struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
> > +        struct xen_domctl_qos_resource      qos_res;
> >          uint8_t                             pad[128];
> >      } u;
> >  };

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 10:09 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 6/8] x86: get per domain CQM information
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> >
> > Retrive CQM information for certain domain, which reflects the L3
cache
> > occupancy for a socket.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > ---
> >  xen/arch/x86/pqos.c             |   57
> ++++++++++++++++++++++++++++++++++
> >  xen/arch/x86/sysctl.c           |   64
> +++++++++++++++++++++++++++++++++++++++
> >  xen/include/asm-x86/msr-index.h |    4 +++
> >  xen/include/asm-x86/pqos.h      |   14 +++++++++
> >  xen/include/public/domctl.h     |   14 +++++++++
> >  xen/include/public/sysctl.h     |   15 +++++++++
> >  6 files changed, 168 insertions(+)
> >
> > diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> > index e294799..d95784c 100644
> > --- a/xen/arch/x86/pqos.c
> > +++ b/xen/arch/x86/pqos.c
> > @@ -19,14 +19,30 @@
> >   * Place - Suite 330, Boston, MA 02111-1307 USA.
> >   */
> >  #include <asm/processor.h>
> > +#include <asm/msr.h>
> >  #include <xen/init.h>
> >  #include <xen/spinlock.h>
> >  #include <xen/sched.h>
> > +#include <public/domctl.h>
> >  #include <asm/pqos.h>
> >
> >  static bool_t pqos_enabled = 1;
> >  boolean_param("pqos", pqos_enabled);
> >
> > +static void read_qm_data(void *arg)
> > +{
> > +    struct qm_element *qm_element = arg;
> > +
> > +    wrmsr(MSR_IA32_QOSEVTSEL, qm_element->evtid,
> qm_element->rmid);
> > +    rdmsrl(MSR_IA32_QMC, qm_element->qm_data);
> > +}
> > +
> > +static void get_generic_qm_info(struct qm_element *qm_element)
> > +{
> > +    unsigned int cpu = qm_element->cpu;
> > +    on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
> > +}
> > +
> >  unsigned int cqm_res_count = 0;
> >  unsigned int cqm_upscaling_factor = 0;
> >  bool_t cqm_enabled = 0;
> > @@ -86,6 +102,23 @@ bool_t system_supports_cqm(void)
> >      return cqm_enabled;
> >  }
> >
> > +unsigned int get_cqm_count(void)
> > +{
> > +    return cqm_res_count;
> > +}
> > +
> > +unsigned int get_cqm_avail(void)
> > +{
> > +    unsigned int cqm_avail = 0;
> > +    int i;
> 
> unsigned int please.  If cqm_res_count has its top bit set, the
> following loop may never terminate.
OK.
> 
> > +
> > +    for ( i = 0; i < cqm_res_count; i++ )
> > +        if ( !cqm_res_array[i].inuse )
> > +            cqm_avail++;
> > +
> > +    return cqm_avail;
> > +}
> > +
> >  int alloc_cqm_rmid(struct domain *d)
> >  {
> >      int rmid, rc = 0;
> > @@ -137,6 +170,30 @@ void free_cqm_rmid(struct domain *d)
> >      d->arch.pqos_cqm_rmid = 0;
> >  }
> >
> > +void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
> 
> A cpumask_t is already quite large, and will get larger in the future.
> Pass by pointer please.
OK.
> 
> > +                  struct xen_domctl_getdomcqminfo *info)
> > +{
> > +    struct qm_element element;
> > +    unsigned int cpu, i;
> > +
> > +    for_each_cpu ( cpu, &cpu_cqmdata_map )
> > +    {
> > +        element.cpu = cpu;
> > +        element.rmid = rmid;
> > +        element.evtid = QOS_MONITOR_EVTID_L3;
> > +
> > +        get_generic_qm_info(&element);
> > +
> > +        i = cpu_to_socket(cpu);
> 
> cpu_to_socket() can return BAD_APICID.
OK, will check the return value correctness.
> 
> > +        info->socket_cqmdata[i].valid > > +           
(element.qm_data & IA32_QM_CTR_ERROR_MASK) ? 0 : 1;
> 
> info->socket_cqmdata[i].valid = !(element.qm_data &
> IA32_QM_CTR_ERROR_MASK);
OK.
> 
> > +        if ( info->socket_cqmdata[i].valid )
> > +            info->socket_cqmdata[i].l3c_occupancy =
element.qm_data *
> cqm_upscaling_factor;
> > +        else
> > +            info->socket_cqmdata[i].l3c_occupancy = 0;
> > +    }
> > +}
> > +
> >  /*
> >   * Local variables:
> >   * mode: C
> > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> > index 15d4b91..d631769 100644
> > --- a/xen/arch/x86/sysctl.c
> > +++ b/xen/arch/x86/sysctl.c
> > @@ -28,6 +28,7 @@
> >  #include <xen/nodemask.h>
> >  #include <xen/cpu.h>
> >  #include <xsm/xsm.h>
> > +#include <asm/pqos.h>
> >
> >  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; } while
(0)
> >
> > @@ -101,6 +102,69 @@ long arch_do_sysctl(
> >      }
> >      break;
> >
> > +    case XEN_SYSCTL_getdomcqminfolist:
> > +    {
> 
> This whole hypercall makes me somewhat uneasy.
> 
> > +        struct domain *d;
> > +        struct xen_domctl_getdomcqminfo info;
> > +        uint32_t resource_count;
> > +        uint32_t resource_avail;
> > +        uint32_t num_domains = 0;
> > +        cpumask_t cpu_cqmdata_map;
> > +        DECLARE_BITMAP(sockets, QOS_MAX_SOCKETS);
> > +        unsigned int cpu;
> > +
> > +        if ( !system_supports_cqm() )
> > +        {
> > +            ret = -ENODEV;
> > +            break;
> > +        }
> > +
> > +        resource_count = get_cqm_count();
> > +        resource_avail = get_cqm_avail();
> > +
> > +        cpumask_clear(&cpu_cqmdata_map);
> > +        bitmap_zero(sockets, QOS_MAX_SOCKETS);
> > +        for_each_online_cpu(cpu)
> > +        {
> > +            int i = cpu_to_socket(cpu);
> > +            if ( test_and_set_bit(i, sockets) )
> > +                continue;
> > +            cpumask_set_cpu(cpu, &cpu_cqmdata_map);
> > +        }
> 
> What is this doing? It appears to be finding the first cpu on each socket.
Yes, the CQM info is per-socket, so just one CPU within the socket is enough to
get/set the data.
> 
> > +
> > +        rcu_read_lock(&domlist_read_lock);
> > +        for_each_domain ( d )
> > +        {
> > +            if ( d->domain_id <
sysctl->u.getdomaininfolist.first_domain )
> > +                continue;
> > +            if ( num_domains ==
sysctl->u.getdomaininfolist.max_domains )
> > +                break;
> > +            if ( d->arch.pqos_cqm_rmid <= 0 )
> > +                continue;
> 
> Is there any case where pqos_cqm_rmid can be negative? alloc_cqm_rmid()
> never assigns a negative number now in v2, in which case
> d->arch.pqos_cqm_rmid can probably be unsigned (and related int
rmid''s
> can be similarly promoted to unsigned)
Yes, it should be changed in the v2 patch, sorry.
> 
> > +            memset(&info, 0, sizeof(struct
xen_domctl_getdomcqminfo));
> > +            info.domain = d->domain_id;
> > +            get_cqm_info(d->arch.pqos_cqm_rmid, cpu_cqmdata_map,
> &info);
> 
> So for a domain the hypercallee is interested in, we get its rmid, and
> ask get_cqm_info() to individually IPI each one cpu from a socket to
> fill in the info field?
> 
> The IPIs are quite expensive, and this system will currently monopolise
> the first cpu on each socket.
Since the L3 cache usage is per-socket wise, so current solution is:
 - Found the first CPU of each socket, and mark them in cpu_cqmdata_map.
 - The get_cqm_info() issues IPI to those CPUs marked in cpu_cqmdata_map to get
the per-socket data.

Do you have proposed solution for this scenario?
> 
> > +
> > +            if (
copy_to_guest_offset(sysctl->u.getdomcqminfolist.buffer,
> > +                                      num_domains, &info, 1) )
> > +            {
> > +                ret = -EFAULT;
> > +                break;
> > +            }
> > +
> > +            num_domains++;
> 
> So this loop is primarily bounded by the number of domains, where each
> domain with a valid rmid will result in a spate of IPI?
> 
> This looks like it needs hypercall continuation logic.
OK, what about a preemption check every domain?
> 
> Also, how well does this intersect with updating the rmid assignment?
It needs to acquire cqm_lock to guarantee the simultaneous access to the cqm
related data.
> 
> > +        }
> > +        rcu_read_unlock(&domlist_read_lock);
> > +
> > +        sysctl->u.getdomcqminfolist.num_domains = num_domains;
> > +        sysctl->u.getdomcqminfolist.resource_count =
resource_count;
> > +        sysctl->u.getdomcqminfolist.resource_avail =
resource_avail;
> > +
> > +        if ( copy_to_guest(u_sysctl, sysctl, 1) )
> > +            ret = -EFAULT;
> > +    }
> > +    break;
> 
> break should be inside the brace.
I refered to other cases in arch/x86/sysctl.c and arch/x86/domctl.c, and the
"break" is mostly placed outside the brace...
> 
> > +
> >      default:
> >          ret = -ENOSYS;
> >          break;
> > diff --git a/xen/include/asm-x86/msr-index.h
> b/xen/include/asm-x86/msr-index.h
> > index e597a28..46ef165 100644
> > --- a/xen/include/asm-x86/msr-index.h
> > +++ b/xen/include/asm-x86/msr-index.h
> > @@ -488,4 +488,8 @@
> >  /* Geode defined MSRs */
> >  #define MSR_GEODE_BUSCONT_CONF0		0x00001900
> >
> > +/* Platform QoS register */
> > +#define MSR_IA32_QOSEVTSEL             0x00000c8d
> > +#define MSR_IA32_QMC                   0x00000c8e
> > +
> >  #endif /* __ASM_MSR_INDEX_H */
> > diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> > index 88de139..5c86c5d 100644
> > --- a/xen/include/asm-x86/pqos.h
> > +++ b/xen/include/asm-x86/pqos.h
> > @@ -27,15 +27,29 @@
> >  /* QoS Monitoring Event ID */
> >  #define QOS_MONITOR_EVTID_L3           0x1
> >
> > +/* IA32_QM_CTR */
> > +#define IA32_QM_CTR_ERROR_MASK         (0x3ul << 62)
> > +
> >  struct cqm_res_struct {
> >      domid_t  domain_id;
> >      bool_t   inuse;
> >  };
> >
> > +struct qm_element {
> > +    uint64_t  qm_data;
> > +    uint32_t  cpu;
> > +    uint32_t  rmid;
> > +    uint8_t   evtid;
> > +};
> > +
> >  void init_platform_qos(void);
> >
> >  bool_t system_supports_cqm(void);
> >  int alloc_cqm_rmid(struct domain *);
> >  void free_cqm_rmid(struct domain *);
> > +unsigned int get_cqm_count(void);
> > +unsigned int get_cqm_avail(void);
> > +void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
> > +                  struct xen_domctl_getdomcqminfo *info);
> >
> >  #endif
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index f5d7062..fe8b37f 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -883,6 +883,20 @@ struct xen_domctl_qos_resource {
> >  typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
> >
> > +struct xen_socket_cqmdata {
> > +    uint64_t l3c_occupancy;
> > +    uint8_t  valid;
> > +};
> > +
> > +struct xen_domctl_getdomcqminfo {
> > +    /* OUT variables. */
> > +    domid_t  domain;
> > +#define QOS_MAX_SOCKETS    128
> 
> Baking this into the ABI seems short sighted, and in this specific case
> looks to blow the 128 byte union size in a domctl structure.
Oh, thanks for catching this, it exceeds 128 bytes.
> 
> The toolstack should be able to find the number of sockets on the
> system, and provide a GUEST_HANDLE to an array of socket_cqmdata''s
of
> the appropriate length.
OK, will pass the structure from tool stack side.
> 
> > +    struct xen_socket_cqmdata socket_cqmdata[QOS_MAX_SOCKETS];
> > +};
> > +typedef struct xen_domctl_getdomcqminfo xen_domctl_getdomcqminfo_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_domctl_getdomcqminfo_t);
> > +
> >  struct xen_domctl {
> >      uint32_t cmd;
> >  #define XEN_DOMCTL_createdomain                   1
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 8437d31..0def306 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -632,6 +632,19 @@ struct xen_sysctl_coverage_op {
> >  typedef struct xen_sysctl_coverage_op xen_sysctl_coverage_op_t;
> >  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
> >
> > +/* XEN_SYSCTL_getdomcqminfolist */
> > +struct xen_sysctl_getdomcqminfolist {
> > +    /* IN variables. */
> > +    domid_t               first_domain;
> > +    uint32_t              max_domains;
> > +    XEN_GUEST_HANDLE_64(xen_domctl_getdomcqminfo_t) buffer;
> > +    /* OUT variables. */
> > +    uint32_t              num_domains;
> 
> num_domains and max_domains can be folded together as both an in and an
> out parameter.  Also, "max_domains" is confusingly close to
"max_domain"
> at a glance.
OK.

Thanks,
Dongxiao
> 
> ~Andrew
> 
> > +    uint32_t              resource_count;
> > +    uint32_t              resource_avail;
> > +};
> > +typedef struct xen_sysctl_getdomcqminfolist
> xen_sysctl_getdomcqminfolist_t;
> > +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_getdomcqminfolist_t);
> >
> >  struct xen_sysctl {
> >      uint32_t cmd;
> > @@ -654,6 +667,7 @@ struct xen_sysctl {
> >  #define XEN_SYSCTL_cpupool_op                    18
> >  #define XEN_SYSCTL_scheduler_op                  19
> >  #define XEN_SYSCTL_coverage_op                   20
> > +#define XEN_SYSCTL_getdomcqminfolist             21
> >      uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
> >      union {
> >          struct xen_sysctl_readconsole       readconsole;
> > @@ -675,6 +689,7 @@ struct xen_sysctl {
> >          struct xen_sysctl_cpupool_op        cpupool_op;
> >          struct xen_sysctl_scheduler_op      scheduler_op;
> >          struct xen_sysctl_coverage_op       coverage_op;
> > +        struct xen_sysctl_getdomcqminfolist getdomcqminfolist;
> >          uint8_t                             pad[128];
> >      } u;
> >  };

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 10:19 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 8/8] x86: enable CQM monitoring for each
> domain RMID
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> >
> > If the CQM service is attached to a domain, its related RMID will be
set
> > to hardware for monitoring when the domain''s vcpu is
scheduled in. When
> > the domain''s vcpu is scheduled out, RMID 0 (system reserved)
will be set
> > for monitoring.
> >
> > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > ---
> >  xen/arch/x86/domain.c           |    5 +++++
> >  xen/arch/x86/pqos.c             |   21 ++++++++++++++++++++-
> >  xen/include/asm-x86/msr-index.h |    1 +
> >  xen/include/asm-x86/pqos.h      |    1 +
> >  4 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> > index 9725649..1eda0ab 100644
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -1372,6 +1372,8 @@ static void __context_switch(void)
> >      {
> >          memcpy(&p->arch.user_regs, stack_regs,
> CTXT_SWITCH_STACK_BYTES);
> >          vcpu_save_fpu(p);
> > +        if ( system_supports_cqm() )
> > +            cqm_assoc_rmid(0);
> 
> So actions from the idle domain are accounted against the previously
> scheduled vcpu?
No. Considering the following cases:
 - Context switch from a normal domain vcpu (p) to an idle domain vcpu (n), then
we will associate RMID=0 to the CPU hardware on ctxt_switch_from(p), so that
idle domain is accounted to RMID=0;
 - Context switch from an idle domain vcpu (p) to a normal domain vcpu (n), then
we will associate the domain''s RMID on ctxt_switch_to(n).
> 
> >          p->arch.ctxt_switch_from(p);
> >      }
> >
> > @@ -1396,6 +1398,9 @@ static void __context_switch(void)
> >          }
> >          vcpu_restore_fpu_eager(n);
> >          n->arch.ctxt_switch_to(n);
> > +
> > +        if ( system_supports_cqm() &&
n->domain->arch.pqos_cqm_rmid >
> 0 )
> 
> n->domain->arch.pqos_cqm_rmid can only be greater than 0 if the
system
> already supports cqm()
This should be changed on v2 patch, sorry.
> 
> > +            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);
> 
> What happens to subsequent Xen accesses before returning to the guest?
> 
> What happens for Xen accesses in interrupt handlers?
The measurement is not that accurate for CQM feature.
CQM is somewhat like xentop, where xentop doesn''t exactly differentiate
the context switch cost and interrupt handling cost, so we adopt the similar
logics to CQM.
> 
> >      }
> >
> >      gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
> > diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> > index d95784c..0cec172 100644
> > --- a/xen/arch/x86/pqos.c
> > +++ b/xen/arch/x86/pqos.c
> > @@ -27,6 +27,7 @@
> >  #include <asm/pqos.h>
> >
> >  static bool_t pqos_enabled = 1;
> > +static unsigned int rmid_bitwidth;
> >  boolean_param("pqos", pqos_enabled);
> >
> >  static void read_qm_data(void *arg)
> > @@ -43,6 +44,14 @@ static void get_generic_qm_info(struct qm_element
> *qm_element)
> >      on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element, 1);
> >  }
> >
> > +void cqm_assoc_rmid(uint16_t rmid)
> > +{
> > +    uint64_t val;
> > +    uint64_t rmid_mask = ~(~0ull << rmid_bitwidth);
> > +    rdmsrl(MSR_IA32_PQR_ASSOC, val);
> > +    wrmsrl(MSR_IA32_PQR_ASSOC, (val & ~(rmid_mask)) | (rmid &
> rmid_mask));
> > +}
> > +
> >  unsigned int cqm_res_count = 0;
> >  unsigned int cqm_upscaling_factor = 0;
> >  bool_t cqm_enabled = 0;
> > @@ -78,13 +87,23 @@ static void __init init_cqm(void)
> >  static void __init init_qos_monitor(void)
> >  {
> >      unsigned int qm_features;
> > -    unsigned int eax, ebx, ecx;
> > +    unsigned int eax, ebx, ecx, i = 0;
> >
> >      if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
> >          return;
> >
> >      cpuid_count(0xf, 0, &eax, &ebx, &ecx,
&qm_features);
> >
> > +    while (1)
> > +    {
> > +        if ( (1 << i) - 1 >= ebx )
> > +        {
> > +            rmid_bitwidth = i;
> > +            break;
> > +        }
> > +        i++;
> > +    }
> 
> Is this some approximation a logarithm ?
> 
> Would it not be better to store rmid_mask (which would appear to be ebx)
> rather than recalculate it from the constant rmid_bitwidth every time
> cqm_assoc_rmid() is called ?
OK.
> 
> > +
> >      if ( qm_features & QOS_MONITOR_TYPE_L3 )
> >          init_cqm();
> >  }
> > diff --git a/xen/include/asm-x86/msr-index.h
> b/xen/include/asm-x86/msr-index.h
> > index 46ef165..45f4918 100644
> > --- a/xen/include/asm-x86/msr-index.h
> > +++ b/xen/include/asm-x86/msr-index.h
> > @@ -491,5 +491,6 @@
> >  /* Platform QoS register */
> >  #define MSR_IA32_QOSEVTSEL             0x00000c8d
> >  #define MSR_IA32_QMC                   0x00000c8e
> > +#define MSR_IA32_PQR_ASSOC             0x00000c8f
> >
> >  #endif /* __ASM_MSR_INDEX_H */
> > diff --git a/xen/include/asm-x86/pqos.h b/xen/include/asm-x86/pqos.h
> > index 5c86c5d..2bb983b 100644
> > --- a/xen/include/asm-x86/pqos.h
> > +++ b/xen/include/asm-x86/pqos.h
> > @@ -51,5 +51,6 @@ unsigned int get_cqm_count(void);
> >  unsigned int get_cqm_avail(void);
> >  void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
> >                    struct xen_domctl_getdomcqminfo *info);
> > +void cqm_assoc_rmid(uint16_t rmid);
> 
> rmid is inconsistently an int, u32 and u16 across this patch series.
> Can this be cleaned up.
OK, will do the cleanup.

Thanks,
Dongxiao
> 
> ~Andrew
> 
> >
> >  #endif

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, November 21, 2013 10:37 PM
> To: Xu, Dongxiao
> Cc: xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] [PATCH v2 0/8] enable Cache QoS Monitoring (CQM)
> feature
> 
> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > From: Dongxiao Xu <dongxiao.xu@intel.com>
> 
> You will need to CC the relevant maintainers.  This would be Jan and
> Keir for the Hypervisor changes, and IanC, IanJ and Stefano for the
> tools changes.
OK, will CC maintainers.
> 
> I would also suggest that you group the tools changes together at the
> end of the series rather than having them interspersed.
OK, will merge the tool side changes.

Thanks,
Dongxiao
> 
> ~Andrew
> 
> >
> > Changes from v1:
> >  - Address comments from Andrew Cooper, including:
> >    * Change function names, e.g., alloc_cqm_rmid(),
system_supports_cqm(),
> etc.
> >    * Change some structure element order to save packing cost.
> >    * Correct some function''s return value.
> >    * Some programming styles change.
> >    * ...
> >
> > Future generations of Intel Xeon processor may offer monitoring
capability in
> > each logical processor to measure specific quality-of-service metric,
> > for example, the Cache QoS Monitoring to get L3 cache occupancy.
> > Detailed information please refer to Intel SDM chapter 17.14.
> >
> > Cache QoS Monitoring provides a layer of abstraction between
applications and
> > logical processors through the use of Resource Monitoring IDs (RMIDs).
> > In Xen design, each guest in the system can be assigned an RMID
> independently,
> > while RMID=0 is reserved for monitoring domains that doesn''t
enable CQM
> service.
> > When any of the domain''s vcpu is scheduled on a logical
processor, the
> domain''s
> > RMID will be activated by programming the value into one specific MSR,
and
> when
> > the vcpu is scheduled out, a RMID=0 will be programmed into that MSR.
> > The Cache QoS Hardware tracks cache utilization of memory accesses
according
> to
> > the RMIDs and reports monitored data via a counter register. With this
solution,
> > we can get the knowledge how much L3 cache is used by a certain guest.
> >
> > To attach CQM service to a certain guest, two approaches are provided:
> > 1) Create the guest with "pqos_cqm=1" set in configuration
file.
> > 2) Use "xl pqos-attach cqm domid" for a running guest.
> >
> > To detached CQM service from a guest, users can:
> > 1) Use "xl pqos-detach cqm domid" for a running guest.
> > 2) Also destroying a guest will detach the CQM service.
> >
> > To get the L3 cache usage, users can use the command of:
> > $ xl pqos-list cqm (domid)
> >
> > The below data is just an example showing how the CQM related data is
> exposed to
> > end user.
> >
> > [root@localhost]# xl pqos-list cqm
> > RMID count    56        RMID available    53
> > Name               ID  SocketID        L3C_Usage       SocketID
> L3C_Usage
> > Domain-0            0         0         20127744              1
> 25231360
> > ExampleHVMDomain    1         0          3211264              1
> 10551296
> >
> > Dongxiao Xu (8):
> >   x86: detect and initialize Cache QoS Monitoring feature
> >   x86: handle CQM resource when creating/destroying guests
> >   tools: handle CQM resource when creating/destroying guests
> >   x86: dynamically attach/detach CQM service for a guest
> >   tools: dynamically attach/detach CQM service for a guest
> >   x86: get per domain CQM information
> >   tools: get per domain CQM information
> >   x86: enable CQM monitoring for each domain RMID
> >
> >  tools/libxc/xc_domain.c          |   51 +++++++++
> >  tools/libxc/xenctrl.h            |   14 +++
> >  tools/libxl/Makefile             |    3 +-
> >  tools/libxl/libxl.h              |    8 ++
> >  tools/libxl/libxl_create.c       |    3 +
> >  tools/libxl/libxl_pqos.c         |  102 +++++++++++++++++
> >  tools/libxl/libxl_types.idl      |    1 +
> >  tools/libxl/xl.h                 |    3 +
> >  tools/libxl/xl_cmdimpl.c         |  129 ++++++++++++++++++++++
> >  tools/libxl/xl_cmdtable.c        |   15 +++
> >  xen/arch/x86/Makefile            |    1 +
> >  xen/arch/x86/cpu/intel.c         |    6 +
> >  xen/arch/x86/domain.c            |   14 +++
> >  xen/arch/x86/domctl.c            |   40 +++++++
> >  xen/arch/x86/pqos.c              |  224
> ++++++++++++++++++++++++++++++++++++++
> >  xen/arch/x86/setup.c             |    3 +
> >  xen/arch/x86/sysctl.c            |   64 +++++++++++
> >  xen/common/domctl.c              |    5 +-
> >  xen/include/asm-x86/cpufeature.h |    1 +
> >  xen/include/asm-x86/domain.h     |    2 +
> >  xen/include/asm-x86/msr-index.h  |    5 +
> >  xen/include/asm-x86/pqos.h       |   56 ++++++++++
> >  xen/include/public/domctl.h      |   31 ++++++
> >  xen/include/public/sysctl.h      |   15 +++
> >  xen/include/xen/sched.h          |    3 +
> >  25 files changed, 797 insertions(+), 2 deletions(-)
> >  create mode 100644 tools/libxl/libxl_pqos.c
> >  create mode 100644 xen/arch/x86/pqos.c
> >  create mode 100644 xen/include/asm-x86/pqos.h
> >

> -----Original Message-----
> From: Xu, Dongxiao
> Sent: Monday, November 25, 2013 11:07 AM
> To: Andrew Cooper
> Cc: xen-devel@lists.xen.org
> Subject: RE: [Xen-devel] [PATCH v2 1/8] x86: detect and initialize Cache
QoS
> Monitoring feature
> 
> > -----Original Message-----
> > From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> > Sent: Thursday, November 21, 2013 8:14 PM
> > To: Xu, Dongxiao
> > Cc: xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [PATCH v2 1/8] x86: detect and initialize
Cache QoS
> > Monitoring feature
> >
> > On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
> > > From: Dongxiao Xu <dongxiao.xu@intel.com>
> > >
> > > Detect platform QoS feature status and enumerate the resource
types,
> > > one of which is to monitor the L3 cache occupancy.
> > >
> > > Also introduce a Xen grub command line parameter to control the
> > > QoS feature status globally.
> > >
> > > Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> > > Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> > > ---
> > >  xen/arch/x86/Makefile            |    1 +
> > >  xen/arch/x86/cpu/intel.c         |    6 +++
> > >  xen/arch/x86/pqos.c              |   89
> > ++++++++++++++++++++++++++++++++++++++
> > >  xen/arch/x86/setup.c             |    3 ++
> > >  xen/include/asm-x86/cpufeature.h |    1 +
> > >  xen/include/asm-x86/pqos.h       |   37 ++++++++++++++++
> > >  6 files changed, 137 insertions(+)
> > >  create mode 100644 xen/arch/x86/pqos.c
> > >  create mode 100644 xen/include/asm-x86/pqos.h
> > >
> > > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> > > index d502bdf..54962e0 100644
> > > --- a/xen/arch/x86/Makefile
> > > +++ b/xen/arch/x86/Makefile
> > > @@ -58,6 +58,7 @@ obj-y += crash.o
> > >  obj-y += tboot.o
> > >  obj-y += hpet.o
> > >  obj-y += xstate.o
> > > +obj-y += pqos.o
> > >
> > >  obj-$(crash_debug) += gdbstub.o
> > >
> > > diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
> > > index 27fe762..f0d83ea 100644
> > > --- a/xen/arch/x86/cpu/intel.c
> > > +++ b/xen/arch/x86/cpu/intel.c
> > > @@ -230,6 +230,12 @@ static void __devinit init_intel(struct
cpuinfo_x86 *c)
> > >  	     ( c->cpuid_level >= 0x00000006 ) &&
> > >  	     ( cpuid_eax(0x00000006) & (1u<<2) ) )
> > >  		set_bit(X86_FEATURE_ARAT, c->x86_capability);
> > > +
> > > +	/* Check platform QoS monitoring capability */
> > > +	if ((c->cpuid_level >= 0x00000007) &&
> > > +	    (cpuid_ebx(0x00000007) & (1u<<12)))
> > > +		set_bit(X86_FEATURE_QOSM, c->x86_capability);
> > > +
> > >  }
> > >
> > >  static struct cpu_dev intel_cpu_dev __cpuinitdata = {
> > > diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
> > > new file mode 100644
> > > index 0000000..e6ab416
> > > --- /dev/null
> > > +++ b/xen/arch/x86/pqos.c
> > > @@ -0,0 +1,89 @@
> > > +/*
> > > + * pqos.c: Platform QoS related service for guest.
> > > + *
> > > + * Copyright (c) 2013, Intel Corporation
> > > + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> > > + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
modify it
> > > + * under the terms and conditions of the GNU General Public
License,
> > > + * version 2, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope it will be useful,
but WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY
> > or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > License for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public
License along
> > with
> > > + * this program; if not, write to the Free Software Foundation,
Inc., 59
> Temple
> > > + * Place - Suite 330, Boston, MA 02111-1307 USA.
> > > + */
> > > +#include <asm/processor.h>
> > > +#include <xen/init.h>
> > > +#include <asm/pqos.h>
> > > +
> > > +static bool_t pqos_enabled = 1;
> >
> > I cant see anywhere in the series where this is used outside of an
init
> > function, in which case it should be __initdata.
> 
> OK.
> 
> >
> > > +boolean_param("pqos", pqos_enabled);
> > > +
> > > +unsigned int cqm_res_count = 0;
> > > +unsigned int cqm_upscaling_factor = 0;
> > > +bool_t cqm_enabled = 0;
> > > +struct cqm_res_struct *cqm_res_array = NULL;
> > > +
> > > +static void __init init_cqm(void)
> > > +{
> > > +    unsigned int eax, edx;
> > > +    unsigned int max_cqm_rmid;
> > > +
> > > +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor,
&max_cqm_rmid,
> > &edx);
> > > +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
> > > +        return;
> > > +
> > > +    cqm_res_count = max_cqm_rmid + 1;
> > > +
> >
> > Range check on cqm_res_count ?  If max_cqm_rmid ends up as -1 from the
> > cpuid, we will allocate a 0 length array and crash later when
reserving
> > RMID 0
> 
> That''s a good point.
> I will add a range check here.
According to the SDM, the biggest RMID value should be 0xffffffff (if possible),
so the biggest cqm_res_count is 0x100000000 (if possible).

So what about define cqm_res_count as "unsigned long"?

Thanks,
Dongxiao
> 
> >
> > > +    cqm_res_array = xzalloc_array(struct cqm_res_struct,
cqm_res_count);
> > > +    if ( !cqm_res_array )
> > > +    {
> > > +        cqm_res_count = 0;
> > > +        return;
> > > +    }
> > > +
> > > +    cqm_enabled = 1;
> > > +
> > > +    /* Reserve RMID 0 for all domains not being monitored */
> > > +    cqm_res_array[0].inuse = 1;
> > > +
> > > +    printk(XENLOG_INFO "Cache QoS Monitoring
Enabled.\n");
> > > +}
> > > +
> > > +static void __init init_qos_monitor(void)
> > > +{
> > > +    unsigned int qm_features;
> > > +    unsigned int eax, ebx, ecx;
> > > +
> > > +    if ( !(boot_cpu_has(X86_FEATURE_QOSM)) )
> > > +        return;
> > > +
> > > +    cpuid_count(0xf, 0, &eax, &ebx, &ecx,
&qm_features);
> > > +
> > > +    if ( qm_features & QOS_MONITOR_TYPE_L3 )
> > > +        init_cqm();
> > > +}
> > > +
> > > +void __init init_platform_qos(void)
> > > +{
> > > +    if ( !pqos_enabled )
> > > +        return;
> > > +
> > > +    init_qos_monitor();
> > > +}
> > > +
> > > +/*
> > > + * Local variables:
> > > + * mode: C
> > > + * c-file-style: "BSD"
> > > + * c-basic-offset: 4
> > > + * tab-width: 4
> > > + * indent-tabs-mode: nil
> > > + * End:
> > > + */
> > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > > index 5bf4ee0..95418e4 100644
> > > --- a/xen/arch/x86/setup.c
> > > +++ b/xen/arch/x86/setup.c
> > > @@ -48,6 +48,7 @@
> > >  #include <asm/setup.h>
> > >  #include <xen/cpu.h>
> > >  #include <asm/nmi.h>
> > > +#include <asm/pqos.h>
> > >
> > >  /* opt_nosmp: If true, secondary processors are ignored. */
> > >  static bool_t __initdata opt_nosmp;
> > > @@ -1402,6 +1403,8 @@ void __init __start_xen(unsigned long
mbi_p)
> > >
> > >      domain_unpause_by_systemcontroller(dom0);
> > >
> > > +    init_platform_qos();
> > > +
> >
> > Does this want to be slightly earlier during startup? Does it make
sense
> > to have dom0 monitored?  Is it possible to monitor Xen''s L3
usage as well?
> 
> In this design, we monitored per domain basis, including all the L3 cache
usage
> within both guest and hypervisor mode.
> Dom0 can also be monitored by "xl pqos-attach cqm 0".
> 
> However for specific Xen''s L3 usage, it is not covered in this
design, since we can
> only write one RMID into the hardware at a time.
> 
> >
> > >      reset_stack_and_jump(init_done);
> > >  }
> > >
> > > diff --git a/xen/include/asm-x86/cpufeature.h
> > b/xen/include/asm-x86/cpufeature.h
> > > index 1cfaf94..ca59668 100644
> > > --- a/xen/include/asm-x86/cpufeature.h
> > > +++ b/xen/include/asm-x86/cpufeature.h
> > > @@ -147,6 +147,7 @@
> > >  #define X86_FEATURE_ERMS	(7*32+ 9) /* Enhanced REP MOVSB/STOSB
*/
> > >  #define X86_FEATURE_INVPCID	(7*32+10) /* Invalidate Process
Context
> > ID */
> > >  #define X86_FEATURE_RTM 	(7*32+11) /* Restricted Transactional
> > Memory */
> > > +#define X86_FEATURE_QOSM	(7*32+12) /* Platform QoS monitoring
> > capability */
> > >  #define X86_FEATURE_NO_FPU_SEL 	(7*32+13) /* FPU CS/DS stored as
> > zero */
> > >  #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access
> > Prevention */
> > >
> > > diff --git a/xen/include/asm-x86/pqos.h
b/xen/include/asm-x86/pqos.h
> > > new file mode 100644
> > > index 0000000..934d68a
> > > --- /dev/null
> > > +++ b/xen/include/asm-x86/pqos.h
> >
> > It might make sense to split this file.  The QOS_* defines could be
> > argued as asm-x86, (perhaps asm-x86/qpos-defines.h)
> >
> > However, the cqm_res_struct and function declaration are very
certainly
> > not asm code, and would probably better live in arch/x86/qpos.h
> >
> > > @@ -0,0 +1,37 @@
> > > +/*
> > > + * pqos.h: Platform QoS related service for guest.
> > > + *
> > > + * Copyright (c) 2013, Intel Corporation
> > > + * Author: Jiongxi Li  <jiongxi.li@intel.com>
> > > + * Author: Dongxiao Xu <dongxiao.xu@intel.com>
> > > + *
> > > + * This program is free software; you can redistribute it and/or
modify it
> > > + * under the terms and conditions of the GNU General Public
License,
> > > + * version 2, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope it will be useful,
but WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY
> > or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > License for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public
License along
> > with
> > > + * this program; if not, write to the Free Software Foundation,
Inc., 59
> Temple
> > > + * Place - Suite 330, Boston, MA 02111-1307 USA.
> > > + */
> > > +#ifndef ASM_PQOS_H
> > > +#define ASM_PQOS_H
> > > +
> > > +/* QoS Resource Type Enumeration */
> > > +#define QOS_MONITOR_TYPE_L3            0x2
> > > +
> > > +/* QoS Monitoring Event ID */
> > > +#define QOS_MONITOR_EVTID_L3           0x1
> > > +
> > > +struct cqm_res_struct {
> >
> > This name is redundant with the extra "_struct" at the end.
How about
> > cqm_res or cqm_resource ?
> 
> OK.
> 
> >
> > > +    domid_t  domain_id;
> >
> > You will need to include one of the types headers.
> 
> Sorry maybe I didn''t quite understand your point here... But,
domid_t''s definition
> header file will be included in the arch/x86/pqos.c file before pqos.h is
included.
> 
> >
> > > +    bool_t   inuse;
> > > +};
> > > +
> > > +void init_platform_qos(void);
> > > +
> > > +#endif

On 25/11/13 03:06, Xu, Dongxiao wrote:
>
>>> +    domid_t  domain_id;
>> You will need to include one of the types headers.
> Sorry maybe I didn''t quite understand your point here... But,
domid_t''s definition header file will be included in the
arch/x86/pqos.c file before pqos.h is included.
Each header file should include its own dependencies, so a source file
doesn''t have to include header files in a specific order to avoid a
compilation error.

~Andrew

On 25/11/13 08:57, Xu, Dongxiao wrote:
>>
>>>> +boolean_param("pqos", pqos_enabled);
>>>> +
>>>> +unsigned int cqm_res_count = 0;
>>>> +unsigned int cqm_upscaling_factor = 0;
>>>> +bool_t cqm_enabled = 0;
>>>> +struct cqm_res_struct *cqm_res_array = NULL;
>>>> +
>>>> +static void __init init_cqm(void)
>>>> +{
>>>> +    unsigned int eax, edx;
>>>> +    unsigned int max_cqm_rmid;
>>>> +
>>>> +    cpuid_count(0xf, 1, &eax, &cqm_upscaling_factor,
&max_cqm_rmid,
>>> &edx);
>>>> +    if ( !(edx & QOS_MONITOR_EVTID_L3) )
>>>> +        return;
>>>> +
>>>> +    cqm_res_count = max_cqm_rmid + 1;
>>>> +
>>> Range check on cqm_res_count ?  If max_cqm_rmid ends up as -1 from
the
>>> cpuid, we will allocate a 0 length array and crash later when
reserving
>>> RMID 0
>> That''s a good point.
>> I will add a range check here.
> According to the SDM, the biggest RMID value should be 0xffffffff (if
possible), so the biggest cqm_res_count is 0x100000000 (if possible).
>
> So what about define cqm_res_count as "unsigned long"?
>
> Thanks,
> Dongxiao
In which case there needs to be a command line resource limit.  2^32
cqm_res_struct''s is unreasonably much to allocate by default.

Perhaps a "qos" custom param with "max-rmid=", set to a
sensible default
such as 256.

~Andrew

On 25/11/13 03:21, Xu, Dongxiao wrote:
>
>>> +
>>> +    spin_lock_irqsave(&cqm_lock, flags);
>>> +    /* We do not free system reserved "RMID=0" */
>>> +    if ( rmid > 0 )
>>> +    {
>>> +        cqm_res_array[rmid].inuse = 0;
>>> +        cqm_res_array[rmid].domain_id = 0;
>> Would DOMID_INVALID be more appropriate here? 0 is valid domain
>> identifier.  It would also mean that you could remove the inuse flag
>> from the structure, and the structure itself degrades to an array of
>> domid_t''s
>>
>> You can then further use cmpxchg() and avoid the spinlock.
>>
>> I guess this all depends on whether you are expecting to add new
>> information into the structure or not.
> Per my understanding, DOMID_xxx is somewhat related with memory management,
e.g., DOMID_INVALID is used to identify pages with unknown owner. Is it
appropriate to use it in CQM feature?
>
> According to your proposal:
>  - DOMID_INVALID is for RMIDs that are not allocated yet;
>  - A valid domain number stands for the RMID is used for a certain domain;
>  - Maybe DOMID_SELF or DOMID_XEN for the system reserved RMID=0?
>
> Do you think it is OK if we introduce extra meanings (CQM specific) for
those macros?
CC''ing the relevant maintainers for their opinion.

I would suggest that DOMID_INVALID should have its formal meaning
expanded to any case where there can logically be a domid needing a
specific invalid state.

~Andrew

On 25/11/13 03:26, Xu, Dongxiao wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Thursday, November 21, 2013 8:50 PM
>> To: Xu, Dongxiao
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH v2 4/8] x86: dynamically attach/detach
CQM
>> service for a guest
>>
>> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
>>> From: Dongxiao Xu <dongxiao.xu@intel.com>
>>>
>>> Add hypervisor side support for dynamically attach and detach CQM
>>> services for a certain guest.
>>>
>>> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
>>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>>> ---
>>>  xen/arch/x86/domctl.c       |   40
>> ++++++++++++++++++++++++++++++++++++++++
>>>  xen/include/public/domctl.h |   14 ++++++++++++++
>>>  2 files changed, 54 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
>>> index f7e4586..5ef21f9 100644
>>> --- a/xen/arch/x86/domctl.c
>>> +++ b/xen/arch/x86/domctl.c
>>> @@ -35,6 +35,7 @@
>>>  #include <asm/mem_sharing.h>
>>>  #include <asm/xstate.h>
>>>  #include <asm/debugger.h>
>>> +#include <asm/pqos.h>
>>>
>>>  static int gdbsx_guest_mem_io(
>>>      domid_t domid, struct xen_domctl_gdbsx_memio *iop)
>>> @@ -1223,6 +1224,45 @@ long arch_do_domctl(
>>>      }
>>>      break;
>>>
>>> +    case XEN_DOMCTL_attach_pqos:
>>> +    {
>>> +        if ( domctl->u.qos_res.flags &
XEN_DOMCTL_ADF_pqos_cqm )
>>> +        {
>>> +            if ( !system_supports_cqm() )
>>> +                ret = -ENODEV;
>>> +            else if ( d->arch.pqos_cqm_rmid > 0 )
>>> +                ret = -EINVAL;
>> EEXISTS perhaps?
> EEXIST stands for "File exists" while EINVAL stands for
"Invalid argument ".
> Here I used EINVAL because if the domain is already CQM enabled, then the
"domid" parameter in "xl pqos-attach cqm domid" should be an
invalid argument.
> Do you think it is OK?
I am looking for some way to distinguish "you passed an invalid flag"
(which is appropriately EINVAL), from "you are trying to attach to a
guest which already has qpos attached".

Personally, I would think that EEXISTS is fine from a hypercall, as Xen
will never have files to report about.  On the other hand, there appear
to be 0 current uses of it in Xen.

~Andrew

On 25/11/13 06:20, Xu, Dongxiao wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Thursday, November 21, 2013 10:09 PM
>> To: Xu, Dongxiao
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH v2 6/8] x86: get per domain CQM
information
>>
>> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
>>> From: Dongxiao Xu <dongxiao.xu@intel.com>
>>>
>>> Retrive CQM information for certain domain, which reflects the L3
cache
>>> occupancy for a socket.
>>>
>>> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
>>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>>> ---
>>>  xen/arch/x86/pqos.c             |   57
>> ++++++++++++++++++++++++++++++++++
>>>  xen/arch/x86/sysctl.c           |   64
>> +++++++++++++++++++++++++++++++++++++++
>>>  xen/include/asm-x86/msr-index.h |    4 +++
>>>  xen/include/asm-x86/pqos.h      |   14 +++++++++
>>>  xen/include/public/domctl.h     |   14 +++++++++
>>>  xen/include/public/sysctl.h     |   15 +++++++++
>>>  6 files changed, 168 insertions(+)
>>>
>>> diff --git a/xen/arch/x86/pqos.c b/xen/arch/x86/pqos.c
>>> index e294799..d95784c 100644
>>> --- a/xen/arch/x86/pqos.c
>>> +++ b/xen/arch/x86/pqos.c
>>> @@ -19,14 +19,30 @@
>>>   * Place - Suite 330, Boston, MA 02111-1307 USA.
>>>   */
>>>  #include <asm/processor.h>
>>> +#include <asm/msr.h>
>>>  #include <xen/init.h>
>>>  #include <xen/spinlock.h>
>>>  #include <xen/sched.h>
>>> +#include <public/domctl.h>
>>>  #include <asm/pqos.h>
>>>
>>>  static bool_t pqos_enabled = 1;
>>>  boolean_param("pqos", pqos_enabled);
>>>
>>> +static void read_qm_data(void *arg)
>>> +{
>>> +    struct qm_element *qm_element = arg;
>>> +
>>> +    wrmsr(MSR_IA32_QOSEVTSEL, qm_element->evtid,
>> qm_element->rmid);
>>> +    rdmsrl(MSR_IA32_QMC, qm_element->qm_data);
>>> +}
>>> +
>>> +static void get_generic_qm_info(struct qm_element *qm_element)
>>> +{
>>> +    unsigned int cpu = qm_element->cpu;
>>> +    on_selected_cpus(cpumask_of(cpu), read_qm_data, qm_element,
1);
>>> +}
>>> +
>>>  unsigned int cqm_res_count = 0;
>>>  unsigned int cqm_upscaling_factor = 0;
>>>  bool_t cqm_enabled = 0;
>>> @@ -86,6 +102,23 @@ bool_t system_supports_cqm(void)
>>>      return cqm_enabled;
>>>  }
>>>
>>> +unsigned int get_cqm_count(void)
>>> +{
>>> +    return cqm_res_count;
>>> +}
>>> +
>>> +unsigned int get_cqm_avail(void)
>>> +{
>>> +    unsigned int cqm_avail = 0;
>>> +    int i;
>> unsigned int please.  If cqm_res_count has its top bit set, the
>> following loop may never terminate.
> OK.
>
>>> +
>>> +    for ( i = 0; i < cqm_res_count; i++ )
>>> +        if ( !cqm_res_array[i].inuse )
>>> +            cqm_avail++;
>>> +
>>> +    return cqm_avail;
>>> +}
>>> +
>>>  int alloc_cqm_rmid(struct domain *d)
>>>  {
>>>      int rmid, rc = 0;
>>> @@ -137,6 +170,30 @@ void free_cqm_rmid(struct domain *d)
>>>      d->arch.pqos_cqm_rmid = 0;
>>>  }
>>>
>>> +void get_cqm_info(uint32_t rmid, cpumask_t cpu_cqmdata_map,
>> A cpumask_t is already quite large, and will get larger in the future.
>> Pass by pointer please.
> OK.
>
>>> +                  struct xen_domctl_getdomcqminfo *info)
>>> +{
>>> +    struct qm_element element;
>>> +    unsigned int cpu, i;
>>> +
>>> +    for_each_cpu ( cpu, &cpu_cqmdata_map )
>>> +    {
>>> +        element.cpu = cpu;
>>> +        element.rmid = rmid;
>>> +        element.evtid = QOS_MONITOR_EVTID_L3;
>>> +
>>> +        get_generic_qm_info(&element);
>>> +
>>> +        i = cpu_to_socket(cpu);
>> cpu_to_socket() can return BAD_APICID.
> OK, will check the return value correctness.
>
>>> +        info->socket_cqmdata[i].valid >>> +           
(element.qm_data & IA32_QM_CTR_ERROR_MASK) ? 0 : 1;
>> info->socket_cqmdata[i].valid = !(element.qm_data &
>> IA32_QM_CTR_ERROR_MASK);
> OK.
>
>>> +        if ( info->socket_cqmdata[i].valid )
>>> +            info->socket_cqmdata[i].l3c_occupancy =
element.qm_data *
>> cqm_upscaling_factor;
>>> +        else
>>> +            info->socket_cqmdata[i].l3c_occupancy = 0;
>>> +    }
>>> +}
>>> +
>>>  /*
>>>   * Local variables:
>>>   * mode: C
>>> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
>>> index 15d4b91..d631769 100644
>>> --- a/xen/arch/x86/sysctl.c
>>> +++ b/xen/arch/x86/sysctl.c
>>> @@ -28,6 +28,7 @@
>>>  #include <xen/nodemask.h>
>>>  #include <xen/cpu.h>
>>>  #include <xsm/xsm.h>
>>> +#include <asm/pqos.h>
>>>
>>>  #define get_xen_guest_handle(val, hnd)  do { val = (hnd).p; }
while (0)
>>>
>>> @@ -101,6 +102,69 @@ long arch_do_sysctl(
>>>      }
>>>      break;
>>>
>>> +    case XEN_SYSCTL_getdomcqminfolist:
>>> +    {
>> This whole hypercall makes me somewhat uneasy.
>>
>>> +        struct domain *d;
>>> +        struct xen_domctl_getdomcqminfo info;
>>> +        uint32_t resource_count;
>>> +        uint32_t resource_avail;
>>> +        uint32_t num_domains = 0;
>>> +        cpumask_t cpu_cqmdata_map;
>>> +        DECLARE_BITMAP(sockets, QOS_MAX_SOCKETS);
>>> +        unsigned int cpu;
>>> +
>>> +        if ( !system_supports_cqm() )
>>> +        {
>>> +            ret = -ENODEV;
>>> +            break;
>>> +        }
>>> +
>>> +        resource_count = get_cqm_count();
>>> +        resource_avail = get_cqm_avail();
>>> +
>>> +        cpumask_clear(&cpu_cqmdata_map);
>>> +        bitmap_zero(sockets, QOS_MAX_SOCKETS);
>>> +        for_each_online_cpu(cpu)
>>> +        {
>>> +            int i = cpu_to_socket(cpu);
>>> +            if ( test_and_set_bit(i, sockets) )
>>> +                continue;
>>> +            cpumask_set_cpu(cpu, &cpu_cqmdata_map);
>>> +        }
>> What is this doing? It appears to be finding the first cpu on each
socket.
> Yes, the CQM info is per-socket, so just one CPU within the socket is
enough to get/set the data.
>
>>> +
>>> +        rcu_read_lock(&domlist_read_lock);
>>> +        for_each_domain ( d )
>>> +        {
>>> +            if ( d->domain_id <
sysctl->u.getdomaininfolist.first_domain )
>>> +                continue;
>>> +            if ( num_domains ==
sysctl->u.getdomaininfolist.max_domains )
>>> +                break;
>>> +            if ( d->arch.pqos_cqm_rmid <= 0 )
>>> +                continue;
>> Is there any case where pqos_cqm_rmid can be negative? alloc_cqm_rmid()
>> never assigns a negative number now in v2, in which case
>> d->arch.pqos_cqm_rmid can probably be unsigned (and related int
rmid''s
>> can be similarly promoted to unsigned)
> Yes, it should be changed in the v2 patch, sorry.
>
>>> +            memset(&info, 0, sizeof(struct
xen_domctl_getdomcqminfo));
>>> +            info.domain = d->domain_id;
>>> +            get_cqm_info(d->arch.pqos_cqm_rmid,
cpu_cqmdata_map,
>> &info);
>>
>> So for a domain the hypercallee is interested in, we get its rmid, and
>> ask get_cqm_info() to individually IPI each one cpu from a socket to
>> fill in the info field?
>>
>> The IPIs are quite expensive, and this system will currently monopolise
>> the first cpu on each socket.
> Since the L3 cache usage is per-socket wise, so current solution is:
>  - Found the first CPU of each socket, and mark them in cpu_cqmdata_map.
>  - The get_cqm_info() issues IPI to those CPUs marked in cpu_cqmdata_map to
get the per-socket data.
>
> Do you have proposed solution for this scenario?
At the very least, capture all the RMID statistics in a single IPI.  If
the target other node was running an HVM guest, this would cause
devastating performance with all the VMexits.

Beyond that, picking a random cpu on the target socket would be better
in cases where VMs are restricted to a subset of possible cpus.


A better method might be to have a per-socket buffer, sized by max
RMIDs, and the IPI fills the entire buffer with the current statistics. 
This allows you to capture the statistics across sockets in parrallel,
rather than serialising the collection while holding the lock.  Then,
the hypercall handler just needs to read the appropriate stats out of
the per-socket buffers when constructing the per-domain information.
>
>>> +
>>> +            if (
copy_to_guest_offset(sysctl->u.getdomcqminfolist.buffer,
>>> +                                      num_domains, &info, 1) )
>>> +            {
>>> +                ret = -EFAULT;
>>> +                break;
>>> +            }
>>> +
>>> +            num_domains++;
>> So this loop is primarily bounded by the number of domains, where each
>> domain with a valid rmid will result in a spate of IPI?
>>
>> This looks like it needs hypercall continuation logic.
> OK, what about a preemption check every domain?
This is awkward, as you can''t perform the continuation with the lock
held, and releasing it causes the stats to become stale, especially if
an RMID gets altered in-between.

A completely different option would be to allow the toolstack to have RO
mappings of the per-socket buffers.  Then, the hypercall itself degrades
to just the IPI to force an update of the per-socket information.

However, this then exposes a memory-based ABI with the toolstack.  One
solution to this would be explicitly define the contents of the pages as
reserved, and provide libxc accessors.  This allows the ABI to be
updated with a commit altering both Xen and libxc at once, while forging
the risk that some third party starts reying on the layout of the magic
pages.

This has the advantage that all real processing is deferred to toolstack
userspace, and Xen is just responsible for dumping the data with enough
structure to be parsed.

~Andrew

On 25/11/13 07:22, Xu, Dongxiao wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: Thursday, November 21, 2013 10:19 PM
>> To: Xu, Dongxiao
>> Cc: xen-devel@lists.xen.org
>> Subject: Re: [Xen-devel] [PATCH v2 8/8] x86: enable CQM monitoring for
each
>> domain RMID
>>
>> On 21/11/13 07:20, dongxiao.xu@intel.com wrote:
>>> From: Dongxiao Xu <dongxiao.xu@intel.com>
>>>
>>> If the CQM service is attached to a domain, its related RMID will
be set
>>> to hardware for monitoring when the domain''s vcpu is
scheduled in. When
>>> the domain''s vcpu is scheduled out, RMID 0 (system
reserved) will be set
>>> for monitoring.
>>>
>>> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
>>> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
>>> ---
>>>  xen/arch/x86/domain.c           |    5 +++++
>>>  xen/arch/x86/pqos.c             |   21 ++++++++++++++++++++-
>>>  xen/include/asm-x86/msr-index.h |    1 +
>>>  xen/include/asm-x86/pqos.h      |    1 +
>>>  4 files changed, 27 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index 9725649..1eda0ab 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1372,6 +1372,8 @@ static void __context_switch(void)
>>>      {
>>>          memcpy(&p->arch.user_regs, stack_regs,
>> CTXT_SWITCH_STACK_BYTES);
>>>          vcpu_save_fpu(p);
>>> +        if ( system_supports_cqm() )
>>> +            cqm_assoc_rmid(0);
>> So actions from the idle domain are accounted against the previously
>> scheduled vcpu?
> No. Considering the following cases:
>  - Context switch from a normal domain vcpu (p) to an idle domain vcpu (n),
then we will associate RMID=0 to the CPU hardware on ctxt_switch_from(p), so
that idle domain is accounted to RMID=0;
>  - Context switch from an idle domain vcpu (p) to a normal domain vcpu (n),
then we will associate the domain''s RMID on ctxt_switch_to(n).
Ah of course.
>
>>>          p->arch.ctxt_switch_from(p);
>>>      }
>>>
>>> @@ -1396,6 +1398,9 @@ static void __context_switch(void)
>>>          }
>>>          vcpu_restore_fpu_eager(n);
>>>          n->arch.ctxt_switch_to(n);
>>> +
>>> +        if ( system_supports_cqm() &&
n->domain->arch.pqos_cqm_rmid >
>> 0 )
>>
>> n->domain->arch.pqos_cqm_rmid can only be greater than 0 if the
system
>> already supports cqm()
> This should be changed on v2 patch, sorry.
>
>>> +            cqm_assoc_rmid(n->domain->arch.pqos_cqm_rmid);
>> What happens to subsequent Xen accesses before returning to the guest?
>>
>> What happens for Xen accesses in interrupt handlers?
> The measurement is not that accurate for CQM feature.
> CQM is somewhat like xentop, where xentop doesn''t exactly
differentiate the context switch cost and interrupt handling cost, so we adopt
the similar logics to CQM.
Ok.

~Andrew

On Thu, Nov 21, 2013 at 03:20:41PM +0800, dongxiao.xu@intel.com
wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
> 
> Add two new xl instruction for attaching and detaching CQM services
> for a running guest.
> 
> Above "qos_res" is the monitoring resource type, and current
supported
> type is "cqm".
> 
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  tools/libxc/xc_domain.c   |   18 ++++++++++
>  tools/libxc/xenctrl.h     |    2 ++
>  tools/libxl/Makefile      |    3 +-
>  tools/libxl/libxl.h       |    3 ++
>  tools/libxl/libxl_pqos.c  |   88
+++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/xl.h          |    2 ++
>  tools/libxl/xl_cmdimpl.c  |   36 +++++++++++++++++++
>  tools/libxl/xl_cmdtable.c |   10 ++++++
>  8 files changed, 161 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_pqos.c
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 1ccafc5..85c2d4d 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -1776,6 +1776,24 @@ int xc_domain_set_max_evtchn(xc_interface *xch,
uint32_t domid,
>      return do_domctl(xch, &domctl);
>  }
>  
> +int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags)
> +{
> +    DECLARE_DOMCTL;
> +    domctl.cmd = XEN_DOMCTL_attach_pqos;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.qos_res.flags = flags;
> +    return do_domctl(xch, &domctl);
> +}
> +
> +int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags)
> +{
> +    DECLARE_DOMCTL;
> +    domctl.cmd = XEN_DOMCTL_detach_pqos;
> +    domctl.domain = (domid_t)domid;
> +    domctl.u.qos_res.flags = flags;
> +    return do_domctl(xch, &domctl);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 4ac6b8a..a57f147 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -2395,4 +2395,6 @@ int xc_kexec_load(xc_interface *xch, uint8_t type,
uint16_t arch,
>   */
>  int xc_kexec_unload(xc_interface *xch, int type);
>  
> +int xc_domain_pqos_attach(xc_interface *xch, uint32_t domid, int flags);
> +int xc_domain_pqos_detach(xc_interface *xch, uint32_t domid, int flags);
>  #endif /* XENCTRL_H */
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index cf214bb..35f0b97 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -74,7 +74,8 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o
libxl_dm.o libxl_pci.o \
>  			libxl_internal.o libxl_utils.o libxl_uuid.o \
>  			libxl_json.o libxl_aoutils.o libxl_numa.o \
>  			libxl_save_callout.o _libxl_save_msgs_callout.o \
> -			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
> +			libxl_qmp.o libxl_event.o libxl_fork.o libxl_pqos.o \
> +			$(LIBXL_OBJS-y)
>  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
>  
>  $(LIBXL_OBJS): CFLAGS += $(CFLAGS_LIBXL) -include
$(XEN_ROOT)/tools/config.h
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index c7dceda..a9a506f 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -1051,6 +1051,9 @@ int libxl_flask_getenforce(libxl_ctx *ctx);
>  int libxl_flask_setenforce(libxl_ctx *ctx, int mode);
>  int libxl_flask_loadpolicy(libxl_ctx *ctx, void *policy, uint32_t size);
>  
> +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char *
qosres);
> +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char *
qosres);
> +
>  /* misc */
>  
>  /* Each of these sets or clears the flag according to whether the
> diff --git a/tools/libxl/libxl_pqos.c b/tools/libxl/libxl_pqos.c
> new file mode 100644
> index 0000000..e92b94b
> --- /dev/null
> +++ b/tools/libxl/libxl_pqos.c
> @@ -0,0 +1,88 @@
> +/*
> + * Copyright (C) 2013      Intel Corporation
> + * Author Jiongxi Li <jiongxi.li@intel.com>
> + * Author Dongxiao Xu <dongxiao.xu@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as
published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_osdeps.h" /* must come before any other headers
*/
> +#include "libxl_internal.h"
> +
> +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
> +{
> +    int rc;
> +    int flags = 0;
> +
> +    if (!strcmp(qosres, "cqm"))
> +        flags |= XEN_DOMCTL_ADF_pqos_cqm;
> +    else {
> +        rc = -EINVAL;
> +        fprintf(stderr, "Invalid QoS resource type! "
> +                        "Supported types are:
\"cqm\".\n");
There are these nice LIBXL__LOG macros. Could you use those instead please?

(And also in the other patch that touches libxl)
> +        return rc;
> +    }
> +
> +    rc = xc_domain_pqos_attach(ctx->xch, domid, flags);
> +    if (rc < 0) {
> +        fprintf(stderr, "Failed to attach CQM service to domain %d.
", domid);
> +        if (errno == ENODEV)
> +            fprintf(stderr, "CQM is not supported in this
system.\n");
> +        else if (errno == EBUSY)
> +            fprintf(stderr, "There is no free CQM RMID
available.\n");
> +        else if (errno == EINVAL)
> +            fprintf(stderr, "Domain %d is already attached with CQM
service.\n", domid);
> +        else if (errno == ESRCH)
> +            fprintf(stderr, "Is Domain %d a valid domain?\n",
domid);
> +        else
> +            fprintf(stderr, "errno: %d\n", errno);
This could be made in a:

static const char * const msg[] = {
	[ENODEV] = "Failed to attach CQM service to domain %d!",
	...
}
and then you just iterate over the msg[] until you 
> +    }
> +
> +    return rc;
> +}
> +
> +int libxl_pqos_detach(libxl_ctx *ctx, uint32_t domid, const char * qosres)
> +{
> +    int rc;
> +    int flags = 0;
> +
> +    if (!strcmp(qosres, "cqm"))
strncmp!
> +        flags |= XEN_DOMCTL_ADF_pqos_cqm;
> +    else {
> +        rc = -EINVAL;
> +        fprintf(stderr, "Invalid QoS resource type! "
> +                        "Supported types are:
\"cqm\".\n");
> +        return rc;
> +    }
> +
> +    rc = xc_domain_pqos_detach(ctx->xch, domid, flags);
> +    if (rc < 0) {
> +        fprintf(stderr, "Failed to detach CQM service to domain %d.
", domid);
> +        if (errno == ENODEV)
> +            fprintf(stderr, "CQM is not supported in this
system.\n");
> +        else if (errno == EINVAL)
> +            fprintf(stderr, "Domain %d is not attached with CQM
service.\n", domid);
> +        else if (errno == ESRCH)
> +            fprintf(stderr, "Is Domain %d a valid domain?\n",
domid);
> +        else
> +            fprintf(stderr, "errno: %d\n", errno);
Again, the thing I mentioned above.
> +    }
> +
> +    return rc;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index e005c39..78738b9 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -105,6 +105,8 @@ int main_getenforce(int argc, char **argv);
>  int main_setenforce(int argc, char **argv);
>  int main_loadpolicy(int argc, char **argv);
>  int main_remus(int argc, char **argv);
> +int main_pqosattach(int argc, char **argv);
> +int main_pqosdetach(int argc, char **argv);
>  
>  void help(const char *command);
>  
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 84a604f..1a12e8f 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7195,6 +7195,42 @@ int main_remus(int argc, char **argv)
>      return -ERROR_FAIL;
>  }
>  
> +int main_pqosattach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt, rc;
> +    const char *qosres = NULL;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-attach",
2) {
> +        /* No options */
> +    }
> +
> +    qosres = argv[optind];
> +    domid = find_domain(argv[optind + 1]);
> +
> +    rc = libxl_pqos_attach(ctx, domid, qosres);
> +
> +    return rc;
> +}
> +
> +int main_pqosdetach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt, rc;
> +    const char *qosres = NULL;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "pqos-detach",
2) {
> +        /* No options */
> +    }
> +
> +    qosres = argv[optind];
> +    domid = find_domain(argv[optind + 1]);
> +
> +    rc = libxl_pqos_detach(ctx, domid, qosres);
> +
> +    return rc;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index 326a660..02a2572 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -488,6 +488,16 @@ struct cmd_spec cmd_table[] = {
>        "                        of the domain."
>  
>      },
> +    { "pqos-attach",
> +      &main_pqosattach, 0, 1,
> +      "Allocate and map qos resource",
> +      "<Resource> <Domain>",
> +    },
> +    { "pqos-detach",
> +      &main_pqosdetach, 0, 1,
> +      "Reliquish qos resource",
> +      "<Resource> <Domain>",
> +    },
>  };
>  
>  int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

> > +int libxl_pqos_attach(libxl_ctx *ctx, uint32_t domid, const char *
qosres)
> > +{
> > +    int rc;
> > +    int flags = 0;
> > +
> > +    if (!strcmp(qosres, "cqm"))
> > +        flags |= XEN_DOMCTL_ADF_pqos_cqm;
So you make ''flags'' an int here, but the hypercall uses
uint32_t.

Could you make it the same type please.

On Thu, Nov 21, 2013 at 03:20:40PM +0800, dongxiao.xu@intel.com
wrote:
> From: Dongxiao Xu <dongxiao.xu@intel.com>
> 
> Add hypervisor side support for dynamically attach and detach CQM
> services for a certain guest.
> 
> Signed-off-by: Jiongxi Li <jiongxi.li@intel.com>
> Signed-off-by: Dongxiao Xu <dongxiao.xu@intel.com>
> ---
>  xen/arch/x86/domctl.c       |   40
++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/domctl.h |   14 ++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index f7e4586..5ef21f9 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -35,6 +35,7 @@
>  #include <asm/mem_sharing.h>
>  #include <asm/xstate.h>
>  #include <asm/debugger.h>
> +#include <asm/pqos.h>
>  
>  static int gdbsx_guest_mem_io(
>      domid_t domid, struct xen_domctl_gdbsx_memio *iop)
> @@ -1223,6 +1224,45 @@ long arch_do_domctl(
>      }
>      break;
>  
> +    case XEN_DOMCTL_attach_pqos:
> +    {
Don''t you need to use to use xsm to check if the operation is
permitted.

Oh, and you also need to add in the XSM the new hypercall I think?

See 8ec7763c807f252e930c9647a0631253db2844a7 or
919f59b3b99e1d845c6a1f30125e79e828805d87 as examples of what needs
to be done.

Oh, and CC Daniel (dgdegra@tycho.nsa.gov)
> +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
> +        {
> +            if ( !system_supports_cqm() )
> +                ret = -ENODEV;
> +            else if ( d->arch.pqos_cqm_rmid > 0 )
> +                ret = -EINVAL;
> +            else
> +            {
> +                ret = alloc_cqm_rmid(d);
> +                if ( ret < 0 )
> +                    ret = -EBUSY;
> +            }
> +        }
> +        else
> +            ret = -EINVAL;
> +    }
> +    break;
> +
> +    case XEN_DOMCTL_detach_pqos:
> +    {
> +        if ( domctl->u.qos_res.flags & XEN_DOMCTL_ADF_pqos_cqm )
> +        {
> +            if ( !system_supports_cqm() )
> +                ret = -ENODEV;
> +            else if ( d->arch.pqos_cqm_rmid > 0 )
> +            {
> +                free_cqm_rmid(d);
> +                ret = 0;
> +            }
> +            else
> +                ret = -EINVAL;
> +        }
> +        else
> +            ret = -EINVAL;
> +    }
> +    break;
> +
>      default:
>          ret = iommu_do_domctl(domctl, d, u_domctl);
>          break;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 47a850a..f5d7062 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -872,6 +872,17 @@ struct xen_domctl_set_max_evtchn {
>  typedef struct xen_domctl_set_max_evtchn xen_domctl_set_max_evtchn_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_max_evtchn_t);
>  
> +/* XEN_DOMCTL_attach_pqos */
> +/* XEN_DOMCTL_detach_pqos */
> +struct xen_domctl_qos_resource {
> + /* Attach or detach flag for cqm */
> +#define _XEN_DOMCTL_ADF_pqos_cqm      0
> +#define XEN_DOMCTL_ADF_pqos_cqm       (1U<<_XEN_DOMCTL_ADF_pqos_cqm)
> +    uint32_t flags;
> +};
> +typedef struct xen_domctl_qos_resource xen_domctl_qos_resource_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_qos_resource_t);
> +
>  struct xen_domctl {
>      uint32_t cmd;
>  #define XEN_DOMCTL_createdomain                   1
> @@ -941,6 +952,8 @@ struct xen_domctl {
>  #define XEN_DOMCTL_setnodeaffinity               68
>  #define XEN_DOMCTL_getnodeaffinity               69
>  #define XEN_DOMCTL_set_max_evtchn                70
> +#define XEN_DOMCTL_attach_pqos                   71
> +#define XEN_DOMCTL_detach_pqos                   72
>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -1001,6 +1014,7 @@ struct xen_domctl {
>          struct xen_domctl_set_broken_page_p2m set_broken_page_p2m;
>          struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu;
>          struct xen_domctl_gdbsx_domstatus   gdbsx_domstatus;
> +        struct xen_domctl_qos_resource      qos_res;
>          uint8_t                             pad[128];
>      } u;
>  };
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel