linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing
@ 2016-10-27 14:01 Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h Grzegorz Andrejczuk
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Grzegorz Andrejczuk @ 2016-10-27 14:01 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bp, dave.hansen, lukasz.daniluk, james.h.cownie, jacob.jun.pan,
	Piotr.Luc, linux-kernel, 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

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 R3MWAIT register and bit to msr-info.h
  x86: Add enabling of the R3MWAIT during boot
  x86: Use HWCAP2 to expose Xeon Phi ring 3 MWAIT
  x86/cpufeature: Add R3MWAIT to CPU features

 Documentation/kernel-parameters.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         | 42 +++++++++++++++++++++++++++++++++++++
 7 files changed, 73 insertions(+)
 create mode 100644 arch/x86/include/uapi/asm/hwcap2.h

-- 
2.5.1

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

* [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h
  2016-10-27 14:01 [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing Grzegorz Andrejczuk
@ 2016-10-27 14:02 ` Grzegorz Andrejczuk
  2016-10-27 14:32   ` Thomas Gleixner
  2016-10-27 14:02 ` [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot Grzegorz Andrejczuk
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Grzegorz Andrejczuk @ 2016-10-27 14:02 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bp, dave.hansen, lukasz.daniluk, james.h.cownie, jacob.jun.pan,
	Piotr.Luc, linux-kernel, Grzegorz Andrejczuk

Intel Xeon Phi x200 (codenamed Knights Landing) has MSR
MISC_THD_FEATURE_ENABLE 0x140.

Setting 2nd bit of this register makes MONITOR and MWAIT instructions
do not cause invalid-opcode exception when called from ring different
than 0.

Hex   Dec  Name                    Scope
140H  320  MISC_THD_FEATURE_ENABLE 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..df9d8d3 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_PHI_MISC_THD_FEATURE	0x00000140
+#define MSR_PHI_MISC_THD_FEATURE_R3MWAIT_BIT	1
+#define MSR_PHI_MISC_THD_FEATURE_R3MWAIT	(1ULL << MSR_PHI_MISC_THD_FEATURE_R3MWAIT_BIT)
+
 #define MSR_IA32_TSC_DEADLINE		0x000006E0
 
 /* P4/Xeon+ specific */
-- 
2.5.1

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

* [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-10-27 14:01 [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h Grzegorz Andrejczuk
@ 2016-10-27 14:02 ` Grzegorz Andrejczuk
  2016-10-27 14:38   ` Thomas Gleixner
  2016-10-27 14:02 ` [PATCH v6: 3/4] x86: Use HWCAP2 to expose Xeon Phi ring 3 MWAIT Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features Grzegorz Andrejczuk
  3 siblings, 1 reply; 17+ messages in thread
From: Grzegorz Andrejczuk @ 2016-10-27 14:02 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bp, dave.hansen, lukasz.daniluk, james.h.cownie, jacob.jun.pan,
	Piotr.Luc, linux-kernel, Grzegorz Andrejczuk

If processor is Intel Xeon Phi x200 we enable user-level mwait feature.
Enabling this feature suppresses invalid-opcode error, when MONITOR/MWAIT
is called from ring 3.

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 Documentation/kernel-parameters.txt |  5 +++++
 arch/x86/kernel/cpu/intel.c         | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a4f4d69..d58915b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3120,6 +3120,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	pg.		[PARIDE]
 			See Documentation/blockdev/paride.txt.
 
+	phir3mwait=	[X86] Disable Intel Xeon Phi x200 ring 3 MONITOR/MWAIT
+			feature for all cpus.
+			Format: { disable }
+			See arch/x86/kernel/cpu/intel.c
+
 	pirq=		[SMP,APIC] Manual mp-table setup
 			See Documentation/x86/i386/IO-APIC.txt.
 
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fcd484d..2140ed3 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -61,6 +61,36 @@ 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;
+	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");
+	return 1;
+}
+__setup("phir3mwait=disable", phir3mwait_disable);
+
+static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
+{
+	u64 msr;
+
+	rdmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
+
+	if (phi_r3mwait_disabled) {
+		msr &= ~MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
+		wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
+	} else {
+		msr |= MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
+		wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
+	}
+}
+
+#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 +595,13 @@ static void init_intel(struct cpuinfo_x86 *c)
 		detect_vmx_virtcap(c);
 
 	init_intel_energy_perf(c);
+
+	/*
+	* Setting ring 3 MONITOR/MWAIT for thread
+	* when CPU is Xeon Phi Family x200 (KnightsLanding).
+	*/
+	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_XEON_PHI_KNL)
+		probe_xeon_phi_r3mwait(c);
 }
 
 #ifdef CONFIG_X86_32
