[PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

[PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

[Grzegorz Andrejczuk]

These patches enable Intel Xeon Phi x200 feature to use MONITOR/MWAIT
instruction in ring 3 (userspace) Patches set MSR 0x140 for all logical CPUs.
Then expose it as CPU feature and introduces elf HWCAP capability for x86.
Reference:
https://software.intel.com/en-us/blogs/2016/10/06/intel-xeon-phi-product-family-x200-knl-user-mode-ring-3-monitor-and-mwait

v9:
Removed PHI from defines

v8:
Fixed commit messages
Removed logging
Used msr_set/clear_bit functions instesd of wrmsrl
Fixed documentation
Renamed HWCAP2_PHIR3MWAIT to HWCAP2_RING3MWAIT

v7:
Change order of the patches, with this code looks cleaner.
Changed the name of MSR to MSR_MISC_FEATURE_ENABLES.
Used Word 3 25th bit to expose feature.

v6: 

v5:
When phir3mwait=disable is cmdline switch off r3 mwait feature
Fix typos

v4:
Wrapped the enabling code by CONFIG_X86_64
Add documentation for phir3mwait=disable cmdline switch
Move probe_ function call from early_intel_init to intel_init
Fixed commit messages

v3:
Included Daves and Thomas comments

v2:
Check MSR before wrmsrl
Shortened names
Used Word 3 for feature init_scattered_cpuid_features()
Fixed commit messages

Grzegorz Andrejczuk (4):
  x86/msr: Add MSR_MISC_FEATURE_ENABLES and RING3MWAIT bit
  x86/elf: Use HWCAP2 to expose ring 3 MWAIT
  x86/cpufeature: Add RING3MWAIT to CPU features
  x86/cpufeatures: Handle RING3MWAIT on Xeon Phi models

 Documentation/kernel-parameters.txt       |  5 ++++
 Documentation/x86/x86_64/boot-options.txt |  5 ++++
 arch/x86/include/asm/cpufeatures.h        |  2 +-
 arch/x86/include/asm/elf.h                |  9 +++++++
 arch/x86/include/asm/msr-index.h          |  5 ++++
 arch/x86/include/uapi/asm/hwcap2.h        |  7 ++++++
 arch/x86/kernel/cpu/common.c              |  3 +++
 arch/x86/kernel/cpu/intel.c               | 39 +++++++++++++++++++++++++++++++
 8 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 arch/x86/include/uapi/asm/hwcap2.h

-- 
2.5.1

[PATCH v9 1/4] x86/msr: add MSR_MISC_FEATURE_ENABLES and RING3MWAIT bit

[Grzegorz Andrejczuk]

Intel Xeon Phi x200 (codenamed Knights Landing) allows to enable
MONITOR and MWAIT instructions outside of ring 0.

The feature is controlled by MSR MISC_FEATURE_ENABLES (0x140).
Setting bit 1 of this register enables it, so MONITOR and MWAIT
instructions do not cause invalid-opcode exceptions when invoked
outside of ring 0.
The feature MSR is not yet documented in the SDM. Here is
the relevant documentation:

Hex   Dec  Name                    Scope
140H  320  MISC_FEATURE_ENABLES    Thread
           0    Reserved
           1    if set to 1, the MONITOR and MWAIT instructions do not
                cause invalid-opcode exceptions when executed with CPL > 0
                or in virtual-8086 mode. If MWAIT is executed when CPL > 0
                or in virtual-8086 mode, and if EAX indicates a C-state
                other than C0 or C1, the instruction operates as if EAX
                indicated the C-state C1.
           63:2 Reserved

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 arch/x86/include/asm/msr-index.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 56f4c66..c95da90 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -540,6 +540,11 @@
 #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT	39
 #define MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE		(1ULL << MSR_IA32_MISC_ENABLE_IP_PREF_DISABLE_BIT)
 
+/* Intel Xeon Phi x200 ring 3 MONITOR/MWAIT */
+#define MSR_MISC_FEATURE_ENABLES	0x00000140
+#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT	1
+#define MSR_MISC_FEATURE_ENABLES_RING3MWAIT	(1ULL << MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT)
+
 #define MSR_IA32_TSC_DEADLINE		0x000006E0
 
 /* P4/Xeon+ specific */
-- 
2.5.1

[PATCH v9 2/4] x86/elf: use HWCAP2 to expose ring 3 MWAIT

[Grzegorz Andrejczuk]

Add HWCAP2 for x86 and reserve its bit 0 to expose
ring 3 mwait.

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 arch/x86/include/asm/elf.h         | 9 +++++++++
 arch/x86/include/uapi/asm/hwcap2.h | 7 +++++++
 arch/x86/kernel/cpu/common.c       | 3 +++
 3 files changed, 19 insertions(+)
 create mode 100644 arch/x86/include/uapi/asm/hwcap2.h

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index e7f155c..59703aa 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -258,6 +258,15 @@ extern int force_personality32;
 
 #define ELF_HWCAP		(boot_cpu_data.x86_capability[CPUID_1_EDX])
 
+extern unsigned int elf_hwcap2;
+
+/*
+ * HWCAP2 supplies mask with kernel enabled CPU features, so that
+ * the application can discover that it can safely use them.
+ * The bits are defined in uapi/asm/hwcap2.h.
+ */
+#define ELF_HWCAP2		elf_hwcap2
+
 /* This yields a string that ld.so will use to load implementation
    specific libraries for optimization.  This is more specific in
    intent than poking at uname or /proc/cpuinfo.
diff --git a/arch/x86/include/uapi/asm/hwcap2.h b/arch/x86/include/uapi/asm/hwcap2.h
new file mode 100644
index 0000000..116cab3
--- /dev/null
+++ b/arch/x86/include/uapi/asm/hwcap2.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_X86_HWCAP2_H
+#define _ASM_X86_HWCAP2_H
+
+/* Kernel enabled Ring 3 MONITOR/MWAIT*/
+#define HWCAP2_RING3MWAIT		(1 << 0)
+
+#endif
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index bcc9ccc..fdbf708 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -35,6 +35,7 @@
 #include <asm/desc.h>
 #include <asm/fpu/internal.h>
 #include <asm/mtrr.h>
+#include <asm/hwcap2.h>
 #include <linux/numa.h>
 #include <asm/asm.h>
 #include <asm/bugs.h>
@@ -51,6 +52,8 @@
 
 #include "cpu.h"
 
+unsigned int elf_hwcap2 __read_mostly;
+
 /* all of these masks are initialized in setup_cpu_local_masks() */
 cpumask_var_t cpu_initialized_mask;
 cpumask_var_t cpu_callout_mask;
-- 
2.5.1

[PATCH v9 3/4] x86/cpufeature: add RING3MWAIT to CPU features

[Grzegorz Andrejczuk]

Add Intel Xeon Phi x200 (KnightsLanding) CPU feature - ring 3 MONITOR/MWAIT.

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 92a8308..5b7f701 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -100,7 +100,7 @@
 #define X86_FEATURE_XTOPOLOGY	( 3*32+22) /* cpu topology enum extensions */
 #define X86_FEATURE_TSC_RELIABLE ( 3*32+23) /* TSC is known to be reliable */
 #define X86_FEATURE_NONSTOP_TSC	( 3*32+24) /* TSC does not stop in C states */
-/* free, was #define X86_FEATURE_CLFLUSH_MONITOR ( 3*32+25) * "" clflush reqd with monitor */
+#define X86_FEATURE_RING3MWAIT	( 3*32+25) /* ring 3 MONITOR/MWAIT */
 #define X86_FEATURE_EXTD_APICID	( 3*32+26) /* has extended APICID (8 bits) */
 #define X86_FEATURE_AMD_DCM     ( 3*32+27) /* multi-node processor */
 #define X86_FEATURE_APERFMPERF	( 3*32+28) /* APERFMPERF */
-- 
2.5.1

[PATCH v9 4/4] x86/cpufeatures: handle RING3MWAIT on Xeon Phi models

[Grzegorz Andrejczuk]

Unfortunately presence of this feature cannot be detected
automatically (by reading some other MSR) therefore it is required
to do explicit check for the family and model of the cpu.

If processor is Intel Xeon Phi x200 RING 3 MONITOR/MWAIT feature is enabled
by setting cpu cap X86_FEATURE_RING3MWAIT and elf HWCAP2_RING3MWAIT.

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 Documentation/kernel-parameters.txt       |  6 +++++
 Documentation/x86/x86_64/boot-options.txt |  5 ++++
 arch/x86/kernel/cpu/intel.c               | 39 +++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a4f4d69..4683cc1 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3120,6 +3120,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	pg.		[PARIDE]
 			See Documentation/blockdev/paride.txt.
 
+	phir3mwait=	[X86-64] Disable ring 3 MONITOR/MWAIT on Intel Xeon Phi
+			codenamed Knight Landing cpus.
+			It has no effect on cpus other than Intel Xeon Phi.
+			Format: { disable }
+			See Documentation/x86/x86_64/boot-options.txt
+
 	pirq=		[SMP,APIC] Manual mp-table setup
 			See Documentation/x86/i386/IO-APIC.txt.
 
diff --git a/Documentation/x86/x86_64/boot-options.txt b/Documentation/x86/x86_64/boot-options.txt
index 0965a71..1a515e8 100644
--- a/Documentation/x86/x86_64/boot-options.txt
+++ b/Documentation/x86/x86_64/boot-options.txt
@@ -281,6 +281,11 @@ Debugging
 
   kstack=N	Print N words from the kernel stack in oops dumps.
 
+  phir3mwait=disable
+  Disables unconditional setting bit 1 of the MSR_MISC_FEATURE_ENABLES
+  for Intel Xeon Phi, this way administrator can switch off ring 3 mwait
+  feature.
+
 Miscellaneous
 
 	nogbpages
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fcd484d..3bfc8e5 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -14,6 +14,8 @@
 #include <asm/bugs.h>
 #include <asm/cpu.h>
 #include <asm/intel-family.h>
+#include <asm/hwcap2.h>
+#include <asm/elf.h>
 
 #ifdef CONFIG_X86_64
 #include <linux/topology.h>
@@ -61,6 +63,41 @@ void check_mpx_erratum(struct cpuinfo_x86 *c)
 	}
 }
 
