* [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs
@ 2015-05-08 8:56 Chao Peng
2015-05-08 8:56 ` [PATCH v7 01/14] x86: add socket_to_cpumask Chao Peng
` (14 more replies)
0 siblings, 15 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Changes in v7:
Address comments from Jan/Ian, mainly:
* Introduce total_cpus to calculate nr_sockets.
* Clear the init/enable flag when a socket going offline.
* Reorder the statements in init_psr_cat.
* Copyback psr_cat_op only for XEN_SYSCTL_PSR_CAT_get_l3_info.
* Broadcast LIBXL_HAVE_SOCKET_BITMAP_ALLOC.
* Add PSR head1 level section and change CMT/CAT as its subsections for xl man page.
Changes in v6:
Address comments from Andrew/Dario/Ian, mainly:
* Introduce cat_socket_init(_enable)_bitmap.
* Merge xl psr-cmt/cat-hwinfo => xl psr-hwinfo.
* Add function header to explain the 'target' parameter.
* Use bitmap instead of TARGETS_ALL.
* Document fix.
Changes in v5:
* Address comments from Andrew and Ian(Detail in patch).
* Add socket_to_cpumask.
* Add xl psr-cmt/cat-hwinfo.
* Add some libxl CMT enhancement.
Changes in v4:
* Address comments from Andrew and Ian(Detail in patch).
* Split COS/CBM management patch into 4 small patches.
* Add documentation xl-psr.markdown.
Changes in v3:
* Address comments from Jan and Ian(Detail in patch).
* Add xl sample output in cover letter.
Changes in v2:
* Address comments from Konrad and Jan(Detail in patch):
* Make all cat unrelated changes into the preparation patches.
This patch serial enables the new Cache Allocation Technology (CAT) feature
found in Intel Broadwell and later server platform. In Xen's implementation,
CAT is used to control cache allocation on VM basis.
Detail hardware spec can be found in section 17.15 of the Intel SDM [1].
The design for XEN can be found at [2].
patch1-2: preparation.
patch3-9: real work for CAT.
patch10-11: enhancement for CMT.
patch12: libxl prepareation
patch13: tools side work for CAT.
patch14: xl document for CMT/MBM/CAT.
[1] Intel SDM (http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf)
[2] CAT design for XEN( http://lists.xen.org/archives/html/xen-devel/2014-12/msg01382.html)
Chao Peng (14):
x86: add socket_to_cpumask
x86: improve psr scheduling code
x86: detect and initialize Intel CAT feature
x86: maintain COS to CBM mapping for each socket
x86: add COS information for each domain
x86: expose CBM length and COS number information
x86: dynamically get/set CBM for a domain
x86: add scheduling support for Intel CAT
xsm: add CAT related xsm policies
tools/libxl: minor name changes for CMT commands
tools/libxl: add command to show PSR hardware info
tools/libxl: introduce some socket helpers
tools: add tools support for Intel CAT
docs: add xl-psr.markdown
docs/man/xl.pod.1 | 76 ++++-
docs/misc/xen-command-line.markdown | 15 +-
docs/misc/xl-psr.markdown | 133 +++++++++
tools/flask/policy/policy/modules/xen/xen.if | 2 +-
tools/flask/policy/policy/modules/xen/xen.te | 4 +-
tools/libxc/include/xenctrl.h | 15 +
tools/libxc/xc_psr.c | 76 +++++
tools/libxl/libxl.h | 42 +++
tools/libxl/libxl_internal.h | 2 +
tools/libxl/libxl_psr.c | 143 ++++++++-
tools/libxl/libxl_types.idl | 10 +
tools/libxl/libxl_utils.c | 46 +++
tools/libxl/libxl_utils.h | 2 +
tools/libxl/xl.h | 5 +
tools/libxl/xl_cmdimpl.c | 262 ++++++++++++++++-
tools/libxl/xl_cmdtable.c | 27 +-
xen/arch/x86/domain.c | 13 +-
xen/arch/x86/domctl.c | 20 ++
xen/arch/x86/mpparse.c | 5 +
xen/arch/x86/psr.c | 422 +++++++++++++++++++++++++--
xen/arch/x86/smpboot.c | 25 +-
xen/arch/x86/sysctl.c | 18 ++
xen/include/asm-x86/cpufeature.h | 1 +
xen/include/asm-x86/domain.h | 5 +-
xen/include/asm-x86/msr-index.h | 1 +
xen/include/asm-x86/psr.h | 13 +-
xen/include/asm-x86/smp.h | 16 +
xen/include/public/domctl.h | 12 +
xen/include/public/sysctl.h | 16 +
xen/xsm/flask/hooks.c | 6 +
xen/xsm/flask/policy/access_vectors | 4 +
31 files changed, 1385 insertions(+), 52 deletions(-)
create mode 100644 docs/misc/xl-psr.markdown
--
1.9.1
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-18 13:21 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 02/14] x86: improve psr scheduling code Chao Peng
` (13 subsequent siblings)
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Maintain socket_to_cpumask which contains all the HT and core siblings
in the same socket.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v7:
* Introduce total_cpus to calculate nr_sockets.
* Minor code sequence improvement in set_cpu_sibling_map.
* Improve comments for nr_sockets.
---
xen/arch/x86/mpparse.c | 5 +++++
xen/arch/x86/smpboot.c | 25 +++++++++++++++++++++++--
xen/include/asm-x86/smp.h | 16 ++++++++++++++++
3 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
index 003c56e..23e6706 100644
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -64,6 +64,9 @@ unsigned int __read_mostly boot_cpu_physical_apicid = BAD_APICID;
static unsigned int __devinitdata num_processors;
static unsigned int __initdata disabled_cpus;
+/* Total detected cpus (may exceed NR_CPUS) */
+unsigned int total_cpus;
+
/* Bitmask of physically existing CPUs */
physid_mask_t phys_cpu_present_map;
@@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct mpc_config_processor *m,
{
int ver, apicid, cpu = 0;
+ total_cpus++;
+
if (!(m->mpc_cpuflag & CPU_ENABLED)) {
if (!hotplug)
++disabled_cpus;
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 116c8f8..c5c25e4 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -59,6 +59,9 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
cpumask_t cpu_online_map __read_mostly;
EXPORT_SYMBOL(cpu_online_map);
+unsigned int nr_sockets __read_mostly;
+cpumask_var_t *socket_to_cpumask __read_mostly;
+
struct cpuinfo_x86 cpu_data[NR_CPUS];
u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
@@ -239,11 +242,14 @@ static void link_thread_siblings(int cpu1, int cpu2)
static void set_cpu_sibling_map(int cpu)
{
- int i;
+ int i, socket = cpu_to_socket(cpu);
struct cpuinfo_x86 *c = cpu_data;
cpumask_set_cpu(cpu, &cpu_sibling_setup_map);
+ if ( socket < nr_sockets )
+ cpumask_set_cpu(cpu, socket_to_cpumask[socket]);
+
if ( c[cpu].x86_num_siblings > 1 )
{
for_each_cpu ( i, &cpu_sibling_setup_map )
@@ -301,6 +307,7 @@ static void set_cpu_sibling_map(int cpu)
}
}
}
+
}
void start_secondary(void *unused)
@@ -704,6 +711,8 @@ static struct notifier_block cpu_smpboot_nfb = {
void __init smp_prepare_cpus(unsigned int max_cpus)
{
+ int socket;
+
register_cpu_notifier(&cpu_smpboot_nfb);
mtrr_aps_sync_begin();
@@ -717,6 +726,15 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
stack_base[0] = stack_start;
+ nr_sockets = DIV_ROUND_UP(total_cpus, boot_cpu_data.x86_max_cores *
+ boot_cpu_data.x86_num_siblings);
+ socket_to_cpumask = xzalloc_array(cpumask_var_t, nr_sockets);
+ if ( !socket_to_cpumask )
+ panic("No memory for socket CPU siblings map");
+ for ( socket = 0; socket < nr_sockets; socket++ )
+ if ( !zalloc_cpumask_var(socket_to_cpumask + socket) )
+ panic("No memory for socket CPU siblings cpumask");
+
if ( !zalloc_cpumask_var(&per_cpu(cpu_sibling_mask, 0)) ||
!zalloc_cpumask_var(&per_cpu(cpu_core_mask, 0)) )
panic("No memory for boot CPU sibling/core maps");
@@ -779,9 +797,12 @@ void __init smp_prepare_boot_cpu(void)
static void
remove_siblinginfo(int cpu)
{
- int sibling;
+ int sibling, socket = cpu_to_socket(cpu);
struct cpuinfo_x86 *c = cpu_data;
+ if ( socket < nr_sockets )
+ cpumask_clear_cpu(cpu, socket_to_cpumask[socket]);
+
for_each_cpu ( sibling, per_cpu(cpu_core_mask, cpu) )
{
cpumask_clear_cpu(cpu, per_cpu(cpu_core_mask, sibling));
diff --git a/xen/include/asm-x86/smp.h b/xen/include/asm-x86/smp.h
index 67518cf..30d8811 100644
--- a/xen/include/asm-x86/smp.h
+++ b/xen/include/asm-x86/smp.h
@@ -58,6 +58,22 @@ int hard_smp_processor_id(void);
void __stop_this_cpu(void);
+/* Total number of cpus in this system (may exceed NR_CPUS) */
+extern unsigned int total_cpus;
+
+/*
+ * This value is calculated by total_cpus/cpus_per_socket with the assumption
+ * that APIC IDs from MP table are continuous. It's possible that this value
+ * is less than the real socket number in the system if the APIC IDs from MP
+ * table are too sparse. Also the value is considered not to change from the
+ * initial startup. Violation of any of these assumptions may result in errors
+ * and requires retrofitting all the relevant places.
+ */
+extern unsigned int nr_sockets;
+
+/* Representing HT and core siblings in each socket */
+extern cpumask_var_t *socket_to_cpumask;
+
#endif /* !__ASSEMBLY__ */
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 02/14] x86: improve psr scheduling code
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
2015-05-08 8:56 ` [PATCH v7 01/14] x86: add socket_to_cpumask Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 10:20 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Chao Peng
` (12 subsequent siblings)
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Switching RMID from previous vcpu to next vcpu only needs to write
MSR_IA32_PSR_ASSOC once. Write it with the value of next vcpu is enough,
no need to write '0' first. Idle domain has RMID set to 0 and because MSR
is already updated lazily, so just switch it as it does.
Also move the initialization of per-CPU variable which used for lazy
update from context switch to CPU starting.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cahnges in v6:
* blankline fix.
Changes in v5:
* use this_cpu() rather than per_cpu().
Changes in v4:
* Move psr_assoc_reg_read/psr_assoc_reg_write into psr_ctxt_switch_to.
* Use 0 instead of smp_processor_id() for boot cpu.
* add cpu parameter to psr_assoc_init.
Changes in v2:
* Move initialization for psr_assoc from context switch to CPU_STARTING.
---
xen/arch/x86/domain.c | 7 ++---
xen/arch/x86/psr.c | 71 +++++++++++++++++++++++++++++++++--------------
xen/include/asm-x86/psr.h | 2 +-
3 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index fcea94b..c26c732 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1444,8 +1444,6 @@ static void __context_switch(void)
{
memcpy(&p->arch.user_regs, stack_regs, CTXT_SWITCH_STACK_BYTES);
vcpu_save_fpu(p);
- if ( psr_cmt_enabled() )
- psr_assoc_rmid(0);
p->arch.ctxt_switch_from(p);
}
@@ -1470,11 +1468,10 @@ static void __context_switch(void)
}
vcpu_restore_fpu_eager(n);
n->arch.ctxt_switch_to(n);
-
- if ( psr_cmt_enabled() && n->domain->arch.psr_rmid > 0 )
- psr_assoc_rmid(n->domain->arch.psr_rmid);
}
+ psr_ctxt_switch_to(n->domain);
+
gdt = !is_pv_32on64_vcpu(n) ? per_cpu(gdt_table, cpu) :
per_cpu(compat_gdt_table, cpu);
if ( need_full_gdt(n) )
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 344de3c..2490d22 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -22,7 +22,6 @@
struct psr_assoc {
uint64_t val;
- bool_t initialized;
};
struct psr_cmt *__read_mostly psr_cmt;
@@ -122,14 +121,6 @@ static void __init init_psr_cmt(unsigned int rmid_max)
printk(XENLOG_INFO "Cache Monitoring Technology enabled\n");
}
-static int __init init_psr(void)
-{
- if ( (opt_psr & PSR_CMT) && opt_rmid_max )
- init_psr_cmt(opt_rmid_max);
- return 0;
-}
-__initcall(init_psr);
-
/* Called with domain lock held, no psr specific lock needed */
int psr_alloc_rmid(struct domain *d)
{
@@ -175,27 +166,65 @@ void psr_free_rmid(struct domain *d)
d->arch.psr_rmid = 0;
}
-void psr_assoc_rmid(unsigned int rmid)
+static inline void psr_assoc_init(void)
{
- uint64_t val;
- uint64_t new_val;
struct psr_assoc *psra = &this_cpu(psr_assoc);
- if ( !psra->initialized )
- {
+ if ( psr_cmt_enabled() )
rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
- psra->initialized = 1;
- }
- val = psra->val;
+}
+
+static inline void psr_assoc_rmid(uint64_t *reg, unsigned int rmid)
+{
+ *reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
+}
+
+void psr_ctxt_switch_to(struct domain *d)
+{
+ struct psr_assoc *psra = &this_cpu(psr_assoc);
+ uint64_t reg = psra->val;
+
+ if ( psr_cmt_enabled() )
+ psr_assoc_rmid(®, d->arch.psr_rmid);
- new_val = (val & ~rmid_mask) | (rmid & rmid_mask);
- if ( val != new_val )
+ if ( reg != psra->val )
{
- wrmsrl(MSR_IA32_PSR_ASSOC, new_val);
- psra->val = new_val;
+ wrmsrl(MSR_IA32_PSR_ASSOC, reg);
+ psra->val = reg;
}
}
+static void psr_cpu_init(void)
+{
+ psr_assoc_init();
+}
+
+static int cpu_callback(
+ struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+ if ( action == CPU_STARTING )
+ psr_cpu_init();
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block cpu_nfb = {
+ .notifier_call = cpu_callback
+};
+
+static int __init psr_presmp_init(void)
+{
+ if ( (opt_psr & PSR_CMT) && opt_rmid_max )
+ init_psr_cmt(opt_rmid_max);
+
+ psr_cpu_init();
+ if ( psr_cmt_enabled() )
+ register_cpu_notifier(&cpu_nfb);
+
+ return 0;
+}
+presmp_initcall(psr_presmp_init);
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index c6076e9..12d593b 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -46,7 +46,7 @@ static inline bool_t psr_cmt_enabled(void)
int psr_alloc_rmid(struct domain *d);
void psr_free_rmid(struct domain *d);
-void psr_assoc_rmid(unsigned int rmid);
+void psr_ctxt_switch_to(struct domain *d);
#endif /* __ASM_PSR_H__ */
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
2015-05-08 8:56 ` [PATCH v7 01/14] x86: add socket_to_cpumask Chao Peng
2015-05-08 8:56 ` [PATCH v7 02/14] x86: improve psr scheduling code Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-18 13:33 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket Chao Peng
` (11 subsequent siblings)
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Detect Intel Cache Allocation Technology(CAT) feature and store the
cpuid information for later use. Currently only L3 cache allocation is
supported. The L3 CAT features may vary among sockets so per-socket
feature information is stored. The initialization can happen either at
boot time or when CPU(s) is hot plugged after booting.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v7:
* Clear the init/enable flag when a socket going offline.
* Reorder the statements in init_psr_cat.
Changes in v6:
* Introduce cat_socket_init(_enable)_bitmap.
Changes in v5:
* Add cos_max boot option.
Changes in v4:
* check X86_FEATURE_CAT available before doing initialization.
Changes in v3:
* Remove num_sockets boot option instead calculate it at boot time.
* Name hardcoded CAT cpuid leaf as PSR_CPUID_LEVEL_CAT.
Changes in v2:
* socket_num => num_sockets and fix several documentaion issues.
* refactor boot line parameters parsing into standlone patch.
* set opt_num_sockets = NR_CPUS when opt_num_sockets > NR_CPUS.
* replace CPU_ONLINE with CPU_STARTING and integrate that into scheduling
improvement patch.
* reimplement get_max_socket() with cpu_to_socket();
* cbm is still uint64 as there is a path forward for supporting long masks.
---
docs/misc/xen-command-line.markdown | 15 ++++-
xen/arch/x86/psr.c | 107 +++++++++++++++++++++++++++++++++++-
xen/include/asm-x86/cpufeature.h | 1 +
xen/include/asm-x86/psr.h | 3 +
4 files changed, 122 insertions(+), 4 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 1dda1f0..a3deb36 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1122,9 +1122,9 @@ This option can be specified more than once (up to 8 times at present).
> `= <integer>`
### psr (Intel)
-> `= List of ( cmt:<boolean> | rmid_max:<integer> )`
+> `= List of ( cmt:<boolean> | rmid_max:<integer> | cat:<boolean> | cos_max:<integer> )`
-> Default: `psr=cmt:0,rmid_max:255`
+> Default: `psr=cmt:0,rmid_max:255,cat:0,cos_max:255`
Platform Shared Resource(PSR) Services. Intel Haswell and later server
platforms offer information about the sharing of resources.
@@ -1134,6 +1134,12 @@ Monitoring ID(RMID) is used to bind the domain to corresponding shared
resource. RMID is a hardware-provided layer of abstraction between software
and logical processors.
+To use the PSR cache allocation service for a certain domain, a capacity
+bitmasks(CBM) is used to bind the domain to corresponding shared resource.
+CBM represents cache capacity and indicates the degree of overlap and isolation
+between domains. In hypervisor a Class of Service(COS) ID is allocated for each
+unique CBM.
+
The following resources are available:
* Cache Monitoring Technology (Haswell and later). Information regarding the
@@ -1144,6 +1150,11 @@ The following resources are available:
total/local memory bandwidth. Follow the same options with Cache Monitoring
Technology.
+* Cache Alllocation Technology (Broadwell and later). Information regarding
+ the cache allocation.
+ * `cat` instructs Xen to enable/disable Cache Allocation Technology.
+ * `cos_max` indicates the max value for COS ID.
+
### reboot
> `= t[riple] | k[bd] | a[cpi] | p[ci] | P[ower] | e[fi] | n[o] [, [w]arm | [c]old]`
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 2490d22..848eacc 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -19,14 +19,26 @@
#include <asm/psr.h>
#define PSR_CMT (1<<0)
+#define PSR_CAT (1<<1)
+
+struct psr_cat_socket_info {
+ unsigned int cbm_len;
+ unsigned int cos_max;
+};
struct psr_assoc {
uint64_t val;
};
struct psr_cmt *__read_mostly psr_cmt;
+
+static unsigned long *__read_mostly cat_socket_init_bitmap;
+static unsigned long *__read_mostly cat_socket_enable_bitmap;
+static struct psr_cat_socket_info *__read_mostly cat_socket_info;
+
static unsigned int __initdata opt_psr;
static unsigned int __initdata opt_rmid_max = 255;
+static unsigned int opt_cos_max = 255;
static uint64_t rmid_mask;
static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
@@ -63,10 +75,14 @@ static void __init parse_psr_param(char *s)
*val_str++ = '\0';
parse_psr_bool(s, val_str, "cmt", PSR_CMT);
+ parse_psr_bool(s, val_str, "cat", PSR_CAT);
if ( val_str && !strcmp(s, "rmid_max") )
opt_rmid_max = simple_strtoul(val_str, NULL, 0);
+ if ( val_str && !strcmp(s, "cos_max") )
+ opt_cos_max = simple_strtoul(val_str, NULL, 0);
+
s = ss + 1;
} while ( ss );
}
@@ -194,16 +210,100 @@ void psr_ctxt_switch_to(struct domain *d)
}
}
+static void cat_cpu_init(void)
+{
+ unsigned int eax, ebx, ecx, edx;
+ struct psr_cat_socket_info *info;
+ unsigned int socket;
+ unsigned int cpu = smp_processor_id();
+ const struct cpuinfo_x86 *c = cpu_data + cpu;
+
+ if ( !cpu_has(c, X86_FEATURE_CAT) )
+ return;
+
+ socket = cpu_to_socket(cpu);
+ if ( socket >= nr_sockets )
+ return;
+
+ /* Avoid initializing more than one times for the same socket. */
+ if ( test_and_set_bit(socket, cat_socket_init_bitmap) )
+ return;
+
+ cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
+ if ( ebx & PSR_RESOURCE_TYPE_L3 )
+ {
+ cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
+ info = cat_socket_info + socket;
+ info->cbm_len = (eax & 0x1f) + 1;
+ info->cos_max = min(opt_cos_max, edx & 0xffff);
+
+ set_bit(socket, cat_socket_enable_bitmap);
+ printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
+ socket, info->cos_max, info->cbm_len);
+ }
+}
+
+static void cat_cpu_fini(void)
+{
+ unsigned int cpu = smp_processor_id();
+ unsigned int socket = cpu_to_socket(cpu);
+
+ if ( socket >= nr_sockets )
+ return;
+
+ /*
+ * Mark the socket as uninitialized if the last cpu of the socket is
+ * getting offline.
+ */
+ if ( cpumask_weight(socket_to_cpumask[socket]) == 1 )
+ {
+ clear_bit(socket, cat_socket_enable_bitmap);
+ clear_bit(socket, cat_socket_init_bitmap);
+ }
+}
+
+static void __init init_psr_cat(void)
+{
+ size_t nlongs = BITS_TO_LONGS(nr_sockets);
+
+ cat_socket_init_bitmap = xzalloc_array(unsigned long, nlongs);
+ cat_socket_enable_bitmap = xzalloc_array(unsigned long, nlongs);
+ cat_socket_info = xzalloc_array(struct psr_cat_socket_info, nr_sockets);
+
+ if ( !cat_socket_init_bitmap ||
+ !cat_socket_enable_bitmap ||
+ !cat_socket_info )
+ {
+ xfree(cat_socket_init_bitmap);
+ cat_socket_init_bitmap = NULL;
+ xfree(cat_socket_enable_bitmap);
+ cat_socket_enable_bitmap = NULL;
+ xfree(cat_socket_info);
+ cat_socket_info = NULL;
+ }
+}
+
static void psr_cpu_init(void)
{
+ if ( cat_socket_info )
+ cat_cpu_init();
+
psr_assoc_init();
}
+static void psr_cpu_fini(void)
+{
+ if ( cat_socket_info )
+ cat_cpu_fini();
+}
+
static int cpu_callback(
struct notifier_block *nfb, unsigned long action, void *hcpu)
{
if ( action == CPU_STARTING )
psr_cpu_init();
+ else if ( action == CPU_DYING )
+ psr_cpu_fini();
return NOTIFY_DONE;
}
@@ -217,9 +317,12 @@ static int __init psr_presmp_init(void)
if ( (opt_psr & PSR_CMT) && opt_rmid_max )
init_psr_cmt(opt_rmid_max);
+ if ( opt_psr & PSR_CAT )
+ init_psr_cat();
+
psr_cpu_init();
- if ( psr_cmt_enabled() )
- register_cpu_notifier(&cpu_nfb);
+ if ( psr_cmt_enabled() || cat_socket_info )
+ register_cpu_notifier(&cpu_nfb);
return 0;
}
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 7963a3a..8c0f0a6 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -149,6 +149,7 @@
#define X86_FEATURE_CMT (7*32+12) /* Cache Monitoring Technology */
#define X86_FEATURE_NO_FPU_SEL (7*32+13) /* FPU CS/DS stored as zero */
#define X86_FEATURE_MPX (7*32+14) /* Memory Protection Extensions */
+#define X86_FEATURE_CAT (7*32+15) /* Cache Allocation Technology */
#define X86_FEATURE_RDSEED (7*32+18) /* RDSEED instruction */
#define X86_FEATURE_ADX (7*32+19) /* ADCX, ADOX instructions */
#define X86_FEATURE_SMAP (7*32+20) /* Supervisor Mode Access Prevention */
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 12d593b..bdda111 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -18,6 +18,9 @@
#include <xen/types.h>
+/* CAT cpuid level */
+#define PSR_CPUID_LEVEL_CAT 0x10
+
/* Resource Type Enumeration */
#define PSR_RESOURCE_TYPE_L3 0x2
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (2 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-18 13:35 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 05/14] x86: add COS information for each domain Chao Peng
` (10 subsequent siblings)
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
For each socket, a COS to CBM mapping structure is maintained for each
COS. The mapping is indexed by COS and the value is the corresponding
CBM. Different VMs may use the same CBM, a reference count is used to
indicate if the CBM is available.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v5:
* rename cos_cbm_map to cos_to_cbm.
---
xen/arch/x86/psr.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 848eacc..b775dbf 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -21,9 +21,15 @@
#define PSR_CMT (1<<0)
#define PSR_CAT (1<<1)
+struct psr_cat_cbm {
+ uint64_t cbm;
+ unsigned int ref;
+};
+
struct psr_cat_socket_info {
unsigned int cbm_len;
unsigned int cos_max;
+ struct psr_cat_cbm *cos_to_cbm;
};
struct psr_assoc {
@@ -237,6 +243,14 @@ static void cat_cpu_init(void)
info->cbm_len = (eax & 0x1f) + 1;
info->cos_max = min(opt_cos_max, edx & 0xffff);
+ info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
+ info->cos_max + 1UL);
+ if ( !info->cos_to_cbm )
+ return;
+
+ /* cos=0 is reserved as default cbm(all ones). */
+ info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
+
set_bit(socket, cat_socket_enable_bitmap);
printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
socket, info->cos_max, info->cbm_len);
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 05/14] x86: add COS information for each domain
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (3 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 06/14] x86: expose CBM length and COS number information Chao Peng
` (9 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
In Xen's implementation, the CAT enforcement granularity is per domain.
Due to the length of CBM and the number of COS may be socket-different,
each domain has COS ID for each socket. The domain get COS=0 by default
and at runtime its COS is then allocated dynamically when user specifies
a CBM for the domain.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v6:
* Add spinlock for cos_to_cbm.
---
xen/arch/x86/domain.c | 6 +++++-
xen/arch/x86/psr.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
xen/include/asm-x86/domain.h | 5 ++++-
xen/include/asm-x86/psr.h | 3 +++
4 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index c26c732..a0b5e25 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -617,6 +617,9 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
/* 64-bit PV guest by default. */
d->arch.is_32bit_pv = d->arch.has_32bit_shinfo = 0;
+ if ( (rc = psr_domain_init(d)) != 0 )
+ goto fail;
+
/* initialize default tsc behavior in case tools don't */
tsc_set_info(d, TSC_MODE_DEFAULT, 0UL, 0, 0);
spin_lock_init(&d->arch.vtsc_lock);
@@ -635,6 +638,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
free_perdomain_mappings(d);
if ( is_pv_domain(d) )
free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
+ psr_domain_free(d);
return rc;
}
@@ -658,7 +662,7 @@ void arch_domain_destroy(struct domain *d)
free_xenheap_page(d->shared_info);
cleanup_domain_irq_mapping(d);
- psr_free_rmid(d);
+ psr_domain_free(d);
}
void arch_domain_shutdown(struct domain *d)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index b775dbf..752499b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -30,6 +30,7 @@ struct psr_cat_socket_info {
unsigned int cbm_len;
unsigned int cos_max;
struct psr_cat_cbm *cos_to_cbm;
+ spinlock_t cbm_lock;
};
struct psr_assoc {
@@ -216,6 +217,52 @@ void psr_ctxt_switch_to(struct domain *d)
}
}
+/* Called with domain lock held, no extra lock needed for 'psr_cos_ids' */
+static void psr_free_cos(struct domain *d)
+{
+ unsigned int socket;
+ unsigned int cos;
+ struct psr_cat_socket_info *info;
+
+ if( !d->arch.psr_cos_ids )
+ return;
+
+ for ( socket = 0; socket < nr_sockets; socket++ )
+ {
+ if ( !test_bit(socket, cat_socket_enable_bitmap) )
+ continue;
+
+ if ( (cos = d->arch.psr_cos_ids[socket]) == 0 )
+ continue;
+
+ info = cat_socket_info + socket;
+ spin_lock(&info->cbm_lock);
+ info->cos_to_cbm[cos].ref--;
+ spin_unlock(&info->cbm_lock);
+ }
+
+ xfree(d->arch.psr_cos_ids);
+ d->arch.psr_cos_ids = NULL;
+}
+
+int psr_domain_init(struct domain *d)
+{
+ if ( cat_socket_info )
+ {
+ d->arch.psr_cos_ids = xzalloc_array(unsigned int, nr_sockets);
+ if ( !d->arch.psr_cos_ids )
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+void psr_domain_free(struct domain *d)
+{
+ psr_free_rmid(d);
+ psr_free_cos(d);
+}
+
static void cat_cpu_init(void)
{
unsigned int eax, ebx, ecx, edx;
@@ -251,6 +298,8 @@ static void cat_cpu_init(void)
/* cos=0 is reserved as default cbm(all ones). */
info->cos_to_cbm[0].cbm = (1ull << info->cbm_len) - 1;
+ spin_lock_init(&info->cbm_lock);
+
set_bit(socket, cat_socket_enable_bitmap);
printk(XENLOG_INFO "CAT: enabled on socket %u, cos_max:%u, cbm_len:%u\n",
socket, info->cos_max, info->cbm_len);
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e5102cc..324011d 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -333,7 +333,10 @@ struct arch_domain
struct e820entry *e820;
unsigned int nr_e820;
- unsigned int psr_rmid; /* RMID assigned to the domain for CMT */
+ /* RMID assigned to the domain for CMT */
+ unsigned int psr_rmid;
+ /* COS assigned to the domain for each socket */
+ unsigned int *psr_cos_ids;
/* Shared page for notifying that explicit PIRQ EOI is required. */
unsigned long *pirq_eoi_map;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index bdda111..1023d5f 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -51,6 +51,9 @@ int psr_alloc_rmid(struct domain *d);
void psr_free_rmid(struct domain *d);
void psr_ctxt_switch_to(struct domain *d);
+int psr_domain_init(struct domain *d);
+void psr_domain_free(struct domain *d);
+
#endif /* __ASM_PSR_H__ */
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 06/14] x86: expose CBM length and COS number information
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (4 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 05/14] x86: add COS information for each domain Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 07/14] x86: dynamically get/set CBM for a domain Chao Peng
` (8 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
General CAT information such as maximum COS and CBM length are exposed to
user space by a SYSCTL hypercall, to help user space to construct the CBM.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes in v7:
* Copyback psr_cat_op only for XEN_SYSCTL_PSR_CAT_get_l3_info.
---
xen/arch/x86/psr.c | 31 +++++++++++++++++++++++++++++++
xen/arch/x86/sysctl.c | 18 ++++++++++++++++++
xen/include/asm-x86/psr.h | 3 +++
xen/include/public/sysctl.h | 16 ++++++++++++++++
4 files changed, 68 insertions(+)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 752499b..1feb2f6 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -217,6 +217,37 @@ void psr_ctxt_switch_to(struct domain *d)
}
}
+static int get_cat_socket_info(unsigned int socket,
+ struct psr_cat_socket_info **info)
+{
+ if ( !cat_socket_info )
+ return -ENODEV;
+
+ if ( socket >= nr_sockets )
+ return -EBADSLT;
+
+ if ( !test_bit(socket, cat_socket_enable_bitmap) )
+ return -ENOENT;
+
+ *info = cat_socket_info + socket;
+ return 0;
+}
+
+int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
+ uint32_t *cos_max)
+{
+ struct psr_cat_socket_info *info;
+ int ret = get_cat_socket_info(socket, &info);
+
+ if ( ret )
+ return ret;
+
+ *cbm_len = info->cbm_len;
+ *cos_max = info->cos_max;
+
+ return 0;
+}
+
/* Called with domain lock held, no extra lock needed for 'psr_cos_ids' */
static void psr_free_cos(struct domain *d)
{
diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
index 611a291..f36b52f 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -171,6 +171,24 @@ long arch_do_sysctl(
break;
+ case XEN_SYSCTL_psr_cat_op:
+ switch ( sysctl->u.psr_cat_op.cmd )
+ {
+ case XEN_SYSCTL_PSR_CAT_get_l3_info:
+ ret = psr_get_cat_l3_info(sysctl->u.psr_cat_op.target,
+ &sysctl->u.psr_cat_op.u.l3_info.cbm_len,
+ &sysctl->u.psr_cat_op.u.l3_info.cos_max);
+
+ if ( !ret && __copy_field_to_guest(u_sysctl, sysctl, u.psr_cat_op) )
+ ret = -EFAULT;
+
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+ break;
+
default:
ret = -ENOSYS;
break;
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index 1023d5f..d364e8c 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -51,6 +51,9 @@ int psr_alloc_rmid(struct domain *d);
void psr_free_rmid(struct domain *d);
void psr_ctxt_switch_to(struct domain *d);
+int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
+ uint32_t *cos_max);
+
int psr_domain_init(struct domain *d);
void psr_domain_free(struct domain *d);
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 711441f..f28e460 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -661,6 +661,20 @@ struct xen_sysctl_psr_cmt_op {
typedef struct xen_sysctl_psr_cmt_op xen_sysctl_psr_cmt_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cmt_op_t);
+#define XEN_SYSCTL_PSR_CAT_get_l3_info 0
+struct xen_sysctl_psr_cat_op {
+ uint32_t cmd; /* IN: XEN_SYSCTL_PSR_CAT_* */
+ uint32_t target; /* IN: socket to be operated on */
+ union {
+ struct {
+ uint32_t cbm_len; /* OUT: CBM length */
+ uint32_t cos_max; /* OUT: Maximum COS */
+ } l3_info;
+ } u;
+};
+typedef struct xen_sysctl_psr_cat_op xen_sysctl_psr_cat_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
+
struct xen_sysctl {
uint32_t cmd;
#define XEN_SYSCTL_readconsole 1
@@ -683,6 +697,7 @@ struct xen_sysctl {
#define XEN_SYSCTL_scheduler_op 19
#define XEN_SYSCTL_coverage_op 20
#define XEN_SYSCTL_psr_cmt_op 21
+#define XEN_SYSCTL_psr_cat_op 22
uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
union {
struct xen_sysctl_readconsole readconsole;
@@ -705,6 +720,7 @@ struct xen_sysctl {
struct xen_sysctl_scheduler_op scheduler_op;
struct xen_sysctl_coverage_op coverage_op;
struct xen_sysctl_psr_cmt_op psr_cmt_op;
+ struct xen_sysctl_psr_cat_op psr_cat_op;
uint8_t pad[128];
} u;
};
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 07/14] x86: dynamically get/set CBM for a domain
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (5 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 06/14] x86: expose CBM length and COS number information Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-14 9:19 ` Dario Faggioli
2015-05-18 13:45 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 08/14] x86: add scheduling support for Intel CAT Chao Peng
` (7 subsequent siblings)
14 siblings, 2 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
For CAT, COS is maintained in hypervisor only while CBM is exposed to
user space directly to allow getting/setting domain's cache capacity.
For each specified CBM, hypervisor will either use a existed COS which
has the same CBM or allocate a new one if the same CBM is not found. If
the allocation fails because of no enough COS available then error is
returned. The getting/setting are always operated on a specified socket.
For multiple sockets system, the interface may be called several times.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v7:
* find => found in psr_set_l3_cbm().
Changes in v6:
* Correct spin_lock scope.
Changes in v5:
* Add spin_lock to protect cbm_map.
---
xen/arch/x86/domctl.c | 20 ++++++
xen/arch/x86/psr.c | 137 ++++++++++++++++++++++++++++++++++++++++
xen/include/asm-x86/msr-index.h | 1 +
xen/include/asm-x86/psr.h | 2 +
xen/include/public/domctl.h | 12 ++++
5 files changed, 172 insertions(+)
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 9450795..7ffa650 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1334,6 +1334,26 @@ long arch_do_domctl(
}
break;
+ case XEN_DOMCTL_psr_cat_op:
+ switch ( domctl->u.psr_cat_op.cmd )
+ {
+ case XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM:
+ ret = psr_set_l3_cbm(d, domctl->u.psr_cat_op.target,
+ domctl->u.psr_cat_op.data);
+ break;
+
+ case XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM:
+ ret = psr_get_l3_cbm(d, domctl->u.psr_cat_op.target,
+ &domctl->u.psr_cat_op.data);
+ copyback = 1;
+ break;
+
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+ break;
+
default:
ret = iommu_do_domctl(domctl, d, u_domctl);
break;
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 1feb2f6..385807b 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -49,6 +49,14 @@ static unsigned int opt_cos_max = 255;
static uint64_t rmid_mask;
static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
+static unsigned int get_socket_cpu(unsigned int socket)
+{
+ if ( socket < nr_sockets )
+ return cpumask_any(socket_to_cpumask[socket]);
+
+ return nr_cpu_ids;
+}
+
static void __init parse_psr_bool(char *s, char *value, char *feature,
unsigned int mask)
{
@@ -248,6 +256,135 @@ int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
return 0;
}
+int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
+{
+ unsigned int cos;
+ struct psr_cat_socket_info *info;
+ int ret = get_cat_socket_info(socket, &info);
+
+ if ( ret )
+ return ret;
+
+ cos = d->arch.psr_cos_ids[socket];
+ *cbm = info->cos_to_cbm[cos].cbm;
+ return 0;
+}
+
+static bool_t psr_check_cbm(unsigned int cbm_len, uint64_t cbm)
+{
+ unsigned int first_bit, zero_bit;
+
+ /* Set bits should only in the range of [0, cbm_len). */
+ if ( cbm & (~0ull << cbm_len) )
+ return 0;
+
+ first_bit = find_first_bit(&cbm, cbm_len);
+ zero_bit = find_next_zero_bit(&cbm, cbm_len, first_bit);
+
+ /* Set bits should be contiguous. */
+ if ( zero_bit < cbm_len &&
+ find_next_bit(&cbm, cbm_len, zero_bit) < cbm_len )
+ return 0;
+
+ return 1;
+}
+
+struct cos_cbm_info
+{
+ unsigned int cos;
+ uint64_t cbm;
+};
+
+static void do_write_l3_cbm(void *data)
+{
+ struct cos_cbm_info *info = data;
+
+ wrmsrl(MSR_IA32_PSR_L3_MASK(info->cos), info->cbm);
+}
+
+static int write_l3_cbm(unsigned int socket, unsigned int cos, uint64_t cbm)
+{
+ struct cos_cbm_info info = { .cos = cos, .cbm = cbm };
+
+ if ( socket == cpu_to_socket(smp_processor_id()) )
+ do_write_l3_cbm(&info);
+ else
+ {
+ unsigned int cpu = get_socket_cpu(socket);
+
+ if ( cpu >= nr_cpu_ids )
+ return -EBADSLT;
+ on_selected_cpus(cpumask_of(cpu), do_write_l3_cbm, &info, 1);
+ }
+
+ return 0;
+}
+
+int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm)
+{
+ unsigned int old_cos, cos;
+ struct psr_cat_cbm *map, *found = NULL;
+ struct psr_cat_socket_info *info;
+ int ret = get_cat_socket_info(socket, &info);
+
+ if ( ret )
+ return ret;
+
+ if ( !psr_check_cbm(info->cbm_len, cbm) )
+ return -EINVAL;
+
+ old_cos = d->arch.psr_cos_ids[socket];
+ map = info->cos_to_cbm;
+
+ spin_lock(&info->cbm_lock);
+
+ for ( cos = 0; cos <= info->cos_max; cos++ )
+ {
+ /* If still not found, then keep unused one. */
+ if ( !found && cos != 0 && map[cos].ref == 0 )
+ found = map + cos;
+ else if ( map[cos].cbm == cbm )
+ {
+ if ( unlikely(cos == old_cos) )
+ {
+ spin_unlock(&info->cbm_lock);
+ return 0;
+ }
+ found = map + cos;
+ break;
+ }
+ }
+
+ /* If old cos is referred only by the domain, then use it. */
+ if ( !found && map[old_cos].ref == 1 )
+ found = map + old_cos;
+
+ if ( !found )
+ {
+ spin_unlock(&info->cbm_lock);
+ return -EUSERS;
+ }
+
+ cos = found - map;
+ if ( found->cbm != cbm )
+ {
+ ret = write_l3_cbm(socket, cos, cbm);
+ if ( ret )
+ {
+ spin_unlock(&info->cbm_lock);
+ return ret;
+ }
+ found->cbm = cbm;
+ }
+
+ found->ref++;
+ map[old_cos].ref--;
+ spin_unlock(&info->cbm_lock);
+
+ d->arch.psr_cos_ids[socket] = cos;
+ return 0;
+}
+
/* Called with domain lock held, no extra lock needed for 'psr_cos_ids' */
static void psr_free_cos(struct domain *d)
{
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index 83f2f70..5425f77 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -327,6 +327,7 @@
#define MSR_IA32_CMT_EVTSEL 0x00000c8d
#define MSR_IA32_CMT_CTR 0x00000c8e
#define MSR_IA32_PSR_ASSOC 0x00000c8f
+#define MSR_IA32_PSR_L3_MASK(n) (0x00000c90 + (n))
/* Intel Model 6 */
#define MSR_P6_PERFCTR(n) (0x000000c1 + (n))
diff --git a/xen/include/asm-x86/psr.h b/xen/include/asm-x86/psr.h
index d364e8c..081750f 100644
--- a/xen/include/asm-x86/psr.h
+++ b/xen/include/asm-x86/psr.h
@@ -53,6 +53,8 @@ void psr_ctxt_switch_to(struct domain *d);
int psr_get_cat_l3_info(unsigned int socket, uint32_t *cbm_len,
uint32_t *cos_max);
+int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm);
+int psr_set_l3_cbm(struct domain *d, unsigned int socket, uint64_t cbm);
int psr_domain_init(struct domain *d);
void psr_domain_free(struct domain *d);
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index d2e8db0..9337bb6 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1001,6 +1001,16 @@ struct xen_domctl_psr_cmt_op {
typedef struct xen_domctl_psr_cmt_op xen_domctl_psr_cmt_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cmt_op_t);
+struct xen_domctl_psr_cat_op {
+#define XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM 0
+#define XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM 1
+ uint32_t cmd; /* IN: XEN_DOMCTL_PSR_CAT_OP_* */
+ uint32_t target; /* IN: socket to be operated on */
+ uint64_t data; /* IN/OUT */
+};
+typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
+
struct xen_domctl {
uint32_t cmd;
#define XEN_DOMCTL_createdomain 1
@@ -1075,6 +1085,7 @@ struct xen_domctl {
#define XEN_DOMCTL_set_vcpu_msrs 73
#define XEN_DOMCTL_setvnumainfo 74
#define XEN_DOMCTL_psr_cmt_op 75
+#define XEN_DOMCTL_psr_cat_op 76
#define XEN_DOMCTL_gdbsx_guestmemio 1000
#define XEN_DOMCTL_gdbsx_pausevcpu 1001
#define XEN_DOMCTL_gdbsx_unpausevcpu 1002
@@ -1137,6 +1148,7 @@ struct xen_domctl {
struct xen_domctl_gdbsx_domstatus gdbsx_domstatus;
struct xen_domctl_vnuma vnuma;
struct xen_domctl_psr_cmt_op psr_cmt_op;
+ struct xen_domctl_psr_cat_op psr_cat_op;
uint8_t pad[128];
} u;
};
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 08/14] x86: add scheduling support for Intel CAT
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (6 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 07/14] x86: dynamically get/set CBM for a domain Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 09/14] xsm: add CAT related xsm policies Chao Peng
` (6 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
On context switch, write the the domain's Class of Service(COS) to MSR
IA32_PQR_ASSOC, to notify hardware to use the new COS.
For performance reason, the COS mask for current cpu is also cached in
the local per-CPU variable.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Changes in v5:
* Remove the need to cache socket.
Changes in v2:
* merge common scheduling changes into scheduling improvement patch.
* use readable expr for psra->cos_mask.
---
xen/arch/x86/psr.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
index 385807b..72721ea 100644
--- a/xen/arch/x86/psr.c
+++ b/xen/arch/x86/psr.c
@@ -35,6 +35,7 @@ struct psr_cat_socket_info {
struct psr_assoc {
uint64_t val;
+ uint64_t cos_mask;
};
struct psr_cmt *__read_mostly psr_cmt;
@@ -201,7 +202,16 @@ static inline void psr_assoc_init(void)
{
struct psr_assoc *psra = &this_cpu(psr_assoc);
- if ( psr_cmt_enabled() )
+ if ( cat_socket_info )
+ {
+ unsigned int socket = cpu_to_socket(smp_processor_id());
+
+ if ( socket < nr_sockets && test_bit(socket, cat_socket_enable_bitmap) )
+ psra->cos_mask = ((1ull << get_count_order(
+ cat_socket_info[socket].cos_max)) - 1) << 32;
+ }
+
+ if ( psr_cmt_enabled() || psra->cos_mask )
rdmsrl(MSR_IA32_PSR_ASSOC, psra->val);
}
@@ -210,6 +220,12 @@ static inline void psr_assoc_rmid(uint64_t *reg, unsigned int rmid)
*reg = (*reg & ~rmid_mask) | (rmid & rmid_mask);
}
+static inline void psr_assoc_cos(uint64_t *reg, unsigned int cos,
+ uint64_t cos_mask)
+{
+ *reg = (*reg & ~cos_mask) | (((uint64_t)cos << 32) & cos_mask);
+}
+
void psr_ctxt_switch_to(struct domain *d)
{
struct psr_assoc *psra = &this_cpu(psr_assoc);
@@ -218,6 +234,11 @@ void psr_ctxt_switch_to(struct domain *d)
if ( psr_cmt_enabled() )
psr_assoc_rmid(®, d->arch.psr_rmid);
+ if ( psra->cos_mask )
+ psr_assoc_cos(®, d->arch.psr_cos_ids ?
+ d->arch.psr_cos_ids[cpu_to_socket(smp_processor_id())] :
+ 0, psra->cos_mask);
+
if ( reg != psra->val )
{
wrmsrl(MSR_IA32_PSR_ASSOC, reg);
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 09/14] xsm: add CAT related xsm policies
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (7 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 08/14] x86: add scheduling support for Intel CAT Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 10/14] tools/libxl: minor name changes for CMT commands Chao Peng
` (5 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Add xsm policies for Cache Allocation Technology(CAT) related hypercalls
to restrict the functions visibility to control domain only.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
tools/flask/policy/policy/modules/xen/xen.if | 2 +-
tools/flask/policy/policy/modules/xen/xen.te | 4 +++-
xen/xsm/flask/hooks.c | 6 ++++++
xen/xsm/flask/policy/access_vectors | 4 ++++
4 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/flask/policy/policy/modules/xen/xen.if b/tools/flask/policy/policy/modules/xen/xen.if
index 620d151..aa5eb72 100644
--- a/tools/flask/policy/policy/modules/xen/xen.if
+++ b/tools/flask/policy/policy/modules/xen/xen.if
@@ -51,7 +51,7 @@ define(`create_domain_common', `
getaffinity setaffinity setvcpuextstate };
allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim
set_max_evtchn set_vnumainfo get_vnumainfo cacheflush
- psr_cmt_op };
+ psr_cmt_op psr_cat_op };
allow $1 $2:security check_context;
allow $1 $2:shadow enable;
allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp };
diff --git a/tools/flask/policy/policy/modules/xen/xen.te b/tools/flask/policy/policy/modules/xen/xen.te
index e555d11..6dcf953 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -67,6 +67,7 @@ allow dom0_t xen_t:xen {
allow dom0_t xen_t:xen2 {
resource_op
psr_cmt_op
+ psr_cat_op
};
allow dom0_t xen_t:mmu memorymap;
@@ -80,7 +81,8 @@ allow dom0_t dom0_t:domain {
getpodtarget setpodtarget set_misc_info set_virq_handler
};
allow dom0_t dom0_t:domain2 {
- set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo get_vnumainfo psr_cmt_op
+ set_cpuid gettsc settsc setscheduler set_max_evtchn set_vnumainfo
+ get_vnumainfo psr_cmt_op psr_cat_op
};
allow dom0_t dom0_t:resource { add remove };
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index ab5141d..72fe9b3 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -729,6 +729,9 @@ static int flask_domctl(struct domain *d, int cmd)
case XEN_DOMCTL_psr_cmt_op:
return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CMT_OP);
+ case XEN_DOMCTL_psr_cat_op:
+ return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__PSR_CAT_OP);
+
default:
printk("flask_domctl: Unknown op %d\n", cmd);
return -EPERM;
@@ -787,6 +790,9 @@ static int flask_sysctl(int cmd)
case XEN_SYSCTL_psr_cmt_op:
return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
XEN2__PSR_CMT_OP, NULL);
+ case XEN_SYSCTL_psr_cat_op:
+ return avc_current_has_perm(SECINITSID_XEN, SECCLASS_XEN2,
+ XEN2__PSR_CAT_OP, NULL);
default:
printk("flask_sysctl: Unknown op %d\n", cmd);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index 128250e..bdf496e 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -84,6 +84,8 @@ class xen2
resource_op
# XEN_SYSCTL_psr_cmt_op
psr_cmt_op
+# XEN_SYSCTL_psr_cat_op
+ psr_cat_op
}
# Classes domain and domain2 consist of operations that a domain performs on
@@ -219,6 +221,8 @@ class domain2
get_vnumainfo
# XEN_DOMCTL_psr_cmt_op
psr_cmt_op
+# XEN_DOMCTL_psr_cat_op
+ psr_cat_op
}
# Similar to class domain, but primarily contains domctls related to HVM domains
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 10/14] tools/libxl: minor name changes for CMT commands
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (8 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 09/14] xsm: add CAT related xsm policies Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 11/14] tools/libxl: add command to show PSR hardware info Chao Peng
` (4 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Use "-" instead of "_" for monitor types.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
tools/libxl/xl_cmdimpl.c | 6 +++---
tools/libxl/xl_cmdtable.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 394b55d..c666d84 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8220,11 +8220,11 @@ int main_psr_cmt_show(int argc, char **argv)
/* No options */
}
- if (!strcmp(argv[optind], "cache_occupancy"))
+ if (!strcmp(argv[optind], "cache-occupancy"))
type = LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY;
- else if (!strcmp(argv[optind], "total_mem_bandwidth"))
+ else if (!strcmp(argv[optind], "total-mem-bandwidth"))
type = LIBXL_PSR_CMT_TYPE_TOTAL_MEM_COUNT;
- else if (!strcmp(argv[optind], "local_mem_bandwidth"))
+ else if (!strcmp(argv[optind], "local-mem-bandwidth"))
type = LIBXL_PSR_CMT_TYPE_LOCAL_MEM_COUNT;
else {
help("psr-cmt-show");
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index 9284887..e0be5b6 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -539,9 +539,9 @@ struct cmd_spec cmd_table[] = {
"Show Cache Monitoring Technology information",
"<PSR-CMT-Type> <Domain>",
"Available monitor types:\n"
- "\"cache_occupancy\": Show L3 cache occupancy(KB)\n"
- "\"total_mem_bandwidth\": Show total memory bandwidth(KB/s)\n"
- "\"local_mem_bandwidth\": Show local memory bandwidth(KB/s)\n",
+ "\"cache-occupancy\": Show L3 cache occupancy(KB)\n"
+ "\"total-mem-bandwidth\": Show total memory bandwidth(KB/s)\n"
+ "\"local-mem-bandwidth\": Show local memory bandwidth(KB/s)\n",
},
#endif
};
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 11/14] tools/libxl: add command to show PSR hardware info
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (9 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 10/14] tools/libxl: minor name changes for CMT commands Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 12/14] tools/libxl: introduce some socket helpers Chao Peng
` (3 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Add dedicated one to show hardware information.
[root@vmm-psr]xl psr-hwinfo
Cache Monitoring Technology (CMT):
Enabled : 1
Total RMID : 63
Supported monitor types:
cache-occupancy
total-mem-bandwidth
local-mem-bandwidth
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v6:
* Add SWITCH_FOREACH_OPT to make '-h' work.
---
docs/man/xl.pod.1 | 4 ++++
tools/libxl/xl.h | 1 +
tools/libxl/xl_cmdimpl.c | 41 +++++++++++++++++++++++++++++++++++++++++
tools/libxl/xl_cmdtable.c | 5 +++++
4 files changed, 51 insertions(+)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 16783c8..7fd9bff 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1495,6 +1495,10 @@ for any of these monitoring types.
=over 4
+=item B<psr-hwinfo>
+
+Show CMT hardware information.
+
=item B<psr-cmt-attach> [I<domain-id>]
attach: Attach the platform shared resource monitoring service to a domain.
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5bc138c..7b56449 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -113,6 +113,7 @@ int main_remus(int argc, char **argv);
#endif
int main_devd(int argc, char **argv);
#ifdef LIBXL_HAVE_PSR_CMT
+int main_psr_hwinfo(int argc, char **argv);
int main_psr_cmt_attach(int argc, char **argv);
int main_psr_cmt_detach(int argc, char **argv);
int main_psr_cmt_show(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index c666d84..59ce257 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8014,6 +8014,36 @@ out:
}
#ifdef LIBXL_HAVE_PSR_CMT
+static int psr_cmt_hwinfo(void)
+{
+ int rc;
+ int enabled;
+ uint32_t total_rmid;
+
+ printf("Cache Monitoring Technology (CMT):\n");
+
+ enabled = libxl_psr_cmt_enabled(ctx);
+ printf("%-16s: %s\n", "Enabled", enabled ? "1" : "0");
+ if (!enabled)
+ return 0;
+
+ rc = libxl_psr_cmt_get_total_rmid(ctx, &total_rmid);
+ if (rc) {
+ fprintf(stderr, "Failed to get max RMID value\n");
+ return rc;
+ }
+ printf("%-16s: %u\n", "Total RMID", total_rmid);
+
+ printf("Supported monitor types:\n");
+ if (libxl_psr_cmt_type_supported(ctx, LIBXL_PSR_CMT_TYPE_CACHE_OCCUPANCY))
+ printf("cache-occupancy\n");
+ if (libxl_psr_cmt_type_supported(ctx, LIBXL_PSR_CMT_TYPE_TOTAL_MEM_COUNT))
+ printf("total-mem-bandwidth\n");
+ if (libxl_psr_cmt_type_supported(ctx, LIBXL_PSR_CMT_TYPE_LOCAL_MEM_COUNT))
+ printf("local-mem-bandwidth\n");
+
+ return rc;
+}
#define MBM_SAMPLE_RETRY_MAX 4
static int psr_cmt_get_mem_bandwidth(uint32_t domid,
@@ -8180,6 +8210,17 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
return 0;
}
+int main_psr_hwinfo(int argc, char **argv)
+{
+ int opt;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "psr-hwinfo", 0) {
+ /* No options */
+ }
+
+ return psr_cmt_hwinfo();
+}
+
int main_psr_cmt_attach(int argc, char **argv)
{
uint32_t domid;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index e0be5b6..be606af 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -524,6 +524,11 @@ struct cmd_spec cmd_table[] = {
"-F Run in the foreground",
},
#ifdef LIBXL_HAVE_PSR_CMT
+ { "psr-hwinfo",
+ &main_psr_hwinfo, 0, 1,
+ "Show hardware information for Platform Shared Resource",
+ "",
+ },
{ "psr-cmt-attach",
&main_psr_cmt_attach, 0, 1,
"Attach Cache Monitoring Technology service to a domain",
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 12/14] tools/libxl: introduce some socket helpers
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (10 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 11/14] tools/libxl: add command to show PSR hardware info Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 13/14] tools: add tools support for Intel CAT Chao Peng
` (2 subsequent siblings)
14 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Add libxl_socket_bitmap_alloc() to allow allocating a socket specific
libxl_bitmap (as it is for cpu/node bitmap).
Internal function libxl__count_physical_sockets() is introduced together
to get the socket count when the size of bitmap is not specified.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v7:
* Broadcast LIBXL_HAVE_SOCKET_BITMAP_ALLOC
---
tools/libxl/libxl.h | 7 +++++++
tools/libxl/libxl_internal.h | 2 ++
tools/libxl/libxl_utils.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
tools/libxl/libxl_utils.h | 2 ++
4 files changed, 57 insertions(+)
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..51bdff4 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -736,6 +736,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
#define LIBXL_HAVE_PSR_MBM 1
#endif
+/*
+ * LIBXL_HAVE_SOCKET_BITMAP_ALLOC
+ *
+ * If this is defined, then libxl_socket_bitmap_alloc exists.
+ */
+#define LIBXL_HAVE_SOCKET_BITMAP_ALLOC 1
+
typedef char **libxl_string_list;
void libxl_string_list_dispose(libxl_string_list *sl);
int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9c22309..32cc725 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3684,6 +3684,8 @@ static inline void libxl__update_config_vtpm(libxl__gc *gc,
*/
void libxl__bitmap_copy_best_effort(libxl__gc *gc, libxl_bitmap *dptr,
const libxl_bitmap *sptr);
+
+int libxl__count_physical_sockets(libxl__gc *gc, int *sockets);
#endif
/*
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 9053b27..5fbace8 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -770,6 +770,52 @@ int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
return rc;
}
+int libxl__count_physical_sockets(libxl__gc *gc, int *sockets)
+{
+ int rc;
+ libxl_physinfo info;
+
+ libxl_physinfo_init(&info);
+
+ rc = libxl_get_physinfo(CTX, &info);
+ if (rc)
+ return rc;
+
+ *sockets = info.nr_cpus / info.threads_per_core
+ / info.cores_per_socket;
+
+ libxl_physinfo_dispose(&info);
+ return 0;
+}
+
+int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *socketmap,
+ int max_sockets)
+{
+ GC_INIT(ctx);
+ int rc = 0;
+
+ if (max_sockets < 0) {
+ rc = ERROR_INVAL;
+ LOG(ERROR, "invalid number of sockets provided");
+ goto out;
+ }
+
+ if (max_sockets == 0) {
+ rc = libxl__count_physical_sockets(gc, &max_sockets);
+ if (rc) {
+ LOGE(ERROR, "failed to get system socket count");
+ goto out;
+ }
+ }
+ /* This can't fail: no need to check and log */
+ libxl_bitmap_alloc(ctx, socketmap, max_sockets);
+
+ out:
+ GC_FREE;
+ return rc;
+
+}
+
int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
const libxl_bitmap *nodemap,
libxl_bitmap *cpumap)
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index 68b5580..36ef67d 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -135,6 +135,8 @@ static inline int libxl_bitmap_equal(const libxl_bitmap *ba,
int libxl_cpu_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *cpumap, int max_cpus);
int libxl_node_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *nodemap,
int max_nodes);
+int libxl_socket_bitmap_alloc(libxl_ctx *ctx, libxl_bitmap *socketmap,
+ int max_sockets);
/* Populate cpumap with the cpus spanned by the nodes in nodemap */
int libxl_nodemap_to_cpumap(libxl_ctx *ctx,
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 13/14] tools: add tools support for Intel CAT
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (11 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 12/14] tools/libxl: introduce some socket helpers Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 13:38 ` Ian Campbell
2015-05-08 8:56 ` [PATCH v7 14/14] docs: add xl-psr.markdown Chao Peng
2015-05-08 13:41 ` [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Ian Campbell
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
This is the xc/xl changes to support Intel Cache Allocation
Technology(CAT).
'xl psr-hwinfo' is updated to show CAT info and two new commands
for CAT are introduced:
- xl psr-cat-cbm-set [-s socket] <domain> <cbm>
Set cache capacity bitmasks(CBM) for a domain.
- xl psr-cat-show <domain>
Show CAT domain information.
Examples:
[root@vmm-psr vmm]# xl psr-hwinfo --cat
Cache Allocation Technology (CAT):
Socket ID : 0
L3 Cache : 12288KB
Maximum COS : 15
CBM length : 12
Default CBM : 0xfff
[root@vmm-psr vmm]# xl psr-cat-cbm-set 0 0xff
[root@vmm-psr vmm]# xl psr-cat-show
Socket ID : 0
L3 Cache : 12288KB
Default CBM : 0xfff
ID NAME CBM
0 Domain-0 0xff
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Changes in v7:
* Add PSR head1 level section and change CMT/CAT as its subsections for xl man page.
* Other minor document changes.
Changes in v6:
* Merge xl psr-cmt/cat-hwinfo => xl psr-hwinfo.
* Add function header to explain the 'target' parameter.
* Use bitmap instead of TARGETS_ALL.
* Remove the need to store the return value form libxc.
* Minor document/commit msg adjustment.
Changes in v5:
* Add psr-cat-hwinfo.
* Add libxl_psr_cat_info_list_free.
* malloc => libxl__malloc
* Other comments from Ian/Wei.
Changes in v4:
* Add example output in commit message.
* Make libxl__count_physical_sockets private to libxl_psr.c.
* Set errno in several error cases.
* Change libxl_psr_cat_get_l3_info to return all sockets information.
* Remove unused libxl_domain_info call.
Changes in v3:
* Add manpage.
* libxl_psr_cat_set/get_domain_data => libxl_psr_cat_set/get_cbm.
* Move libxl_count_physical_sockets into seperate patch.
* Support LIBXL_PSR_TARGET_ALL for libxl_psr_cat_set_cbm.
* Clean up the print codes.
---
docs/man/xl.pod.1 | 75 +++++++++++--
tools/libxc/include/xenctrl.h | 15 +++
tools/libxc/xc_psr.c | 76 ++++++++++++++
tools/libxl/libxl.h | 35 +++++++
tools/libxl/libxl_psr.c | 143 +++++++++++++++++++++++--
tools/libxl/libxl_types.idl | 10 ++
tools/libxl/xl.h | 4 +
tools/libxl/xl_cmdimpl.c | 237 ++++++++++++++++++++++++++++++++++++++++--
tools/libxl/xl_cmdtable.c | 18 +++-
9 files changed, 584 insertions(+), 29 deletions(-)
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 7fd9bff..795ce91 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1477,28 +1477,52 @@ policy. Loading new security policy will reset runtime changes to device labels.
=back
-=head1 CACHE MONITORING TECHNOLOGY
+=head1 PLATFORM SHARED RESOURCE MONITORING/CONTROL
+
+Intel Haswell and later server platforms offer shared resource monitoring
+and control technologies. The availability of these technologies and the
+hardware capabilities can be shown with B<psr-hwinfo>.
+
+=over 4
+
+=item B<psr-hwinfo> [I<OPTIONS>]
+
+Show Platform Shared Resource (PSR) hardware information.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-m>, B<--cmt>
+
+Show Cache Monitoring Technology (CMT) hardware information.
+
+=item B<-a>, B<--cat>
+
+Show Cache Allocation Technology (CAT) hardware information.
+
+=back
+
+=back
+
+=head2 CACHE MONITORING TECHNOLOGY
Intel Haswell and later server platforms offer monitoring capability in each
logical processor to measure specific platform shared resource metric, for
-example, L3 cache occupancy. In Xen implementation, the monitoring granularity
-is domain level. To monitor a specific domain, just attach the domain id with
-the monitoring service. When the domain doesn't need to be monitored any more,
-detach the domain id from the monitoring service.
+example, L3 cache occupancy. In the Xen implementation, the monitoring
+granularity is domain level. To monitor a specific domain, just attach the
+domain id with the monitoring service. When the domain doesn't need to be
+monitored any more, detach the domain id from the monitoring service.
Intel Broadwell and later server platforms also offer total/local memory
bandwidth monitoring. Xen supports per-domain monitoring for these two
additional monitoring types. Both memory bandwidth monitoring and L3 cache
occupancy monitoring share the same set of underlying monitoring service. Once
-a domain is attached to the monitoring service, monitoring data can be showed
+a domain is attached to the monitoring service, monitoring data can be shown
for any of these monitoring types.
=over 4
-=item B<psr-hwinfo>
-
-Show CMT hardware information.
-
=item B<psr-cmt-attach> [I<domain-id>]
attach: Attach the platform shared resource monitoring service to a domain.
@@ -1517,6 +1541,37 @@ monitor types are:
=back
+=head2 CACHE ALLOCATION TECHNOLOGY
+
+Intel Broadwell and later server platforms offer capabilities to configure and
+make use of the Cache Allocation Technology (CAT) mechanisms, which enable more
+cache resources (i.e. L3 cache) to be made available for high priority
+applications. In the Xen implementation, CAT is used to control cache allocation
+on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
+(CBM) for the domain.
+
+=over 4
+
+=item B<psr-cat-cbm-set> [I<OPTIONS>] I<domain-id> I<cbm>
+
+Set cache capacity bitmasks(CBM) for a domain.
+
+B<OPTIONS>
+
+=over 4
+
+=item B<-s SOCKET>, B<--socket=SOCKET>
+
+Specify the socket to process, otherwise all sockets are processed.
+
+=back
+
+=item B<psr-cat-show> [I<domain-id>]
+
+Show CAT settings for a certain domain or all domains.
+
+=back
+
=head1 TO BE DOCUMENTED
We need better documentation for:
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 02d0db8..077cc1b 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2696,6 +2696,12 @@ enum xc_psr_cmt_type {
XC_PSR_CMT_LOCAL_MEM_COUNT,
};
typedef enum xc_psr_cmt_type xc_psr_cmt_type;
+
+enum xc_psr_cat_type {
+ XC_PSR_CAT_L3_CBM = 1,
+};
+typedef enum xc_psr_cat_type xc_psr_cat_type;
+
int xc_psr_cmt_attach(xc_interface *xch, uint32_t domid);
int xc_psr_cmt_detach(xc_interface *xch, uint32_t domid);
int xc_psr_cmt_get_domain_rmid(xc_interface *xch, uint32_t domid,
@@ -2710,6 +2716,15 @@ int xc_psr_cmt_get_data(xc_interface *xch, uint32_t rmid, uint32_t cpu,
uint32_t psr_cmt_type, uint64_t *monitor_data,
uint64_t *tsc);
int xc_psr_cmt_enabled(xc_interface *xch);
+
+int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
+ xc_psr_cat_type type, uint32_t target,
+ uint64_t data);
+int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
+ xc_psr_cat_type type, uint32_t target,
+ uint64_t *data);
+int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
+ uint32_t *cos_max, uint32_t *cbm_len);
#endif
#endif /* XENCTRL_H */
diff --git a/tools/libxc/xc_psr.c b/tools/libxc/xc_psr.c
index e367a80..d8b3a51 100644
--- a/tools/libxc/xc_psr.c
+++ b/tools/libxc/xc_psr.c
@@ -248,6 +248,82 @@ int xc_psr_cmt_enabled(xc_interface *xch)
return 0;
}
+int xc_psr_cat_set_domain_data(xc_interface *xch, uint32_t domid,
+ xc_psr_cat_type type, uint32_t target,
+ uint64_t data)
+{
+ DECLARE_DOMCTL;
+ uint32_t cmd;
+
+ switch ( type )
+ {
+ case XC_PSR_CAT_L3_CBM:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_SET_L3_CBM;
+ break;
+ default:
+ errno = EINVAL;
+ return -1;
+ }
+
+ domctl.cmd = XEN_DOMCTL_psr_cat_op;
+ domctl.domain = (domid_t)domid;
+ domctl.u.psr_cat_op.cmd = cmd;
+ domctl.u.psr_cat_op.target = target;
+ domctl.u.psr_cat_op.data = data;
+
+ return do_domctl(xch, &domctl);
+}
+
+int xc_psr_cat_get_domain_data(xc_interface *xch, uint32_t domid,
+ xc_psr_cat_type type, uint32_t target,
+ uint64_t *data)
+{
+ int rc;
+ DECLARE_DOMCTL;
+ uint32_t cmd;
+
+ switch ( type )
+ {
+ case XC_PSR_CAT_L3_CBM:
+ cmd = XEN_DOMCTL_PSR_CAT_OP_GET_L3_CBM;
+ break;
+ default:
+ errno = EINVAL;
+ return -1;
+ }
+
+ domctl.cmd = XEN_DOMCTL_psr_cat_op;
+ domctl.domain = (domid_t)domid;
+ domctl.u.psr_cat_op.cmd = cmd;
+ domctl.u.psr_cat_op.target = target;
+
+ rc = do_domctl(xch, &domctl);
+
+ if ( !rc )
+ *data = domctl.u.psr_cat_op.data;
+
+ return rc;
+}
+
+int xc_psr_cat_get_l3_info(xc_interface *xch, uint32_t socket,
+ uint32_t *cos_max, uint32_t *cbm_len)
+{
+ int rc;
+ DECLARE_SYSCTL;
+
+ sysctl.cmd = XEN_SYSCTL_psr_cat_op;
+ sysctl.u.psr_cat_op.cmd = XEN_SYSCTL_PSR_CAT_get_l3_info;
+ sysctl.u.psr_cat_op.target = socket;
+
+ rc = xc_sysctl(xch, &sysctl);
+ if ( !rc )
+ {
+ *cos_max = sysctl.u.psr_cat_op.u.l3_info.cos_max;
+ *cbm_len = sysctl.u.psr_cat_op.u.l3_info.cbm_len;
+ }
+
+ return rc;
+}
/*
* Local variables:
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 51bdff4..c45fd71 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -734,6 +734,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
* If this is defined, the Memory Bandwidth Monitoring feature is supported.
*/
#define LIBXL_HAVE_PSR_MBM 1
+
+/*
+ * LIBXL_HAVE_PSR_CAT
+ *
+ * If this is defined, the Cache Allocation Technology feature is supported.
+ */
+#define LIBXL_HAVE_PSR_CAT 1
#endif
/*
@@ -1540,6 +1547,34 @@ int libxl_psr_cmt_get_sample(libxl_ctx *ctx,
uint64_t *tsc_r);
#endif
+#ifdef LIBXL_HAVE_PSR_CAT
+/*
+ * Function to set a domain's cbm. It operates on a single or multiple
+ * target(s) defined in 'target_map'. The definition of 'target_map' is
+ * related to 'type':
+ * 'L3_CBM': 'target_map' specifies all the sockets to be operated on.
+ */
+int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
+ libxl_psr_cbm_type type, libxl_bitmap *target_map,
+ uint64_t cbm);
+/*
+ * Function to get a domain's cbm. It operates on a single 'target'.
+ * The definition of 'target' is related to 'type':
+ * 'L3_CBM': 'target' specifies which socket to be operated on.
+ */
+int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
+ libxl_psr_cbm_type type, uint32_t target,
+ uint64_t *cbm_r);
+
+/*
+ * On success, the function returns an array of elements in 'info',
+ * and the length in 'nr'.
+ */
+int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
+ int *nr);
+void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr);
+#endif
+
/* misc */
/* Each of these sets or clears the flag according to whether the
diff --git a/tools/libxl/libxl_psr.c b/tools/libxl/libxl_psr.c
index 3e1c792..313ec86 100644
--- a/tools/libxl/libxl_psr.c
+++ b/tools/libxl/libxl_psr.c
@@ -19,14 +19,37 @@
#define IA32_QM_CTR_ERROR_MASK (0x3ul << 62)
-static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
+static void libxl__psr_log_err_msg(libxl__gc *gc, int err)
{
char *msg;
switch (err) {
case ENOSYS:
+ case EOPNOTSUPP:
msg = "unsupported operation";
break;
+ case ESRCH:
+ msg = "invalid domain ID";
+ break;
+ case EBADSLT:
+ msg = "socket is not supported";
+ break;
+ case EFAULT:
+ msg = "failed to exchange data with Xen";
+ break;
+ default:
+ msg = "unknown error";
+ break;
+ }
+
+ LOGE(ERROR, "%s", msg);
+}
+
+static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
+{
+ char *msg;
+
+ switch (err) {
case ENODEV:
msg = "CMT is not supported in this system";
break;
@@ -39,15 +62,35 @@ static void libxl__psr_cmt_log_err_msg(libxl__gc *gc, int err)
case EUSERS:
msg = "no free RMID available";
break;
- case ESRCH:
- msg = "invalid domain ID";
+ default:
+ libxl__psr_log_err_msg(gc, err);
+ return;
+ }
+
+ LOGE(ERROR, "%s", msg);
+}
+
+static void libxl__psr_cat_log_err_msg(libxl__gc *gc, int err)
+{
+ char *msg;
+
+ switch (err) {
+ case ENODEV:
+ msg = "CAT is not supported in this system";
break;
- case EFAULT:
- msg = "failed to exchange data with Xen";
+ case ENOENT:
+ msg = "CAT is not enabled on the socket";
break;
- default:
- msg = "unknown error";
+ case EUSERS:
+ msg = "no free COS available";
break;
+ case EEXIST:
+ msg = "The same CBM is already set to this domain";
+ break;
+
+ default:
+ libxl__psr_log_err_msg(gc, err);
+ return;
}
LOGE(ERROR, "%s", msg);
@@ -247,6 +290,92 @@ out:
return rc;
}
+int libxl_psr_cat_set_cbm(libxl_ctx *ctx, uint32_t domid,
+ libxl_psr_cbm_type type, libxl_bitmap *target_map,
+ uint64_t cbm)
+{
+ GC_INIT(ctx);
+ int rc;
+ int socket, nr_sockets;
+
+ rc = libxl__count_physical_sockets(gc, &nr_sockets);
+ if (rc) {
+ LOGE(ERROR, "failed to get system socket count");
+ goto out;
+ }
+
+ libxl_for_each_set_bit(socket, *target_map) {
+ if (socket >= nr_sockets)
+ break;
+ if (xc_psr_cat_set_domain_data(ctx->xch, domid, type, socket, cbm)) {
+ libxl__psr_cat_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ }
+ }
+
+out:
+ GC_FREE;
+ return rc;
+}
+
+int libxl_psr_cat_get_cbm(libxl_ctx *ctx, uint32_t domid,
+ libxl_psr_cbm_type type, uint32_t target,
+ uint64_t *cbm_r)
+{
+ GC_INIT(ctx);
+ int rc = 0;
+
+ if (xc_psr_cat_get_domain_data(ctx->xch, domid, type, target, cbm_r)) {
+ libxl__psr_cat_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ }
+
+ GC_FREE;
+ return rc;
+}
+
+int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, libxl_psr_cat_info **info,
+ int *nr)
+{
+ GC_INIT(ctx);
+ int rc;
+ int i, nr_sockets;
+ libxl_psr_cat_info *ptr;
+
+ rc = libxl__count_physical_sockets(gc, &nr_sockets);
+ if (rc) {
+ LOGE(ERROR, "failed to get system socket count");
+ goto out;
+ }
+
+ ptr = libxl__malloc(NOGC, nr_sockets * sizeof(libxl_psr_cat_info));
+
+ for (i = 0; i < nr_sockets; i++) {
+ if (xc_psr_cat_get_l3_info(ctx->xch, i, &ptr[i].cos_max,
+ &ptr[i].cbm_len)) {
+ libxl__psr_cat_log_err_msg(gc, errno);
+ rc = ERROR_FAIL;
+ free(ptr);
+ goto out;
+ }
+ }
+
+ *info = ptr;
+ *nr = nr_sockets;
+out:
+ GC_FREE;
+ return rc;
+}
+
+void libxl_psr_cat_info_list_free(libxl_psr_cat_info *list, int nr)
+{
+ int i;
+
+ for (i = 0; i < nr; i++)
+ libxl_psr_cat_info_dispose(&list[i]);
+ free(list);
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 0866433..ee8d469 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -709,3 +709,13 @@ libxl_psr_cmt_type = Enumeration("psr_cmt_type", [
(2, "TOTAL_MEM_COUNT"),
(3, "LOCAL_MEM_COUNT"),
])
+
+libxl_psr_cbm_type = Enumeration("psr_cbm_type", [
+ (0, "UNKNOWN"),
+ (1, "L3_CBM"),
+ ])
+
+libxl_psr_cat_info = Struct("psr_cat_info", [
+ ("cos_max", uint32),
+ ("cbm_len", uint32),
+ ])
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 7b56449..bef32d7 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -118,6 +118,10 @@ int main_psr_cmt_attach(int argc, char **argv);
int main_psr_cmt_detach(int argc, char **argv);
int main_psr_cmt_show(int argc, char **argv);
#endif
+#ifdef LIBXL_HAVE_PSR_CAT
+int main_psr_cat_cbm_set(int argc, char **argv);
+int main_psr_cat_show(int argc, char **argv);
+#endif
void help(const char *command);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 59ce257..2ac9046 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8210,17 +8210,6 @@ static int psr_cmt_show(libxl_psr_cmt_type type, uint32_t domid)
return 0;
}
-int main_psr_hwinfo(int argc, char **argv)
-{
- int opt;
-
- SWITCH_FOREACH_OPT(opt, "", NULL, "psr-hwinfo", 0) {
- /* No options */
- }
-
- return psr_cmt_hwinfo();
-}
-
int main_psr_cmt_attach(int argc, char **argv)
{
uint32_t domid;
@@ -8287,6 +8276,232 @@ int main_psr_cmt_show(int argc, char **argv)
}
#endif
+#ifdef LIBXL_HAVE_PSR_CAT
+static int psr_cat_hwinfo(void)
+{
+ int rc;
+ int socket, nr_sockets;
+ uint32_t l3_cache_size;
+ libxl_psr_cat_info *info;
+
+ printf("Cache Allocation Technology (CAT):\n");
+
+ rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+ if (rc) {
+ fprintf(stderr, "Failed to get cat info\n");
+ return rc;
+ }
+
+ for (socket = 0; socket < nr_sockets; socket++) {
+ rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
+ if (rc) {
+ fprintf(stderr, "Failed to get l3 cache size for socket:%d\n",
+ socket);
+ goto out;
+ }
+ printf("%-16s: %u\n", "Socket ID", socket);
+ printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+ printf("%-16s: %u\n", "Maximum COS", info->cos_max);
+ printf("%-16s: %u\n", "CBM length", info->cbm_len);
+ printf("%-16s: %#"PRIx64"\n", "Default CBM",
+ (1ul << info->cbm_len) - 1);
+ }
+
+out:
+ libxl_psr_cat_info_list_free(info, nr_sockets);
+ return rc;
+}
+
+static void psr_cat_print_one_domain_cbm(uint32_t domid, uint32_t socket)
+{
+ char *domain_name;
+ uint64_t cbm;
+
+ domain_name = libxl_domid_to_name(ctx, domid);
+ printf("%5d%25s", domid, domain_name);
+ free(domain_name);
+
+ if (!libxl_psr_cat_get_cbm(ctx, domid, LIBXL_PSR_CBM_TYPE_L3_CBM,
+ socket, &cbm))
+ printf("%#16"PRIx64, cbm);
+
+ printf("\n");
+}
+
+static int psr_cat_print_domain_cbm(uint32_t domid, uint32_t socket)
+{
+ int i, nr_domains;
+ libxl_dominfo *list;
+
+ if (domid != INVALID_DOMID) {
+ psr_cat_print_one_domain_cbm(domid, socket);
+ return 0;
+ }
+
+ if (!(list = libxl_list_domain(ctx, &nr_domains))) {
+ fprintf(stderr, "Failed to get domain list for cbm display\n");
+ return -1;
+ }
+
+ for (i = 0; i < nr_domains; i++)
+ psr_cat_print_one_domain_cbm(list[i].domid, socket);
+ libxl_dominfo_list_free(list, nr_domains);
+
+ return 0;
+}
+
+static int psr_cat_print_socket(uint32_t domid, uint32_t socket,
+ libxl_psr_cat_info *info)
+{
+ int rc;
+ uint32_t l3_cache_size;
+
+ rc = libxl_psr_cmt_get_l3_cache_size(ctx, socket, &l3_cache_size);
+ if (rc) {
+ fprintf(stderr, "Failed to get l3 cache size for socket:%d\n", socket);
+ return -1;
+ }
+
+ printf("%-16s: %u\n", "Socket ID", socket);
+ printf("%-16s: %uKB\n", "L3 Cache", l3_cache_size);
+ printf("%-16s: %#"PRIx64"\n", "Default CBM", (1ul << info->cbm_len) - 1);
+ printf("%5s%25s%16s\n", "ID", "NAME", "CBM");
+
+ return psr_cat_print_domain_cbm(domid, socket);
+}
+
+static int psr_cat_show(uint32_t domid)
+{
+ int socket, nr_sockets;
+ int rc;
+ libxl_psr_cat_info *info;
+
+ rc = libxl_psr_cat_get_l3_info(ctx, &info, &nr_sockets);
+ if (rc) {
+ fprintf(stderr, "Failed to get cat info\n");
+ return rc;
+ }
+
+ for (socket = 0; socket < nr_sockets; socket++) {
+ rc = psr_cat_print_socket(domid, socket, info + socket);
+ if (rc)
+ goto out;
+ }
+
+out:
+ libxl_psr_cat_info_list_free(info, nr_sockets);
+ return rc;
+}
+
+int main_psr_cat_cbm_set(int argc, char **argv)
+{
+ uint32_t domid;
+ libxl_psr_cbm_type type = LIBXL_PSR_CBM_TYPE_L3_CBM;
+ uint64_t cbm;
+ int ret, opt = 0;
+ libxl_bitmap target_map;
+ char *value;
+ libxl_string_list socket_list;
+ unsigned long start, end;
+ int i, j, len;
+
+ static struct option opts[] = {
+ {"socket", 0, 0, 's'},
+ COMMON_LONG_OPTS,
+ {0, 0, 0, 0}
+ };
+
+ libxl_socket_bitmap_alloc(ctx, &target_map, 0);
+ libxl_bitmap_set_none(&target_map);
+
+ SWITCH_FOREACH_OPT(opt, "s", opts, "psr-cat-cbm-set", 1) {
+ case 's':
+ trim(isspace, optarg, &value);
+ split_string_into_string_list(value, ",", &socket_list);
+ len = libxl_string_list_length(&socket_list);
+ for (i = 0; i < len; i++) {
+ parse_range(socket_list[i], &start, &end);
+ for (j = start; j < end; j++)
+ libxl_bitmap_set(&target_map, j);
+ }
+
+ libxl_string_list_dispose(&socket_list);
+ free(value);
+ break;
+ }
+
+ if (libxl_bitmap_is_empty(&target_map))
+ libxl_bitmap_set_any(&target_map);
+
+ domid = find_domain(argv[optind]);
+ cbm = strtoll(argv[optind + 1], NULL , 0);
+
+ ret = libxl_psr_cat_set_cbm(ctx, domid, type, &target_map, cbm);
+
+ libxl_bitmap_dispose(&target_map);
+ return ret;
+}
+
+int main_psr_cat_show(int argc, char **argv)
+{
+ int opt;
+ uint32_t domid;
+
+ SWITCH_FOREACH_OPT(opt, "", NULL, "psr-cat-show", 0) {
+ /* No options */
+ }
+
+ if (optind >= argc)
+ domid = INVALID_DOMID;
+ else if (optind == argc - 1)
+ domid = find_domain(argv[optind]);
+ else {
+ help("psr-cat-show");
+ return 2;
+ }
+
+ return psr_cat_show(domid);
+}
+
+int main_psr_hwinfo(int argc, char **argv)
+{
+ int opt, ret;
+ int cmt = 0, cat = 0;
+ static struct option opts[] = {
+ {"cmt", 0, 0, 'm'},
+ {"cat", 0, 0, 'a'},
+ COMMON_LONG_OPTS,
+ {0, 0, 0, 0}
+ };
+
+ SWITCH_FOREACH_OPT(opt, "ma", opts, "psr-hwinfo", 0) {
+ case 'm':
+ cmt = 1;
+ break;
+ case 'a':
+ cat = 1;
+ break;
+ }
+
+ if (!(cmt | cat)) {
+ cmt = 1;
+ cat = 1;
+ }
+
+ if (cmt)
+ ret = psr_cmt_hwinfo();
+
+ if (ret)
+ return ret;
+
+ if (cat)
+ ret = psr_cat_hwinfo();
+
+ return ret;
+}
+
+#endif
+
/*
* Local variables:
* mode: C
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index be606af..529eae0 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -527,7 +527,9 @@ struct cmd_spec cmd_table[] = {
{ "psr-hwinfo",
&main_psr_hwinfo, 0, 1,
"Show hardware information for Platform Shared Resource",
- "",
+ "[options]",
+ "-m, --cmt Show Cache Monitoring Technology (CMT) hardware info\n"
+ "-a, --cat Show Cache Allocation Technology (CAT) hardware info\n"
},
{ "psr-cmt-attach",
&main_psr_cmt_attach, 0, 1,
@@ -549,6 +551,20 @@ struct cmd_spec cmd_table[] = {
"\"local-mem-bandwidth\": Show local memory bandwidth(KB/s)\n",
},
#endif
+#ifdef LIBXL_HAVE_PSR_CAT
+ { "psr-cat-cbm-set",
+ &main_psr_cat_cbm_set, 0, 1,
+ "Set cache capacity bitmasks(CBM) for a domain",
+ "[options] <Domain> <CBM>",
+ "-s <socket> Specify the socket to process, otherwise all sockets are processed\n"
+ },
+ { "psr-cat-show",
+ &main_psr_cat_show, 0, 1,
+ "Show Cache Allocation Technology information",
+ "<Domain>",
+ },
+
+#endif
};
int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v7 14/14] docs: add xl-psr.markdown
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (12 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 13/14] tools: add tools support for Intel CAT Chao Peng
@ 2015-05-08 8:56 ` Chao Peng
2015-05-08 13:39 ` Ian Campbell
2015-05-08 13:41 ` [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Ian Campbell
14 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-08 8:56 UTC (permalink / raw)
To: xen-devel
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, will.auld, JBeulich, Chao Peng, wei.liu2, dgdegra
Add document to introduce basic concepts and terms in PSR family
technologies and the xl interfaces.
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
Changes in v7:
* Correct 'xl psr-hwinfo'.
Changes in v6:
* Address comments from Ian.
Changes in v5:
* Address comments from Andrew/Ian.
---
docs/man/xl.pod.1 | 7 ++-
docs/misc/xl-psr.markdown | 133 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 139 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/xl-psr.markdown
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 795ce91..1454431 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1483,6 +1483,9 @@ Intel Haswell and later server platforms offer shared resource monitoring
and control technologies. The availability of these technologies and the
hardware capabilities can be shown with B<psr-hwinfo>.
+See L<http://xenbits.xen.org/docs/unstable/misc/xl-psr.html> for more
+information.
+
=over 4
=item B<psr-hwinfo> [I<OPTIONS>]
@@ -1554,7 +1557,8 @@ on VM basis. To enforce cache on a specific domain, just set capacity bitmasks
=item B<psr-cat-cbm-set> [I<OPTIONS>] I<domain-id> I<cbm>
-Set cache capacity bitmasks(CBM) for a domain.
+Set cache capacity bitmasks(CBM) for a domain. For how to specify I<cbm>
+please refer to L<http://xenbits.xen.org/docs/unstable/misc/xl-psr.html>.
B<OPTIONS>
@@ -1595,6 +1599,7 @@ And the following documents on the xen.org website:
L<http://xenbits.xen.org/docs/unstable/misc/xl-network-configuration.html>
L<http://xenbits.xen.org/docs/unstable/misc/xl-disk-configuration.txt>
L<http://xenbits.xen.org/docs/unstable/misc/xsm-flask.txt>
+L<http://xenbits.xen.org/docs/unstable/misc/xl-psr.html>
For systems that don't automatically bring CPU online:
diff --git a/docs/misc/xl-psr.markdown b/docs/misc/xl-psr.markdown
new file mode 100644
index 0000000..3545912
--- /dev/null
+++ b/docs/misc/xl-psr.markdown
@@ -0,0 +1,133 @@
+# Intel Platform Shared Resource Monitoring/Control in xl
+
+This document introduces Intel Platform Shared Resource Monitoring/Control
+technologies, their basic concepts and the xl interfaces.
+
+## Cache Monitoring Technology (CMT)
+
+Cache Monitoring Technology (CMT) is a new feature available on Intel Haswell
+and later server platforms that allows an OS or Hypervisor/VMM to determine
+the usage of cache (currently only L3 cache supported) by applications running
+on the platform. A Resource Monitoring ID (RMID) is the abstraction of the
+application(s) that will be monitored for its cache usage. The CMT hardware
+tracks cache utilization of memory accesses according to the RMID and reports
+monitored data via a counter register.
+
+For more detailed information please refer to Intel SDM chapter
+"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+
+In Xen's implementation, each domain in the system can be assigned a RMID
+independently, while RMID=0 is reserved for monitoring domains that don't
+have CMT service attached. RMID is opaque for xl/libxl and is only used in
+hypervisor.
+
+### xl interfaces
+
+A domain is assigned a RMID implicitly by attaching it to CMT service:
+
+`xl psr-cmt-attach <domid>`
+
+After that, cache usage for the domain can be shown by:
+
+`xl psr-cmt-show cache-occupancy <domid>`
+
+Once monitoring is not needed any more, the domain can be detached from the
+CMT service by:
+
+`xl psr-cmt-detach <domid>`
+
+An attach may fail because of no free RMID available. In such case unused
+RMID(s) can be freed by detaching corresponding domains from CMT service.
+
+Maximum RMID and supported monitor types in the system can be obtained by:
+
+`xl psr-hwinfo --cmt`
+
+## Memory Bandwidth Monitoring (MBM)
+
+Memory Bandwidth Monitoring(MBM) is a new hardware feature available on Intel
+Broadwell and later server platforms which builds on the CMT infrastructure to
+allow monitoring of system memory bandwidth. It introduces two new monitoring
+event type to monitor system total/local memory bandwidth. The same RMID can
+be used to monitor both cache usage and memory bandwidth at the same time.
+
+For more detailed information please refer to Intel SDM chapter
+"17.14 - Platform Shared Resource Monitoring: Cache Monitoring Technology".
+
+In Xen's implementation, MBM shares the same set of underlying monitoring
+service with CMT and can be used to monitor memory bandwidth on a per domain
+basis.
+
+The xl interfaces are the same with that of CMT. The difference is the
+monitor type is corresponding memory monitoring type (local-mem-bandwidth/
+total-mem-bandwidth instead of cache-occupancy). E.g. after a `xl psr-cmt-attach`:
+
+`xl psr-cmt-show local-mem-bandwidth <domid>`
+
+`xl psr-cmt-show total-mem-bandwidth <domid>`
+
+## Cache Allocation Technology (CAT)
+
+Cache Allocation Technology (CAT) is a new feature available on Intel
+Broadwell and later server platforms that allows an OS or Hypervisor/VMM to
+partition cache allocation (i.e. L3 cache) based on application priority or
+Class of Service (COS). Each COS is configured using capacity bitmasks (CBM)
+which represent cache capacity and indicate the degree of overlap and
+isolation between classes. System cache resource is divided into numbers of
+minimum portions which is then made up into subset for cache partition. Each
+portion corresponds to a bit in CBM and the set bit represents the
+corresponding cache portion is available.
+
+For example, assuming a system with 8 portions and 3 domains:
+
+ * A CBM of 0xff for every domain means each domain can access the whole cache.
+ This is the default.
+
+ * Giving one domain a CBM of 0x0f and the other two domain's 0xf0 means that
+ the first domain gets exclusive access to half of the cache (half of the
+ portions) and the other two will share the other half.
+
+ * Giving one domain a CBM of 0x0f, one 0x30 and the last 0xc0 would give the
+ first domain exclusive access to half the cache, and the other two exclusive
+ access to one quarter each.
+
+For more detailed information please refer to Intel SDM chapter
+"17.15 - Platform Shared Resource Control: Cache Allocation Technology".
+
+In Xen's implementation, CBM can be configured with libxl/xl interfaces but
+COS is maintained in hypervisor only. The cache partition granularity is per
+domain, each domain has COS=0 assigned by default, the corresponding CBM is
+all-ones, which means all the cache resource can be used by default.
+
+### xl interfaces
+
+System CAT information such as maximum COS and CBM length can be obtained by:
+
+`xl psr-hwinfo --cat`
+
+The simplest way to change a domain's CBM from its default is running:
+
+`xl psr-cat-cbm-set [OPTIONS] <domid> <cbm>`
+
+where cbm is a number to represent the corresponding cache subset can be used.
+A cbm is valid only when:
+
+ * Set bits only exist in the range of [0, cbm_len), where cbm_len can be
+ obtained with `xl psr-hwinfo --cat`.
+ * All the set bits are contiguous.
+
+In a multi-socket system, the same cbm will be set on each socket by default.
+Per socket cbm can be specified with the `--socket SOCKET` option.
+
+Setting the CBM may not be successful if insufficient COS is available. In
+such case unused COS(es) may be freed by setting CBM of all related domains to
+its default value(all-ones).
+
+Per domain CBM settings can be shown by:
+
+`xl psr-cat-show`
+
+## Reference
+
+[1] Intel SDM
+(http://www.intel.com/content/www/us/en/processors/architectures-software-developer-manuals.html).
--
1.9.1
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v7 02/14] x86: improve psr scheduling code
2015-05-08 8:56 ` [PATCH v7 02/14] x86: improve psr scheduling code Chao Peng
@ 2015-05-08 10:20 ` Jan Beulich
2015-05-11 1:35 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-08 10:20 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> Switching RMID from previous vcpu to next vcpu only needs to write
> MSR_IA32_PSR_ASSOC once. Write it with the value of next vcpu is enough,
> no need to write '0' first. Idle domain has RMID set to 0 and because MSR
> is already updated lazily, so just switch it as it does.
>
> Also move the initialization of per-CPU variable which used for lazy
> update from context switch to CPU starting.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Please avoid sending again changes that got applied already.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 13/14] tools: add tools support for Intel CAT
2015-05-08 8:56 ` [PATCH v7 13/14] tools: add tools support for Intel CAT Chao Peng
@ 2015-05-08 13:38 ` Ian Campbell
0 siblings, 0 replies; 48+ messages in thread
From: Ian Campbell @ 2015-05-08 13:38 UTC (permalink / raw)
To: Chao Peng
Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
will.auld, JBeulich, wei.liu2, dgdegra
On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> This is the xc/xl changes to support Intel Cache Allocation
> Technology(CAT).
>
> 'xl psr-hwinfo' is updated to show CAT info and two new commands
> for CAT are introduced:
> - xl psr-cat-cbm-set [-s socket] <domain> <cbm>
> Set cache capacity bitmasks(CBM) for a domain.
> - xl psr-cat-show <domain>
> Show CAT domain information.
>
> Examples:
> [root@vmm-psr vmm]# xl psr-hwinfo --cat
> Cache Allocation Technology (CAT):
> Socket ID : 0
> L3 Cache : 12288KB
> Maximum COS : 15
> CBM length : 12
> Default CBM : 0xfff
>
> [root@vmm-psr vmm]# xl psr-cat-cbm-set 0 0xff
>
> [root@vmm-psr vmm]# xl psr-cat-show
> Socket ID : 0
> L3 Cache : 12288KB
> Default CBM : 0xfff
> ID NAME CBM
> 0 Domain-0 0xff
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 14/14] docs: add xl-psr.markdown
2015-05-08 8:56 ` [PATCH v7 14/14] docs: add xl-psr.markdown Chao Peng
@ 2015-05-08 13:39 ` Ian Campbell
0 siblings, 0 replies; 48+ messages in thread
From: Ian Campbell @ 2015-05-08 13:39 UTC (permalink / raw)
To: Chao Peng
Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
will.auld, JBeulich, wei.liu2, dgdegra
On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> Add document to introduce basic concepts and terms in PSR family
> technologies and the xl interfaces.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
` (13 preceding siblings ...)
2015-05-08 8:56 ` [PATCH v7 14/14] docs: add xl-psr.markdown Chao Peng
@ 2015-05-08 13:41 ` Ian Campbell
2015-05-08 13:48 ` Jan Beulich
14 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2015-05-08 13:41 UTC (permalink / raw)
To: Chao Peng
Cc: keir, stefano.stabellini, andrew.cooper3, Ian.Jackson, xen-devel,
will.auld, JBeulich, wei.liu2, dgdegra
On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> Changes in v7:
I've now acked the last of the tools stuff.
Jan, will you pick that up along with the remaining hypervisor stuff
whenever it is ready please?
Chao, If there are any major changes to the tools bit please ping me
since I may not be paying close attention to this thread any more.
Cheers,
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs
2015-05-08 13:41 ` [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Ian Campbell
@ 2015-05-08 13:48 ` Jan Beulich
2015-05-08 14:11 ` Ian Campbell
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-08 13:48 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, andrew.cooper3, Ian.Jackson,
xen-devel, will.auld, Chao Peng, dgdegra, keir
>>> On 08.05.15 at 15:41, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
>> Changes in v7:
>
> I've now acked the last of the tools stuff.
>
> Jan, will you pick that up along with the remaining hypervisor stuff
> whenever it is ready please?
Sure, I'll try to remember.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs
2015-05-08 13:48 ` Jan Beulich
@ 2015-05-08 14:11 ` Ian Campbell
2015-05-11 1:37 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Ian Campbell @ 2015-05-08 14:11 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, stefano.stabellini, andrew.cooper3, Ian.Jackson,
xen-devel, will.auld, Chao Peng, dgdegra, keir
On Fri, 2015-05-08 at 14:48 +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 15:41, <ian.campbell@citrix.com> wrote:
> > On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> >> Changes in v7:
> >
> > I've now acked the last of the tools stuff.
> >
> > Jan, will you pick that up along with the remaining hypervisor stuff
> > whenever it is ready please?
>
> Sure, I'll try to remember.
Thanks.
I'm sure Chao will prod me if you don't ;-)
Ian.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 02/14] x86: improve psr scheduling code
2015-05-08 10:20 ` Jan Beulich
@ 2015-05-11 1:35 ` Chao Peng
0 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-11 1:35 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Fri, May 08, 2015 at 11:20:45AM +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> > Switching RMID from previous vcpu to next vcpu only needs to write
> > MSR_IA32_PSR_ASSOC once. Write it with the value of next vcpu is enough,
> > no need to write '0' first. Idle domain has RMID set to 0 and because MSR
> > is already updated lazily, so just switch it as it does.
> >
> > Also move the initialization of per-CPU variable which used for lazy
> > update from context switch to CPU starting.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
>
> Please avoid sending again changes that got applied already.
>
Just noticed it's already get merged. Sorry for the noise and thanks.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs
2015-05-08 14:11 ` Ian Campbell
@ 2015-05-11 1:37 ` Chao Peng
0 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-11 1:37 UTC (permalink / raw)
To: Ian Campbell
Cc: wei.liu2, stefano.stabellini, andrew.cooper3, Ian.Jackson,
xen-devel, will.auld, Jan Beulich, keir, dgdegra
On Fri, May 08, 2015 at 03:11:27PM +0100, Ian Campbell wrote:
> On Fri, 2015-05-08 at 14:48 +0100, Jan Beulich wrote:
> > >>> On 08.05.15 at 15:41, <ian.campbell@citrix.com> wrote:
> > > On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> > >> Changes in v7:
> > >
> > > I've now acked the last of the tools stuff.
> > >
> > > Jan, will you pick that up along with the remaining hypervisor stuff
> > > whenever it is ready please?
> >
> > Sure, I'll try to remember.
>
> Thanks.
>
> I'm sure Chao will prod me if you don't ;-)
I will absolutely :)
Thanks for your help made on this.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 07/14] x86: dynamically get/set CBM for a domain
2015-05-08 8:56 ` [PATCH v7 07/14] x86: dynamically get/set CBM for a domain Chao Peng
@ 2015-05-14 9:19 ` Dario Faggioli
2015-05-15 1:35 ` Chao Peng
2015-05-18 13:45 ` Jan Beulich
1 sibling, 1 reply; 48+ messages in thread
From: Dario Faggioli @ 2015-05-14 9:19 UTC (permalink / raw)
To: Chao Peng
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra
[-- Attachment #1.1: Type: text/plain, Size: 1293 bytes --]
On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> For CAT, COS is maintained in hypervisor only while CBM is exposed to
> user space directly to allow getting/setting domain's cache capacity.
> For each specified CBM, hypervisor will either use a existed COS which
> has the same CBM or allocate a new one if the same CBM is not found. If
> the allocation fails because of no enough COS available then error is
> returned. The getting/setting are always operated on a specified socket.
> For multiple sockets system, the interface may be called several times.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Just, one minor thing, only if you end up resending...
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 1feb2f6..385807b 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -49,6 +49,14 @@ static unsigned int opt_cos_max = 255;
> static uint64_t rmid_mask;
> static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
>
> +static unsigned int get_socket_cpu(unsigned int socket)
> +{
> + if ( socket < nr_sockets )
> + return cpumask_any(socket_to_cpumask[socket]);
> +
... What about
if ( likely(socket < nr_sockets) )
Regards,
Dario
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 07/14] x86: dynamically get/set CBM for a domain
2015-05-14 9:19 ` Dario Faggioli
@ 2015-05-15 1:35 ` Chao Peng
0 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-15 1:35 UTC (permalink / raw)
To: Dario Faggioli
Cc: keir, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, JBeulich, wei.liu2, dgdegra
On Thu, May 14, 2015 at 11:19:17AM +0200, Dario Faggioli wrote:
> On Fri, 2015-05-08 at 16:56 +0800, Chao Peng wrote:
> > For CAT, COS is maintained in hypervisor only while CBM is exposed to
> > user space directly to allow getting/setting domain's cache capacity.
> > For each specified CBM, hypervisor will either use a existed COS which
> > has the same CBM or allocate a new one if the same CBM is not found. If
> > the allocation fails because of no enough COS available then error is
> > returned. The getting/setting are always operated on a specified socket.
> > For multiple sockets system, the interface may be called several times.
> >
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> >
> Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
>
> Just, one minor thing, only if you end up resending...
>
> > diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> > index 1feb2f6..385807b 100644
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -49,6 +49,14 @@ static unsigned int opt_cos_max = 255;
> > static uint64_t rmid_mask;
> > static DEFINE_PER_CPU(struct psr_assoc, psr_assoc);
> >
> > +static unsigned int get_socket_cpu(unsigned int socket)
> > +{
> > + if ( socket < nr_sockets )
> > + return cpumask_any(socket_to_cpumask[socket]);
> > +
> ... What about
>
> if ( likely(socket < nr_sockets) )
Agreed, it can be an optimization chance.
Let's see what others think.
Thanks for review.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-08 8:56 ` [PATCH v7 01/14] x86: add socket_to_cpumask Chao Peng
@ 2015-05-18 13:21 ` Jan Beulich
2015-05-19 6:12 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-18 13:21 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/mpparse.c
> +++ b/xen/arch/x86/mpparse.c
> @@ -64,6 +64,9 @@ unsigned int __read_mostly boot_cpu_physical_apicid = BAD_APICID;
> static unsigned int __devinitdata num_processors;
> static unsigned int __initdata disabled_cpus;
>
> +/* Total detected cpus (may exceed NR_CPUS) */
> +unsigned int total_cpus;
> +
> /* Bitmask of physically existing CPUs */
> physid_mask_t phys_cpu_present_map;
>
> @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct mpc_config_processor *m,
> {
> int ver, apicid, cpu = 0;
>
> + total_cpus++;
> +
> if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> if (!hotplug)
> ++disabled_cpus;
Is there a reason you can't use disabled_cpus and avoid adding yet
another variable?
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -59,6 +59,9 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
> cpumask_t cpu_online_map __read_mostly;
> EXPORT_SYMBOL(cpu_online_map);
>
> +unsigned int nr_sockets __read_mostly;
> +cpumask_var_t *socket_to_cpumask __read_mostly;
I'd really like to see the "to" dropped from the name. It has been
confusing me not for the first time. I'd also prefer the section
annotations to be at their mandated place, between type and
variable name.
> @@ -239,11 +242,14 @@ static void link_thread_siblings(int cpu1, int cpu2)
>
> static void set_cpu_sibling_map(int cpu)
> {
> - int i;
> + int i, socket = cpu_to_socket(cpu);
unsigned int
> struct cpuinfo_x86 *c = cpu_data;
>
> cpumask_set_cpu(cpu, &cpu_sibling_setup_map);
>
> + if ( socket < nr_sockets )
> + cpumask_set_cpu(cpu, socket_to_cpumask[socket]);
> +
> if ( c[cpu].x86_num_siblings > 1 )
> {
> for_each_cpu ( i, &cpu_sibling_setup_map )
> @@ -301,6 +307,7 @@ static void set_cpu_sibling_map(int cpu)
> }
> }
> }
> +
> }
???
> @@ -704,6 +711,8 @@ static struct notifier_block cpu_smpboot_nfb = {
>
> void __init smp_prepare_cpus(unsigned int max_cpus)
> {
> + int socket;
unsigned int
> @@ -717,6 +726,15 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>
> stack_base[0] = stack_start;
>
> + nr_sockets = DIV_ROUND_UP(total_cpus, boot_cpu_data.x86_max_cores *
> + boot_cpu_data.x86_num_siblings);
> + socket_to_cpumask = xzalloc_array(cpumask_var_t, nr_sockets);
> + if ( !socket_to_cpumask )
> + panic("No memory for socket CPU siblings map");
> + for ( socket = 0; socket < nr_sockets; socket++ )
> + if ( !zalloc_cpumask_var(socket_to_cpumask + socket) )
> + panic("No memory for socket CPU siblings cpumask");
You might be allocating quite a bit too much memory now that you
overestimate nr_sockets. Hence at least this second part of the
change here would better be switched to an on demand allocation
model.
> @@ -779,9 +797,12 @@ void __init smp_prepare_boot_cpu(void)
> static void
> remove_siblinginfo(int cpu)
> {
> - int sibling;
> + int sibling, socket = cpu_to_socket(cpu);
unsigned int
> --- a/xen/include/asm-x86/smp.h
> +++ b/xen/include/asm-x86/smp.h
> @@ -58,6 +58,22 @@ int hard_smp_processor_id(void);
>
> void __stop_this_cpu(void);
>
> +/* Total number of cpus in this system (may exceed NR_CPUS) */
> +extern unsigned int total_cpus;
> +
> +/*
> + * This value is calculated by total_cpus/cpus_per_socket with the assumption
> + * that APIC IDs from MP table are continuous. It's possible that this value
> + * is less than the real socket number in the system if the APIC IDs from MP
> + * table are too sparse. Also the value is considered not to change from the
> + * initial startup. Violation of any of these assumptions may result in errors
> + * and requires retrofitting all the relevant places.
> + */
This all reads pretty frightening. How about using a better estimate of
core and thread count (i.e. ones matching actually observed values
instead of the nearest larger powers of two) in the nr_sockets
calculation? Overestimating nr_sockets is surely better than using too
small a value, as the alternative of remembering to always bounds
check socket values before use (not only in your series, but also in
future changes) is going to be pretty fragile.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-08 8:56 ` [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Chao Peng
@ 2015-05-18 13:33 ` Jan Beulich
2015-05-19 7:40 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-18 13:33 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -19,14 +19,26 @@
> #include <asm/psr.h>
>
> #define PSR_CMT (1<<0)
> +#define PSR_CAT (1<<1)
> +
> +struct psr_cat_socket_info {
> + unsigned int cbm_len;
> + unsigned int cos_max;
> +};
>
> struct psr_assoc {
> uint64_t val;
> };
>
> struct psr_cmt *__read_mostly psr_cmt;
> +
> +static unsigned long *__read_mostly cat_socket_init_bitmap;
> +static unsigned long *__read_mostly cat_socket_enable_bitmap;
Didn't we agree to fold these two into one? Apart from that the
_bitmap name tag doesn't seem very useful...
> @@ -194,16 +210,100 @@ void psr_ctxt_switch_to(struct domain *d)
> }
> }
>
> +static void cat_cpu_init(void)
> +{
> + unsigned int eax, ebx, ecx, edx;
> + struct psr_cat_socket_info *info;
> + unsigned int socket;
> + unsigned int cpu = smp_processor_id();
> + const struct cpuinfo_x86 *c = cpu_data + cpu;
> +
> + if ( !cpu_has(c, X86_FEATURE_CAT) )
> + return;
> +
> + socket = cpu_to_socket(cpu);
> + if ( socket >= nr_sockets )
> + return;
> +
> + /* Avoid initializing more than one times for the same socket. */
> + if ( test_and_set_bit(socket, cat_socket_init_bitmap) )
> + return;
> +
> + cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> + if ( ebx & PSR_RESOURCE_TYPE_L3 )
> + {
> + cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> + info = cat_socket_info + socket;
> + info->cbm_len = (eax & 0x1f) + 1;
> + info->cos_max = min(opt_cos_max, edx & 0xffff);
Is opt_cos_max being zero (or even one) going to result in a useful /
working environment? I.e. shouldn't you rather disable CAT in that
case?
> +static void cat_cpu_fini(void)
> +{
> + unsigned int cpu = smp_processor_id();
> + unsigned int socket = cpu_to_socket(cpu);
This is the only use of "cpu", i.e. the variable is pretty pointless.
> static int cpu_callback(
> struct notifier_block *nfb, unsigned long action, void *hcpu)
> {
> if ( action == CPU_STARTING )
> psr_cpu_init();
> + else if ( action == CPU_DYING )
> + psr_cpu_fini();
Are these the right notifiers for doing things involving memory
allocation / freeing?
> @@ -217,9 +317,12 @@ static int __init psr_presmp_init(void)
> if ( (opt_psr & PSR_CMT) && opt_rmid_max )
> init_psr_cmt(opt_rmid_max);
>
> + if ( opt_psr & PSR_CAT )
> + init_psr_cat();
> +
> psr_cpu_init();
> - if ( psr_cmt_enabled() )
> - register_cpu_notifier(&cpu_nfb);
> + if ( psr_cmt_enabled() || cat_socket_info )
> + register_cpu_notifier(&cpu_nfb);
Please don't corrupt indentation here.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket
2015-05-08 8:56 ` [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket Chao Peng
@ 2015-05-18 13:35 ` Jan Beulich
2015-05-19 7:41 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-18 13:35 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> @@ -237,6 +243,14 @@ static void cat_cpu_init(void)
> info->cbm_len = (eax & 0x1f) + 1;
> info->cos_max = min(opt_cos_max, edx & 0xffff);
>
> + info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
> + info->cos_max + 1UL);
How does the array dimension here match the "for each socket"
in the title?
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 07/14] x86: dynamically get/set CBM for a domain
2015-05-08 8:56 ` [PATCH v7 07/14] x86: dynamically get/set CBM for a domain Chao Peng
2015-05-14 9:19 ` Dario Faggioli
@ 2015-05-18 13:45 ` Jan Beulich
1 sibling, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2015-05-18 13:45 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> +int psr_get_l3_cbm(struct domain *d, unsigned int socket, uint64_t *cbm)
> +{
> + unsigned int cos;
> + struct psr_cat_socket_info *info;
> + int ret = get_cat_socket_info(socket, &info);
> +
> + if ( ret )
> + return ret;
> +
> + cos = d->arch.psr_cos_ids[socket];
> + *cbm = info->cos_to_cbm[cos].cbm;
> + return 0;
> +}
Blank line before a function's final return statement please. With
that and Dario's comment addressed
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-18 13:21 ` Jan Beulich
@ 2015-05-19 6:12 ` Chao Peng
2015-05-19 6:28 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 6:12 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> > --- a/xen/arch/x86/mpparse.c
> > +++ b/xen/arch/x86/mpparse.c
> > @@ -64,6 +64,9 @@ unsigned int __read_mostly boot_cpu_physical_apicid = BAD_APICID;
> > static unsigned int __devinitdata num_processors;
> > static unsigned int __initdata disabled_cpus;
> >
> > +/* Total detected cpus (may exceed NR_CPUS) */
> > +unsigned int total_cpus;
> > +
> > /* Bitmask of physically existing CPUs */
> > physid_mask_t phys_cpu_present_map;
> >
> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct mpc_config_processor *m,
> > {
> > int ver, apicid, cpu = 0;
> >
> > + total_cpus++;
> > +
> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> > if (!hotplug)
> > ++disabled_cpus;
>
> Is there a reason you can't use disabled_cpus and avoid adding yet
> another variable?
The problem is not with disabled_cpus but with num_processors, which
does not keep the original detected cpus in current code.
Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
in some cases.
>
> > --- a/xen/arch/x86/smpboot.c
> > +++ b/xen/arch/x86/smpboot.c
> > @@ -59,6 +59,9 @@ DEFINE_PER_CPU_READ_MOSTLY(cpumask_var_t, cpu_core_mask);
> > cpumask_t cpu_online_map __read_mostly;
> > EXPORT_SYMBOL(cpu_online_map);
> >
> > +unsigned int nr_sockets __read_mostly;
> > +cpumask_var_t *socket_to_cpumask __read_mostly;
>
> I'd really like to see the "to" dropped from the name. It has been
> confusing me not for the first time. I'd also prefer the section
> annotations to be at their mandated place, between type and
> variable name.
Agreed, I also disliked this name while I just copied that from
node_to_cpumask.
> >
> > + nr_sockets = DIV_ROUND_UP(total_cpus, boot_cpu_data.x86_max_cores *
> > + boot_cpu_data.x86_num_siblings);
> > + socket_to_cpumask = xzalloc_array(cpumask_var_t, nr_sockets);
> > + if ( !socket_to_cpumask )
> > + panic("No memory for socket CPU siblings map");
> > + for ( socket = 0; socket < nr_sockets; socket++ )
> > + if ( !zalloc_cpumask_var(socket_to_cpumask + socket) )
> > + panic("No memory for socket CPU siblings cpumask");
>
> You might be allocating quite a bit too much memory now that you
> overestimate nr_sockets. Hence at least this second part of the
> change here would better be switched to an on demand allocation
> model.
Agreed.
> > +/*
> > + * This value is calculated by total_cpus/cpus_per_socket with the assumption
> > + * that APIC IDs from MP table are continuous. It's possible that this value
> > + * is less than the real socket number in the system if the APIC IDs from MP
> > + * table are too sparse. Also the value is considered not to change from the
> > + * initial startup. Violation of any of these assumptions may result in errors
> > + * and requires retrofitting all the relevant places.
> > + */
>
> This all reads pretty frightening. How about using a better estimate of
> core and thread count (i.e. ones matching actually observed values
> instead of the nearest larger powers of two) in the nr_sockets
> calculation? Overestimating nr_sockets is surely better than using too
> small a value, as the alternative of remembering to always bounds
> check socket values before use (not only in your series, but also in
> future changes) is going to be pretty fragile.
OK I will try this.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 6:12 ` Chao Peng
@ 2015-05-19 6:28 ` Jan Beulich
2015-05-19 6:47 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 6:28 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
> On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
>> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct mpc_config_processor *m,
>> > {
>> > int ver, apicid, cpu = 0;
>> >
>> > + total_cpus++;
>> > +
>> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
>> > if (!hotplug)
>> > ++disabled_cpus;
>>
>> Is there a reason you can't use disabled_cpus and avoid adding yet
>> another variable?
>
> The problem is not with disabled_cpus but with num_processors, which
> does not keep the original detected cpus in current code.
> Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
> in some cases.
Please be more specific about when this is a problem (I do note that
I'm aware that the equation will not always hold, but during my
inspection while reviewing your change I didn't see that this would
ever become problematic).
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 6:28 ` Jan Beulich
@ 2015-05-19 6:47 ` Chao Peng
2015-05-19 6:52 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 6:47 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct mpc_config_processor *m,
> >> > {
> >> > int ver, apicid, cpu = 0;
> >> >
> >> > + total_cpus++;
> >> > +
> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> >> > if (!hotplug)
> >> > ++disabled_cpus;
> >>
> >> Is there a reason you can't use disabled_cpus and avoid adding yet
> >> another variable?
> >
> > The problem is not with disabled_cpus but with num_processors, which
> > does not keep the original detected cpus in current code.
> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
> > in some cases.
>
> Please be more specific about when this is a problem (I do note that
> I'm aware that the equation will not always hold, but during my
> inspection while reviewing your change I didn't see that this would
> ever become problematic).
What I really need is the original cpu count enumerated from MADT. If
not introduce total_cpus then the only way getting it AFAICS is
'disabled_cpus + num_processors'.
The problem is that MP_processor_info_x() have some earlier returns
before increasing num_processors. In those cases, the cpu detected will
neither counted to disabled_cpus nor num_processors, which means
'disabled_cpus + num_processors' is potentially small than what I need.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 6:47 ` Chao Peng
@ 2015-05-19 6:52 ` Jan Beulich
2015-05-19 7:10 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 6:52 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
> On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
>> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
>> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
> mpc_config_processor *m,
>> >> > {
>> >> > int ver, apicid, cpu = 0;
>> >> >
>> >> > + total_cpus++;
>> >> > +
>> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
>> >> > if (!hotplug)
>> >> > ++disabled_cpus;
>> >>
>> >> Is there a reason you can't use disabled_cpus and avoid adding yet
>> >> another variable?
>> >
>> > The problem is not with disabled_cpus but with num_processors, which
>> > does not keep the original detected cpus in current code.
>> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
>> > in some cases.
>>
>> Please be more specific about when this is a problem (I do note that
>> I'm aware that the equation will not always hold, but during my
>> inspection while reviewing your change I didn't see that this would
>> ever become problematic).
>
> What I really need is the original cpu count enumerated from MADT. If
> not introduce total_cpus then the only way getting it AFAICS is
> 'disabled_cpus + num_processors'.
>
> The problem is that MP_processor_info_x() have some earlier returns
> before increasing num_processors. In those cases, the cpu detected will
> neither counted to disabled_cpus nor num_processors, which means
> 'disabled_cpus + num_processors' is potentially small than what I need.
As said - I understand this. But you still fail to explain under what
(realistic, i.e. other than someone bogusly setting NR_CPUS=1)
conditions this ends up being a problem.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 6:52 ` Jan Beulich
@ 2015-05-19 7:10 ` Chao Peng
2015-05-19 7:31 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 7:10 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
> >> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
> > mpc_config_processor *m,
> >> >> > {
> >> >> > int ver, apicid, cpu = 0;
> >> >> >
> >> >> > + total_cpus++;
> >> >> > +
> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> >> >> > if (!hotplug)
> >> >> > ++disabled_cpus;
> >> >>
> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet
> >> >> another variable?
> >> >
> >> > The problem is not with disabled_cpus but with num_processors, which
> >> > does not keep the original detected cpus in current code.
> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
> >> > in some cases.
> >>
> >> Please be more specific about when this is a problem (I do note that
> >> I'm aware that the equation will not always hold, but during my
> >> inspection while reviewing your change I didn't see that this would
> >> ever become problematic).
> >
> > What I really need is the original cpu count enumerated from MADT. If
> > not introduce total_cpus then the only way getting it AFAICS is
> > 'disabled_cpus + num_processors'.
> >
> > The problem is that MP_processor_info_x() have some earlier returns
> > before increasing num_processors. In those cases, the cpu detected will
> > neither counted to disabled_cpus nor num_processors, which means
> > 'disabled_cpus + num_processors' is potentially small than what I need.
>
> As said - I understand this. But you still fail to explain under what
> (realistic, i.e. other than someone bogusly setting NR_CPUS=1)
> conditions this ends up being a problem.
As we calculate nr_sockets with:
nr_sockets = total_cpus / _cpus_per_socket__
If the calculated total_cpus is smaller than the actual cpu count on the
hardware, then the nr_sockets is also potentially smaller than the
actual socket count on the hardware. This is not the expectation.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 7:10 ` Chao Peng
@ 2015-05-19 7:31 ` Jan Beulich
2015-05-19 9:51 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 7:31 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 09:10, <chao.p.peng@linux.intel.com> wrote:
> On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
>> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
>> >> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
>> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
>> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
>> > mpc_config_processor *m,
>> >> >> > {
>> >> >> > int ver, apicid, cpu = 0;
>> >> >> >
>> >> >> > + total_cpus++;
>> >> >> > +
>> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
>> >> >> > if (!hotplug)
>> >> >> > ++disabled_cpus;
>> >> >>
>> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet
>> >> >> another variable?
>> >> >
>> >> > The problem is not with disabled_cpus but with num_processors, which
>> >> > does not keep the original detected cpus in current code.
>> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
>> >> > in some cases.
>> >>
>> >> Please be more specific about when this is a problem (I do note that
>> >> I'm aware that the equation will not always hold, but during my
>> >> inspection while reviewing your change I didn't see that this would
>> >> ever become problematic).
>> >
>> > What I really need is the original cpu count enumerated from MADT. If
>> > not introduce total_cpus then the only way getting it AFAICS is
>> > 'disabled_cpus + num_processors'.
>> >
>> > The problem is that MP_processor_info_x() have some earlier returns
>> > before increasing num_processors. In those cases, the cpu detected will
>> > neither counted to disabled_cpus nor num_processors, which means
>> > 'disabled_cpus + num_processors' is potentially small than what I need.
>>
>> As said - I understand this. But you still fail to explain under what
>> (realistic, i.e. other than someone bogusly setting NR_CPUS=1)
>> conditions this ends up being a problem.
>
> As we calculate nr_sockets with:
>
> nr_sockets = total_cpus / _cpus_per_socket__
>
> If the calculated total_cpus is smaller than the actual cpu count on the
> hardware, then the nr_sockets is also potentially smaller than the
> actual socket count on the hardware. This is not the expectation.
Sure - but you still don't say what is going to go wrong. Remember,
when I asked you to change to the total count I gave an explicit
example: Use of "nosmp" would have yielded a zero nr_sockets in
the earlier code. Yet with the sum of num_processors and
disabled_cpus this can't happen anymore afaict. Hence I'm looking
forward to you detailing the conditions under which you would see
an issue without introducing total_cpus.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-18 13:33 ` Jan Beulich
@ 2015-05-19 7:40 ` Chao Peng
2015-05-19 8:42 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 7:40 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> > --- a/xen/arch/x86/psr.c
> > +++ b/xen/arch/x86/psr.c
> > @@ -19,14 +19,26 @@
> > #include <asm/psr.h>
> >
> > #define PSR_CMT (1<<0)
> > +#define PSR_CAT (1<<1)
> > +
> > +struct psr_cat_socket_info {
> > + unsigned int cbm_len;
> > + unsigned int cos_max;
> > +};
> >
> > struct psr_assoc {
> > uint64_t val;
> > };
> >
> > struct psr_cmt *__read_mostly psr_cmt;
> > +
> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>
> Didn't we agree to fold these two into one? Apart from that the
> _bitmap name tag doesn't seem very useful...
Looks like this?
static unsigned long *__read_mostly cat_socket_init,
*__read_mostly cat_socket_enable;
> > + cpuid_count(PSR_CPUID_LEVEL_CAT, 0, &eax, &ebx, &ecx, &edx);
> > + if ( ebx & PSR_RESOURCE_TYPE_L3 )
> > + {
> > + cpuid_count(PSR_CPUID_LEVEL_CAT, 1, &eax, &ebx, &ecx, &edx);
> > + info = cat_socket_info + socket;
> > + info->cbm_len = (eax & 0x1f) + 1;
> > + info->cos_max = min(opt_cos_max, edx & 0xffff);
>
>
> Is opt_cos_max being zero (or even one) going to result in a useful /
> working environment? I.e. shouldn't you rather disable CAT in that
> case?
OK, I will disable CAT in init_psr_cat() for this case.
>
> > +static void cat_cpu_fini(void)
> > +{
> > + unsigned int cpu = smp_processor_id();
> > + unsigned int socket = cpu_to_socket(cpu);
>
> This is the only use of "cpu", i.e. the variable is pretty pointless.
>
> > static int cpu_callback(
> > struct notifier_block *nfb, unsigned long action, void *hcpu)
> > {
> > if ( action == CPU_STARTING )
> > psr_cpu_init();
> > + else if ( action == CPU_DYING )
> > + psr_cpu_fini();
>
> Are these the right notifiers for doing things involving memory
> allocation / freeing?
psr_cpu_fini() does not really have memory allocation/freeing but
psr_cpu_init() does have.
While one thing to clarify is: psr_cpu_init() will not propagate
the error even when the memory allocation fails(instead it disables
the CAT on the socket).
If there is still problem then perhaps I need change CPU_STARTING back
to CPU_ONLINE and make on_selected_cpus() call on target cpu.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket
2015-05-18 13:35 ` Jan Beulich
@ 2015-05-19 7:41 ` Chao Peng
0 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-19 7:41 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Mon, May 18, 2015 at 02:35:39PM +0100, Jan Beulich wrote:
> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> > @@ -237,6 +243,14 @@ static void cat_cpu_init(void)
> > info->cbm_len = (eax & 0x1f) + 1;
> > info->cos_max = min(opt_cos_max, edx & 0xffff);
> >
> > + info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
> > + info->cos_max + 1UL);
>
> How does the array dimension here match the "for each socket"
> in the title?
info is already per-socket data so the dimension here is for only one
socket.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-19 7:40 ` Chao Peng
@ 2015-05-19 8:42 ` Jan Beulich
2015-05-19 8:46 ` Jan Beulich
2015-05-19 9:33 ` Chao Peng
0 siblings, 2 replies; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 8:42 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
> On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
>> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> > --- a/xen/arch/x86/psr.c
>> > +++ b/xen/arch/x86/psr.c
>> > @@ -19,14 +19,26 @@
>> > #include <asm/psr.h>
>> >
>> > #define PSR_CMT (1<<0)
>> > +#define PSR_CAT (1<<1)
>> > +
>> > +struct psr_cat_socket_info {
>> > + unsigned int cbm_len;
>> > + unsigned int cos_max;
>> > +};
>> >
>> > struct psr_assoc {
>> > uint64_t val;
>> > };
>> >
>> > struct psr_cmt *__read_mostly psr_cmt;
>> > +
>> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
>> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>>
>> Didn't we agree to fold these two into one? Apart from that the
>> _bitmap name tag doesn't seem very useful...
>
> Looks like this?
>
> static unsigned long *__read_mostly cat_socket_init,
> *__read_mostly cat_socket_enable;
Yes, except that you will want to drop one (or make clear why you
need both).
>> > static int cpu_callback(
>> > struct notifier_block *nfb, unsigned long action, void *hcpu)
>> > {
>> > if ( action == CPU_STARTING )
>> > psr_cpu_init();
>> > + else if ( action == CPU_DYING )
>> > + psr_cpu_fini();
>>
>> Are these the right notifiers for doing things involving memory
>> allocation / freeing?
>
> psr_cpu_fini() does not really have memory allocation/freeing but
> psr_cpu_init() does have.
Hmm, wait - where did I see this allocation? Looking again, I don't see
it now. But if there was one, then surely it would be wrong for _fini
to not free it.
> While one thing to clarify is: psr_cpu_init() will not propagate
> the error even when the memory allocation fails(instead it disables
> the CAT on the socket).
>
> If there is still problem then perhaps I need change CPU_STARTING back
> to CPU_ONLINE and make on_selected_cpus() call on target cpu.
No, if there was any allocation needed, you should do the allocation
in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
looks like this is moot now anyway.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-19 8:42 ` Jan Beulich
@ 2015-05-19 8:46 ` Jan Beulich
2015-05-19 9:33 ` Chao Peng
1 sibling, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 8:46 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 10:42, <JBeulich@suse.com> wrote:
>>>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
>> On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
>>> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>>> > --- a/xen/arch/x86/psr.c
>>> > +++ b/xen/arch/x86/psr.c
>>> > @@ -19,14 +19,26 @@
>>> > #include <asm/psr.h>
>>> >
>>> > #define PSR_CMT (1<<0)
>>> > +#define PSR_CAT (1<<1)
>>> > +
>>> > +struct psr_cat_socket_info {
>>> > + unsigned int cbm_len;
>>> > + unsigned int cos_max;
>>> > +};
>>> >
>>> > struct psr_assoc {
>>> > uint64_t val;
>>> > };
>>> >
>>> > struct psr_cmt *__read_mostly psr_cmt;
>>> > +
>>> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
>>> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>>>
>>> Didn't we agree to fold these two into one? Apart from that the
>>> _bitmap name tag doesn't seem very useful...
>>
>> Looks like this?
>>
>> static unsigned long *__read_mostly cat_socket_init,
>> *__read_mostly cat_socket_enable;
>
> Yes, except that you will want to drop one (or make clear why you
> need both).
>
>>> > static int cpu_callback(
>>> > struct notifier_block *nfb, unsigned long action, void *hcpu)
>>> > {
>>> > if ( action == CPU_STARTING )
>>> > psr_cpu_init();
>>> > + else if ( action == CPU_DYING )
>>> > + psr_cpu_fini();
>>>
>>> Are these the right notifiers for doing things involving memory
>>> allocation / freeing?
>>
>> psr_cpu_fini() does not really have memory allocation/freeing but
>> psr_cpu_init() does have.
>
> Hmm, wait - where did I see this allocation? Looking again, I don't see
> it now. But if there was one, then surely it would be wrong for _fini
> to not free it.
Ah, it's actually patch 4 where an allocation gets added. So
adjustments seem to be needed there.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-19 8:42 ` Jan Beulich
2015-05-19 8:46 ` Jan Beulich
@ 2015-05-19 9:33 ` Chao Peng
2015-05-19 10:22 ` Jan Beulich
1 sibling, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 9:33 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
> >>
> >> Didn't we agree to fold these two into one? Apart from that the
> >> _bitmap name tag doesn't seem very useful...
> >
> > Looks like this?
> >
> > static unsigned long *__read_mostly cat_socket_init,
> > *__read_mostly cat_socket_enable;
>
> Yes, except that you will want to drop one (or make clear why you
> need both).
As said in the previous version, cat_socket_enable is the actual CAT
enabled status.
And cat_socket_init is used for performance optimization purpose only.
It indicate if the socket has been initialized, so that initialization
happens only once for each socket, but not __cpus_in_socket__ times.
There are three possibilities:
1) Not initialized;
2) Initialized, CAT disabled;
3) Initialized, CAT enabled;
So it's not possible to use only one bit to represent three values.
>
> >> > static int cpu_callback(
> >> > struct notifier_block *nfb, unsigned long action, void *hcpu)
> >> > {
> >> > if ( action == CPU_STARTING )
> >> > psr_cpu_init();
> >> > + else if ( action == CPU_DYING )
> >> > + psr_cpu_fini();
> >>
> >> Are these the right notifiers for doing things involving memory
> >> allocation / freeing?
> >
> > psr_cpu_fini() does not really have memory allocation/freeing but
> > psr_cpu_init() does have.
>
> Hmm, wait - where did I see this allocation? Looking again, I don't see
> it now. But if there was one, then surely it would be wrong for _fini
> to not free it.
>
> > While one thing to clarify is: psr_cpu_init() will not propagate
> > the error even when the memory allocation fails(instead it disables
> > the CAT on the socket).
> >
> > If there is still problem then perhaps I need change CPU_STARTING back
> > to CPU_ONLINE and make on_selected_cpus() call on target cpu.
>
> No, if there was any allocation needed, you should do the allocation
> in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
> looks like this is moot now anyway.
As I will add allocation in patch 4, so I will move psr_cpu_init() to
CPU_UP_PREPARE(As the allocation length is based on the cpuid result, so
the initialization will also be done here), but still need to move
psr_cpu_fini(which will add the freeing code) to CPU_DOWN_PREPARE?
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 7:31 ` Jan Beulich
@ 2015-05-19 9:51 ` Chao Peng
2015-05-19 10:18 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-19 9:51 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 08:31:53AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 09:10, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote:
> >> >>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
> >> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
> >> >> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
> >> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
> >> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
> >> > mpc_config_processor *m,
> >> >> >> > {
> >> >> >> > int ver, apicid, cpu = 0;
> >> >> >> >
> >> >> >> > + total_cpus++;
> >> >> >> > +
> >> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> >> >> >> > if (!hotplug)
> >> >> >> > ++disabled_cpus;
> >> >> >>
> >> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet
> >> >> >> another variable?
> >> >> >
> >> >> > The problem is not with disabled_cpus but with num_processors, which
> >> >> > does not keep the original detected cpus in current code.
> >> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
> >> >> > in some cases.
> >> >>
> >> >> Please be more specific about when this is a problem (I do note that
> >> >> I'm aware that the equation will not always hold, but during my
> >> >> inspection while reviewing your change I didn't see that this would
> >> >> ever become problematic).
> >> >
> >> > What I really need is the original cpu count enumerated from MADT. If
> >> > not introduce total_cpus then the only way getting it AFAICS is
> >> > 'disabled_cpus + num_processors'.
> >> >
> >> > The problem is that MP_processor_info_x() have some earlier returns
> >> > before increasing num_processors. In those cases, the cpu detected will
> >> > neither counted to disabled_cpus nor num_processors, which means
> >> > 'disabled_cpus + num_processors' is potentially small than what I need.
> >>
> >> As said - I understand this. But you still fail to explain under what
> >> (realistic, i.e. other than someone bogusly setting NR_CPUS=1)
> >> conditions this ends up being a problem.
> >
> > As we calculate nr_sockets with:
> >
> > nr_sockets = total_cpus / _cpus_per_socket__
> >
> > If the calculated total_cpus is smaller than the actual cpu count on the
> > hardware, then the nr_sockets is also potentially smaller than the
> > actual socket count on the hardware. This is not the expectation.
>
> Sure - but you still don't say what is going to go wrong. Remember,
> when I asked you to change to the total count I gave an explicit
> example: Use of "nosmp" would have yielded a zero nr_sockets in
> the earlier code. Yet with the sum of num_processors and
> disabled_cpus this can't happen anymore afaict.
"nosmp" only has side effect on max_cpus and nr_cpu_ids, but they are
never used at all when calculating nr_sockets. So I can't see any reason
why with "num_processors + disabled_cpus" the nr_sockets would not be
zero, I think this is a bug that I should fix in nosmp case.
> Hence I'm looking
> forward to you detailing the conditions under which you would see
> an issue without introducing total_cpus.
As said before, with "num_processors + disabled_cpus" I may get a
smaller nr_sockets than the machine actually has. This is my exact
problem: I may miss enumerating some CAT-enabled sockets. While the
assumption is that I will follow your suggestion to make nr_socket >=
the socket count that the machine actually has.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 9:51 ` Chao Peng
@ 2015-05-19 10:18 ` Jan Beulich
2015-05-20 3:13 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 10:18 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 11:51, <chao.p.peng@linux.intel.com> wrote:
> On Tue, May 19, 2015 at 08:31:53AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 09:10, <chao.p.peng@linux.intel.com> wrote:
>> > On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote:
>> >> >>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
>> >> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
>> >> >> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
>> >> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
>> >> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> >> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
>> >> > mpc_config_processor *m,
>> >> >> >> > {
>> >> >> >> > int ver, apicid, cpu = 0;
>> >> >> >> >
>> >> >> >> > + total_cpus++;
>> >> >> >> > +
>> >> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
>> >> >> >> > if (!hotplug)
>> >> >> >> > ++disabled_cpus;
>> >> >> >>
>> >> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet
>> >> >> >> another variable?
>> >> >> >
>> >> >> > The problem is not with disabled_cpus but with num_processors, which
>> >> >> > does not keep the original detected cpus in current code.
>> >> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
>> >> >> > in some cases.
>> >> >>
>> >> >> Please be more specific about when this is a problem (I do note that
>> >> >> I'm aware that the equation will not always hold, but during my
>> >> >> inspection while reviewing your change I didn't see that this would
>> >> >> ever become problematic).
>> >> >
>> >> > What I really need is the original cpu count enumerated from MADT. If
>> >> > not introduce total_cpus then the only way getting it AFAICS is
>> >> > 'disabled_cpus + num_processors'.
>> >> >
>> >> > The problem is that MP_processor_info_x() have some earlier returns
>> >> > before increasing num_processors. In those cases, the cpu detected will
>> >> > neither counted to disabled_cpus nor num_processors, which means
>> >> > 'disabled_cpus + num_processors' is potentially small than what I need.
>> >>
>> >> As said - I understand this. But you still fail to explain under what
>> >> (realistic, i.e. other than someone bogusly setting NR_CPUS=1)
>> >> conditions this ends up being a problem.
>> >
>> > As we calculate nr_sockets with:
>> >
>> > nr_sockets = total_cpus / _cpus_per_socket__
>> >
>> > If the calculated total_cpus is smaller than the actual cpu count on the
>> > hardware, then the nr_sockets is also potentially smaller than the
>> > actual socket count on the hardware. This is not the expectation.
>>
>> Sure - but you still don't say what is going to go wrong. Remember,
>> when I asked you to change to the total count I gave an explicit
>> example: Use of "nosmp" would have yielded a zero nr_sockets in
>> the earlier code. Yet with the sum of num_processors and
>> disabled_cpus this can't happen anymore afaict.
>
> "nosmp" only has side effect on max_cpus and nr_cpu_ids, but they are
> never used at all when calculating nr_sockets. So I can't see any reason
> why with "num_processors + disabled_cpus" the nr_sockets would not be
> zero, I think this is a bug that I should fix in nosmp case.
Did you really mean to say "would _not_ be zero"? Because afaict
the above resolves to "would ever be zero".
>> Hence I'm looking
>> forward to you detailing the conditions under which you would see
>> an issue without introducing total_cpus.
>
> As said before, with "num_processors + disabled_cpus" I may get a
> smaller nr_sockets than the machine actually has. This is my exact
> problem: I may miss enumerating some CAT-enabled sockets. While the
> assumption is that I will follow your suggestion to make nr_socket >=
> the socket count that the machine actually has.
Please can you stop repeating yourself? Yes, nr_sockets can get
underestimated this way, but the cases where total_cpus !=
num_processors + disabled_cpus are - afaict - such that this in
the end does no harm. Other than the original case of nr_sockets
ending up being zero when "nosmp". So to convince me of the
opposite I'm afraid you won't get around constructing an explicit
example where things break (and hence my earlier hint regarding
NR_CPUS=1, as I wouldn't count this as a valid example).
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-19 9:33 ` Chao Peng
@ 2015-05-19 10:22 ` Jan Beulich
2015-05-20 3:21 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-19 10:22 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 19.05.15 at 11:33, <chao.p.peng@linux.intel.com> wrote:
> On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
>> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
>> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
>> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>> >>
>> >> Didn't we agree to fold these two into one? Apart from that the
>> >> _bitmap name tag doesn't seem very useful...
>> >
>> > Looks like this?
>> >
>> > static unsigned long *__read_mostly cat_socket_init,
>> > *__read_mostly cat_socket_enable;
>>
>> Yes, except that you will want to drop one (or make clear why you
>> need both).
>
> As said in the previous version, cat_socket_enable is the actual CAT
> enabled status.
>
> And cat_socket_init is used for performance optimization purpose only.
> It indicate if the socket has been initialized, so that initialization
> happens only once for each socket, but not __cpus_in_socket__ times.
>
> There are three possibilities:
> 1) Not initialized;
> 2) Initialized, CAT disabled;
> 3) Initialized, CAT enabled;
>
> So it's not possible to use only one bit to represent three values.
Why? The two bits get set together, and get cleared together. Even
if they have formally different meaning, there is - afaict - no case
where their values will differ (except for the brief period between
setting one and then the other).
>> >> > static int cpu_callback(
>> >> > struct notifier_block *nfb, unsigned long action, void *hcpu)
>> >> > {
>> >> > if ( action == CPU_STARTING )
>> >> > psr_cpu_init();
>> >> > + else if ( action == CPU_DYING )
>> >> > + psr_cpu_fini();
>> >>
>> >> Are these the right notifiers for doing things involving memory
>> >> allocation / freeing?
>> >
>> > psr_cpu_fini() does not really have memory allocation/freeing but
>> > psr_cpu_init() does have.
>>
>> Hmm, wait - where did I see this allocation? Looking again, I don't see
>> it now. But if there was one, then surely it would be wrong for _fini
>> to not free it.
>>
>> > While one thing to clarify is: psr_cpu_init() will not propagate
>> > the error even when the memory allocation fails(instead it disables
>> > the CAT on the socket).
>> >
>> > If there is still problem then perhaps I need change CPU_STARTING back
>> > to CPU_ONLINE and make on_selected_cpus() call on target cpu.
>>
>> No, if there was any allocation needed, you should do the allocation
>> in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
>> looks like this is moot now anyway.
>
> As I will add allocation in patch 4, so I will move psr_cpu_init() to
> CPU_UP_PREPARE(As the allocation length is based on the cpuid result, so
> the initialization will also be done here), but still need to move
> psr_cpu_fini(which will add the freeing code) to CPU_DOWN_PREPARE?
CPU_DEAD you mean.
But again - only the allocation/freeing should be done in the
alternative named notifications, not the actual data setup (as that
iirc needs to run on the CPU).
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-19 10:18 ` Jan Beulich
@ 2015-05-20 3:13 ` Chao Peng
2015-05-20 9:00 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-20 3:13 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 11:18:20AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 11:51, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 19, 2015 at 08:31:53AM +0100, Jan Beulich wrote:
> >> >>> On 19.05.15 at 09:10, <chao.p.peng@linux.intel.com> wrote:
> >> > On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote:
> >> >> >>> On 19.05.15 at 08:47, <chao.p.peng@linux.intel.com> wrote:
> >> >> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote:
> >> >> >> >>> On 19.05.15 at 08:12, <chao.p.peng@linux.intel.com> wrote:
> >> >> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote:
> >> >> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> >> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct
> >> >> > mpc_config_processor *m,
> >> >> >> >> > {
> >> >> >> >> > int ver, apicid, cpu = 0;
> >> >> >> >> >
> >> >> >> >> > + total_cpus++;
> >> >> >> >> > +
> >> >> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) {
> >> >> >> >> > if (!hotplug)
> >> >> >> >> > ++disabled_cpus;
> >> >> >> >>
> >> >> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet
> >> >> >> >> another variable?
> >> >> >> >
> >> >> >> > The problem is not with disabled_cpus but with num_processors, which
> >> >> >> > does not keep the original detected cpus in current code.
> >> >> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct
> >> >> >> > in some cases.
> >> >> >>
> >> >> >> Please be more specific about when this is a problem (I do note that
> >> >> >> I'm aware that the equation will not always hold, but during my
> >> >> >> inspection while reviewing your change I didn't see that this would
> >> >> >> ever become problematic).
> >> >> >
> >> >> > What I really need is the original cpu count enumerated from MADT. If
> >> >> > not introduce total_cpus then the only way getting it AFAICS is
> >> >> > 'disabled_cpus + num_processors'.
> >> >> >
> >> >> > The problem is that MP_processor_info_x() have some earlier returns
> >> >> > before increasing num_processors. In those cases, the cpu detected will
> >> >> > neither counted to disabled_cpus nor num_processors, which means
> >> >> > 'disabled_cpus + num_processors' is potentially small than what I need.
> >> >>
> >> >> As said - I understand this. But you still fail to explain under what
> >> >> (realistic, i.e. other than someone bogusly setting NR_CPUS=1)
> >> >> conditions this ends up being a problem.
> >> >
> >> > As we calculate nr_sockets with:
> >> >
> >> > nr_sockets = total_cpus / _cpus_per_socket__
> >> >
> >> > If the calculated total_cpus is smaller than the actual cpu count on the
> >> > hardware, then the nr_sockets is also potentially smaller than the
> >> > actual socket count on the hardware. This is not the expectation.
> >>
> >> Sure - but you still don't say what is going to go wrong. Remember,
> >> when I asked you to change to the total count I gave an explicit
> >> example: Use of "nosmp" would have yielded a zero nr_sockets in
> >> the earlier code. Yet with the sum of num_processors and
> >> disabled_cpus this can't happen anymore afaict.
> >
> > "nosmp" only has side effect on max_cpus and nr_cpu_ids, but they are
> > never used at all when calculating nr_sockets. So I can't see any reason
> > why with "num_processors + disabled_cpus" the nr_sockets would not be
> > zero, I think this is a bug that I should fix in nosmp case.
>
> Did you really mean to say "would _not_ be zero"? Because afaict
> the above resolves to "would ever be zero".
Yes, I mean "would ever be zero".
>
> >> Hence I'm looking
> >> forward to you detailing the conditions under which you would see
> >> an issue without introducing total_cpus.
> >
> > As said before, with "num_processors + disabled_cpus" I may get a
> > smaller nr_sockets than the machine actually has. This is my exact
> > problem: I may miss enumerating some CAT-enabled sockets. While the
> > assumption is that I will follow your suggestion to make nr_socket >=
> > the socket count that the machine actually has.
>
> Please can you stop repeating yourself? Yes, nr_sockets can get
> underestimated this way, but the cases where total_cpus !=
> num_processors + disabled_cpus are - afaict - such that this in
> the end does no harm. Other than the original case of nr_sockets
> ending up being zero when "nosmp". So to convince me of the
> opposite I'm afraid you won't get around constructing an explicit
> example where things break (and hence my earlier hint regarding
> NR_CPUS=1, as I wouldn't count this as a valid example).
My original thought is that with newly introduced total_cpus, nr_sockets
is less likely to be underestimated than with "num_processors +
disabled_cpus", if not now but for the future. But perhaps I understood
it incorrectly.
While this is really not a big deal for me. And "num_processors +
disabled_cpus" does work for me now so I feel comfortable to use it.
I do have a problem when coming up with the final codes, there are two
possible ways:
1) Make both num_processors and disabled_cpus public so that they can
be used when calculating nr_sockets.
2) Add another public function set_nr_sockets in mpparse.c (just like
set_nr_cpu_ids does).
Which do you prefer?
Thanks,
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-19 10:22 ` Jan Beulich
@ 2015-05-20 3:21 ` Chao Peng
2015-05-20 9:06 ` Jan Beulich
0 siblings, 1 reply; 48+ messages in thread
From: Chao Peng @ 2015-05-20 3:21 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Tue, May 19, 2015 at 11:22:54AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 11:33, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
> >> >>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
> >> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> >> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
> >> >>
> >> >> Didn't we agree to fold these two into one? Apart from that the
> >> >> _bitmap name tag doesn't seem very useful...
> >> >
> >> > Looks like this?
> >> >
> >> > static unsigned long *__read_mostly cat_socket_init,
> >> > *__read_mostly cat_socket_enable;
> >>
> >> Yes, except that you will want to drop one (or make clear why you
> >> need both).
> >
> > As said in the previous version, cat_socket_enable is the actual CAT
> > enabled status.
> >
> > And cat_socket_init is used for performance optimization purpose only.
> > It indicate if the socket has been initialized, so that initialization
> > happens only once for each socket, but not __cpus_in_socket__ times.
> >
> > There are three possibilities:
> > 1) Not initialized;
> > 2) Initialized, CAT disabled;
> > 3) Initialized, CAT enabled;
> >
> > So it's not possible to use only one bit to represent three values.
>
> Why? The two bits get set together, and get cleared together. Even
> if they have formally different meaning, there is - afaict - no case
> where their values will differ (except for the brief period between
> setting one and then the other).
Do you want to try the failure initialization(both because CAT is not
even supported on the machine or fails to allocate cos_to_cbm) on and
on?
>
> >> >> > static int cpu_callback(
> >> >> > struct notifier_block *nfb, unsigned long action, void *hcpu)
> >> >> > {
> >> >> > if ( action == CPU_STARTING )
> >> >> > psr_cpu_init();
> >> >> > + else if ( action == CPU_DYING )
> >> >> > + psr_cpu_fini();
> >> >>
> >> >> Are these the right notifiers for doing things involving memory
> >> >> allocation / freeing?
> >> >
> >> > psr_cpu_fini() does not really have memory allocation/freeing but
> >> > psr_cpu_init() does have.
> >>
> >> Hmm, wait - where did I see this allocation? Looking again, I don't see
> >> it now. But if there was one, then surely it would be wrong for _fini
> >> to not free it.
> >>
> >> > While one thing to clarify is: psr_cpu_init() will not propagate
> >> > the error even when the memory allocation fails(instead it disables
> >> > the CAT on the socket).
> >> >
> >> > If there is still problem then perhaps I need change CPU_STARTING back
> >> > to CPU_ONLINE and make on_selected_cpus() call on target cpu.
> >>
> >> No, if there was any allocation needed, you should do the allocation
> >> in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
> >> looks like this is moot now anyway.
> >
> > As I will add allocation in patch 4, so I will move psr_cpu_init() to
> > CPU_UP_PREPARE(As the allocation length is based on the cpuid result, so
> > the initialization will also be done here), but still need to move
> > psr_cpu_fini(which will add the freeing code) to CPU_DOWN_PREPARE?
>
> CPU_DEAD you mean.
Yes, CPU_DEAD.
>
> But again - only the allocation/freeing should be done in the
> alternative named notifications, not the actual data setup (as that
> iirc needs to run on the CPU).
I know this is the general rules, but for this case, I don't think it's
going to work to allocate memory in CPU_UP_PREPARE while do the
initialization later(in CPU_STARTING).
As said, the allocation has dependency on the result of the
initialization. E.g. before
info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
info->cos_max + 1UL);
info->cos_max must have been known.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 01/14] x86: add socket_to_cpumask
2015-05-20 3:13 ` Chao Peng
@ 2015-05-20 9:00 ` Jan Beulich
0 siblings, 0 replies; 48+ messages in thread
From: Jan Beulich @ 2015-05-20 9:00 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 20.05.15 at 05:13, <chao.p.peng@linux.intel.com> wrote:
> I do have a problem when coming up with the final codes, there are two
> possible ways:
> 1) Make both num_processors and disabled_cpus public so that they can
> be used when calculating nr_sockets.
> 2) Add another public function set_nr_sockets in mpparse.c (just like
> set_nr_cpu_ids does).
>
> Which do you prefer?
The latter.
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-20 3:21 ` Chao Peng
@ 2015-05-20 9:06 ` Jan Beulich
2015-05-20 9:26 ` Chao Peng
0 siblings, 1 reply; 48+ messages in thread
From: Jan Beulich @ 2015-05-20 9:06 UTC (permalink / raw)
To: Chao Peng
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
>>> On 20.05.15 at 05:21, <chao.p.peng@linux.intel.com> wrote:
> On Tue, May 19, 2015 at 11:22:54AM +0100, Jan Beulich wrote:
>> >>> On 19.05.15 at 11:33, <chao.p.peng@linux.intel.com> wrote:
>> > On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
>> >> >>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
>> >> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
>> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
>> >> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
>> >> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
>> >> >>
>> >> >> Didn't we agree to fold these two into one? Apart from that the
>> >> >> _bitmap name tag doesn't seem very useful...
>> >> >
>> >> > Looks like this?
>> >> >
>> >> > static unsigned long *__read_mostly cat_socket_init,
>> >> > *__read_mostly cat_socket_enable;
>> >>
>> >> Yes, except that you will want to drop one (or make clear why you
>> >> need both).
>> >
>> > As said in the previous version, cat_socket_enable is the actual CAT
>> > enabled status.
>> >
>> > And cat_socket_init is used for performance optimization purpose only.
>> > It indicate if the socket has been initialized, so that initialization
>> > happens only once for each socket, but not __cpus_in_socket__ times.
>> >
>> > There are three possibilities:
>> > 1) Not initialized;
>> > 2) Initialized, CAT disabled;
>> > 3) Initialized, CAT enabled;
>> >
>> > So it's not possible to use only one bit to represent three values.
>>
>> Why? The two bits get set together, and get cleared together. Even
>> if they have formally different meaning, there is - afaict - no case
>> where their values will differ (except for the brief period between
>> setting one and then the other).
>
> Do you want to try the failure initialization(both because CAT is not
> even supported on the machine or fails to allocate cos_to_cbm) on and
> on?
I don't see any harm - all you save is a cpuid_count() invocation.
>> But again - only the allocation/freeing should be done in the
>> alternative named notifications, not the actual data setup (as that
>> iirc needs to run on the CPU).
>
> I know this is the general rules, but for this case, I don't think it's
> going to work to allocate memory in CPU_UP_PREPARE while do the
> initialization later(in CPU_STARTING).
>
> As said, the allocation has dependency on the result of the
> initialization. E.g. before
>
> info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
> info->cos_max + 1UL);
>
> info->cos_max must have been known.
Ah, yes. Otoh, considering that everywhere else we expect
symmetry across CPUs, do you really think there are going to be
systems with different CPUs having different cos_max values?
Jan
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature
2015-05-20 9:06 ` Jan Beulich
@ 2015-05-20 9:26 ` Chao Peng
0 siblings, 0 replies; 48+ messages in thread
From: Chao Peng @ 2015-05-20 9:26 UTC (permalink / raw)
To: Jan Beulich
Cc: wei.liu2, Ian.Campbell, stefano.stabellini, andrew.cooper3,
Ian.Jackson, xen-devel, will.auld, keir, dgdegra
On Wed, May 20, 2015 at 10:06:32AM +0100, Jan Beulich wrote:
> >>> On 20.05.15 at 05:21, <chao.p.peng@linux.intel.com> wrote:
> > On Tue, May 19, 2015 at 11:22:54AM +0100, Jan Beulich wrote:
> >> >>> On 19.05.15 at 11:33, <chao.p.peng@linux.intel.com> wrote:
> >> > On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
> >> >> >>> On 19.05.15 at 09:40, <chao.p.peng@linux.intel.com> wrote:
> >> >> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >> >> >> >>> On 08.05.15 at 10:56, <chao.p.peng@linux.intel.com> wrote:
> >> >> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> >> >> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
> >> >> >>
> >> >> >> Didn't we agree to fold these two into one? Apart from that the
> >> >> >> _bitmap name tag doesn't seem very useful...
> >> >> >
> >> >> > Looks like this?
> >> >> >
> >> >> > static unsigned long *__read_mostly cat_socket_init,
> >> >> > *__read_mostly cat_socket_enable;
> >> >>
> >> >> Yes, except that you will want to drop one (or make clear why you
> >> >> need both).
> >> >
> >> > As said in the previous version, cat_socket_enable is the actual CAT
> >> > enabled status.
> >> >
> >> > And cat_socket_init is used for performance optimization purpose only.
> >> > It indicate if the socket has been initialized, so that initialization
> >> > happens only once for each socket, but not __cpus_in_socket__ times.
> >> >
> >> > There are three possibilities:
> >> > 1) Not initialized;
> >> > 2) Initialized, CAT disabled;
> >> > 3) Initialized, CAT enabled;
> >> >
> >> > So it's not possible to use only one bit to represent three values.
> >>
> >> Why? The two bits get set together, and get cleared together. Even
> >> if they have formally different meaning, there is - afaict - no case
> >> where their values will differ (except for the brief period between
> >> setting one and then the other).
> >
> > Do you want to try the failure initialization(both because CAT is not
> > even supported on the machine or fails to allocate cos_to_cbm) on and
> > on?
>
> I don't see any harm - all you save is a cpuid_count() invocation.
Then I will remove cat_socket_init.
>
> >> But again - only the allocation/freeing should be done in the
> >> alternative named notifications, not the actual data setup (as that
> >> iirc needs to run on the CPU).
> >
> > I know this is the general rules, but for this case, I don't think it's
> > going to work to allocate memory in CPU_UP_PREPARE while do the
> > initialization later(in CPU_STARTING).
> >
> > As said, the allocation has dependency on the result of the
> > initialization. E.g. before
> >
> > info->cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
> > info->cos_max + 1UL);
> >
> > info->cos_max must have been known.
>
> Ah, yes. Otoh, considering that everywhere else we expect
> symmetry across CPUs, do you really think there are going to be
> systems with different CPUs having different cos_max values?
I agree, such systems should be rare, or even doesn't exist in the real
world. But as we are trying to support it, then I have to make it working.
Chao
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2015-05-20 9:26 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-08 8:56 [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Chao Peng
2015-05-08 8:56 ` [PATCH v7 01/14] x86: add socket_to_cpumask Chao Peng
2015-05-18 13:21 ` Jan Beulich
2015-05-19 6:12 ` Chao Peng
2015-05-19 6:28 ` Jan Beulich
2015-05-19 6:47 ` Chao Peng
2015-05-19 6:52 ` Jan Beulich
2015-05-19 7:10 ` Chao Peng
2015-05-19 7:31 ` Jan Beulich
2015-05-19 9:51 ` Chao Peng
2015-05-19 10:18 ` Jan Beulich
2015-05-20 3:13 ` Chao Peng
2015-05-20 9:00 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 02/14] x86: improve psr scheduling code Chao Peng
2015-05-08 10:20 ` Jan Beulich
2015-05-11 1:35 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Chao Peng
2015-05-18 13:33 ` Jan Beulich
2015-05-19 7:40 ` Chao Peng
2015-05-19 8:42 ` Jan Beulich
2015-05-19 8:46 ` Jan Beulich
2015-05-19 9:33 ` Chao Peng
2015-05-19 10:22 ` Jan Beulich
2015-05-20 3:21 ` Chao Peng
2015-05-20 9:06 ` Jan Beulich
2015-05-20 9:26 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 04/14] x86: maintain COS to CBM mapping for each socket Chao Peng
2015-05-18 13:35 ` Jan Beulich
2015-05-19 7:41 ` Chao Peng
2015-05-08 8:56 ` [PATCH v7 05/14] x86: add COS information for each domain Chao Peng
2015-05-08 8:56 ` [PATCH v7 06/14] x86: expose CBM length and COS number information Chao Peng
2015-05-08 8:56 ` [PATCH v7 07/14] x86: dynamically get/set CBM for a domain Chao Peng
2015-05-14 9:19 ` Dario Faggioli
2015-05-15 1:35 ` Chao Peng
2015-05-18 13:45 ` Jan Beulich
2015-05-08 8:56 ` [PATCH v7 08/14] x86: add scheduling support for Intel CAT Chao Peng
2015-05-08 8:56 ` [PATCH v7 09/14] xsm: add CAT related xsm policies Chao Peng
2015-05-08 8:56 ` [PATCH v7 10/14] tools/libxl: minor name changes for CMT commands Chao Peng
2015-05-08 8:56 ` [PATCH v7 11/14] tools/libxl: add command to show PSR hardware info Chao Peng
2015-05-08 8:56 ` [PATCH v7 12/14] tools/libxl: introduce some socket helpers Chao Peng
2015-05-08 8:56 ` [PATCH v7 13/14] tools: add tools support for Intel CAT Chao Peng
2015-05-08 13:38 ` Ian Campbell
2015-05-08 8:56 ` [PATCH v7 14/14] docs: add xl-psr.markdown Chao Peng
2015-05-08 13:39 ` Ian Campbell
2015-05-08 13:41 ` [PATCH v7 00/14] enable Cache Allocation Technology (CAT) for VMs Ian Campbell
2015-05-08 13:48 ` Jan Beulich
2015-05-08 14:11 ` Ian Campbell
2015-05-11 1:37 ` Chao Peng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.