-- 
2.5.1

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

* [PATCH v6: 3/4] x86: Use HWCAP2 to expose Xeon Phi ring 3 MWAIT
  2016-10-27 14:01 [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot Grzegorz Andrejczuk
@ 2016-10-27 14:02 ` Grzegorz Andrejczuk
  2016-10-27 14:02 ` [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features Grzegorz Andrejczuk
  3 siblings, 0 replies; 17+ messages in thread
From: Grzegorz Andrejczuk @ 2016-10-27 14:02 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bp, dave.hansen, lukasz.daniluk, james.h.cownie, jacob.jun.pan,
	Piotr.Luc, linux-kernel, Grzegorz Andrejczuk

Add HWCAP2 for x86 and reserve its 1st bit to expose
Xeon Phi ring 3 monitor/mwait to userspace apps.

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..90ef445
--- /dev/null
+++ b/arch/x86/include/uapi/asm/hwcap2.h
@@ -0,0 +1,7 @@
+#ifndef _ASM_HWCAP2_H
+#define _ASM_HWCAP2_H
+
+/* Kernel enabled Ring 3 MWAIT for Xeon Phi*/
+#define HWCAP2_PHIR3MWAIT		(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

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

* [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features
  2016-10-27 14:01 [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing Grzegorz Andrejczuk
                   ` (2 preceding siblings ...)
  2016-10-27 14:02 ` [PATCH v6: 3/4] x86: Use HWCAP2 to expose Xeon Phi ring 3 MWAIT Grzegorz Andrejczuk
@ 2016-10-27 14:02 ` Grzegorz Andrejczuk
  2016-10-27 14:30   ` Borislav Petkov
  3 siblings, 1 reply; 17+ messages in thread
From: Grzegorz Andrejczuk @ 2016-10-27 14:02 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bp, dave.hansen, lukasz.daniluk, james.h.cownie, jacob.jun.pan,
	Piotr.Luc, linux-kernel, Grzegorz Andrejczuk

Add cpu feature for ring 3 monitor/mwait.
Set HWCAP2 1st bit during init.

Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
---
 arch/x86/include/asm/cpufeatures.h | 2 ++
 arch/x86/kernel/cpu/intel.c        | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 92a8308..d430200 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -71,6 +71,8 @@
 #define X86_FEATURE_RECOVERY	( 2*32+ 0) /* CPU in recovery mode */
 #define X86_FEATURE_LONGRUN	( 2*32+ 1) /* Longrun power control */
 #define X86_FEATURE_LRTI	( 2*32+ 3) /* LongRun table interface */
+/* Xeon Phi x200 ring 3 MONITOR/MWAIT enabled */
+#define X86_FEATURE_PHIR3MWAIT	( 2*32+ 4)
 
 /* Other features, Linux-defined mapping, word 3 */
 /* This range is used for feature bits which conflict or are synthesized */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 2140ed3..3f02c7e 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>
@@ -84,6 +86,8 @@ static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
 	} else {
 		msr |= MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
 		wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
+		set_cpu_cap(c, X86_FEATURE_PHIR3MWAIT);
+		ELF_HWCAP2 |= HWCAP2_PHIR3MWAIT;
 	}
 }
 
-- 
2.5.1

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

* Re: [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features
  2016-10-27 14:02 ` [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features Grzegorz Andrejczuk
@ 2016-10-27 14:30   ` Borislav Petkov
  2016-10-27 16:46     ` Andrejczuk, Grzegorz
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2016-10-27 14:30 UTC (permalink / raw)
  To: Grzegorz Andrejczuk
  Cc: tglx, mingo, hpa, x86, dave.hansen, lukasz.daniluk,
	james.h.cownie, jacob.jun.pan, Piotr.Luc, linux-kernel

On Thu, Oct 27, 2016 at 04:02:03PM +0200, Grzegorz Andrejczuk wrote:
> Add cpu feature for ring 3 monitor/mwait.
> Set HWCAP2 1st bit during init.
> 
> Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
> ---
>  arch/x86/include/asm/cpufeatures.h | 2 ++
>  arch/x86/kernel/cpu/intel.c        | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 92a8308..d430200 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -71,6 +71,8 @@
>  #define X86_FEATURE_RECOVERY	( 2*32+ 0) /* CPU in recovery mode */
>  #define X86_FEATURE_LONGRUN	( 2*32+ 1) /* Longrun power control */
>  #define X86_FEATURE_LRTI	( 2*32+ 3) /* LongRun table interface */
> +/* Xeon Phi x200 ring 3 MONITOR/MWAIT enabled */
> +#define X86_FEATURE_PHIR3MWAIT	( 2*32+ 4)

This leaf is for Transmeta CPUs:

/* Transmeta-defined CPU features, CPUID level 0x80860001, word 2 */

is KNL close to some Transmeta design, per chance or so?

I mean, for this to work, it would have to implement CPUID leaf
0x80860001...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

* Re: [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h
  2016-10-27 14:02 ` [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h Grzegorz Andrejczuk
@ 2016-10-27 14:32   ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-10-27 14:32 UTC (permalink / raw)
  To: Grzegorz Andrejczuk
  Cc: mingo, H. Peter Anvin, x86, bp, dave.hansen, lukasz.daniluk,
	james.h.cownie, Pan, Jacob jun, Piotr.Luc, LKML

On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> Intel Xeon Phi x200 (codenamed Knights Landing) has MSR
> MISC_THD_FEATURE_ENABLE 0x140.

Oh well. I just reviewed another patch which names this register:

   MSR_MISC_FEATURE_ENABLES

and that's how it's named in this document:

http://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf
 
Can Intel please get its act together and document that register proper in
the SDM?

Thanks,

	tglx

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

* Re: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-10-27 14:02 ` [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot Grzegorz Andrejczuk
@ 2016-10-27 14:38   ` Thomas Gleixner
  2016-10-27 16:53     ` Andrejczuk, Grzegorz
  2016-11-26 13:15     ` Pavel Machek
  0 siblings, 2 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-10-27 14:38 UTC (permalink / raw)
  To: Grzegorz Andrejczuk
  Cc: mingo, hpa, x86, bp, dave.hansen, lukasz.daniluk, james.h.cownie,
	jacob.jun.pan, Piotr.Luc, linux-kernel

On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> +#ifdef CONFIG_X86_64
> +static int phi_r3mwait_disabled __read_mostly;
> +
> +static int __init phir3mwait_disable(char *__unused)
> +{
> +	phi_r3mwait_disabled = 1;
> +	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");

Why would that be a warning? The sysadmin added the command line switch, so
why does he needs to be warned?

> +	return 1;
> +}
> +__setup("phir3mwait=disable", phir3mwait_disable);
> +
> +static void probe_xeon_phi_r3mwait(struct cpuinfo_x86 *c)
> +{
> +	u64 msr;
> +
> +	rdmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
> +
> +	if (phi_r3mwait_disabled) {
> +		msr &= ~MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
> +		wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
> +	} else {
> +		msr |= MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
> +		wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);
> +	}

	if (phi_r3mwait_disabled)
		msr &= ~MSR_PHI_MISC_THD_FEATURE_R3MWAIT;
	else
		msr |= MSR_PHI_MISC_THD_FEATURE_R3MWAIT;

	wrmsrl(MSR_PHI_MISC_THD_FEATURE, msr);

Would be too simple and obvious, right? You still can add the extra bits of
setting the capability flag into the else path.

>  	init_intel_energy_perf(c);
> +
> +	/*
> +	* Setting ring 3 MONITOR/MWAIT for thread
> +	* when CPU is Xeon Phi Family x200 (KnightsLanding).
> +	*/
> +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_XEON_PHI_KNL)

Please move this conditional into the probe function.

> +		probe_xeon_phi_r3mwait(c);

Can you please check with your hardware people, whether this function is
somewhere detectable. bit 0 of the MISC_*FEATURE* MSR (Ring 3 CPUID fault
enable) is detectable via the PLATFORM_INFO MSR. I would be surprised if
this thing is not detectable in some way.

I really prefer detectable things over hardcoded crap which depends on
model information.

Thanks,

	tglx

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

* RE: [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features
  2016-10-27 14:30   ` Borislav Petkov
@ 2016-10-27 16:46     ` Andrejczuk, Grzegorz
  2016-10-27 17:01       ` Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Andrejczuk, Grzegorz @ 2016-10-27 16:46 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: tglx, mingo, hpa, x86, dave.hansen, Daniluk, Lukasz, Cownie,
	James H, Pan, Jacob jun, Luc, Piotr, linux-kernel

> On Thu, Oct 27, 2016 at 04:02:03PM +0200, Grzegorz Andrejczuk wrote:
> > Add cpu feature for ring 3 monitor/mwait.
> > Set HWCAP2 1st bit during init.
> > 
> > Signed-off-by: Grzegorz Andrejczuk <grzegorz.andrejczuk@intel.com>
> > ---
> >  arch/x86/include/asm/cpufeatures.h | 2 ++
> >  arch/x86/kernel/cpu/intel.c        | 4 ++++
> >  2 files changed, 6 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/cpufeatures.h 
> > b/arch/x86/include/asm/cpufeatures.h
> > index 92a8308..d430200 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -71,6 +71,8 @@
> >  #define X86_FEATURE_RECOVERY	( 2*32+ 0) /* CPU in recovery mode */
> >  #define X86_FEATURE_LONGRUN	( 2*32+ 1) /* Longrun power control */
> >  #define X86_FEATURE_LRTI	( 2*32+ 3) /* LongRun table interface */
> > +/* Xeon Phi x200 ring 3 MONITOR/MWAIT enabled */
> > +#define X86_FEATURE_PHIR3MWAIT	( 2*32+ 4)
>
> This leaf is for Transmeta CPUs:
>
> /* Transmeta-defined CPU features, CPUID level 0x80860001, word 2 */
>
> is KNL close to some Transmeta design, per chance or so?
>
> I mean, for this to work, it would have to implement CPUID leaf 0x80860001...
>

I was reusing Word 3 as you suggested in patch rev. 1. There was idea to use init_scattered_cpuid_features.
What is good place for this feature? 

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

* RE: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-10-27 14:38   ` Thomas Gleixner
@ 2016-10-27 16:53     ` Andrejczuk, Grzegorz
  2016-10-27 18:22       ` Thomas Gleixner
  2016-11-26 13:15     ` Pavel Machek
  1 sibling, 1 reply; 17+ messages in thread
From: Andrejczuk, Grzegorz @ 2016-10-27 16:53 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: mingo, hpa, x86, bp, dave.hansen, Daniluk, Lukasz, Cownie,
	James H, Pan, Jacob jun, Luc, Piotr, linux-kernel

> >  	init_intel_energy_perf(c);
> > +
> > +	/*
> > +	* Setting ring 3 MONITOR/MWAIT for thread
> > +	* when CPU is Xeon Phi Family x200 (KnightsLanding).
> > +	*/
> > +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_XEON_PHI_KNL)
>
> Please move this conditional into the probe function.
>
> > +		probe_xeon_phi_r3mwait(c);
>
> Can you please check with your hardware people, whether this function is somewhere detectable. bit 0 of the MISC_*FEATURE* MSR (Ring 3 CPUID fault
> enable) is detectable via the PLATFORM_INFO MSR. I would be surprised if this thing is not detectable in some way.
>
> I really prefer detectable things over hardcoded crap which depends on model information.

I asked hardware people and MSR 0x140 should be called MSR_MISC_FEATURE_ENABLES and there is no other feature MSR indicating that this bit can be set. 
Unfortunately hardcoded crap has to be used. 

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

* Re: [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features
  2016-10-27 16:46     ` Andrejczuk, Grzegorz
@ 2016-10-27 17:01       ` Borislav Petkov
  0 siblings, 0 replies; 17+ messages in thread
From: Borislav Petkov @ 2016-10-27 17:01 UTC (permalink / raw)
  To: Andrejczuk, Grzegorz
  Cc: tglx, mingo, hpa, x86, dave.hansen, Daniluk, Lukasz, Cownie,
	James H, Pan, Jacob jun, Luc, Piotr, linux-kernel

On Thu, Oct 27, 2016 at 04:46:54PM +0000, Andrejczuk, Grzegorz wrote:
> I was reusing Word 3 as you suggested in patch rev. 1. There was idea
> to use init_scattered_cpuid_features.

I meant this word 3:

/* Other features, Linux-defined mapping, word 3 */
/* This range is used for feature bits which conflict or are synthesized */
#define X86_FEATURE_CXMMX       ( 3*32+ 0) /* Cyrix MMX extensions */
#define X86_FEATURE_K6_MTRR     ( 3*32+ 1) /* AMD K6 nonstandard MTRRs */
#define X86_FEATURE_CYRIX_ARR   ( 3*32+ 2) /* Cyrix ARRs (= MTRRs) */
...

i.e., the 4th word, if that makes more sense.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 

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

* RE: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-10-27 16:53     ` Andrejczuk, Grzegorz
@ 2016-10-27 18:22       ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-10-27 18:22 UTC (permalink / raw)
  To: Andrejczuk, Grzegorz
  Cc: mingo, hpa, x86, bp, dave.hansen, Daniluk, Lukasz, Cownie,
	James H, Pan, Jacob jun, Luc, Piotr, linux-kernel

On Thu, 27 Oct 2016, Andrejczuk, Grzegorz wrote:
> > >  	init_intel_energy_perf(c);
> > > +
> > > +	/*
> > > +	* Setting ring 3 MONITOR/MWAIT for thread
> > > +	* when CPU is Xeon Phi Family x200 (KnightsLanding).
> > > +	*/
> > > +	if (c->x86 == 6 && c->x86_model == INTEL_FAM6_XEON_PHI_KNL)
> >
> > Please move this conditional into the probe function.
> >
> > > +		probe_xeon_phi_r3mwait(c);
> >
> > Can you please check with your hardware people, whether this function
> > is somewhere detectable. bit 0 of the MISC_*FEATURE* MSR (Ring 3 CPUID
> > fault enable) is detectable via the PLATFORM_INFO MSR. I would be
> > surprised if this thing is not detectable in some way.
> >
> > I really prefer detectable things over hardcoded crap which depends on
> > model information.
>
> I asked hardware people and MSR 0x140 should be called
> MSR_MISC_FEATURE_ENABLES and there is no other feature MSR indicating
> that this bit can be set.  Unfortunately hardcoded crap has to be used.

Can you please tell your hardware folks, that non discoverable features are
a horror? We really need unique detection of features across all the
various cpu platforms. Making stuff depend on models, stepping results in a
nightmare.

Thanks,

	tglx

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

* Re: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-10-27 14:38   ` Thomas Gleixner
  2016-10-27 16:53     ` Andrejczuk, Grzegorz
@ 2016-11-26 13:15     ` Pavel Machek
  2016-11-26 13:20       ` Thomas Gleixner
  1 sibling, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2016-11-26 13:15 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Grzegorz Andrejczuk, mingo, hpa, x86, bp, dave.hansen,
	lukasz.daniluk, james.h.cownie, jacob.jun.pan, Piotr.Luc,
	linux-kernel

On Thu 2016-10-27 16:38:13, Thomas Gleixner wrote:
> On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> > +#ifdef CONFIG_X86_64
> > +static int phi_r3mwait_disabled __read_mostly;
> > +
> > +static int __init phir3mwait_disable(char *__unused)
> > +{
> > +	phi_r3mwait_disabled = 1;
> > +	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");
> 
> Why would that be a warning? The sysadmin added the command line switch, so
> why does he needs to be warned?

Its telling admin he did not make a typo on the command line. We often do that....

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

* Re: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-11-26 13:15     ` Pavel Machek
@ 2016-11-26 13:20       ` Thomas Gleixner
  2016-12-09 12:49         ` Andrejczuk, Grzegorz
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2016-11-26 13:20 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Grzegorz Andrejczuk, mingo, hpa, x86, bp, dave.hansen,
	lukasz.daniluk, james.h.cownie, jacob.jun.pan, Piotr.Luc,
	linux-kernel

On Sat, 26 Nov 2016, Pavel Machek wrote:

> On Thu 2016-10-27 16:38:13, Thomas Gleixner wrote:
> > On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> > > +#ifdef CONFIG_X86_64
> > > +static int phi_r3mwait_disabled __read_mostly;
> > > +
> > > +static int __init phir3mwait_disable(char *__unused)
> > > +{
> > > +	phi_r3mwait_disabled = 1;
> > > +	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");
> > 
> > Why would that be a warning? The sysadmin added the command line switch, so
> > why does he needs to be warned?
> 
> Its telling admin he did not make a typo on the command line. We often do that....

Definitely not at the warning level.

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

* RE: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-11-26 13:20       ` Thomas Gleixner
@ 2016-12-09 12:49         ` Andrejczuk, Grzegorz
  2017-02-03 10:47           ` Pavel Machek
  0 siblings, 1 reply; 17+ messages in thread
From: Andrejczuk, Grzegorz @ 2016-12-09 12:49 UTC (permalink / raw)
  To: Thomas Gleixner, Pavel Machek
  Cc: mingo, hpa, x86, bp, dave.hansen, Daniluk, Lukasz, Cownie,
	James H, Pan, Jacob jun, Luc, Piotr, linux-kernel

> > On Thu 2016-10-27 16:38:13, Thomas Gleixner wrote:
> > > On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> > > > +#ifdef CONFIG_X86_64
> > > > +static int phi_r3mwait_disabled __read_mostly;
> > > > +
> > > > +static int __init phir3mwait_disable(char *__unused) {
> > > > +	phi_r3mwait_disabled = 1;
> > > > +	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");
> > > 
> > > Why would that be a warning? The sysadmin added the command line 
> > > switch, so why does he needs to be warned?
> > 
> > Its telling admin he did not make a typo on the command line. We often do that....
>
> Definitely not at the warning level.

Is info level OK for that message?

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

* Re: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2016-12-09 12:49         ` Andrejczuk, Grzegorz
@ 2017-02-03 10:47           ` Pavel Machek
  2017-02-03 11:28             ` Andrejczuk, Grzegorz
  0 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2017-02-03 10:47 UTC (permalink / raw)
  To: Andrejczuk, Grzegorz
  Cc: Thomas Gleixner, mingo, hpa, x86, bp, dave.hansen, Daniluk,
	Lukasz, Cownie, James H, Pan, Jacob jun, Luc, Piotr,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Fri 2016-12-09 12:49:33, Andrejczuk, Grzegorz wrote:
> > > On Thu 2016-10-27 16:38:13, Thomas Gleixner wrote:
> > > > On Thu, 27 Oct 2016, Grzegorz Andrejczuk wrote:
> > > > > +#ifdef CONFIG_X86_64
> > > > > +static int phi_r3mwait_disabled __read_mostly;
> > > > > +
> > > > > +static int __init phir3mwait_disable(char *__unused) {
> > > > > +	phi_r3mwait_disabled = 1;
> > > > > +	pr_warn("x86/phir3mwait: Disabled ring 3 MWAIT for Xeon Phi");
> > > > 
> > > > Why would that be a warning? The sysadmin added the command line 
> > > > switch, so why does he needs to be warned?
> > > 
> > > Its telling admin he did not make a typo on the command line. We often do that....
> >
> > Definitely not at the warning level.
> 
> Is info level OK for that message?

I'd say so.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* RE: [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot
  2017-02-03 10:47           ` Pavel Machek
@ 2017-02-03 11:28             ` Andrejczuk, Grzegorz
  0 siblings, 0 replies; 17+ messages in thread
From: Andrejczuk, Grzegorz @ 2017-02-03 11:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Thomas Gleixner, mingo, hpa, x86, bp, dave.hansen, Daniluk,
	Lukasz, Cownie, James H, Pan, Jacob jun, Luc, Piotr,
	linux-kernel

On Friday, February 3, 2017 11:48 AM Pavel wrote:
> > Is info level OK for that message?
>
> I'd say so.

Thanks, I will update patch.

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

end of thread, other threads:[~2017-02-03 11:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 14:01 [PATCH v6 0/4] Enabling Ring 3 MONITOR/MWAIT feature for Knights Landing Grzegorz Andrejczuk
2016-10-27 14:02 ` [PATCH v6: 1/4] x86/msr: Add R3MWAIT register and bit to msr-info.h Grzegorz Andrejczuk
2016-10-27 14:32   ` Thomas Gleixner
2016-10-27 14:02 ` [PATCH v6: 2/4] x86: Add enabling of the R3MWAIT during boot Grzegorz Andrejczuk
2016-10-27 14:38   ` Thomas Gleixner
2016-10-27 16:53     ` Andrejczuk, Grzegorz
2016-10-27 18:22       ` Thomas Gleixner
2016-11-26 13:15     ` Pavel Machek
2016-11-26 13:20       ` Thomas Gleixner
2016-12-09 12:49         ` Andrejczuk, Grzegorz
2017-02-03 10:47           ` Pavel Machek
2017-02-03 11:28             ` Andrejczuk, Grzegorz
2016-10-27 14:02 ` [PATCH v6: 3/4] x86: Use HWCAP2 to expose Xeon Phi ring 3 MWAIT Grzegorz Andrejczuk
2016-10-27 14:02 ` [PATCH v6: 4/4] x86/cpufeature: Add R3MWAIT to CPU features Grzegorz Andrejczuk
2016-10-27 14:30   ` Borislav Petkov
2016-10-27 16:46     ` Andrejczuk, Grzegorz
2016-10-27 17:01       ` Borislav Petkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).