[RFC PATCH 0/3] x86/cpufeatures: Re-arrange a few features and enumerate AVX512 BFLOAT16 intructions

[RFC PATCH 0/3] x86/cpufeatures: Re-arrange a few features and enumerate AVX512 BFLOAT16 intructions

[Fenghua Yu]

To enumerate AVX512 BFLOAT16 feature CPUID.7.1:EAX[5] and other future
features in CPUID.7.1:EAX, Boris suggests to create a new pure feature
bits word.

Boris further suggests to re-define word 11 as scattered features word
and move the four X86_FEATURE_CQM_* features in existing word 11 and
word 12 into the new word 11. Then use word 12 to hold features in
CPUID.7.1:EAX including AVX512 BFLOAT16 instructions.

Also remove x86_cache_max_rmid and x86_cache_occ_scale in cpuinfo_x86
to save memory space because they are only read once by resctrl during
boot time. Get the info directly from CPUID in resctrl initialization.

Fenghua Yu (3):
  x86/resctrl: Get max rmid and occupancy scale directly from CPUID
    instead of cpuinfo_x86
  x86/cpufeatures: Combine word 11 and 12 into new scattered features
    word 11
  x86/cpufeatures: Enumerate new AVX512 BFLOAT16 instructions

 arch/x86/include/asm/cpufeature.h      |  4 +--
 arch/x86/include/asm/cpufeatures.h     | 18 +++++++----
 arch/x86/include/asm/processor.h       |  3 --
 arch/x86/kernel/cpu/common.c           | 45 ++------------------------
 arch/x86/kernel/cpu/cpuid-deps.c       |  4 +++
 arch/x86/kernel/cpu/resctrl/internal.h |  2 +-
 arch/x86/kernel/cpu/resctrl/monitor.c  | 28 ++++++++++++++--
 arch/x86/kernel/cpu/scattered.c        |  4 +++
 arch/x86/kvm/cpuid.h                   |  2 --
 9 files changed, 51 insertions(+), 59 deletions(-)

-- 
2.19.1

[RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale directly from CPUID instead of cpuinfo_x86

[Fenghua Yu]

Although x86_cache_max_rmid and x86_cache_occ_scale are read only once
during resctrl initialization, they are always stored in cpuinfo_x86 on
each CPU during run time even if resctrl is not configured.

To save cpuinfo_x86 space and make CPU and resctrl initialization simpler,
remove the two fields from cpuinfo_x86 and get max rmid and occupancy
scale directly from CPUID during resctrl initialization. And since each
known platform that supports resctrl has same max rmid on all CPUs, no
need to scan all CPUs to find minimum of max rmid values, i.e. getting
max rmid from CPUID on the current CPU is fine.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/processor.h       |  3 ---
 arch/x86/kernel/cpu/common.c           | 28 --------------------------
 arch/x86/kernel/cpu/resctrl/internal.h |  2 +-
 arch/x86/kernel/cpu/resctrl/monitor.c  | 28 +++++++++++++++++++++++---
 4 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index c34a35c78618..27e875d4ca7d 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -99,9 +99,6 @@ struct cpuinfo_x86 {
 	/* in KB - valid for CPUS which support this call: */
 	unsigned int		x86_cache_size;
 	int			x86_cache_alignment;	/* In bytes */
-	/* Cache QoS architectural values: */
-	int			x86_cache_max_rmid;	/* max index */
-	int			x86_cache_occ_scale;	/* scale to bytes */
 	int			x86_power;
 	unsigned long		loops_per_jiffy;
 	/* cpuid returned max cores value: */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2c57fffebf9b..38e4b1a9005e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -840,22 +840,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_F_0_EDX] = edx;
 
 		if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
-			/* will be overridden if occupancy monitoring exists */
-			c->x86_cache_max_rmid = ebx;
-
 			/* QoS sub-leaf, EAX=0Fh, ECX=1 */
 			cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
 			c->x86_capability[CPUID_F_1_EDX] = edx;
-
-			if ((cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC)) ||
-			      ((cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL)) ||
-			       (cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)))) {
-				c->x86_cache_max_rmid = ecx;
-				c->x86_cache_occ_scale = ebx;
-			}
-		} else {
-			c->x86_cache_max_rmid = -1;
-			c->x86_cache_occ_scale = -1;
 		}
 	}
 
@@ -1269,20 +1256,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
 #endif
 }
 
-static void x86_init_cache_qos(struct cpuinfo_x86 *c)
-{
-	/*
-	 * The heavy lifting of max_rmid and cache_occ_scale are handled
-	 * in get_cpu_cap().  Here we just set the max_rmid for the boot_cpu
-	 * in case CQM bits really aren't there in this CPU.
-	 */
-	if (c != &boot_cpu_data) {
-		boot_cpu_data.x86_cache_max_rmid =
-			min(boot_cpu_data.x86_cache_max_rmid,
-			    c->x86_cache_max_rmid);
-	}
-}
-
 /*
  * Validate that ACPI/mptables have the same information about the
  * effective APIC id and update the package map.
@@ -1391,7 +1364,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 #endif
 
 	x86_init_rdrand(c);
-	x86_init_cache_qos(c);
 	setup_pku(c);
 
 	/*
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index e49b77283924..474a7090d2dd 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -579,7 +579,7 @@ int closids_supported(void);
 void closid_free(int closid);
 int alloc_rmid(void);
 void free_rmid(u32 rmid);
-int rdt_get_mon_l3_config(struct rdt_resource *r);
+int __init rdt_get_mon_l3_config(struct rdt_resource *r);
 void mon_event_count(void *info);
 int rdtgroup_mondata_show(struct seq_file *m, void *arg);
 void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 1573a0a6b525..e9d876c25703 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -617,13 +617,35 @@ static void l3_mon_evt_init(struct rdt_resource *r)
 		list_add_tail(&mbm_local_event.list, &r->evt_list);
 }
 
-int rdt_get_mon_l3_config(struct rdt_resource *r)
+static void __init get_cqm_info(struct rdt_resource *r)
+{
+	u32 eax, ebx, ecx, edx;
+
+	/*
+	 * At this point, CQM LLC and one of occupancy, MBM total, and
+	 * MBM local monitoring features must be supported.
+	 */
+	cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
+	/* will be overridden if occupancy monitoring exists */
+	r->num_rmid = ebx + 1;
+
+	cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
+
+	if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
+		r->num_rmid = ecx + 1;
+
+	if (boot_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) || boot_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
+		r->mon_scale = ebx;
+	else
+		r->mon_scale = -1;
+}
+
+int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 {
 	unsigned int cl_size = boot_cpu_data.x86_cache_size;
 	int ret;
 
-	r->mon_scale = boot_cpu_data.x86_cache_occ_scale;
-	r->num_rmid = boot_cpu_data.x86_cache_max_rmid + 1;
+	get_cqm_info(r);
 
 	/*
 	 * A reasonable upper limit on the max threshold is the number
-- 
2.19.1

[RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

It's a waste for the four X86_FEATURE_CQM_* features to occupy two
pure feature bits words. To better utilize feature words, re-define
word 11 to host scattered features and move the four X86_FEATURE_CQM_*
features into word 11. More scattered features can be added in word 11
in the future.

KVM doesn't support resctrl now. So it's safe to move the
X86_FEATURE_CQM_* features to scattered features word 11 for KVM.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpufeature.h  |  3 +--
 arch/x86/include/asm/cpufeatures.h | 17 ++++++++++-------
 arch/x86/kernel/cpu/common.c       | 14 --------------
 arch/x86/kernel/cpu/cpuid-deps.c   |  3 +++
 arch/x86/kernel/cpu/scattered.c    |  4 ++++
 arch/x86/kvm/cpuid.h               |  2 --
 6 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 1d337c51f7e6..526619906305 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -22,8 +22,7 @@ enum cpuid_leafs
 	CPUID_LNX_3,
 	CPUID_7_0_EBX,
 	CPUID_D_1_EAX,
-	CPUID_F_0_EDX,
-	CPUID_F_1_EDX,
+	CPUID_LNX_4,
 	CPUID_8000_0008_EBX,
 	CPUID_6_EAX,
 	CPUID_8000_000A_EDX,
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 75f27ee2c263..4f0a3d093794 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -269,13 +269,16 @@
 #define X86_FEATURE_XGETBV1		(10*32+ 2) /* XGETBV with ECX = 1 instruction */
 #define X86_FEATURE_XSAVES		(10*32+ 3) /* XSAVES/XRSTORS instructions */
 
-/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:0 (EDX), word 11 */
-#define X86_FEATURE_CQM_LLC		(11*32+ 1) /* LLC QoS if 1 */
-
-/* Intel-defined CPU QoS Sub-leaf, CPUID level 0x0000000F:1 (EDX), word 12 */
-#define X86_FEATURE_CQM_OCCUP_LLC	(12*32+ 0) /* LLC occupancy monitoring */
-#define X86_FEATURE_CQM_MBM_TOTAL	(12*32+ 1) /* LLC Total MBM monitoring */
-#define X86_FEATURE_CQM_MBM_LOCAL	(12*32+ 2) /* LLC Local MBM monitoring */
+/*
+ * Extended auxiliary flags: Linux defined - For features scattered in various
+ * CPUID levels like 0xf, word 11.
+ *
+ * Reuse free bits when adding new feature flags!
+ */
+#define X86_FEATURE_CQM_LLC		(11*32+ 0) /* LLC QoS if 1 */
+#define X86_FEATURE_CQM_OCCUP_LLC	(11*32+ 1) /* LLC occupancy monitoring */
+#define X86_FEATURE_CQM_MBM_TOTAL	(11*32+ 2) /* LLC Total MBM monitoring */
+#define X86_FEATURE_CQM_MBM_LOCAL	(11*32+ 3) /* LLC Local MBM monitoring */
 
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 38e4b1a9005e..5b0e9d869ce5 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -832,20 +832,6 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_D_1_EAX] = eax;
 	}
 
