[PATCH v3] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction

[PATCH v3] arch_prctl,x86 Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction

[Kyle Huey]

rr (http://rr-project.org/), a userspace record-and-replay reverse-
execution debugger, would like to trap and emulate the CPUID instruction.
This would allow us to a) mask away certain hardware features that rr does
not support (e.g. RDRAND) and b) enable trace portability across machines
by providing constant results.

The following changes have been	made since v2.

Patch 1:
- Use of compat_sys_arch_prctl and separate do_arch_prctl_[common|64]
  functions to separate generic and 64-bit only arch_prctls.

Patch 2:
- The hack to suppress the mistakenly advertised CPUID faulting support in
  Xen guests is removed. Doing this for both PV and HVM guests is quite
  tricky, and likely more trouble than it's worth. Instead I'll submit a
  patch to Xen.

Patch 3:
- TIF_NOCPUID is now droppped on exec. I added the arch_post_exec hook
  as I didn't see any existing place to run arch-specific code during
  exec. The test is updated for the new exec behavior.

[PATCH v3 1/3] syscalls,x86 Expose arch_prctl on x86-32.

[Kyle Huey]

arch_prctl is currently 64-bit only. Wire it up for 32-bits, as a no-op for
now. Rename the second arg to a more generic name.

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/entry/syscalls/syscall_32.tbl |  1 +
 arch/x86/include/asm/proto.h           |  5 ++++-
 arch/x86/kernel/process.c              | 10 ++++++++++
 arch/x86/kernel/process_64.c           | 33 +++++++++++++++++++++------------
 arch/x86/kernel/ptrace.c               |  8 ++++----
 arch/x86/um/Makefile                   |  2 +-
 arch/x86/um/syscalls_32.c              |  7 +++++++
 arch/x86/um/syscalls_64.c              |  4 ++--
 include/linux/compat.h                 |  2 ++
 9 files changed, 52 insertions(+), 20 deletions(-)
 create mode 100644 arch/x86/um/syscalls_32.c

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index f848572..666fa61 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -386,3 +386,4 @@
 377	i386	copy_file_range		sys_copy_file_range
 378	i386	preadv2			sys_preadv2			compat_sys_preadv2
 379	i386	pwritev2		sys_pwritev2			compat_sys_pwritev2
+380	i386	arch_prctl		compat_sys_arch_prctl		compat_sys_arch_prctl
diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h
index 9b9b30b..f0e86aa 100644
--- a/arch/x86/include/asm/proto.h
+++ b/arch/x86/include/asm/proto.h
@@ -30,6 +30,9 @@ void x86_report_nx(void);
 
 extern int reboot_force;
 
-long do_arch_prctl(struct task_struct *task, int code, unsigned long addr);
+long do_arch_prctl_common(struct task_struct *task, int code, unsigned long addr);
+#ifdef CONFIG_X86_64
+long do_arch_prctl_64(struct task_struct *task, int code, unsigned long addr);
+#endif
 
 #endif /* _ASM_X86_PROTO_H */
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 62c0b0e..1421451 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -567,3 +567,13 @@ unsigned long get_wchan(struct task_struct *p)
 	} while (count++ < 16 && p->state != TASK_RUNNING);
 	return 0;
 }
+
+long do_arch_prctl_common(struct task_struct *task, int code, unsigned long arg2)
+{
+	return -EINVAL;
+}
+
+asmlinkage long compat_sys_arch_prctl(int code, unsigned long arg2)
+{
+	return do_arch_prctl_common(current, code, arg2);
+}
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 63236d8..0e44608 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -35,6 +35,7 @@
 #include <linux/uaccess.h>
 #include <linux/io.h>
 #include <linux/ftrace.h>
+#include <linux/syscalls.h>
 
 #include <asm/pgtable.h>
 #include <asm/processor.h>
@@ -196,7 +197,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long sp,
 				(struct user_desc __user *)tls, 0);
 		else
 #endif
-			err = do_arch_prctl(p, ARCH_SET_FS, tls);
+			err = do_arch_prctl_64(p, ARCH_SET_FS, tls);
 		if (err)
 			goto out;
 	}
@@ -524,7 +525,7 @@ void set_personality_ia32(bool x32)
 }
 EXPORT_SYMBOL_GPL(set_personality_ia32);
 
-long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
+long do_arch_prctl_64(struct task_struct *task, int code, unsigned long arg2)
 {
 	int ret = 0;
 	int doit = task == current;
@@ -532,48 +533,50 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
 
 	switch (code) {
 	case ARCH_SET_GS:
-		if (addr >= TASK_SIZE_MAX)
+		if (arg2 >= TASK_SIZE_MAX)
 			return -EPERM;
 		cpu = get_cpu();
 		task->thread.gsindex = 0;
-		task->thread.gsbase = addr;
+		task->thread.gsbase = arg2;
 		if (doit) {
 			load_gs_index(0);
-			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
+			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, arg2);
 		}
 		put_cpu();
 		break;
 	case ARCH_SET_FS:
 		/* Not strictly needed for fs, but do it for symmetry
 		   with gs */
-		if (addr >= TASK_SIZE_MAX)
+		if (arg2 >= TASK_SIZE_MAX)
 			return -EPERM;
 		cpu = get_cpu();
 		task->thread.fsindex = 0;
-		task->thread.fsbase = addr;
+		task->thread.fsbase = arg2;
 		if (doit) {
 			/* set the selector to 0 to not confuse __switch_to */
 			loadsegment(fs, 0);
-			ret = wrmsrl_safe(MSR_FS_BASE, addr);
+			ret = wrmsrl_safe(MSR_FS_BASE, arg2);
 		}
 		put_cpu();
 		break;
 	case ARCH_GET_FS: {
 		unsigned long base;
+
 		if (doit)
 			rdmsrl(MSR_FS_BASE, base);
 		else
 			base = task->thread.fsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
+		ret = put_user(base, (unsigned long __user *)arg2);
 		break;
 	}
 	case ARCH_GET_GS: {
 		unsigned long base;
+
 		if (doit)
 			rdmsrl(MSR_KERNEL_GS_BASE, base);
 		else
 			base = task->thread.gsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
+		ret = put_user(base, (unsigned long __user *)arg2);
 		break;
 	}
 
@@ -585,9 +588,15 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
 	return ret;
 }
 