+#ifdef CONFIG_X86_64
+static int phi_r3mwait_disabled __read_mostly;
+
+static int __init phir3mwait_disable(char *__unused)
+{
+	phi_r3mwait_disabled = 1;
+	return 1;
+}
+__setup("phir3mwait=disable", phir3mwait_disable);
+
+static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
+{
+	/*
+	 * Ring 3 MONITOR/MWAIT feature cannot be detected without
+	 * cpu model and family comparison.
+	 */
+	if (c->x86 != 6 || c->x86_model != INTEL_FAM6_XEON_PHI_KNL)
+		return;
+
+	if (phi_r3mwait_disabled) {
+		msr_clear_bit(MSR_MISC_FEATURE_ENABLES,
+			      MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
+		return;
+	}
+
+	msr_set_bit(MSR_MISC_FEATURE_ENABLES,
+		    MSR_MISC_FEATURE_ENABLES_RING3MWAIT_BIT);
+	set_cpu_cap(c, X86_FEATURE_RING3MWAIT);
+	ELF_HWCAP2 |= HWCAP2_RING3MWAIT;
+}
+
+#else
+static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *__unused) {}
+#endif
+
 static void early_init_intel(struct cpuinfo_x86 *c)
 {
 	u64 misc_enable;
@@ -565,6 +602,8 @@ static void init_intel(struct cpuinfo_x86 *c)
 		detect_vmx_virtcap(c);
 
 	init_intel_energy_perf(c);
+
+	probe_xeon_phi_r3mwait(c);
 }
 
 #ifdef CONFIG_X86_32