-	/* Additional Intel-defined flags: level 0x0000000F */
-	if (c->cpuid_level >= 0x0000000F) {
-
-		/* QoS sub-leaf, EAX=0Fh, ECX=0 */
-		cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
-		c->x86_capability[CPUID_F_0_EDX] = edx;
-
-		if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
-			/* QoS sub-leaf, EAX=0Fh, ECX=1 */
-			cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
-			c->x86_capability[CPUID_F_1_EDX] = edx;
-		}
-	}
-
 	/* AMD-defined flags: level 0x80000001 */
 	eax = cpuid_eax(0x80000000);
 	c->extended_cpuid_level = eax;
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index 2c0bd38a44ab..fa07a224e7b9 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -59,6 +59,9 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_AVX512_4VNNIW,	X86_FEATURE_AVX512F   },
 	{ X86_FEATURE_AVX512_4FMAPS,	X86_FEATURE_AVX512F   },
 	{ X86_FEATURE_AVX512_VPOPCNTDQ, X86_FEATURE_AVX512F   },
+	{ X86_FEATURE_CQM_OCCUP_LLC,	X86_FEATURE_CQM_LLC   },
+	{ X86_FEATURE_CQM_MBM_TOTAL,	X86_FEATURE_CQM_LLC   },
+	{ X86_FEATURE_CQM_MBM_LOCAL,	X86_FEATURE_CQM_LLC   },
 	{}
 };
 
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 94aa1c72ca98..adf9b71386ef 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -26,6 +26,10 @@ struct cpuid_bit {
 static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_APERFMPERF,       CPUID_ECX,  0, 0x00000006, 0 },
 	{ X86_FEATURE_EPB,		CPUID_ECX,  3, 0x00000006, 0 },
+	{ X86_FEATURE_CQM_LLC,		CPUID_EDX,  1, 0x0000000f, 0 },
+	{ X86_FEATURE_CQM_OCCUP_LLC,	CPUID_EDX,  0, 0x0000000f, 1 },
+	{ X86_FEATURE_CQM_MBM_TOTAL,	CPUID_EDX,  1, 0x0000000f, 1 },
+	{ X86_FEATURE_CQM_MBM_LOCAL,	CPUID_EDX,  2, 0x0000000f, 1 },
 	{ X86_FEATURE_CAT_L3,		CPUID_EBX,  1, 0x00000010, 0 },
 	{ X86_FEATURE_CAT_L2,		CPUID_EBX,  2, 0x00000010, 0 },
 	{ X86_FEATURE_CDP_L3,		CPUID_ECX,  2, 0x00000010, 1 },
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 9a327d5b6d1f..d78a61408243 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -47,8 +47,6 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
 	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
 	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