-long sys_arch_prctl(int code, unsigned long addr)
+SYSCALL_DEFINE2(arch_prctl, int, code, unsigned long, arg2)
 {
-	return do_arch_prctl(current, code, addr);
+	long ret;
+
+	ret = do_arch_prctl_64(current, code, arg2);
+	if (ret == -EINVAL)
+		ret = do_arch_prctl_common(current, code, arg2);
+
+	return ret;
 }
 
 unsigned long KSTK_ESP(struct task_struct *task)
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index f79576a..030cbc5 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -395,12 +395,12 @@ static int putreg(struct task_struct *child,
 		if (value >= TASK_SIZE_MAX)
 			return -EIO;
 		/*
-		 * When changing the segment base, use do_arch_prctl
+		 * When changing the segment base, use do_arch_prctl_64
 		 * to set either thread.fs or thread.fsindex and the
 		 * corresponding GDT slot.
 		 */
 		if (child->thread.fsbase != value)
-			return do_arch_prctl(child, ARCH_SET_FS, value);
+			return do_arch_prctl_64(child, ARCH_SET_FS, value);
 		return 0;
 	case offsetof(struct user_regs_struct,gs_base):
 		/*
@@ -409,7 +409,7 @@ static int putreg(struct task_struct *child,
 		if (value >= TASK_SIZE_MAX)
 			return -EIO;
 		if (child->thread.gsbase != value)
-			return do_arch_prctl(child, ARCH_SET_GS, value);
+			return do_arch_prctl_64(child, ARCH_SET_GS, value);
 		return 0;
 #endif
 	}
@@ -868,7 +868,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		   Works just like arch_prctl, except that the arguments
 		   are reversed. */
 	case PTRACE_ARCH_PRCTL:
-		ret = do_arch_prctl(child, data, addr);
+		ret = do_arch_prctl_64(child, data, addr);
 		break;
 #endif
 
diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile
index 3ee2bb6..5e039d6 100644
--- a/arch/x86/um/Makefile
+++ b/arch/x86/um/Makefile
@@ -16,7 +16,7 @@ obj-y = bug.o bugs_$(BITS).o delay.o fault.o ksyms.o ldt.o \
 
 ifeq ($(CONFIG_X86_32),y)
 
-obj-y += checksum_32.o
+obj-y += checksum_32.o syscalls_32.o
 obj-$(CONFIG_ELF_CORE) += elfcore.o
 
 subarch-y = ../lib/string_32.o ../lib/atomic64_32.o ../lib/atomic64_cx8_32.o
diff --git a/arch/x86/um/syscalls_32.c b/arch/x86/um/syscalls_32.c
new file mode 100644
index 0000000..c6812c1
--- /dev/null
+++ b/arch/x86/um/syscalls_32.c
@@ -0,0 +1,7 @@
+#include <linux/syscalls.h>
+#include <os.h>
+
+long compat_sys_arch_prctl(int code, unsigned long arg2)
+{
+	return -EINVAL;
+}
diff --git a/arch/x86/um/syscalls_64.c b/arch/x86/um/syscalls_64.c
index e655227..d0a7160 100644
--- a/arch/x86/um/syscalls_64.c
+++ b/arch/x86/um/syscalls_64.c
@@ -72,9 +72,9 @@ long arch_prctl(struct task_struct *task, int code, unsigned long __user *addr)
 	return ret;
 }
 
-long sys_arch_prctl(int code, unsigned long addr)
+long sys_arch_prctl(int code, unsigned long arg2)
 {
-	return arch_prctl(current, code, (unsigned long __user *) addr);
+	return arch_prctl(current, code, (unsigned long __user *) arg2);
 }
 
 void arch_switch_to(struct task_struct *to)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index f964ef7..0039d53 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -722,6 +722,8 @@ asmlinkage long compat_sys_sched_rr_get_interval(compat_pid_t pid,
 asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
 					    int, const char __user *);
 
+asmlinkage long compat_sys_arch_prctl(int, unsigned long);
+
 /*
  * For most but not all architectures, "am I in a compat syscall?" and
  * "am I a compat task?" are the same question.  For architectures on which
-- 
2.9.3

base-commit: 4cea8776571b18db7485930cb422faa739580c8c

[PATCH v3 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo

[Kyle Huey]

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/include/asm/cpufeatures.h |  1 +
 arch/x86/include/asm/msr-index.h   |  1 +
 arch/x86/kernel/cpu/scattered.c    | 14 ++++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 92a8308..78b9d06 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -190,6 +190,7 @@
 
 #define X86_FEATURE_CPB		( 7*32+ 2) /* AMD Core Performance Boost */
 #define X86_FEATURE_EPB		( 7*32+ 3) /* IA32_ENERGY_PERF_BIAS support */
+#define X86_FEATURE_CPUID_FAULT ( 7*32+ 4) /* Intel CPUID faulting */
 
 #define X86_FEATURE_HW_PSTATE	( 7*32+ 8) /* AMD HW-PState */
 #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 56f4c66..83908d5 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -41,6 +41,7 @@
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
 #define MSR_PLATFORM_INFO		0x000000ce
+#define CPUID_FAULTING_SUPPORT		(1UL << 31)
 
 #define MSR_NHM_SNB_PKG_CST_CFG_CTL	0x000000e2
 #define NHM_C3_AUTO_DEMOTE		(1UL << 25)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 8cb57df..d502da1 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -24,6 +24,17 @@ enum cpuid_regs {
 	CR_EBX
 };
 
+static int supports_cpuid_faulting(void)
+{
+	unsigned int lo, hi;
+
+	if (rdmsr_safe(MSR_PLATFORM_INFO, &lo, &hi) == 0 &&
+	    (lo & CPUID_FAULTING_SUPPORT))
+		return 1;
+	else
+		return 0;
+}
+
 void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 {
 	u32 max_level;
@@ -54,4 +65,7 @@ void init_scattered_cpuid_features(struct cpuinfo_x86 *c)
 		if (regs[cb->reg] & (1 << cb->bit))
 			set_cpu_cap(c, cb->feature);
 	}