-- 
2.5.1

Re: [PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

[Thomas Gleixner]

On Wed, 9 Nov 2016, Grzegorz Andrejczuk wrote:

> These patches enable Intel Xeon Phi x200 feature to use MONITOR/MWAIT
> instruction in ring 3 (userspace) Patches set MSR 0x140 for all logical CPUs.
> Then expose it as CPU feature and introduces elf HWCAP capability for x86.
> Reference:
> https://software.intel.com/en-us/blogs/2016/10/06/intel-xeon-phi-product-family-x200-knl-user-mode-ring-3-monitor-and-mwait
> 
> v9:
> Removed PHI from defines

Do I really have to spell out everything? I asked you several times to get
rid of all PHI associations except for the feature detection logic.

But no, you still insist on it being a PHI special feature and once it
becomes available on other models, which can be expected, we can deal with
the cleanup and a PHI specific kernel parameter which we have to support
forever.

I'm slowly starting to get really grumpy. Your attitude of just addressing
review comments in the most minimal way w/o thinking about the big picture
is annoying.

This is hillarious. 9 versions of that simple thing, just because you
insist on slapping PHI to everything despite being told otherwise.

I do not care about the time you waste with this, but I very much care
about the time you steal from me. 

If you can't be bothered to send something which addresses _ALL_ my review
comments, then so be it. The next version is the last one I'm going to look
at.

Thanks,

	tglx

RE: [PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

[Andrejczuk, Grzegorz]

-----Original Message-----
From: Thomas Gleixner [mailto:tglx@linutronix.de] 
Sent: Wednesday, November 9, 2016 3:32 PM
To: Andrejczuk, Grzegorz <grzegorz.andrejczuk@intel.com>
Cc: mingo@redhat.com; hpa@zytor.com; x86@kernel.org; bp@suse.de; dave.hansen@linux.intel.com; Daniluk, Lukasz <lukasz.daniluk@intel.com>; Cownie, James H <james.h.cownie@intel.com>; Pan, Jacob jun <jacob.jun.pan@intel.com>; Luc, Piotr <Piotr.Luc@intel.com>; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

On Wed, 9 Nov 2016, Grzegorz Andrejczuk wrote:

> These patches enable Intel Xeon Phi x200 feature to use MONITOR/MWAIT 
> instruction in ring 3 (userspace) Patches set MSR 0x140 for all logical CPUs.
> Then expose it as CPU feature and introduces elf HWCAP capability for x86.
> Reference:
> https://software.intel.com/en-us/blogs/2016/10/06/intel-xeon-phi-produ
> ct-family-x200-knl-user-mode-ring-3-monitor-and-mwait
> 
> v9:
> Removed PHI from defines
>
> Do I really have to spell out everything? I asked you several times to get rid of all PHI associations except for the feature detection logic.
>
> But no, you still insist on it being a PHI special feature and once it becomes available on other models, which can be expected, we can deal with the cleanup and a PHI specific kernel parameter which we have to support forever.
>
> I'm slowly starting to get really grumpy. Your attitude of just addressing review comments in the most minimal way w/o thinking about the big picture is annoying.
>
> This is hillarious. 9 versions of that simple thing, just because you insist on slapping PHI to everything despite being told otherwise.
>
> I do not care about the time you waste with this, but I very much care about the time you steal from me. 
>
> If you can't be bothered to send something which addresses _ALL_ my review comments, then so be it. The next version is the last one I'm going to look at.
>
> Thanks,
>
>	Tglx


Hi, 

Sorry we end up in this situation. 

I have removed PHI from:
- MSR definition,
- HWCAP2 bit,
- X86_CPU_FEATURE

Making kernel parameter non-phi would require implementing the ring3mwait=disable for any other non-ring 0 MWAIT (i.e AMD MWAITX). 
My concern is that kernel will have to maintain various non architectural model specific stuff in single kernel parameter. 

Best Regards,
Grzegorz

RE: [PATCH v9 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing

[Thomas Gleixner]

On Wed, 9 Nov 2016, Andrejczuk, Grzegorz wrote:

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de] 

Can you pretty please use a mail client which does not copy the whole mail
header into the mail body? That's just annoying.
 
> Sorry we end up in this situation. 
> 
> I have removed PHI from:
> - MSR definition,
> - HWCAP2 bit,
> - X86_CPU_FEATURE
> 
> Making kernel parameter non-phi would require implementing the
> ring3mwait=disable for any other non-ring 0 MWAIT (i.e AMD MWAITX).

No, it would not. Simply because AMD does not yet provide any means to
enable ring3 MWAIT in the kernel and adding this parameter does not require
to implement it at all. There are gazillions of generic kernel parameters
which are only effective on particular systems. They describe a general
feature not a particular implementation.

If that support for AMD gets ever implemented then it definitely wants to
reuse ring3wait and not phiring3mwait and neiter it wants to add
amdF16ring3mwait..

That feature is going to show up on other Intel cpus sooner than later.
Are we supposed to have xeonring3mwait and atomring3mwait command line
options as well?

Can you see how that gets silly very fast?

> My concern is that kernel will have to maintain various non architectural
> model specific stuff in single kernel parameter.

The feature of enabling ring3 mwait is the same whether it's on PHI, xeon,
atom or some AMD model. Why on earth should they use a different parameter?

If you think I'm wrong, then discuss that instead of silently ignoring my
review requests and resending the same stuff over and over. We could have
been done with that thing weeks ago.

Thanks,

	tglx