-	[CPUID_F_0_EDX]       = {       0xf, 0, CPUID_EDX},
-	[CPUID_F_1_EDX]       = {       0xf, 1, CPUID_EDX},
 	[CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
 	[CPUID_6_EAX]         = {         6, 0, CPUID_EAX},
 	[CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
-- 
2.19.1

[RFC PATCH 3/3] x86/cpufeatures: Enumerate new AVX512 BFLOAT16 instructions

[Fenghua Yu]

AVX512 Vector Neural Network Instructions (VNNI) in Intel Deep Learning
Boost support BFLOAT16 format (BF16). BF16 is a short version of FP32 and
has several advantages over FP16. BF16 offers more than enough range for
deep learning training tasks and doesn't need to handle hardware exception
as this is a performance optimization. FP32 accumulation after the
multiply is essential to achieve sufficient numerical behavior on an
application level.

AVX512 BFLOAT16 instructions can be enumerated by:
	CPUID.7.1:EAX[bit 5] AVX512_BF16

Add word 12 to hold features in CPUID.7.1:EAX including AVX512_BF16.

Detailed information of the CPUID bit and AVX512 BFLOAT16 instructions
can be found in the latest Intel Architecture Instruction Set Extensions
and Future Features Programming Reference.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/cpufeature.h  | 1 +
 arch/x86/include/asm/cpufeatures.h | 3 +++
 arch/x86/kernel/cpu/common.c       | 3 +++
 arch/x86/kernel/cpu/cpuid-deps.c   | 1 +
 4 files changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 526619906305..58acda503817 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -23,6 +23,7 @@ enum cpuid_leafs
 	CPUID_7_0_EBX,
 	CPUID_D_1_EAX,
 	CPUID_LNX_4,
+	CPUID_7_1_EAX,
 	CPUID_8000_0008_EBX,
 	CPUID_6_EAX,
 	CPUID_8000_000A_EDX,
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4f0a3d093794..625191ceb214 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -280,6 +280,9 @@
 #define X86_FEATURE_CQM_MBM_TOTAL	(11*32+ 2) /* LLC Total MBM monitoring */
 #define X86_FEATURE_CQM_MBM_LOCAL	(11*32+ 3) /* LLC Local MBM monitoring */
 
+/* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
+#define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
+
 /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
 #define X86_FEATURE_CLZERO		(13*32+ 0) /* CLZERO instruction */
 #define X86_FEATURE_IRPERF		(13*32+ 1) /* Instructions Retired Count */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5b0e9d869ce5..6a4e948c7580 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -823,6 +823,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
 		c->x86_capability[CPUID_7_0_EBX] = ebx;
 		c->x86_capability[CPUID_7_ECX] = ecx;
 		c->x86_capability[CPUID_7_EDX] = edx;
+
+		cpuid_count(0x00000007, 1, &eax, &ebx, &ecx, &edx);
+		c->x86_capability[CPUID_7_1_EAX] = eax;
 	}
 
 	/* Extended state features: level 0x0000000d */
diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
index fa07a224e7b9..a444028d8145 100644
--- a/arch/x86/kernel/cpu/cpuid-deps.c
+++ b/arch/x86/kernel/cpu/cpuid-deps.c
@@ -62,6 +62,7 @@ static const struct cpuid_dep cpuid_deps[] = {
 	{ X86_FEATURE_CQM_OCCUP_LLC,	X86_FEATURE_CQM_LLC   },
 	{ X86_FEATURE_CQM_MBM_TOTAL,	X86_FEATURE_CQM_LLC   },
 	{ X86_FEATURE_CQM_MBM_LOCAL,	X86_FEATURE_CQM_LLC   },
+	{ X86_FEATURE_AVX512_BF16,	X86_FEATURE_AVX512VL  },
 	{}
 };
 
-- 
2.19.1

Re: [RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale directly from CPUID instead of cpuinfo_x86

[Borislav Petkov]

On Thu, Jun 13, 2019 at 01:51:02PM -0700, Fenghua Yu wrote:
> Although x86_cache_max_rmid and x86_cache_occ_scale are read only once
> during resctrl initialization, they are always stored in cpuinfo_x86 on
> each CPU during run time even if resctrl is not configured.
> 
> To save cpuinfo_x86 space and make CPU and resctrl initialization simpler,
> remove the two fields from cpuinfo_x86 and get max rmid and occupancy
> scale directly from CPUID during resctrl initialization. And since each
> known platform that supports resctrl has same max rmid on all CPUs, no
> need to scan all CPUs to find minimum of max rmid values, i.e. getting
> max rmid from CPUID on the current CPU is fine.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/include/asm/processor.h       |  3 ---
>  arch/x86/kernel/cpu/common.c           | 28 --------------------------
>  arch/x86/kernel/cpu/resctrl/internal.h |  2 +-
>  arch/x86/kernel/cpu/resctrl/monitor.c  | 28 +++++++++++++++++++++++---
>  4 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index c34a35c78618..27e875d4ca7d 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -99,9 +99,6 @@ struct cpuinfo_x86 {
>  	/* in KB - valid for CPUS which support this call: */
>  	unsigned int		x86_cache_size;
>  	int			x86_cache_alignment;	/* In bytes */
> -	/* Cache QoS architectural values: */
> -	int			x86_cache_max_rmid;	/* max index */
> -	int			x86_cache_occ_scale;	/* scale to bytes */
>  	int			x86_power;
>  	unsigned long		loops_per_jiffy;
>  	/* cpuid returned max cores value: */
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 2c57fffebf9b..38e4b1a9005e 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -840,22 +840,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
>  		c->x86_capability[CPUID_F_0_EDX] = edx;
>  
>  		if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
> -			/* will be overridden if occupancy monitoring exists */
> -			c->x86_cache_max_rmid = ebx;
> -
>  			/* QoS sub-leaf, EAX=0Fh, ECX=1 */
>  			cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
>  			c->x86_capability[CPUID_F_1_EDX] = edx;
> -
> -			if ((cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC)) ||
> -			      ((cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL)) ||
> -			       (cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)))) {
> -				c->x86_cache_max_rmid = ecx;
> -				c->x86_cache_occ_scale = ebx;
> -			}
> -		} else {
> -			c->x86_cache_max_rmid = -1;
> -			c->x86_cache_occ_scale = -1;
>  		}
>  	}
>  
> @@ -1269,20 +1256,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
>  #endif
>  }
>  
> -static void x86_init_cache_qos(struct cpuinfo_x86 *c)
> -{
> -	/*
> -	 * The heavy lifting of max_rmid and cache_occ_scale are handled
> -	 * in get_cpu_cap().  Here we just set the max_rmid for the boot_cpu
> -	 * in case CQM bits really aren't there in this CPU.
> -	 */
> -	if (c != &boot_cpu_data) {
> -		boot_cpu_data.x86_cache_max_rmid =
> -			min(boot_cpu_data.x86_cache_max_rmid,
> -			    c->x86_cache_max_rmid);
> -	}
> -}
> -
>  /*
>   * Validate that ACPI/mptables have the same information about the
>   * effective APIC id and update the package map.
> @@ -1391,7 +1364,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>  #endif
>  
>  	x86_init_rdrand(c);
> -	x86_init_cache_qos(c);
>  	setup_pku(c);
>  
>  	/*
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index e49b77283924..474a7090d2dd 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -579,7 +579,7 @@ int closids_supported(void);
>  void closid_free(int closid);
>  int alloc_rmid(void);
>  void free_rmid(u32 rmid);
> -int rdt_get_mon_l3_config(struct rdt_resource *r);
> +int __init rdt_get_mon_l3_config(struct rdt_resource *r);
>  void mon_event_count(void *info);
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg);
>  void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index 1573a0a6b525..e9d876c25703 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -617,13 +617,35 @@ static void l3_mon_evt_init(struct rdt_resource *r)
>  		list_add_tail(&mbm_local_event.list, &r->evt_list);
>  }
>  
> -int rdt_get_mon_l3_config(struct rdt_resource *r)
> +static void __init get_cqm_info(struct rdt_resource *r)
> +{
> +	u32 eax, ebx, ecx, edx;
> +
> +	/*
> +	 * At this point, CQM LLC and one of occupancy, MBM total, and
> +	 * MBM local monitoring features must be supported.
> +	 */
> +	cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
> +	/* will be overridden if occupancy monitoring exists */
> +	r->num_rmid = ebx + 1;
> +
> +	cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);

Those CPUID accesses should be done *after* testing features, not
before.

> +
> +	if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))

That is already done in get_rdt_mon_resources() and rdt_mon_features
caches those bits. I think you wanna test QOS_L3_OCCUP_EVENT_ID in there
and then read CPUID 0xf and set ->num_rmid.

> +		r->num_rmid = ecx + 1;
> +
> +	if (boot_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) || boot_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))

Ditto.

Other than that, I like where this cleanup is going...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Thu, Jun 13, 2019 at 01:51:03PM -0700, Fenghua Yu wrote:
> It's a waste for the four X86_FEATURE_CQM_* features to occupy two
> pure feature bits words. To better utilize feature words, re-define
> word 11 to host scattered features and move the four X86_FEATURE_CQM_*
> features into word 11. More scattered features can be added in word 11
> in the future.
> 
> KVM doesn't support resctrl now. So it's safe to move the
> X86_FEATURE_CQM_* features to scattered features word 11 for KVM.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>

...

> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index 9a327d5b6d1f..d78a61408243 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -47,8 +47,6 @@ static const struct cpuid_reg reverse_cpuid[] = {
>  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
>  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
>  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> -	[CPUID_F_0_EDX]       = {       0xf, 0, CPUID_EDX},
> -	[CPUID_F_1_EDX]       = {       0xf, 1, CPUID_EDX},

I think you're going to have to change those to:

        [CPUID_LNX_4]         = {         0, 0, 0},
        [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX },

instead of removing them because kvm is basically hardcoding the feature
words and then bitches when array elements in the middle get removed:

In file included from ./include/linux/export.h:45,
                 from ./include/linux/linkage.h:7,
                 from ./include/linux/preempt.h:10,
                 from ./include/linux/hardirq.h:5,
                 from ./include/linux/kvm_host.h:10,
                 from arch/x86/kvm/x86.c:22:
In function ‘x86_feature_cpuid’,
    inlined from ‘guest_cpuid_get_register’ at arch/x86/kvm/cpuid.h:71:33,
    inlined from ‘guest_cpuid_has’ at arch/x86/kvm/cpuid.h:100:8,
    inlined from ‘kvm_get_msr_common’ at arch/x86/kvm/x86.c:2804:8:
./include/linux/compiler.h:345:38: error: call to ‘__compiletime_assert_62’ declared with attribute error: BUILD_BUG_ON failed: x86_leaf >= ARRAY_SIZE(reverse_cpuid)
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                      ^
./include/linux/compiler.h:326:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^~~~~~
./include/linux/compiler.h:345:2: note: in expansion of macro ‘_compiletime_assert’
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
  ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  ^~~~~~~~~~~~~~~~
arch/x86/kvm/cpuid.h:62:2: note: in expansion of macro ‘BUILD_BUG_ON’
  BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
  ^~~~~~~~~~~~
./include/linux/compiler.h:345:38: error: call to ‘__compiletime_assert_63’ declared with attribute error: BUILD_BUG_ON failed: reverse_cpuid[x86_leaf].function == 0
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
                                      ^
./include/linux/compiler.h:326:4: note: in definition of macro ‘__compiletime_assert’
    prefix ## suffix();    \
    ^~~~~~
./include/linux/compiler.h:345:2: note: in expansion of macro ‘_compiletime_assert’
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
  ^~~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
 #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                     ^~~~~~~~~~~~~~~~~~
./include/linux/build_bug.h:50:2: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
  ^~~~~~~~~~~~~~~~
arch/x86/kvm/cpuid.h:63:2: note: in expansion of macro ‘BUILD_BUG_ON’
  BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
  ^~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:278: arch/x86/kvm/x86.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [scripts/Makefile.build:489: arch/x86/kvm] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1071: arch/x86] Error 2