+
+	if (supports_cpuid_faulting())
+		set_cpu_cap(c, X86_FEATURE_CPUID_FAULT);
 }
-- 
2.9.3

[PATCH v3 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction

[Kyle Huey]

Intel supports faulting on the CPUID instruction in newer processors. Bit
31 of MSR_PLATFORM_INFO advertises support for this feature. It is
documented in detail in Section 2.3.2 of
http://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf

Support for this is implemented as a new pair of arch_prctls, available on both x86-32 and x86-64.  The structure mirrors PR_[GET|SET]_TSC.  Like the TSC flag, CPUID faulting is propagated across forks.  Unlike the TSC flag, it is reset (to CPUID enabled) on exec.

Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/include/asm/msr-index.h          |   1 +
 arch/x86/include/asm/thread_info.h        |   5 +-
 arch/x86/include/uapi/asm/prctl.h         |   6 +
 arch/x86/kernel/process.c                 |  98 ++++++++++++-
 fs/exec.c                                 |   6 +
 tools/testing/selftests/x86/Makefile      |   2 +-
 tools/testing/selftests/x86/cpuid-fault.c | 234 ++++++++++++++++++++++++++++++
 7 files changed, 349 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/x86/cpuid-fault.c

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 83908d5..4aebec2 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -53,6 +53,7 @@
 #define MSR_MTRRcap			0x000000fe
 #define MSR_IA32_BBL_CR_CTL		0x00000119
 #define MSR_IA32_BBL_CR_CTL3		0x0000011e
+#define MSR_MISC_FEATURES_ENABLES	0x00000140
 
 #define MSR_IA32_SYSENTER_CS		0x00000174
 #define MSR_IA32_SYSENTER_ESP		0x00000175
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index 8b7c8d8..e3c40c6 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -93,6 +93,7 @@ struct thread_info {
 #define TIF_SECCOMP		8	/* secure computing */
 #define TIF_USER_RETURN_NOTIFY	11	/* notify kernel of userspace return */
 #define TIF_UPROBE		12	/* breakpointed or singlestepping */
+#define TIF_NOCPUID		15	/* CPUID is not accessible in userland */
 #define TIF_NOTSC		16	/* TSC is not accessible in userland */
 #define TIF_IA32		17	/* IA32 compatibility process */
 #define TIF_FORK		18	/* ret_from_fork */
@@ -117,6 +118,7 @@ struct thread_info {
 #define _TIF_SECCOMP		(1 << TIF_SECCOMP)
 #define _TIF_USER_RETURN_NOTIFY	(1 << TIF_USER_RETURN_NOTIFY)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
+#define _TIF_NOCPUID		(1 << TIF_NOCPUID)
 #define _TIF_NOTSC		(1 << TIF_NOTSC)
 #define _TIF_IA32		(1 << TIF_IA32)
 #define _TIF_FORK		(1 << TIF_FORK)
@@ -146,7 +148,7 @@ struct thread_info {
 
 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW							\
-	(_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP)
+	(_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
 
 #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
 #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
@@ -293,6 +295,7 @@ static inline bool in_ia32_syscall(void)
 extern void arch_task_cache_init(void);
 extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 extern void arch_release_task_struct(struct task_struct *tsk);
+extern void arch_post_exec(void);
 #endif	/* !__ASSEMBLY__ */
 
 #endif /* _ASM_X86_THREAD_INFO_H */
diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
index 3ac5032..c087e55 100644
--- a/arch/x86/include/uapi/asm/prctl.h
+++ b/arch/x86/include/uapi/asm/prctl.h
@@ -6,4 +6,10 @@
 #define ARCH_GET_FS 0x1003
 #define ARCH_GET_GS 0x1004
 
+/* Get/set the process' ability to use the CPUID instruction */
+#define ARCH_GET_CPUID 0x1005
+#define ARCH_SET_CPUID 0x1006
+# define ARCH_CPUID_ENABLE		1	/* allow the use of the CPUID instruction */
+# define ARCH_CPUID_SIGSEGV		2	/* throw a SIGSEGV instead of reading the CPUID */
+
 #endif /* _ASM_X86_PRCTL_H */
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 1421451..f307d5c 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -32,6 +32,7 @@
 #include <asm/tlbflush.h>
 #include <asm/mce.h>
 #include <asm/vm86.h>
+#include <asm/prctl.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -191,6 +192,75 @@ int set_tsc_mode(unsigned int val)
 	return 0;
 }
 
+static void switch_cpuid_faulting(bool on)
+{
+	if (on)
+		msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0);
+	else
+		msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0);
+}
+
+static void disable_cpuid(void)
+{
+	preempt_disable();
+	if (!test_and_set_thread_flag(TIF_NOCPUID))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOCPUID in the current running context.
+		 */
+		switch_cpuid_faulting(true);
+	preempt_enable();
+}
+
+static void enable_cpuid(void)
+{
+	preempt_disable();
+	if (test_and_clear_thread_flag(TIF_NOCPUID))
+		/*
+		 * Must flip the CPU state synchronously with
+		 * TIF_NOCPUID in the current running context.
+		 */
+		switch_cpuid_faulting(false);
+	preempt_enable();
+}
+
+int get_cpuid_mode(unsigned long adr)
+{
+	unsigned int val;
+
+	if (test_thread_flag(TIF_NOCPUID))
+		val = ARCH_CPUID_SIGSEGV;
+	else
+		val = ARCH_CPUID_ENABLE;
+
+	return put_user(val, (unsigned int __user *)adr);
+}
+
+int set_cpuid_mode(struct task_struct *task, unsigned long val)
+{
+	/* Only disable/enable_cpuid() if it is supported on this hardware. */
+	bool cpuid_fault_supported = static_cpu_has(X86_FEATURE_CPUID_FAULT);
+
+	if (val == ARCH_CPUID_ENABLE && cpuid_fault_supported)
+		enable_cpuid();
+	else if (val == ARCH_CPUID_SIGSEGV && cpuid_fault_supported)
+		disable_cpuid();
+	else
+		return -EINVAL;
+
+	return 0;
+}
+
+/*
+ * Called immediately after a successful exec.
+ */
+void arch_post_exec()
+{
+	/* If cpuid was previously disabled for this task, re-enable it. */
+	if (test_thread_flag(TIF_NOCPUID))
+		enable_cpuid();
+}
+
 void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		      struct tss_struct *tss)
 {
@@ -210,6 +280,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,
 		update_debugctlmsr(debugctl);
 	}
 
+	if (test_tsk_thread_flag(prev_p, TIF_NOCPUID) ^
+	    test_tsk_thread_flag(next_p, TIF_NOCPUID)) {
+		/* prev and next are different */
+		if (test_tsk_thread_flag(next_p, TIF_NOCPUID))
+			switch_cpuid_faulting(true);
+		else
+			switch_cpuid_faulting(false);
+	}
+
 	if (test_tsk_thread_flag(prev_p, TIF_NOTSC) ^
 	    test_tsk_thread_flag(next_p, TIF_NOTSC)) {
 		/* prev and next are different */
@@ -570,7 +649,24 @@ unsigned long get_wchan(struct task_struct *p)
 
 long do_arch_prctl_common(struct task_struct *task, int code, unsigned long arg2)
 {
-	return -EINVAL;
+	int ret = 0;
+
+	switch (code) {
+	case ARCH_GET_CPUID: {
+		ret = get_cpuid_mode(arg2);
+		break;
+	}
+	case ARCH_SET_CPUID: {
+		ret = set_cpuid_mode(task, arg2);
+		break;
+	}
+
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
 }
 
 asmlinkage long compat_sys_arch_prctl(int code, unsigned long arg2)
diff --git a/fs/exec.c b/fs/exec.c
index 6fcfb3f..a0fca09 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1627,6 +1627,11 @@ static int exec_binprm(struct linux_binprm *bprm)
 	return ret;
 }
 
+void __weak arch_post_exec(void)
+{
+	/* Do nothing by default */
+}
+
 /*
  * sys_execve() executes a new program.
  */
@@ -1743,6 +1748,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 	/* execve succeeded */
 	current->fs->in_exec = 0;
 	current->in_execve = 0;
+	arch_post_exec();
 	acct_update_integrals(current);
 	task_numa_free(current);
 	free_bprm(bprm);
diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 4f747ee..fbf34d3 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -5,7 +5,7 @@ include ../lib.mk
 .PHONY: all all_32 all_64 warn_32bit_failure clean
 
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_syscall test_mremap_vdso \
-			check_initial_reg_state sigreturn ldt_gdt iopl mpx-mini-test
+			check_initial_reg_state sigreturn ldt_gdt iopl mpx-mini-test cpuid-fault
 TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/cpuid-fault.c b/tools/testing/selftests/x86/cpuid-fault.c
new file mode 100644
index 0000000..a9f3f68
--- /dev/null
+++ b/tools/testing/selftests/x86/cpuid-fault.c
@@ -0,0 +1,234 @@
+
+/*
+ * Tests for arch_prctl(ARCH_GET_CPUID, ...) / prctl(ARCH_SET_CPUID, ...)
+ *
+ * Basic test to test behaviour of ARCH_GET_CPUID and ARCH_SET_CPUID
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <signal.h>
+#include <inttypes.h>
+#include <cpuid.h>
+#include <errno.h>
+#include <sys/wait.h>
+
+#include <sys/prctl.h>
+#include <linux/prctl.h>
+
+const char *cpuid_names[] = {
+	[0] = "[not set]",
+	[ARCH_CPUID_ENABLE] = "ARCH_CPUID_ENABLE",
+	[ARCH_CPUID_SIGSEGV] = "ARCH_CPUID_SIGSEGV",
+};
+
+int arch_prctl(int code, unsigned long arg2)
+{
+	return syscall(SYS_arch_prctl, code, arg2);
+}
+
+int cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
+	  unsigned int *edx)
+{
+	return __get_cpuid(0, eax, ebx, ecx, edx);
+}
+
+int do_child_exec_test(int eax, int ebx, int ecx, int edx)
+{
+	int cpuid_val = 0, child = 0, status = 0;
+
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	if (cpuid_val != ARCH_CPUID_SIGSEGV)
+		exit(42);
+
+	if ((child = fork()) == 0) {
+		printf("exec\n");
+		fflush(stdout);
+		execl("/proc/self/exe", "cpuid-fault", "-early-return", NULL);
+	}
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != 0)
+		exit(42);
+
+	return 0;
+}
+
+int child_received_signal;
+
+void child_sigsegv_cb(int sig)
+{
+	int cpuid_val = 0;
+
+	child_received_signal = 1;
+	printf("[ SIG_SEGV ]\n");
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0)
+		exit(errno);
+
+	printf("cpuid() == ");
+}
+
+int do_child_test(void)
+{
+	unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+	signal(SIGSEGV, child_sigsegv_cb);
+
+	/* the child starts out with cpuid disabled, the signal handler
+	 * attempts to enable and retry
+	 */
+	printf("cpuid() == ");
+	fflush(stdout);
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("{%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	return child_received_signal ? 0 : 42;
+}
+
+int signal_count;
+
+void sigsegv_cb(int sig)
+{
+	int cpuid_val = 0;
+
+	signal_count++;
+	printf("[ SIG_SEGV ]\n");
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
+		exit(42);
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	printf("arch_prctl(ARC_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0)
+		exit(42);
+
+	printf("cpuid() == ");
+}
+
+int main(int argc, char** argv)
+{
+	int cpuid_val = 0, child = 0, status = 0;
+	unsigned int eax = 0, ebx = 0, ecx = 0, edx = 0;
+
+	signal(SIGSEGV, sigsegv_cb);
+
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0) {
+		if (errno == EINVAL) {
+			printf("ARCH_GET_CPUID is unsupported on this system.");
+			fflush(stdout);
+			exit(0); /* no ARCH_GET_CPUID on this system */
+		} else {
+			exit(42);
+		}
+	}
+
+	printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE)\n");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0) {
+		if (errno == EINVAL) {
+			printf("ARCH_SET_CPUID is unsupported on this system.");
+			fflush(stdout);
+			exit(0); /* no ARCH_SET_CPUID on this system */
+		} else {
+			exit(42);
+		}
+	}
+
+
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("cpuid() == {%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV)\n");
+	fflush(stdout);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV) == -1)
+		exit(42);
+
+	printf("cpuid() == ");
+	fflush(stdout);
+	eax = ebx = ecx = edx = 0;
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("{%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV)\n");
+	fflush(stdout);
+
+	if (signal_count != 1)
+		exit(42);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV) == -1)
+		exit(42);
+
+	if (argc > 1)
+		exit(0); /* Don't run the whole test again if we were execed */
+
+	printf("do_child_test\n");
+	fflush(stdout);
+	if ((child = fork()) == 0)
+		return do_child_test();
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != 0)
+		exit(42);
+
+	/* The child enabling cpuid should not have affected us */
+	printf("cpuid() == ");
+	fflush(stdout);
+	eax = ebx = ecx = edx = 0;
+	cpuid(&eax, &ebx, &ecx, &edx);
+	printf("{%x, %x, %x, %x}\n", eax, ebx, ecx, edx);
+	printf("arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV)\n");
+	fflush(stdout);
+
+	if (signal_count != 2)
+		exit(42);
+
+	if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_SIGSEGV) == -1)
+		exit(42);
+
+	/* Our ARCH_CPUID_SIGSEGV should not propagate through exec */
+	printf("do_child_exec_test\n");
+	fflush(stdout);
+	if ((child = fork()) == 0)
+		return do_child_exec_test(eax, ebx, ecx, edx);
+
+	if (child != waitpid(child, &status, 0))
+		exit(42);
+
+	if (WEXITSTATUS(status) != 0)
+		exit(42);
+
+	printf("All tests passed!\n");
+	fflush(stdout);
+	exit(EXIT_SUCCESS);
+}
+
-- 
2.9.3