make: *** Waiting for unfinished jobs....

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 01:44:10PM +0200, Borislav Petkov wrote:
> On Thu, Jun 13, 2019 at 01:51:03PM -0700, Fenghua Yu wrote:
> > It's a waste for the four X86_FEATURE_CQM_* features to occupy two
> > pure feature bits words. To better utilize feature words, re-define
> > word 11 to host scattered features and move the four X86_FEATURE_CQM_*
> > features into word 11. More scattered features can be added in word 11
> > in the future.
> > 
> > KVM doesn't support resctrl now. So it's safe to move the
> > X86_FEATURE_CQM_* features to scattered features word 11 for KVM.
> > 
> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> 
> ...
> 
> > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> > index 9a327d5b6d1f..d78a61408243 100644
> > --- a/arch/x86/kvm/cpuid.h
> > +++ b/arch/x86/kvm/cpuid.h
> > @@ -47,8 +47,6 @@ static const struct cpuid_reg reverse_cpuid[] = {
> >  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
> >  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
> >  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> > -	[CPUID_F_0_EDX]       = {       0xf, 0, CPUID_EDX},
> > -	[CPUID_F_1_EDX]       = {       0xf, 1, CPUID_EDX},
> 
> I think you're going to have to change those to:
> 
>         [CPUID_LNX_4]         = {         0, 0, 0},
>         [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX },
> 
> instead of removing them because kvm is basically hardcoding the feature
> words and then bitches when array elements in the middle get removed:

Alternatively - and what I think is the better solution - would be to
remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
Linux-defined leafs dynamically. This way the array won't have holes in
it.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

On Fri, Jun 14, 2019 at 02:27:50PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 01:44:10PM +0200, Borislav Petkov wrote:
> > On Thu, Jun 13, 2019 at 01:51:03PM -0700, Fenghua Yu wrote:
> > > It's a waste for the four X86_FEATURE_CQM_* features to occupy two
> > > pure feature bits words. To better utilize feature words, re-define
> > > word 11 to host scattered features and move the four X86_FEATURE_CQM_*
> > > features into word 11. More scattered features can be added in word 11
> > > in the future.
> > > 
> > > KVM doesn't support resctrl now. So it's safe to move the
> > > X86_FEATURE_CQM_* features to scattered features word 11 for KVM.
> > > 
> > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> > 
> > ...
> > 
> > > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> > > index 9a327d5b6d1f..d78a61408243 100644
> > > --- a/arch/x86/kvm/cpuid.h
> > > +++ b/arch/x86/kvm/cpuid.h
> > > @@ -47,8 +47,6 @@ static const struct cpuid_reg reverse_cpuid[] = {
> > >  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
> > >  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
> > >  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> > > -	[CPUID_F_0_EDX]       = {       0xf, 0, CPUID_EDX},
> > > -	[CPUID_F_1_EDX]       = {       0xf, 1, CPUID_EDX},
> > 
> > I think you're going to have to change those to:
> > 
> >         [CPUID_LNX_4]         = {         0, 0, 0},
> >         [CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX },
> > 
> > instead of removing them because kvm is basically hardcoding the feature
> > words and then bitches when array elements in the middle get removed:
> 
> Alternatively - and what I think is the better solution - would be to
> remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
> Linux-defined leafs dynamically. This way the array won't have holes in
> it.

Maybe adding a dummy slot in cpuid_leafs in patch 0002 to avoid the
compilation errors?

Those KVM BUILD_BUG_ON() want to find out any empty leaf in
reverse_cpuid (and also cpuid_leafs).

The patch 0002 actually creates such empty leaf 12 in cpuid_leafs. So
CPUID_7_EDX=17 instead of NCAPINTS-1=18 in cpuid_leafs. It's not
desired.

After applying patch 0003, the hole is filled in and there is no
compilation error from the KVM BUILD_BUG_ON() checks. So the compilation
errors only happens in bisect.

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 526619906305..403f70c2e431 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -23,6 +23,7 @@ enum cpuid_leafs
        CPUID_7_0_EBX,
        CPUID_D_1_EAX,
        CPUID_LNX_4,
+       CPUID_DUMMY,
        CPUID_8000_0008_EBX,
        CPUID_6_EAX,
        CPUID_8000_000A_EDX,

Is it OK to add the above patch to the patch 0002 to solve the
compilation errors? If it's ok, I will explain in commit message why add
CPUID_DUMMY.

Thanks.

-Fenghua

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

+ Radim and Paolo. See upthread for context.

On Fri, Jun 14, 2019 at 06:17:02AM -0700, Fenghua Yu wrote:
> > Alternatively - and what I think is the better solution - would be to
> > remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
> > Linux-defined leafs dynamically. This way the array won't have holes in
> > it.
> 
> Maybe adding a dummy slot in cpuid_leafs in patch 0002 to avoid the
> compilation errors?

Maybe you didn't read what you're replying to: "This way the array
won't have holes in it". Ontop of yours:

diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index d78a61408243..03d6f3f7b27c 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -47,6 +47,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
 	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
 	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
 	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
+	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
 	[CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
 	[CPUID_6_EAX]         = {         6, 0, CPUID_EAX},
 	[CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
@@ -59,8 +60,9 @@ static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
 {
 	unsigned x86_leaf = x86_feature / 32;
 
-	BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
-	BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
+	if (x86_leaf == CPUID_LNX_1 ||
+	    x86_leaf == CPUID_LNX_4)
+		return NULL;
 
 	return reverse_cpuid[x86_leaf];
 }

That's what I mean with filter out dynamically.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

On Fri, Jun 14, 2019 at 03:41:23PM +0200, Borislav Petkov wrote:
> + Radim and Paolo. See upthread for context.
> 
> On Fri, Jun 14, 2019 at 06:17:02AM -0700, Fenghua Yu wrote:
> > > Alternatively - and what I think is the better solution - would be to
> > > remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
> > > Linux-defined leafs dynamically. This way the array won't have holes in
> > > it.
> > 
> > Maybe adding a dummy slot in cpuid_leafs in patch 0002 to avoid the
> > compilation errors?
> 
> Maybe you didn't read what you're replying to: "This way the array
> won't have holes in it". Ontop of yours:
> 
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index d78a61408243..03d6f3f7b27c 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -47,6 +47,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
>  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
>  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> +	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},

CPUID_7_1_EAX is defined in patch 0003. Should I combine patch 0002 and 0003
into one patch?

>  	[CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
>  	[CPUID_6_EAX]         = {         6, 0, CPUID_EAX},
>  	[CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
> @@ -59,8 +60,9 @@ static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
>  {
>  	unsigned x86_leaf = x86_feature / 32;
>  
> -	BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
> -	BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
> +	if (x86_leaf == CPUID_LNX_1 ||
> +	    x86_leaf == CPUID_LNX_4)
> +		return NULL;

Need to check CPUID_LNX_2 and CPUID_LNX_3 as well?

>  
>  	return reverse_cpuid[x86_leaf];
>  }
> 
> That's what I mean with filter out dynamically.

Thanks.

-Fenghua

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 06:51:05AM -0700, Fenghua Yu wrote:
> CPUID_7_1_EAX is defined in patch 0003. Should I combine patch 0002 and 0003
> into one patch?

That was just an example diff. Generally, patches should do one logical
thing. In this case, I'd suggest you make the kvm change the first and
separate patch, from the other three.

> Need to check CPUID_LNX_2 and CPUID_LNX_3 as well?

Yes, all linux-defined feature words.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Sean Christopherson]

On Fri, Jun 14, 2019 at 03:41:23PM +0200, Borislav Petkov wrote:
> + Radim and Paolo. See upthread for context.
> 
> On Fri, Jun 14, 2019 at 06:17:02AM -0700, Fenghua Yu wrote:
> > > Alternatively - and what I think is the better solution - would be to
> > > remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
> > > Linux-defined leafs dynamically. This way the array won't have holes in
> > > it.
> > 
> > Maybe adding a dummy slot in cpuid_leafs in patch 0002 to avoid the
> > compilation errors?
> 
> Maybe you didn't read what you're replying to: "This way the array
> won't have holes in it". Ontop of yours:
> 
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index d78a61408243..03d6f3f7b27c 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -47,6 +47,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
>  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
>  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
>  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> +	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
>  	[CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
>  	[CPUID_6_EAX]         = {         6, 0, CPUID_EAX},
>  	[CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
> @@ -59,8 +60,9 @@ static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
>  {
>  	unsigned x86_leaf = x86_feature / 32;
>  
> -	BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
> -	BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
> +	if (x86_leaf == CPUID_LNX_1 ||
> +	    x86_leaf == CPUID_LNX_4)
> +		return NULL;
>  
>  	return reverse_cpuid[x86_leaf];
>  }
> 
> That's what I mean with filter out dynamically.

This is wrong.  KVM isn't complaining about shuffling the order of feature
words, it's complaining that code is trying to do a reverse CPUID lookup
to a feature that isn't in the reverse_cpuid table.   Filtering out
checks dynamically is just hiding bugs.

>In function ‘x86_feature_cpuid’,
>     inlined from ‘guest_cpuid_get_register’ at arch/x86/kvm/cpuid.h:71:33,
>     inlined from ‘guest_cpuid_has’ at arch/x86/kvm/cpuid.h:100:8,
>     inlined from ‘kvm_get_msr_common’ at arch/x86/kvm/x86.c:2804:8:

This corresponds to "guest_cpuid_has(vcpu, X86_FEATURE_ARCH_CAPABILITIES)",
i.e. KVM is trying to query X86_FEATURE_ARCH_CAPABILITIES and is yelling
that there is no reverse_cpuid entry for CPUID_7_EDX.

The problem is that 'enum cpuid_leafs' no longer matches up with the
word numbers defined in cpufeatures.h, e.g. CPUID_7_EDX == 17 or so, but
the entries in cpufeatures.h defined CPUID_7_EDX flags using word 18.

This patch also needs to modify NCAPINTS.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

On Fri, Jun 14, 2019 at 07:14:24AM -0700, Sean Christopherson wrote:
> On Fri, Jun 14, 2019 at 03:41:23PM +0200, Borislav Petkov wrote:
> > + Radim and Paolo. See upthread for context.
> > 
> > On Fri, Jun 14, 2019 at 06:17:02AM -0700, Fenghua Yu wrote:
> > > > Alternatively - and what I think is the better solution - would be to
> > > > remove those BUILD_BUG_ONs in x86_feature_cpuid and filter out the
> > > > Linux-defined leafs dynamically. This way the array won't have holes in
> > > > it.
> > > 
> > > Maybe adding a dummy slot in cpuid_leafs in patch 0002 to avoid the
> > > compilation errors?
> > 
> > Maybe you didn't read what you're replying to: "This way the array
> > won't have holes in it". Ontop of yours:
> > 
> > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> > index d78a61408243..03d6f3f7b27c 100644
> > --- a/arch/x86/kvm/cpuid.h
> > +++ b/arch/x86/kvm/cpuid.h
> > @@ -47,6 +47,7 @@ static const struct cpuid_reg reverse_cpuid[] = {
> >  	[CPUID_8000_0001_ECX] = {0x80000001, 0, CPUID_ECX},
> >  	[CPUID_7_0_EBX]       = {         7, 0, CPUID_EBX},
> >  	[CPUID_D_1_EAX]       = {       0xd, 1, CPUID_EAX},
> > +	[CPUID_7_1_EAX]       = {         7, 1, CPUID_EAX},
> >  	[CPUID_8000_0008_EBX] = {0x80000008, 0, CPUID_EBX},
> >  	[CPUID_6_EAX]         = {         6, 0, CPUID_EAX},
> >  	[CPUID_8000_000A_EDX] = {0x8000000a, 0, CPUID_EDX},
> > @@ -59,8 +60,9 @@ static __always_inline struct cpuid_reg x86_feature_cpuid(unsigned x86_feature)
> >  {
> >  	unsigned x86_leaf = x86_feature / 32;
> >  
> > -	BUILD_BUG_ON(x86_leaf >= ARRAY_SIZE(reverse_cpuid));
> > -	BUILD_BUG_ON(reverse_cpuid[x86_leaf].function == 0);
> > +	if (x86_leaf == CPUID_LNX_1 ||
> > +	    x86_leaf == CPUID_LNX_4)
> > +		return NULL;
> >  
> >  	return reverse_cpuid[x86_leaf];
> >  }
> > 
> > That's what I mean with filter out dynamically.
> 
> This is wrong.  KVM isn't complaining about shuffling the order of feature
> words, it's complaining that code is trying to do a reverse CPUID lookup
> to a feature that isn't in the reverse_cpuid table.   Filtering out
> checks dynamically is just hiding bugs.
> 
> >In function ‘x86_feature_cpuid’,
> >     inlined from ‘guest_cpuid_get_register’ at arch/x86/kvm/cpuid.h:71:33,
> >     inlined from ‘guest_cpuid_has’ at arch/x86/kvm/cpuid.h:100:8,
> >     inlined from ‘kvm_get_msr_common’ at arch/x86/kvm/x86.c:2804:8:
> 
> This corresponds to "guest_cpuid_has(vcpu, X86_FEATURE_ARCH_CAPABILITIES)",
> i.e. KVM is trying to query X86_FEATURE_ARCH_CAPABILITIES and is yelling
> that there is no reverse_cpuid entry for CPUID_7_EDX.
> 
> The problem is that 'enum cpuid_leafs' no longer matches up with the
> word numbers defined in cpufeatures.h, e.g. CPUID_7_EDX == 17 or so, but
> the entries in cpufeatures.h defined CPUID_7_EDX flags using word 18.
> 
> This patch also needs to modify NCAPINTS.

Changing NCAPINTS is heavy lifting to only solve this biset issue. After
applying patch 0003, the word 12 hole generated in patch 0002 is filled
in and no issue reported. If changing NCAPINTS in patch 0002, it needs to
be changed back again after applying 0003.

How about add this patch in patch 0002? I posted this patch in this thread
just now and post it here again:

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 526619906305..403f70c2e431 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -23,6 +23,7 @@ enum cpuid_leafs
 	CPUID_7_0_EBX,
 	CPUID_D_1_EAX,
 	CPUID_LNX_4,
+	CPUID_DUMMY,
 	CPUID_8000_0008_EBX,
 	CPUID_6_EAX,
 	CPUID_8000_000A_EDX,

Adding this small patch into patch 0002 will solve the build errors without
changing the build checks.

Thanks.

-Fenghua

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 07:14:24AM -0700, Sean Christopherson wrote:
> This is wrong.  KVM isn't complaining about shuffling the order of feature
> words, it's complaining that code is trying to do a reverse CPUID lookup
> to a feature that isn't in the reverse_cpuid table.   Filtering out
> checks dynamically is just hiding bugs.

No no, reverse_cpuid is hardcoding our feature leafs. This is wrong as
we want to be able to change those. And reverse_cpuid[] should be able
to handle that.

KVM is complaining because he removed one leaf. He adds it later in
patch 3 as a Linux-defined leaf.

All that doesn't matter for KVM - if KVM wants to do reverse lookup,
then it should handle Linux-defined leafs just fine.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

On Fri, Jun 14, 2019 at 04:26:11PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 07:15:20AM -0700, Fenghua Yu wrote:
> > Adding this small patch into patch 0002 will solve the build errors without
> > changing the build checks.
> 
> There's no need for that if you remove the BUILD_BUG_ON checks first.

But without this small patch, CPUID_7_EDX is 17 instead of
NCAPINTS(19)-1=18 in patch 0002. Of course CPUID_7_EDX is 18 correctly
evetually after applying patch 0003 which add the word 12 back.

KVM reports this CPUID_7_EDX is not 18 in bisect (as Sena also pointed
out). I think even native kernel should have some check to report this
error (similar to other checks of NCAPINTS).

Thanks.

-Fenghua

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 07:15:20AM -0700, Fenghua Yu wrote:
> Adding this small patch into patch 0002 will solve the build errors without
> changing the build checks.

There's no need for that if you remove the BUILD_BUG_ON checks first.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Sean Christopherson]

On Fri, Jun 14, 2019 at 04:21:39PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 07:14:24AM -0700, Sean Christopherson wrote:
> > This is wrong.  KVM isn't complaining about shuffling the order of feature
> > words, it's complaining that code is trying to do a reverse CPUID lookup
> > to a feature that isn't in the reverse_cpuid table.   Filtering out
> > checks dynamically is just hiding bugs.
> 
> No no, reverse_cpuid is hardcoding our feature leafs. This is wrong as
> we want to be able to change those. And reverse_cpuid[] should be able
> to handle that.
> 
> KVM is complaining because he removed one leaf. He adds it later in
> patch 3 as a Linux-defined leaf.

Yes, because removing that leaf breaks 'enum cpuid_leafs'.  Patch 3/3
"fixes" it by re-inserting a leaf, which causes 'enum cpuid_leafs' to
align with the CPU features.

For example, this assertion also fails:

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5b0e9d869ce5..c273b99702d0 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -823,6 +823,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_capability[CPUID_7_0_EBX] = ebx;
                c->x86_capability[CPUID_7_ECX] = ecx;
                c->x86_capability[CPUID_7_EDX] = edx;
+               BUILD_BUG_ON(CPUID_7_EDX != X86_FEATURE_ARCH_CAPABILITIES/32);
        }
 
        /* Extended state features: level 0x0000000d */

In function ‘x86_feature_cpuid’,
    inlined from ‘guest_cpuid_get_register’ at arch/x86/kvm/cpuid.h:71:25,
    inlined from ‘guest_cpuid_has’ at arch/x86/kvm/cpuid.h:100:6,
    inlined from ‘kvm_get_msr_common’ at arch/x86/kvm/x86.c:2824:8:
include/linux/compiler.h:345:38: error: call to ‘__compiletime_assert_62’ declared with attribute error: BUILD_BUG_ON failed: x86_leaf >= ARRAY_SIZE(reverse_cpuid)
  _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)


But this assertion passes because its word is 10, i.e. below the 11/12
words that are getting mucked with.

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5b0e9d869ce5..aada9d2fa4df 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -830,6 +830,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                cpuid_count(0x0000000d, 1, &eax, &ebx, &ecx, &edx);
 
                c->x86_capability[CPUID_D_1_EAX] = eax;
+               BUILD_BUG_ON(CPUID_D_1_EAX != X86_FEATURE_XSAVES/32);
        }
 
        /* AMD-defined flags: level 0x80000001 */


> All that doesn't matter for KVM - if KVM wants to do reverse lookup,
> then it should handle Linux-defined leafs just fine.

KVM can't handle Linux-defined leafs without extra tricks, which is why
I removed get_scattered_cpuid_leaf() or whatever it was called.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 07:39:12AM -0700, Sean Christopherson wrote:
> KVM can't handle Linux-defined leafs without extra tricks

and that's what I'm proposing - an extra trick.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 07:25:51AM -0700, Fenghua Yu wrote:
> But without this small patch, CPUID_7_EDX is 17 instead of
> NCAPINTS(19)-1=18 in patch 0002. Of course CPUID_7_EDX is 18 correctly
> evetually after applying patch 0003 which add the word 12 back.

Ok, then I guess the easiest should be if not remove defines from
cpuid_leafs but rename them, no?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Sean Christopherson]

On Fri, Jun 14, 2019 at 04:57:34PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 07:39:12AM -0700, Sean Christopherson wrote:
> > KVM can't handle Linux-defined leafs without extra tricks
> 
> and that's what I'm proposing - an extra trick.

It's not a trick, it's bug suppression.

Try running a kernel built with only patches 1/2 and 2/2 applied, along
with KVM's assertions removed.  It'll probably boot fine since most of the
affected features are option things, but Linux's feature reporting will be
all kinds of screwed up.

E.g. this WARN triggers because CPUID_7_EDX is 17, not 18 as expected,
and so the word at c->x86_capability[18] is never written.  Run the kernel
again after applying patch 3/3 and the warning magically disappears...

/* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
#define X86_FEATURE_SPEC_CTRL_SSBD	(18*32+31) /* "" Speculative Store Bypass Disable */

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 5b0e9d869ce5..6c30fb244594 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -823,6 +823,8 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
                c->x86_capability[CPUID_7_0_EBX] = ebx;
                c->x86_capability[CPUID_7_ECX] = ecx;
                c->x86_capability[CPUID_7_EDX] = edx;
+
+               WARN_ON((edx & BIT(31)) && !cpu_has(c, X86_FEATURE_SPEC_CTRL_SSBD));
        }
 
        /* Extended state features: level 0x0000000d */


[    0.085583] WARNING: CPU: 0 PID: 0 at /home/sean/go/src/kernel.org/sinux/arch/x86/kernel/cpu/common.c:827 get_cpu_cap+0x1c9/0x1d0
[    0.085583] Modules linked in:
[    0.085585] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W         5.2.0-rc2+ #4
[    0.085585] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
[    0.085586] RIP: 0010:get_cpu_cap+0x1c9/0x1d0
[    0.085587] Code: <0f> 0b e9 7f fe ff ff 48 8b 05 d9 39 fe 00 ba 01 00 00 00 41 54 b9
[    0.085588] RSP: 0000:ffffffff82003e80 EFLAGS: 00010246
[    0.085588] RAX: 0000000000000000 RBX: 00000000009c4fbb RCX: 0000000000000004
[    0.085589] RDX: 0000000084000000 RSI: 0000000000000000 RDI: ffffffff820f71e0
[    0.085589] RBP: ffffffff820f71e0 R08: 0000000000000001 R09: 0000000000000001
[    0.085589] R10: 000000000002c4f8 R11: 0000000000000002 R12: ffffffff82486920
[    0.085590] R13: ffffffff8248d2e0 R14: ffff88827fff73c0 R15: 000000000000010f
[    0.085590] FS:  0000000000000000(0000) GS:ffff888277a00000(0000) knlGS:0000000000000000
[    0.085592] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.085592] CR2: 00000000ffffffff CR3: 0000000005009001 CR4: 00000000000606b0
[    0.085592] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    0.085593] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    0.085593] Call Trace:
[    0.085595]  identify_cpu+0xbb/0x540
[    0.085597]  identify_boot_cpu+0xc/0x6f
[    0.085598]  check_bugs+0x26/0x852
[    0.085599]  start_kernel+0x47a/0x4ab
[    0.085601]  secondary_startup_64+0xa4/0xb0
[    0.085602] ---[ end trace 33fbe952b06dbb68 ]---

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Borislav Petkov]