Re: [PATCH v3 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo

[Andy Lutomirski]

On Thu, Sep 15, 2016 at 4:33 PM, Kyle Huey <me@kylehuey.com> wrote:

Reviewed-by: Andy Lutomirski <luto@kernel.org>

although this is really Borislav's domain.

OTOH, if you're planning on changing Linux's Xen MSR helpers to mask
the feature out, that should be in the same patch or an earlier patch.

Re: [PATCH v3 1/3] syscalls,x86 Expose arch_prctl on x86-32.

[Andy Lutomirski]

On Thu, Sep 15, 2016 at 4:33 PM, Kyle Huey <me@kylehuey.com> wrote:
> arch_prctl is currently 64-bit only. Wire it up for 32-bits, as a no-op for
> now. Rename the second arg to a more generic name.
>
> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> ---
>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>  arch/x86/include/asm/proto.h           |  5 ++++-
>  arch/x86/kernel/process.c              | 10 ++++++++++
>  arch/x86/kernel/process_64.c           | 33 +++++++++++++++++++++------------
>  arch/x86/kernel/ptrace.c               |  8 ++++----
>  arch/x86/um/Makefile                   |  2 +-
>  arch/x86/um/syscalls_32.c              |  7 +++++++
>  arch/x86/um/syscalls_64.c              |  4 ++--
>  include/linux/compat.h                 |  2 ++
>  9 files changed, 52 insertions(+), 20 deletions(-)
>  create mode 100644 arch/x86/um/syscalls_32.c
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index f848572..666fa61 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -386,3 +386,4 @@
>  377    i386    copy_file_range         sys_copy_file_range
>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
> +380    i386    arch_prctl              compat_sys_arch_prctl           compat_sys_arch_prctl

Let's call this sys_arch_prctl_32, even if it's unconventional.  See below.

> diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h
> index 9b9b30b..f0e86aa 100644
> --- a/arch/x86/include/asm/proto.h
> +++ b/arch/x86/include/asm/proto.h
> @@ -30,6 +30,9 @@ void x86_report_nx(void);
>
>  extern int reboot_force;
>
> -long do_arch_prctl(struct task_struct *task, int code, unsigned long addr);
> +long do_arch_prctl_common(struct task_struct *task, int code, unsigned long addr);
> +#ifdef CONFIG_X86_64
> +long do_arch_prctl_64(struct task_struct *task, int code, unsigned long addr);
> +#endif
>
>  #endif /* _ASM_X86_PROTO_H */
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 62c0b0e..1421451 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -567,3 +567,13 @@ unsigned long get_wchan(struct task_struct *p)
>         } while (count++ < 16 && p->state != TASK_RUNNING);
>         return 0;
>  }
> +
> +long do_arch_prctl_common(struct task_struct *task, int code, unsigned long arg2)
> +{
> +       return -EINVAL;
> +}
> +
> +asmlinkage long compat_sys_arch_prctl(int code, unsigned long arg2)