On Fri, Jun 14, 2019 at 08:24:58AM -0700, Sean Christopherson wrote:
> On Fri, Jun 14, 2019 at 04:57:34PM +0200, Borislav Petkov wrote:
> > On Fri, Jun 14, 2019 at 07:39:12AM -0700, Sean Christopherson wrote:
> > > KVM can't handle Linux-defined leafs without extra tricks
> > 
> > and that's what I'm proposing - an extra trick.
> 
> It's not a trick, it's bug suppression.
> 
> Try running a kernel built with only patches 1/2 and 2/2 applied, along
> with KVM's assertions removed.  It'll probably boot fine since most of the
> affected features are option things, but Linux's feature reporting will be
> all kinds of screwed up.
> 
> E.g. this WARN triggers because CPUID_7_EDX is 17, not 18 as expected,

We can decrement NCAPINTS and word 18 in the header. The BUILD_BUG_ONs
should not fire then too.

But the easier thing is to not remove any defines in the enum
cpuid_leafs thing so that the capabilities array has the proper size for
after patch 2.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Sean Christopherson]

On Fri, Jun 14, 2019 at 06:10:12PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 08:24:58AM -0700, Sean Christopherson wrote:
> > On Fri, Jun 14, 2019 at 04:57:34PM +0200, Borislav Petkov wrote:
> > > On Fri, Jun 14, 2019 at 07:39:12AM -0700, Sean Christopherson wrote:
> > > > KVM can't handle Linux-defined leafs without extra tricks
> > > 
> > > and that's what I'm proposing - an extra trick.
> > 
> > It's not a trick, it's bug suppression.
> > 
> > Try running a kernel built with only patches 1/2 and 2/2 applied, along
> > with KVM's assertions removed.  It'll probably boot fine since most of the
> > affected features are option things, but Linux's feature reporting will be
> > all kinds of screwed up.
> > 
> > E.g. this WARN triggers because CPUID_7_EDX is 17, not 18 as expected,
> 
> We can decrement NCAPINTS and word 18 in the header. The BUILD_BUG_ONs
> should not fire then too.
> 
> But the easier thing is to not remove any defines in the enum
> cpuid_leafs thing so that the capabilities array has the proper size for
> after patch 2.

Agreed, Fenghua's proposed CPUID_DUMMY is way easier.

Re: [RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale directly from CPUID instead of cpuinfo_x86

[Fenghua Yu]

On Fri, Jun 14, 2019 at 01:16:33PM +0200, Borislav Petkov wrote:
> On Thu, Jun 13, 2019 at 01:51:02PM -0700, Fenghua Yu wrote:
> > Although x86_cache_max_rmid and x86_cache_occ_scale are read only once
> > during resctrl initialization, they are always stored in cpuinfo_x86 on
> > each CPU during run time even if resctrl is not configured.
> > 
> > To save cpuinfo_x86 space and make CPU and resctrl initialization simpler,
> > remove the two fields from cpuinfo_x86 and get max rmid and occupancy
> > scale directly from CPUID during resctrl initialization. And since each
> > known platform that supports resctrl has same max rmid on all CPUs, no
> > need to scan all CPUs to find minimum of max rmid values, i.e. getting
> > max rmid from CPUID on the current CPU is fine.
> > 
> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> > ---
> >  arch/x86/include/asm/processor.h       |  3 ---
> >  arch/x86/kernel/cpu/common.c           | 28 --------------------------
> >  arch/x86/kernel/cpu/resctrl/internal.h |  2 +-
> >  arch/x86/kernel/cpu/resctrl/monitor.c  | 28 +++++++++++++++++++++++---
> >  4 files changed, 26 insertions(+), 35 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> > index c34a35c78618..27e875d4ca7d 100644
> > --- a/arch/x86/include/asm/processor.h
> > +++ b/arch/x86/include/asm/processor.h
> > @@ -99,9 +99,6 @@ struct cpuinfo_x86 {
> >  	/* in KB - valid for CPUS which support this call: */
> >  	unsigned int		x86_cache_size;
> >  	int			x86_cache_alignment;	/* In bytes */
> > -	/* Cache QoS architectural values: */
> > -	int			x86_cache_max_rmid;	/* max index */
> > -	int			x86_cache_occ_scale;	/* scale to bytes */
> >  	int			x86_power;
> >  	unsigned long		loops_per_jiffy;
> >  	/* cpuid returned max cores value: */
> > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > index 2c57fffebf9b..38e4b1a9005e 100644
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -840,22 +840,9 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
> >  		c->x86_capability[CPUID_F_0_EDX] = edx;
> >  
> >  		if (cpu_has(c, X86_FEATURE_CQM_LLC)) {
> > -			/* will be overridden if occupancy monitoring exists */
> > -			c->x86_cache_max_rmid = ebx;
> > -
> >  			/* QoS sub-leaf, EAX=0Fh, ECX=1 */
> >  			cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
> >  			c->x86_capability[CPUID_F_1_EDX] = edx;
> > -
> > -			if ((cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC)) ||
> > -			      ((cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL)) ||
> > -			       (cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)))) {
> > -				c->x86_cache_max_rmid = ecx;
> > -				c->x86_cache_occ_scale = ebx;
> > -			}
> > -		} else {
> > -			c->x86_cache_max_rmid = -1;
> > -			c->x86_cache_occ_scale = -1;
> >  		}
> >  	}
> >  
> > @@ -1269,20 +1256,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
> >  #endif
> >  }
> >  
> > -static void x86_init_cache_qos(struct cpuinfo_x86 *c)
> > -{
> > -	/*
> > -	 * The heavy lifting of max_rmid and cache_occ_scale are handled
> > -	 * in get_cpu_cap().  Here we just set the max_rmid for the boot_cpu
> > -	 * in case CQM bits really aren't there in this CPU.
> > -	 */
> > -	if (c != &boot_cpu_data) {
> > -		boot_cpu_data.x86_cache_max_rmid =
> > -			min(boot_cpu_data.x86_cache_max_rmid,
> > -			    c->x86_cache_max_rmid);
> > -	}
> > -}
> > -
> >  /*
> >   * Validate that ACPI/mptables have the same information about the
> >   * effective APIC id and update the package map.
> > @@ -1391,7 +1364,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> >  #endif
> >  
> >  	x86_init_rdrand(c);
> > -	x86_init_cache_qos(c);
> >  	setup_pku(c);
> >  
> >  	/*
> > diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> > index e49b77283924..474a7090d2dd 100644
> > --- a/arch/x86/kernel/cpu/resctrl/internal.h
> > +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> > @@ -579,7 +579,7 @@ int closids_supported(void);
> >  void closid_free(int closid);
> >  int alloc_rmid(void);
> >  void free_rmid(u32 rmid);
> > -int rdt_get_mon_l3_config(struct rdt_resource *r);
> > +int __init rdt_get_mon_l3_config(struct rdt_resource *r);
> >  void mon_event_count(void *info);
> >  int rdtgroup_mondata_show(struct seq_file *m, void *arg);
> >  void rmdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
> > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> > index 1573a0a6b525..e9d876c25703 100644
> > --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> > @@ -617,13 +617,35 @@ static void l3_mon_evt_init(struct rdt_resource *r)
> >  		list_add_tail(&mbm_local_event.list, &r->evt_list);
> >  }
> >  
> > -int rdt_get_mon_l3_config(struct rdt_resource *r)
> > +static void __init get_cqm_info(struct rdt_resource *r)
> > +{
> > +	u32 eax, ebx, ecx, edx;
> > +
> > +	/*
> > +	 * At this point, CQM LLC and one of occupancy, MBM total, and
> > +	 * MBM local monitoring features must be supported.
> > +	 */
> > +	cpuid_count(0x0000000F, 0, &eax, &ebx, &ecx, &edx);
> > +	/* will be overridden if occupancy monitoring exists */
> > +	r->num_rmid = ebx + 1;
> > +
> > +	cpuid_count(0x0000000F, 1, &eax, &ebx, &ecx, &edx);
> 
> Those CPUID accesses should be done *after* testing features, not
> before.
> 
> > +
> > +	if (boot_cpu_has(X86_FEATURE_CQM_OCCUP_LLC))
> 
> That is already done in get_rdt_mon_resources() and rdt_mon_features
> caches those bits. I think you wanna test QOS_L3_OCCUP_EVENT_ID in there