I believe you mean COMPAT_SYSCALL_DEFINE2 here.

But I see what you're doing here.  Could you instead do:

#if defined(CONFIG_IA32_EMULATION) || defined(CONFIG_X86_32)
#ifdef CONFIG_X86_32
COMPAT_SYSCALL_DEFINE2(...)
#else
SYSCALL_DEFINE2(...)
#endif

... body here ...
#endif

and name the thing do_arch_prctl_32?

It's too bad we don't have a SYSCALL_DEFINE_32 macro.  But you could add one...


> diff --git a/arch/x86/um/syscalls_32.c b/arch/x86/um/syscalls_32.c
> new file mode 100644
> index 0000000..c6812c1
> --- /dev/null
> +++ b/arch/x86/um/syscalls_32.c
> @@ -0,0 +1,7 @@
> +#include <linux/syscalls.h>
> +#include <os.h>
> +
> +long compat_sys_arch_prctl(int code, unsigned long arg2)

COMPAT_SYSCALL_DEFINE2

Also, does this really need a new file?

> -long sys_arch_prctl(int code, unsigned long addr)
> +long sys_arch_prctl(int code, unsigned long arg2)

SYSCALL_DEFINE2

Re: [PATCH v3 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction

[Andy Lutomirski]

On Thu, Sep 15, 2016 at 4:33 PM, Kyle Huey <me@kylehuey.com> wrote:
> Intel supports faulting on the CPUID instruction in newer processors. Bit
> 31 of MSR_PLATFORM_INFO advertises support for this feature. It is
> documented in detail in Section 2.3.2 of
> http://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf
>
> Support for this is implemented as a new pair of arch_prctls, available on both x86-32 and x86-64.  The structure mirrors PR_[GET|SET]_TSC.  Like the TSC flag, CPUID faulting is propagated across forks.  Unlike the TSC flag, it is reset (to CPUID enabled) on exec.
>
> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> ---
>  arch/x86/include/asm/msr-index.h          |   1 +
>  arch/x86/include/asm/thread_info.h        |   5 +-
>  arch/x86/include/uapi/asm/prctl.h         |   6 +
>  arch/x86/kernel/process.c                 |  98 ++++++++++++-
>  fs/exec.c                                 |   6 +
>  tools/testing/selftests/x86/Makefile      |   2 +-
>  tools/testing/selftests/x86/cpuid-fault.c | 234 ++++++++++++++++++++++++++++++
>  7 files changed, 349 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/x86/cpuid-fault.c
>
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 83908d5..4aebec2 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -53,6 +53,7 @@
>  #define MSR_MTRRcap                    0x000000fe
>  #define MSR_IA32_BBL_CR_CTL            0x00000119
>  #define MSR_IA32_BBL_CR_CTL3           0x0000011e
> +#define MSR_MISC_FEATURES_ENABLES      0x00000140
>
>  #define MSR_IA32_SYSENTER_CS           0x00000174
>  #define MSR_IA32_SYSENTER_ESP          0x00000175
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index 8b7c8d8..e3c40c6 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -93,6 +93,7 @@ struct thread_info {
>  #define TIF_SECCOMP            8       /* secure computing */
>  #define TIF_USER_RETURN_NOTIFY 11      /* notify kernel of userspace return */
>  #define TIF_UPROBE             12      /* breakpointed or singlestepping */
> +#define TIF_NOCPUID            15      /* CPUID is not accessible in userland */
>  #define TIF_NOTSC              16      /* TSC is not accessible in userland */
>  #define TIF_IA32               17      /* IA32 compatibility process */
>  #define TIF_FORK               18      /* ret_from_fork */
> @@ -117,6 +118,7 @@ struct thread_info {
>  #define _TIF_SECCOMP           (1 << TIF_SECCOMP)
>  #define _TIF_USER_RETURN_NOTIFY        (1 << TIF_USER_RETURN_NOTIFY)
>  #define _TIF_UPROBE            (1 << TIF_UPROBE)
> +#define _TIF_NOCPUID           (1 << TIF_NOCPUID)
>  #define _TIF_NOTSC             (1 << TIF_NOTSC)
>  #define _TIF_IA32              (1 << TIF_IA32)
>  #define _TIF_FORK              (1 << TIF_FORK)
> @@ -146,7 +148,7 @@ struct thread_info {
>
>  /* flags to check in __switch_to() */
>  #define _TIF_WORK_CTXSW                                                        \
> -       (_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP)
> +       (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP)
>
>  #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)
>  #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
> @@ -293,6 +295,7 @@ static inline bool in_ia32_syscall(void)
>  extern void arch_task_cache_init(void);
>  extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
>  extern void arch_release_task_struct(struct task_struct *tsk);
> +extern void arch_post_exec(void);
>  #endif /* !__ASSEMBLY__ */
>
>  #endif /* _ASM_X86_THREAD_INFO_H */
> diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h
> index 3ac5032..c087e55 100644
> --- a/arch/x86/include/uapi/asm/prctl.h
> +++ b/arch/x86/include/uapi/asm/prctl.h
> @@ -6,4 +6,10 @@
>  #define ARCH_GET_FS 0x1003
>  #define ARCH_GET_GS 0x1004
>
> +/* Get/set the process' ability to use the CPUID instruction */
> +#define ARCH_GET_CPUID 0x1005
> +#define ARCH_SET_CPUID 0x1006
> +# define ARCH_CPUID_ENABLE             1       /* allow the use of the CPUID instruction */
> +# define ARCH_CPUID_SIGSEGV            2       /* throw a SIGSEGV instead of reading the CPUID */
> +
>  #endif /* _ASM_X86_PRCTL_H */
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 1421451..f307d5c 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -32,6 +32,7 @@
>  #include <asm/tlbflush.h>
>  #include <asm/mce.h>
>  #include <asm/vm86.h>
> +#include <asm/prctl.h>
>
>  /*
>   * per-CPU TSS segments. Threads are completely 'soft' on Linux,
> @@ -191,6 +192,75 @@ int set_tsc_mode(unsigned int val)
>         return 0;
>  }
>
> +static void switch_cpuid_faulting(bool on)
> +{
> +       if (on)
> +               msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0);
> +       else
> +               msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0);
> +}
> +
> +static void disable_cpuid(void)
> +{
> +       preempt_disable();
> +       if (!test_and_set_thread_flag(TIF_NOCPUID))
> +               /*
> +                * Must flip the CPU state synchronously with
> +                * TIF_NOCPUID in the current running context.
> +                */
> +               switch_cpuid_faulting(true);
> +       preempt_enable();
> +}
> +
> +static void enable_cpuid(void)
> +{
> +       preempt_disable();
> +       if (test_and_clear_thread_flag(TIF_NOCPUID))
> +               /*
> +                * Must flip the CPU state synchronously with
> +                * TIF_NOCPUID in the current running context.
> +                */
> +               switch_cpuid_faulting(false);
> +       preempt_enable();
> +}
> +
> +int get_cpuid_mode(unsigned long adr)
> +{
> +       unsigned int val;
> +
> +       if (test_thread_flag(TIF_NOCPUID))
> +               val = ARCH_CPUID_SIGSEGV;
> +       else
> +               val = ARCH_CPUID_ENABLE;
> +
> +       return put_user(val, (unsigned int __user *)adr);
> +}

Can we just do:

if (arg2 != 0)
  return -EINVAL;
else
 return test_thread_flag(TIF_NOCPUID) ? ARCH_CPUID_SIGSEGBV : ARCH_CPUID_ENABLE;


> +
> +int set_cpuid_mode(struct task_struct *task, unsigned long val)
> +{
> +       /* Only disable/enable_cpuid() if it is supported on this hardware. */
> +       bool cpuid_fault_supported = static_cpu_has(X86_FEATURE_CPUID_FAULT);
> +
> +       if (val == ARCH_CPUID_ENABLE && cpuid_fault_supported)
> +               enable_cpuid();

No need to check cpuid_fault_supported in this branch.

> +/*
> + * Called immediately after a successful exec.
> + */
> +void arch_post_exec()
> +{
> +       /* If cpuid was previously disabled for this task, re-enable it. */
> +       if (test_thread_flag(TIF_NOCPUID))
> +               enable_cpuid();
> +}

Ugh, do we seriously not have anything that does this yet?
start_thread is almost the right thing.  So is elf_common_init.

>  asmlinkage long compat_sys_arch_prctl(int code, unsigned long arg2)
> diff --git a/fs/exec.c b/fs/exec.c
> index 6fcfb3f..a0fca09 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1627,6 +1627,11 @@ static int exec_binprm(struct linux_binprm *bprm)
>         return ret;
>  }
>
> +void __weak arch_post_exec(void)
> +{
> +       /* Do nothing by default */
> +}
> +

I personally prefer:

#ifndef arch_post_exec
static inline void arch_post_exec(void) {}
#endif

in linux/whatever.h and:

#define arch_post_exec arch_post_exec

in arch/x86/include/asm/whatever.h

It avoids bloating other architectures.

>  /*
>   * sys_execve() executes a new program.
>   */
> @@ -1743,6 +1748,7 @@ static int do_execveat_common(int fd, struct filename *filename,
>         /* execve succeeded */
>         current->fs->in_exec = 0;
>         current->in_execve = 0;
> +       arch_post_exec();

Hrm.  This could also go in setup_new_exec().  I think I prefer that
so that all of this type of stuff stays together.

> diff --git a/tools/testing/selftests/x86/cpuid-fault.c b/tools/testing/selftests/x86/cpuid-fault.c
> new file mode 100644
> index 0000000..a9f3f68
> --- /dev/null
> +++ b/tools/testing/selftests/x86/cpuid-fault.c
> @@ -0,0 +1,234 @@
> +
> +/*
> + * Tests for arch_prctl(ARCH_GET_CPUID, ...) / prctl(ARCH_SET_CPUID, ...)
> + *
> + * Basic test to test behaviour of ARCH_GET_CPUID and ARCH_SET_CPUID
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <signal.h>
> +#include <inttypes.h>
> +#include <cpuid.h>
> +#include <errno.h>
> +#include <sys/wait.h>
> +
> +#include <sys/prctl.h>
> +#include <linux/prctl.h>
> +
> +const char *cpuid_names[] = {
> +       [0] = "[not set]",

Is 0 even possible?

> +       [ARCH_CPUID_ENABLE] = "ARCH_CPUID_ENABLE",
> +       [ARCH_CPUID_SIGSEGV] = "ARCH_CPUID_SIGSEGV",
> +};
> +
> +int arch_prctl(int code, unsigned long arg2)
> +{
> +       return syscall(SYS_arch_prctl, code, arg2);
> +}
> +
> +int cpuid(unsigned int *eax, unsigned int *ebx, unsigned int *ecx,
> +         unsigned int *edx)
> +{
> +       return __get_cpuid(0, eax, ebx, ecx, edx);
> +}
> +
> +int do_child_exec_test(int eax, int ebx, int ecx, int edx)
> +{
> +       int cpuid_val = 0, child = 0, status = 0;
> +
> +       printf("arch_prctl(ARCH_GET_CPUID, &cpuid_val); ");
> +       fflush(stdout);
> +
> +       if (arch_prctl(ARCH_GET_CPUID, (unsigned long)&cpuid_val) != 0)
> +               exit(42);

Let's exit with 1 and print something informative.  I like using the
errx macro for this -- see some of the other tests in this directory.

> +
> +       printf("cpuid_val == %s\n", cpuid_names[cpuid_val]);
> +       if (cpuid_val != ARCH_CPUID_SIGSEGV)
> +               exit(42);
> +
> +       if ((child = fork()) == 0) {

Please check that we're still ARCH_CPU_SIGSEGV in here.

> +               printf("exec\n");
> +               fflush(stdout);
> +               execl("/proc/self/exe", "cpuid-fault", "-early-return", NULL);
> +       }
> +
> +       if (child != waitpid(child, &status, 0))
> +               exit(42);
> +
> +       if (WEXITSTATUS(status) != 0)
> +               exit(42);
> +
> +       return 0;
> +}
> +


> +       if (arch_prctl(ARCH_SET_CPUID, ARCH_CPUID_ENABLE) != 0)
> +               exit(42);

You are sneaky.  Nice hack to avoid siglongjmp :)

General comment: all of these fflush calls are ugly.  How about
calling setvbuf early in main() instead?

Re: [PATCH v3 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction

[Kyle Huey]

On Thu, Sep 15, 2016 at 5:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Sep 15, 2016 at 4:33 PM, Kyle Huey <me@kylehuey.com> wrote:
>> +int get_cpuid_mode(unsigned long adr)
>> +{
>> +       unsigned int val;
>> +
>> +       if (test_thread_flag(TIF_NOCPUID))
>> +               val = ARCH_CPUID_SIGSEGV;
>> +       else
>> +               val = ARCH_CPUID_ENABLE;
>> +
>> +       return put_user(val, (unsigned int __user *)adr);
>> +}
>
> Can we just do:
>
> if (arg2 != 0)
>   return -EINVAL;
> else
>  return test_thread_flag(TIF_NOCPUID) ? ARCH_CPUID_SIGSEGBV : ARCH_CPUID_ENABLE;

We could.  I copied the pattern of PR_GET_TSC here, but I don't feel
strongly about it.

>> diff --git a/tools/testing/selftests/x86/cpuid-fault.c b/tools/testing/selftests/x86/cpuid-fault.c
>> new file mode 100644
>> index 0000000..a9f3f68
>> --- /dev/null
>> +++ b/tools/testing/selftests/x86/cpuid-fault.c
>> @@ -0,0 +1,234 @@
>> +
>> +/*
>> + * Tests for arch_prctl(ARCH_GET_CPUID, ...) / prctl(ARCH_SET_CPUID, ...)
>> + *
>> + * Basic test to test behaviour of ARCH_GET_CPUID and ARCH_SET_CPUID
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <unistd.h>
>> +#include <signal.h>
>> +#include <inttypes.h>
>> +#include <cpuid.h>
>> +#include <errno.h>
>> +#include <sys/wait.h>
>> +
>> +#include <sys/prctl.h>
>> +#include <linux/prctl.h>
>> +
>> +const char *cpuid_names[] = {
>> +       [0] = "[not set]",
>
> Is 0 even possible?

Only if the call fails.

- Kyle

Re: [PATCH v3 1/3] syscalls,x86 Expose arch_prctl on x86-32.

[Thomas Gleixner]

On Thu, 15 Sep 2016, Kyle Huey wrote:

First of all, please add a cover letter [PATCH 0/N] to your patch series
and send it with something which provides proper mail threading. 
See: git-send-email, quilt 

> arch_prctl is currently 64-bit only. Wire it up for 32-bits, as a no-op for
> now. Rename the second arg to a more generic name.

This changelog is useless.

- it does not provide any rationale for this change, i.e. why this is
  required. Just because its 64bit only is not a reason.

- "Rename the second arg to a more generic name" does not give
  any useful information.

Misleading information is worse than no information.

Further your patch does 5 things at once. It wants to be split into parts:

1) Rename do_arch_prctl() and change the argument name,

> -long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
> +long do_arch_prctl_64(struct task_struct *task, int code, unsigned long arg2)

2) Provide do_arch_prctl_common() and hook it up to the arch_prctl syscall

> -long sys_arch_prctl(int code, unsigned long addr)
> +SYSCALL_DEFINE2(arch_prctl, int, code, unsigned long, arg2)
>  {
> -	return do_arch_prctl(current, code, addr);
> +	long ret;
> +
> +	ret = do_arch_prctl_64(current, code, arg2);
> +	if (ret == -EINVAL)
> +		ret = do_arch_prctl_common(current, code, arg2);
> +
> +	return ret;
>  }

3) Implement the compat version
  
Thanks,

	tflx

Re: [PATCH v3 2/3] x86 Test and expose CPUID faulting capabilities in /proc/cpuinfo

[Thomas Gleixner]

On Thu, 15 Sep 2016, Kyle Huey wrote:

Please use proper prefixes for your patch:

git-log arch/x86/kernel/cpu/scattered.c will give you the hint:

x86/cpufeature: Move some of the scattered feature bits to x86_capability
x86/cpufeature: Correct spelling of the HWP_NOTIFY flag

and make the subject line short. Long sentences belong into the body of the
changelog.

And again this changelog does not tell anything.

What is this CPUID faulting support?
Which CPUs do support this?
Why do we want this?

>  #define X86_FEATURE_CPB		( 7*32+ 2) /* AMD Core Performance Boost */
>  #define X86_FEATURE_EPB		( 7*32+ 3) /* IA32_ENERGY_PERF_BIAS support */
> +#define X86_FEATURE_CPUID_FAULT ( 7*32+ 4) /* Intel CPUID faulting */

Boris?

>  #define X86_FEATURE_HW_PSTATE	( 7*32+ 8) /* AMD HW-PState */
>  #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 56f4c66..83908d5 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -41,6 +41,7 @@
>  #define MSR_IA32_PERFCTR1		0x000000c2
>  #define MSR_FSB_FREQ			0x000000cd
>  #define MSR_PLATFORM_INFO		0x000000ce
> +#define CPUID_FAULTING_SUPPORT		(1UL << 31)

If you look at the other MSR bit defines then they have always a prefix
which links them to the MSR....

What's the name of this bit in the Documentation?

> +static int supports_cpuid_faulting(void)

bool please

> +{
> +	unsigned int lo, hi;
> +
> +	if (rdmsr_safe(MSR_PLATFORM_INFO, &lo, &hi) == 0 &&
> +	    (lo & CPUID_FAULTING_SUPPORT))
> +		return 1;
> +	else
> +		return 0;

  	if (rdmsr_safe(MSR_PLATFORM_INFO, &lo, &hi))
		return false;

	return lo & PLATINFO_CPUID_FAULT;

would make it readable without linebreaks.

Thanks,

	tglx
 

Re: [PATCH v3 1/3] syscalls,x86 Expose arch_prctl on x86-32.

[Kyle Huey]

On Fri, Sep 16, 2016 at 12:50 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 15 Sep 2016, Kyle Huey wrote:
>
> First of all, please add a cover letter [PATCH 0/N] to your patch series
> and send it with something which provides proper mail threading.
> See: git-send-email, quilt

I did ... seems like using git-send-email with
--cc-cmd=scripts/get_maintainer.pl is not a good idea since people get
CCd to some parts of the thread and not others.

https://lkml.org/lkml/2016/9/15/811

>> arch_prctl is currently 64-bit only. Wire it up for 32-bits, as a no-op for
>> now. Rename the second arg to a more generic name.
>
> This changelog is useless.
>
> - it does not provide any rationale for this change, i.e. why this is
>   required. Just because its 64bit only is not a reason.
>
> - "Rename the second arg to a more generic name" does not give
>   any useful information.
>
> Misleading information is worse than no information.
>
> Further your patch does 5 things at once. It wants to be split into parts:
>
> 1) Rename do_arch_prctl() and change the argument name,
>
>> -long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
>> +long do_arch_prctl_64(struct task_struct *task, int code, unsigned long arg2)
>
> 2) Provide do_arch_prctl_common() and hook it up to the arch_prctl syscall
>
>> -long sys_arch_prctl(int code, unsigned long addr)
>> +SYSCALL_DEFINE2(arch_prctl, int, code, unsigned long, arg2)
>>  {
>> -     return do_arch_prctl(current, code, addr);
>> +     long ret;
>> +
>> +     ret = do_arch_prctl_64(current, code, arg2);
>> +     if (ret == -EINVAL)
>> +             ret = do_arch_prctl_common(current, code, arg2);
>> +
>> +     return ret;
>>  }
>
> 3) Implement the compat version

Ok.

- Kyle