Sure. I can test QOS_L3_OCCUP_EVENT_ID instead of cpuinfo_x86.

> and then read CPUID 0xf and set ->num_rmid.

When this function is called, X86_FEATURE_CQM_LLC must be supported and
one of X86_FEATURE_CQM_OCCUP_LLC, X86_FEATURE_CQM_MBM_LOCAL, and
X86_FEATURE_CQM_MBM_TOTAL must be supported. Otherwise,
get_rdt_mon_resource() is returned before calling rdt_get_mon_l3_config().

So CPUID.f.0 and CPUID.f.1 must be readable and return meaningful
data without testing the features.

But the problem is it's unknown which ones of X86_FEATURE_CQM_OCCUP_LLC,
X86_FEATURE_CQM_MBM_LOCAL, and X86_FEATURE_CQM_MBM_TOTAL are supported.

So after reading CPUID.f.1, test X86_FEATURE_CQM_OCCUP_LLC in order to
override r->num_rmid from ecx. And test X86_FEATURE_CQM_MBM_TOTAL or
X86_FEATURE_CQM_MBM_LOCAL in order to get r>mon_scale from ebx.

Is this sequence OK? If the current sequence is OK, I only need to
change code to test the QOS_L3_*_EVENT_ID, right?

> 
> > +		r->num_rmid = ecx + 1;
> > +
> > +	if (boot_cpu_has(X86_FEATURE_CQM_MBM_TOTAL) || boot_cpu_has(X86_FEATURE_CQM_MBM_LOCAL))
> 
> Ditto.

Thanks.

-Fenghua

Re: [RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale directly from CPUID instead of cpuinfo_x86

[Borislav Petkov]

On Fri, Jun 14, 2019 at 09:55:28AM -0700, Fenghua Yu wrote:
> When this function is called, X86_FEATURE_CQM_LLC must be supported and
> one of X86_FEATURE_CQM_OCCUP_LLC, X86_FEATURE_CQM_MBM_LOCAL, and
> X86_FEATURE_CQM_MBM_TOTAL must be supported. Otherwise,
> get_rdt_mon_resource() is returned before calling rdt_get_mon_l3_config().
> 
> So CPUID.f.0 and CPUID.f.1 must be readable and return meaningful
> data without testing the features.

How's that?

static void __init get_cqm_info(struct rdt_resource *r)
{
        u32 eax, ebx, ecx, edx;

        /*
         * At this point, CQM LLC and one of occupancy, MBM total, and
         * MBM local monitoring features must be supported.
         */
        cpuid_count(0xf, 1, &eax, &ebx, &ecx, &edx);

        if (rdt_mon_features & QOS_L3_OCCUP_EVENT_ID)
                r->num_rmid = ecx + 1;
        else
                /*
                 * Fallback maximum range (zero-based) of RMID within
                 * this physical processor of all types, in subleaf 0,
                 * EBX.
                 */
                r->num_rmid = cpuid_ebx(0xf) + 1;

        if (rdt_mon_features & (QOS_L3_MBM_TOTAL_EVENT_ID |
                                QOS_L3_MBM_LOCAL_EVENT_ID))
                r->mon_scale = ebx;
	else
                r->mon_scale = -1;
}

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [RFC PATCH 1/3] x86/resctrl: Get max rmid and occupancy scale directly from CPUID instead of cpuinfo_x86

[Fenghua Yu]

On Fri, Jun 14, 2019 at 07:47:01PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 09:55:28AM -0700, Fenghua Yu wrote:
> > When this function is called, X86_FEATURE_CQM_LLC must be supported and
> > one of X86_FEATURE_CQM_OCCUP_LLC, X86_FEATURE_CQM_MBM_LOCAL, and
> > X86_FEATURE_CQM_MBM_TOTAL must be supported. Otherwise,
> > get_rdt_mon_resource() is returned before calling rdt_get_mon_l3_config().
> > 
> > So CPUID.f.0 and CPUID.f.1 must be readable and return meaningful
> > data without testing the features.
> 
> How's that?
> 
> static void __init get_cqm_info(struct rdt_resource *r)
> {
>         u32 eax, ebx, ecx, edx;
> 
>         /*
>          * At this point, CQM LLC and one of occupancy, MBM total, and
>          * MBM local monitoring features must be supported.
>          */
>         cpuid_count(0xf, 1, &eax, &ebx, &ecx, &edx);
> 
>         if (rdt_mon_features & QOS_L3_OCCUP_EVENT_ID)
>                 r->num_rmid = ecx + 1;
>         else
>                 /*
>                  * Fallback maximum range (zero-based) of RMID within
>                  * this physical processor of all types, in subleaf 0,
>                  * EBX.
>                  */
>                 r->num_rmid = cpuid_ebx(0xf) + 1;
> 
>         if (rdt_mon_features & (QOS_L3_MBM_TOTAL_EVENT_ID |
>                                 QOS_L3_MBM_LOCAL_EVENT_ID))
>                 r->mon_scale = ebx;
> 	else
>                 r->mon_scale = -1;
> }

This is much better and cleaner! I will copy your code to patch 0001
in the next version.

Thanks.

-Fenghua

Re: [RFC PATCH 2/3] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

[Fenghua Yu]

On Fri, Jun 14, 2019 at 05:02:19PM +0200, Borislav Petkov wrote:
> On Fri, Jun 14, 2019 at 07:25:51AM -0700, Fenghua Yu wrote:
> > But without this small patch, CPUID_7_EDX is 17 instead of
> > NCAPINTS(19)-1=18 in patch 0002. Of course CPUID_7_EDX is 18 correctly
> > evetually after applying patch 0003 which add the word 12 back.
> 
> Ok, then I guess the easiest should be if not remove defines from
> cpuid_leafs but rename them, no?

So I will re-name the defines in cpuid_leafs to CPUID_LNX_4 and CPUID_DUMMY
to match status of word 11 and word 12.

Hopefully that's ok?

Thanks.

-Fenghua