[Devel] [PATCH hci-8.1 v2 0/5] x86/ve: CPUID override follow-ups: arm at once, prefixes, selftests

[Devel] [PATCH hci-8.1 v2 0/5] x86/ve: CPUID override follow-ups: arm at once, prefixes, selftests

[Konstantin Khorenko]

VSTOR-148656, every Container start dying with SIGSEGV once
/proc/vz/cpuid_override is non-empty, is fixed in 22.8 by 07338fbae5a3
("x86/traps: emulate CPUID faults for user tasks in
exc_general_protection()"). Working on it turned up two more defects in
the same feature, and the fact that nothing had ever exercised it. This
series takes care of those. None of them is a 22.x regression, all have
been there since the vz9 port.

Patch 2: ve_setup_task() only sets TIF_CPUID_OVERRIDE, the MSR follows
at the next context switch. A task entering a VE through unshare() or
setns() executes CPUID natively until it is scheduled out and sees the
host values. set_cpuid_override() flips the MSR synchronously for the
current task, the way disable_cpuid() does.

Patch 4: check_cpuid_fault() compares two bytes at ip with 0f a2. A
CPUID with a legacy or REX prefix faults the same way, but the handler
sees the prefix and the task gets SIGSEGV. Use the instruction decoder,
as fixup_umip_exception() does, and advance ip by the decoded length.

Patches 1, 3, 5: selftests/ve/ve_cpuid_override_test. Each kernel patch
gets its own case. The test saves and restores the table around every
case and tears down in the parent, because the kselftest harness skips
a child-side teardown after a failed ASSERT.

Tested on sqvm2-4060, the node of the failing CI run (Broadwell VM with
cpuid_fault, cpupools-features 9.1.7, so the table is armed at every
Container start). With only the 22.8 fix, cases 3 and 4 fail the way
their patches describe: host values, and a child killed by signal 11.
With the whole series all four cases pass, repeatedly and on a
non-empty table, and the vzctl-pytest start cases from the original
ticket pass as well.

https://virtuozzo.atlassian.net/browse/VSTOR-148843

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Changes in v2:
- Patch 1: the child stops itself with SIGSTOP after entering the VE
  and the parent resumes it, so faulting is armed by a real context
  switch; the pipe handshake, whose read could return without sleeping,
  is gone (Sashiko).
- Patch 1: setup creates the VE cgroup last, so a failed ASSERT in
  setup leaves nothing behind; the table snapshot buffer fits the
  largest table the kernel prints and a truncated read fails the setup;
  a failed restore in teardown is logged (Sashiko).
- Patch 1: drop the EAX comparison after the fallback to hardware, the
  child is not pinned and hybrid CPUs may differ per core (Sashiko).
- Patch 1: build the test on x86 only, add the binary to .gitignore
  (Sashiko).
- Patch 3: the no-reschedule case repeats the sequence, a tick can
  still preempt the child in the window; with the handshake gone a
  child that dies before reporting is reaped where the result is read
  and its signal logged (Sashiko).
- Retested on sqvm2-2084 with the whole series: all cases pass and the
  table is restored; on sqvm2-4060 with the stock kernel all cases fail
  with the expected diagnostics.
- Link to v1: http://lore.virtuozzo.com/20261006-b4-prep-khorenko-vstor-148656-cpuid-v1-0-741609f4f03a@virtuozzo.com

---
Konstantin Khorenko (5):
      selftests/ve: check CPUID override emulation for VE tasks
      x86/ve: enable CPUID faulting at once when the current task enters a VE
      selftests/ve: check CPUID override right after entering a VE
      x86/traps: decode the faulting CPUID with the instruction decoder
      selftests/ve: check CPUID override emulation for a prefixed CPUID

 arch/x86/kernel/process.c                          |  12 +
 arch/x86/kernel/traps.c                            |  16 +-
 include/linux/cpuid_override.h                     |   3 +
 kernel/ve/ve.c                                     |   2 +-
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   7 +
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 291 +++++++++++++++++++++
 7 files changed, 325 insertions(+), 7 deletions(-)
---
base-commit: f5d1fdcfffe101fe2bafa7df09d95d2e811c5c5e
change-id: 20261006-b4-prep-khorenko-vstor-148656-cpuid-a5c5993e1f83

Best regards,
--  
Konstantin Khorenko <khorenko@virtuozzo.com>

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 1/5] selftests/ve: check CPUID override emulation for VE tasks

[Konstantin Khorenko]

Write a one-entry /proc/vz/cpuid_override table that clears one leaf 1
feature bit and compare what a task in a fresh VE and the host see:
the VE task must get the table values, the host the hardware ones. A
second test flushes the table after the task was armed and expects the
hardware values again, since faulting stays on for the task and the
emulation has to fall back to the real CPUID.

The child stops itself with SIGSTOP after entering the VE and the
parent resumes it before the CPUID: faulting is switched on for a task
at its next context switch, not when the flag is set, and a stopped
task is off the CPU for sure. For the second test the table is flushed
while the child stands still.

Without the fix both children die with SIGSEGV. The tests skip on CPUs
without CPUID faulting and restore the previous table on exit. The
teardown runs in the parent because the harness skips a child-side
teardown once an ASSERT has fired. The test is built on x86 only.

https://virtuozzo.atlassian.net/browse/VSTOR-148843

Feature: x86: cpu pools (cpuid override)
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/ve/.gitignore              |   1 +
 tools/testing/selftests/ve/Makefile                |   7 +
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 236 +++++++++++++++++++++
 3 files changed, 244 insertions(+)

diff --git a/tools/testing/selftests/ve/.gitignore b/tools/testing/selftests/ve/.gitignore
index 3df4d05888dce..bdc322c645d95 100644
--- a/tools/testing/selftests/ve/.gitignore
+++ b/tools/testing/selftests/ve/.gitignore
@@ -1,3 +1,4 @@
 ve_ns_owner_test
 ve_perms_test
 ve_mount_accounting_test
+ve_cpuid_override_test
diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
index c6efe7c4b4fba..cd50a86fdb226 100644
--- a/tools/testing/selftests/ve/Makefile
+++ b/tools/testing/selftests/ve/Makefile
@@ -6,4 +6,11 @@ TEST_GEN_PROGS += ve_ns_owner_test
 TEST_GEN_PROGS += ve_perms_test
 TEST_GEN_PROGS += ve_mount_accounting_test
 
+ifeq ($(ARCH),)
+ARCH := $(shell uname -m 2>/dev/null || echo not)
+endif
+ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
+TEST_GEN_PROGS += ve_cpuid_override_test
+endif
+
 include ../lib.mk
diff --git a/tools/testing/selftests/ve/ve_cpuid_override_test.c b/tools/testing/selftests/ve/ve_cpuid_override_test.c
new file mode 100644
index 0000000000000..291b88404a53e
--- /dev/null
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -0,0 +1,236 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * /proc/vz/cpuid_override selftests
+ *
+ * A non-empty override table turns CPUID faulting on for every task that
+ * enters a VE, and the #GP handler has to emulate the instruction for
+ * them. The tests check that a task in a VE sees the table values, that
+ * the host keeps seeing the hardware values, and that a task which was
+ * armed while the table was set falls back to the hardware once the
+ * table is flushed. A kernel that does not emulate the fault kills the
+ * child with SIGSEGV instead.
+ */
+#define _GNU_SOURCE
+#include <sched.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/wait.h>
+
+#include "../kselftest_harness.h"
+#include "ve_selftest.h"
+
+#define CPUID_OVERRIDE_PATH	"/proc/vz/cpuid_override"
+#define CPUID_LEAF		1
+#define CPUID_TABLE_LEN		128
+/* 128 entries of up to 67 characters each, as the kernel prints them. */
+#define CPUID_TABLE_MAX		16384
+
+struct cpuid_regs {
+	unsigned int eax, ebx, ecx, edx;
+};
+
+/* x86_64 only, so ebx is not the PIC base and "=b" is safe. */
+static void do_cpuid(unsigned int leaf, struct cpuid_regs *r)
+{
+	asm volatile("cpuid"
+		     : "=a" (r->eax), "=b" (r->ebx), "=c" (r->ecx), "=d" (r->edx)
+		     : "a" (leaf), "c" (0));
+}
+
+static int write_cpuid_override(const char *table)
+{
+	int fd, ret;
+
+	fd = open(CPUID_OVERRIDE_PATH, O_WRONLY);
+	if (fd < 0)
+		return -1;
+	ret = write(fd, table, strlen(table));
+	close(fd);
+	return ret == (int)strlen(table) ? 0 : -1;
+}
+
+/*
+ * The child enters a fresh VE, so it is armed with the table state of that
+ * moment, then reports its view of the leaf to the parent over a pipe. It
+ * stops itself in between: a stopped task is off the CPU, so the resume is
+ * a context switch, and the parent may change the table while the child
+ * stands still.
+ */
+struct child_ctx {
+	int result_pipe[2];
+};
+
+static int cpuid_child(struct child_ctx *ctx)
+{
+	struct cpuid_regs r;
+
+	close(ctx->result_pipe[0]);
+
+	if (unshare(CLONE_NEWVE) < 0)
+		return 21;
+	if (raise(SIGSTOP))
+		return 22;
+
+	do_cpuid(CPUID_LEAF, &r);
+
+	if (write(ctx->result_pipe[1], &r, sizeof(r)) != sizeof(r))
+		return 24;
+	return 0;
+}
+
+FIXTURE(ve_cpuid_override)
+{
+	int cgv2_fd;
+	int ctid;
+	struct cpuid_regs host;
+	unsigned int bit;
+	char table[CPUID_TABLE_LEN];
+	char saved[CPUID_TABLE_MAX];
+};
+
+FIXTURE_SETUP(ve_cpuid_override)
+{
+	int fd, len;
+
+	fd = open(CPUID_OVERRIDE_PATH, O_RDONLY);
+	if (fd < 0)
+		SKIP(return, "%s: %s", CPUID_OVERRIDE_PATH, strerror(errno));
+	close(fd);
+
+	len = read_file_at(AT_FDCWD, CPUID_OVERRIDE_PATH, self->saved,
+			   sizeof(self->saved));
+	ASSERT_GE(len, 0);
+	ASSERT_LT(len, sizeof(self->saved) - 1);
+
+	/* Mask one feature bit the host has; SSE2 when present. */
+	do_cpuid(CPUID_LEAF, &self->host);
+	self->bit = 1u << 26;
+	if (!(self->host.edx & self->bit))
+		self->bit = self->host.edx & -self->host.edx;
+	ASSERT_NE(self->bit, 0);
+
+	snprintf(self->table, sizeof(self->table),
+		 "0x%08x: 0x%08x 0x%08x 0x%08x 0x%08x\n", CPUID_LEAF,
+		 self->host.eax, self->host.ebx, self->host.ecx,
+		 self->host.edx & ~self->bit);
+
+	/* Last, so that a failed ASSERT above leaves no VE cgroup behind. */
+	self->cgv2_fd = mount_cg2_fd();
+	ASSERT_GE(self->cgv2_fd, 0);
+	ASSERT_EQ(write_file_at(self->cgv2_fd, "cgroup.subtree_control",
+				VE_CONTROLLERS), 0);
+	self->ctid = make_ve(self->cgv2_fd, CTID_MIN);
+	ASSERT_GE(self->ctid, 0);
+}
+
+/*
+ * Runs in the parent: a child-side teardown is skipped when an ASSERT fires,
+ * and the table must be restored even then. The fd from setup belongs to the
+ * child, so the cgroup2 tree is mounted again here.
+ */
+FIXTURE_TEARDOWN_PARENT(ve_cpuid_override)
+{
+	int cgv2_fd;
+
+	if (write_cpuid_override(self->saved[0] ? self->saved : "\n"))
+		TH_LOG("could not restore %s: %s", CPUID_OVERRIDE_PATH,
+		       strerror(errno));
+
+	cgv2_fd = mount_cg2_fd();
+	if (cgv2_fd < 0)
+		return;
+	destroy_ve(cgv2_fd, self->ctid);
+	close(cgv2_fd);
+}
+
+/* Fork the child into the VE and return once it stands still there. */
+static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
+			       int cgv2_fd, int ctid, struct child_ctx *ctx)
+{
+	int status;
+	pid_t pid;
+
+	ASSERT_EQ(pipe(ctx->result_pipe), 0);
+	ASSERT_GE(enter_cgroup(cgv2_fd, ctid), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0)
+		_exit(cpuid_child(ctx));
+
+	ASSERT_GE(enter_cgroup(cgv2_fd, 0), 0);
+	close(ctx->result_pipe[1]);
+
+	ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
+	if (WIFEXITED(status))
+		TH_LOG("child exited with %d before stopping",
+		       WEXITSTATUS(status));
+	ASSERT_TRUE(WIFSTOPPED(status));
+	return pid;
+}
+
+static void resume_cpuid_child(struct __test_metadata *_metadata, pid_t pid)
+{
+	ASSERT_EQ(kill(pid, SIGCONT), 0);
+}
+
+static void collect_cpuid_child(struct __test_metadata *_metadata, pid_t pid,
+				struct child_ctx *ctx, struct cpuid_regs *r)
+{
+	int status, len;
+
+	len = read(ctx->result_pipe[0], r, sizeof(*r));
+	ASSERT_GE(waitpid(pid, &status, 0), 0);
+	if (WIFSIGNALED(status))
+		TH_LOG("child killed by signal %d", WTERMSIG(status));
+	ASSERT_TRUE(WIFEXITED(status));
+	ASSERT_EQ(WEXITSTATUS(status), 0);
+	ASSERT_EQ(len, sizeof(*r));
+
+	close(ctx->result_pipe[0]);
+}
+
+TEST_F(ve_cpuid_override, ve_sees_table_host_sees_hardware)
+{
+	struct child_ctx ctx;
+	struct cpuid_regs r;
+	pid_t pid;
+
+	ASSERT_EQ(write_cpuid_override(self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self->cgv2_fd, self->ctid, &ctx);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, pid, &ctx, &r);
+
+	EXPECT_EQ(r.eax, self->host.eax);
+	EXPECT_EQ(r.ebx, self->host.ebx);
+	EXPECT_EQ(r.ecx, self->host.ecx);
+	EXPECT_EQ(r.edx, self->host.edx & ~self->bit);
+
+	do_cpuid(CPUID_LEAF, &r);
+	EXPECT_NE(r.edx & self->bit, 0);
+}
+
+TEST_F(ve_cpuid_override, flushed_table_falls_back_to_hardware)
+{
+	struct child_ctx ctx;
+	struct cpuid_regs r;
+	pid_t pid;
+
+	ASSERT_EQ(write_cpuid_override(self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self->cgv2_fd, self->ctid, &ctx);
+
+	ASSERT_EQ(write_cpuid_override("\n"), 0);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, pid, &ctx, &r);
+
+	EXPECT_NE(r.edx & self->bit, 0);
+}
+
+TEST_HARNESS_MAIN

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 2/5] x86/ve: enable CPUID faulting at once when the current task enters a VE

[Konstantin Khorenko]

ve_setup_task() only sets TIF_CPUID_OVERRIDE. The MSR follows the flag
in __switch_to_xtra(), so a task that enters a VE through unshare() or
setns() keeps executing CPUID natively until it is scheduled out for the
first time and sees the host values instead of the override table.

Add set_cpuid_override(), which sets the flag and, like disable_cpuid(),
flips the CPU state synchronously when the task is the current one.
Tasks set up through clone() are not running yet and are still handled
at their first switch-in.

https://virtuozzo.atlassian.net/browse/VSTOR-148843

Feature: x86: cpu pools (cpuid override)
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 arch/x86/kernel/process.c      | 12 ++++++++++++
 include/linux/cpuid_override.h |  3 +++
 kernel/ve/ve.c                 |  2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 28fc4168b6c9f..2673f2e51a833 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -23,6 +23,7 @@
 #include <linux/utsname.h>
 #include <linux/stackprotector.h>
 #include <linux/cpuidle.h>
+#include <linux/cpuid_override.h>
 #include <linux/acpi.h>
 #include <linux/elf-randomize.h>
 #include <linux/static_call.h>
@@ -381,6 +382,17 @@ static void enable_cpuid(void)
 	preempt_enable();
 }
 
+#ifdef CONFIG_VE
+void set_cpuid_override(struct task_struct *tsk)
+{
+	preempt_disable();
+	if (!test_and_set_tsk_thread_flag(tsk, TIF_CPUID_OVERRIDE) &&
+	    tsk == current)
+		set_cpuid_faulting(true);
+	preempt_enable();
+}
+#endif
+
 static int get_cpuid_mode(void)
 {
 	return !test_thread_flag(TIF_NOCPUID);
diff --git a/include/linux/cpuid_override.h b/include/linux/cpuid_override.h
index 1f5223728c9ba..83a955b628cdf 100644
--- a/include/linux/cpuid_override.h
+++ b/include/linux/cpuid_override.h
@@ -35,4 +35,7 @@ static inline bool cpuid_override_on(void)
 {
 	return rcu_access_pointer(cpuid_override);
 }
+
+struct task_struct;
+void set_cpuid_override(struct task_struct *tsk);
 #endif
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 579628a8a310a..8df881039c3c6 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -1857,7 +1857,7 @@ void ve_setup_task(struct task_struct *task, struct ve_struct *ve)
 
 #ifdef CONFIG_X86
 	if (cpuid_override_on())
-		set_tsk_thread_flag(task, TIF_CPUID_OVERRIDE);
+		set_cpuid_override(task);
 #endif
 }
 EXPORT_SYMBOL(ve_setup_task);

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 3/5] selftests/ve: check CPUID override right after entering a VE

[Konstantin Khorenko]

Add a case where the child executes CPUID straight after unshare(),
without stopping or blocking in between, and expects the table values.
Before the kernel change the override took effect only after the first
context switch, so the child saw the hardware values.

A tick can still preempt the child in that window, so the case runs
the sequence several times: a kernel that arms faulting only at the
switch has to get lucky every time.

https://virtuozzo.atlassian.net/browse/VSTOR-148843

Feature: x86: cpu pools (cpuid override)
---
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 54 ++++++++++++++++------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_cpuid_override_test.c b/tools/testing/selftests/ve/ve_cpuid_override_test.c
index 291b88404a53e..0e748d4fd5f33 100644
--- a/tools/testing/selftests/ve/ve_cpuid_override_test.c
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -27,6 +27,7 @@
 #define CPUID_OVERRIDE_PATH	"/proc/vz/cpuid_override"
 #define CPUID_LEAF		1
 #define CPUID_TABLE_LEN		128
+#define NO_RESCHED_ROUNDS	20
 /* 128 entries of up to 67 characters each, as the kernel prints them. */
 #define CPUID_TABLE_MAX		16384
 
@@ -56,13 +57,14 @@ static int write_cpuid_override(const char *table)
 
 /*
  * The child enters a fresh VE, so it is armed with the table state of that
- * moment, then reports its view of the leaf to the parent over a pipe. It
- * stops itself in between: a stopped task is off the CPU, so the resume is
- * a context switch, and the parent may change the table while the child
- * stands still.
+ * moment, then reports its view of the leaf to the parent over a pipe. With
+ * @stop it stops itself in between: a stopped task is off the CPU, so the
+ * resume is a context switch, and the parent may change the table while
+ * the child stands still.
  */
 struct child_ctx {
 	int result_pipe[2];
+	int stop;
 };
 
 static int cpuid_child(struct child_ctx *ctx)
@@ -73,7 +75,7 @@ static int cpuid_child(struct child_ctx *ctx)
 
 	if (unshare(CLONE_NEWVE) < 0)
 		return 21;
-	if (raise(SIGSTOP))
+	if (ctx->stop && raise(SIGSTOP))
 		return 22;
 
 	do_cpuid(CPUID_LEAF, &r);
@@ -148,7 +150,7 @@ FIXTURE_TEARDOWN_PARENT(ve_cpuid_override)
 	close(cgv2_fd);
 }
 
-/* Fork the child into the VE and return once it stands still there. */
+/* Fork the child into the VE; with @stop return once it stands still. */
 static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
 			       int cgv2_fd, int ctid, struct child_ctx *ctx)
 {
@@ -166,11 +168,13 @@ static pid_t spawn_cpuid_child(struct __test_metadata *_metadata,
 	ASSERT_GE(enter_cgroup(cgv2_fd, 0), 0);
 	close(ctx->result_pipe[1]);
 
-	ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
-	if (WIFEXITED(status))
-		TH_LOG("child exited with %d before stopping",
-		       WEXITSTATUS(status));
-	ASSERT_TRUE(WIFSTOPPED(status));
+	if (ctx->stop) {
+		ASSERT_GE(waitpid(pid, &status, WUNTRACED), 0);
+		if (WIFEXITED(status))
+			TH_LOG("child exited with %d before stopping",
+			       WEXITSTATUS(status));
+		ASSERT_TRUE(WIFSTOPPED(status));
+	}
 	return pid;
 }
 
@@ -197,7 +201,7 @@ static void collect_cpuid_child(struct __test_metadata *_metadata, pid_t pid,
 
 TEST_F(ve_cpuid_override, ve_sees_table_host_sees_hardware)
 {
-	struct child_ctx ctx;
+	struct child_ctx ctx = { .stop = 1 };
 	struct cpuid_regs r;
 	pid_t pid;
 
@@ -218,7 +222,7 @@ TEST_F(ve_cpuid_override, ve_sees_table_host_sees_hardware)
 
 TEST_F(ve_cpuid_override, flushed_table_falls_back_to_hardware)
 {
-	struct child_ctx ctx;
+	struct child_ctx ctx = { .stop = 1 };
 	struct cpuid_regs r;
 	pid_t pid;
 
@@ -233,4 +237,28 @@ TEST_F(ve_cpuid_override, flushed_table_falls_back_to_hardware)
 	EXPECT_NE(r.edx & self->bit, 0);
 }
 
+/*
+ * The child does not block between entering the VE and the CPUID, but a
+ * tick can still preempt it there. Repeat, so that a kernel which arms
+ * faulting only at the switch has to get lucky every time.
+ */
+TEST_F(ve_cpuid_override, armed_before_first_reschedule)
+{
+	struct cpuid_regs r;
+	pid_t pid;
+	int i;
+
+	ASSERT_EQ(write_cpuid_override(self->table), 0);
+
+	for (i = 0; i < NO_RESCHED_ROUNDS; i++) {
+		struct child_ctx ctx = { .stop = 0 };
+
+		pid = spawn_cpuid_child(_metadata, self->cgv2_fd, self->ctid,
+					&ctx);
+		collect_cpuid_child(_metadata, pid, &ctx, &r);
+		ASSERT_EQ(r.edx, self->host.edx & ~self->bit)
+			TH_LOG("round %d", i);
+	}
+}
+
 TEST_HARNESS_MAIN

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 4/5] x86/traps: decode the faulting CPUID with the instruction decoder

[Konstantin Khorenko]

check_cpuid_fault() reads two bytes at the faulting ip and emulates
only when they are 0f a2, the bare CPUID encoding.

CPUID has no operands, so the CPU accepts and ignores every legacy
and REX prefix in front of it. All of these are valid CPUID and raise
the same #GP(0) when CPUID faulting is on:

  0f a2           cpuid
  66 0f a2        data16 cpuid       (operand-size override)
  f3 0f a2        repz cpuid
  2e 0f a2        cs cpuid           (segment override)
  48 0f a2        rex.W cpuid

Only the first one is recognised. For the others the handler reads
the prefix byte instead of the opcode, declines, and the task is
killed with SIGSEGV although the instruction is legal and the host
would have executed it.
Such encodings could come from hand-written and generated assembly, from
binary translators and JITs that emit prefixes for alignment or as
padding, and from any binary that disassembles as "data16 cpuid" rather
than "cpuid".

Fetch and decode the instruction with insn_fetch_from_user() and
insn_decode_from_regs(), as fixup_umip_exception() does, match the
opcode bytes and advance ip by the decoded length. The decoder derives
the linear ip from the code segment itself, so convert_ip_to_linear()
is not needed any more.

Fixes: 1033fe455434 ("arch/x86: introduce cpuid override")
https://virtuozzo.atlassian.net/browse/VSTOR-148843

Feature: x86: cpu pools (cpuid override)
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 arch/x86/kernel/traps.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 8074387bf4f43..fa12da67adbf0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -633,8 +633,9 @@ static enum kernel_gp_hint get_kernel_gp_address(struct pt_regs *regs,
 
 static int check_cpuid_fault(struct pt_regs *regs, long error_code)
 {
-	unsigned long addr;
-	unsigned short opcode;
+	unsigned char buf[MAX_INSN_SIZE];
+	struct insn insn;
+	int nr_copied;
 
 	if (error_code != 0)
 		return 0;
@@ -642,16 +643,19 @@ static int check_cpuid_fault(struct pt_regs *regs, long error_code)
 	if (test_thread_flag(TIF_NOCPUID))
 		return 0;
 
-	addr = convert_ip_to_linear(current, regs);
-	if (get_user(opcode, (unsigned short __user *)addr))
+	nr_copied = insn_fetch_from_user(regs, buf);
+	if (nr_copied <= 0)
+		return 0;
+
+	if (!insn_decode_from_regs(&insn, regs, buf, nr_copied))
 		return 0;
 
-	if (opcode != 0xa20f)
+	if (insn.opcode.bytes[0] != 0x0f || insn.opcode.bytes[1] != 0xa2)
 		return 0;
 
 	do_cpuid_fault(regs);
 
-	regs->ip += 2;
+	regs->ip += insn.length;
 	return 1;
 }
 

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH hci-8.1 v2 5/5] selftests/ve: check CPUID override emulation for a prefixed CPUID

[Konstantin Khorenko]

Execute CPUID with an operand-size prefix, which the CPU ignores, and
expect the table values. Before the kernel change the #GP handler
matched only the bare opcode and the child died with SIGSEGV.

https://virtuozzo.atlassian.net/browse/VSTOR-148843

Feature: x86: cpu pools (cpuid override)
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 .../testing/selftests/ve/ve_cpuid_override_test.c  | 29 +++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ve/ve_cpuid_override_test.c b/tools/testing/selftests/ve/ve_cpuid_override_test.c
index 0e748d4fd5f33..67ab9f02442df 100644
--- a/tools/testing/selftests/ve/ve_cpuid_override_test.c
+++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c
@@ -43,6 +43,14 @@ static void do_cpuid(unsigned int leaf, struct cpuid_regs *r)
 		     : "a" (leaf), "c" (0));
 }
 
+/* CPUID with an operand-size prefix, which the CPU accepts and ignores. */
+static void do_cpuid_prefixed(unsigned int leaf, struct cpuid_regs *r)
+{
+	asm volatile(".byte 0x66, 0x0f, 0xa2"
+		     : "=a" (r->eax), "=b" (r->ebx), "=c" (r->ecx), "=d" (r->edx)
+		     : "a" (leaf), "c" (0));
+}
+
 static int write_cpuid_override(const char *table)
 {
 	int fd, ret;
@@ -65,6 +73,7 @@ static int write_cpuid_override(const char *table)
 struct child_ctx {
 	int result_pipe[2];
 	int stop;
+	int prefixed;
 };
 
 static int cpuid_child(struct child_ctx *ctx)
@@ -78,7 +87,10 @@ static int cpuid_child(struct child_ctx *ctx)
 	if (ctx->stop && raise(SIGSTOP))
 		return 22;
 
-	do_cpuid(CPUID_LEAF, &r);
+	if (ctx->prefixed)
+		do_cpuid_prefixed(CPUID_LEAF, &r);
+	else
+		do_cpuid(CPUID_LEAF, &r);
 
 	if (write(ctx->result_pipe[1], &r, sizeof(r)) != sizeof(r))
 		return 24;
@@ -261,4 +273,19 @@ TEST_F(ve_cpuid_override, armed_before_first_reschedule)
 	}
 }
 
+TEST_F(ve_cpuid_override, prefixed_cpuid_is_emulated)
+{
+	struct child_ctx ctx = { .stop = 1, .prefixed = 1 };
+	struct cpuid_regs r;
+	pid_t pid;
+
+	ASSERT_EQ(write_cpuid_override(self->table), 0);
+
+	pid = spawn_cpuid_child(_metadata, self->cgv2_fd, self->ctid, &ctx);
+	resume_cpuid_child(_metadata, pid);
+	collect_cpuid_child(_metadata, pid, &ctx, &r);
+
+	EXPECT_EQ(r.edx, self->host.edx & ~self->bit);
+}
+
 TEST_HARNESS_MAIN

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 2/5] x86/ve: enable CPUID faulting at once when the current task enters a VE

[no-reply]

> ve_setup_task() only sets TIF_CPUID_OVERRIDE. The MSR follows the flag
> in __switch_to_xtra(), so a task that enters a VE through unshare() or
> setns() keeps executing CPUID natively until it is scheduled out for the
> first time and sees the host values instead of the override table.
> 
> Add set_cpuid_override(), which sets the flag and, like disable_cpuid(),
> flips the CPU state synchronously when the task is the current one.
> Tasks set up through clone() are not running yet and are still handled
> at their first switch-in.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
> Feature: x86: cpu pools (cpuid override)
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@virtuozzo.com?part=2


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 3/5] selftests/ve: check CPUID override right after entering a VE

[no-reply]

> Add a case where the child executes CPUID straight after unshare(),
> without stopping or blocking in between, and expects the table values.
> Before the kernel change the override took effect only after the first
> context switch, so the child saw the hardware values.
> 
> A tick can still preempt the child in that window, so the case runs
> the sequence several times: a kernel that arms faulting only at the
> switch has to get lucky every time.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
> Feature: x86: cpu pools (cpuid override)

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@virtuozzo.com?part=3


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 4/5] x86/traps: decode the faulting CPUID with the instruction decoder

[no-reply]

> check_cpuid_fault() reads two bytes at the faulting ip and emulates
> only when they are 0f a2, the bare CPUID encoding.
> 
> CPUID has no operands, so the CPU accepts and ignores every legacy
> and REX prefix in front of it. All of these are valid CPUID and raise
> the same #GP(0) when CPUID faulting is on:
> 
>   0f a2           cpuid
>   66 0f a2        data16 cpuid       (operand-size override)
>   f3 0f a2        repz cpuid
>   2e 0f a2        cs cpuid           (segment override)
>   48 0f a2        rex.W cpuid
> 
> Only the first one is recognised. For the others the handler reads
> the prefix byte instead of the opcode, declines, and the task is
> [ ... ]
> Fixes: 1033fe455434 ("arch/x86: introduce cpuid override")
> https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
> Feature: x86: cpu pools (cpuid override)
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@virtuozzo.com?part=4


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 5/5] selftests/ve: check CPUID override emulation for a prefixed CPUID

[no-reply]

> Execute CPUID with an operand-size prefix, which the CPU ignores, and
> expect the table values. Before the kernel change the #GP handler
> matched only the bare opcode and the child died with SIGSEGV.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-148843
> 
> Feature: x86: cpu pools (cpuid override)
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@virtuozzo.com?part=5


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 v2 1/5] selftests/ve: check CPUID override emulation for VE tasks

[no-reply]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] selftests/ve: restore of saved cpuid_override table fails for tables of 4096 bytes or more
- [Low] selftests/ve: stopped child is never killed or reaped when an ASSERT fires
- [Low] selftests/ve: Makefile overwrites global ARCH, breaking native LLVM=1 builds on non-x86
--

commit 6db1939d0f0292549e9feb0b4de5043c4eac6ba7
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

selftests/ve: check CPUID override emulation for VE tasks

This adds a selftest that writes a one-entry /proc/vz/cpuid_override table
and checks that a task in a fresh VE sees the table values while the host
sees the hardware values. A second test flushes the table after the task
was armed and expects the hardware values again. The test is built on x86
only.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-148843

> diff --git a/tools/testing/selftests/ve/Makefile b/tools/testing/selftests/ve/Makefile
> --- a/tools/testing/selftests/ve/Makefile
> +++ b/tools/testing/selftests/ve/Makefile
> @@ -6,4 +6,11 @@ TEST_GEN_PROGS += ve_ns_owner_test
>  TEST_GEN_PROGS += ve_perms_test
>  TEST_GEN_PROGS += ve_mount_accounting_test
>  
> +ifeq ($(ARCH),)
> +ARCH := $(shell uname -m 2>/dev/null || echo not)
> +endif
> +ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
> +TEST_GEN_PROGS += ve_cpuid_override_test
> +endif
> +
>  include ../lib.mk

[Severity: Low]
Can setting ARCH here break native LLVM=1 builds on non-x86 hosts?

The block assigns ARCH from uname -m before include ../lib.mk.  In lib.mk
the clang target selection depends on ARCH being empty:

    ifeq ($(ARCH),)
    CLANG_TARGET_FLAGS := $(shell $(CLANG) -print-target-triple)
    else
    CLANG_TARGET_FLAGS := $(CLANG_TARGET_FLAGS_$(ARCH))
    endif

With "make LLVM=1 -C tools/testing/selftests/ve" on a host such as aarch64
or ppc64le, with ARCH and CROSS_COMPILE unset, ARCH is now non-empty and
CLANG_TARGET_FLAGS_$(ARCH) is empty.  lib.mk then hits "$(error Specify
CROSS_COMPILE or add '--target=' option to lib.mk)", so
ve_ns_owner_test, ve_perms_test and ve_mount_accounting_test also stop
building.

Would a private variable such as uname_M be better than overwriting ARCH?
The series-end diff of tools/testing/selftests/ve/ shows no later change to
this Makefile.

> diff --git a/tools/testing/selftests/ve/ve_cpuid_override_test.c b/tools/testing/selftests/ve/ve_cpuid_override_test.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/ve/ve_cpuid_override_test.c

[ ... ]

> +/* 128 entries of up to 67 characters each, as the kernel prints them. */
> +#define CPUID_TABLE_MAX		16384

[ ... ]

> +FIXTURE_TEARDOWN_PARENT(ve_cpuid_override)
> +{
> +	int cgv2_fd;
> +
> +	if (write_cpuid_override(self->saved[0] ? self->saved : "\n"))
> +		TH_LOG("could not restore %s: %s", CPUID_OVERRIDE_PATH,
> +		       strerror(errno));
> +
> +	cgv2_fd = mount_cg2_fd();
> +	if (cgv2_fd < 0)
> +		return;
> +	destroy_ve(cgv2_fd, self->ctid);
> +	close(cgv2_fd);
> +}

[Severity: Medium]
Can the restore of the saved table fail for large tables?

FIXTURE_SETUP reads the printed table into the 16384-byte saved[] buffer
and accepts anything below 16383 bytes.  FIXTURE_TEARDOWN_PARENT then
writes it back with a single write() through write_cpuid_override().

In arch/x86/kernel/cpuid_fault.c, cpuid_override_write() starts with:

    err = -E2BIG;
    if (count >= PAGE_SIZE)
            goto out;

A table that prints to 4096 bytes or more (roughly 61 or more entries at
about 67 characters each) is read fine but rejected with -E2BIG on
restore.  Such a table can only have been loaded by writing it in chunks.

Teardown only logs the failure with TH_LOG, so the host-wide table stays
replaced after the test and later VE tasks see the wrong leaf 1 values.
That also contradicts the commit message statement "restore the previous
table on exit".  Should the restore write the table in chunks below
PAGE_SIZE, or should setup skip when the saved table is too large to
restore?  The later patches in the series do not change the restore logic.

[Severity: Low]
Can a stopped child be left behind when an ASSERT fires?

FIXTURE_TEARDOWN_PARENT neither kills nor reaps the child, and the pid
returned by spawn_cpuid_child() is not kept in the fixture.  The child has
no PR_SET_PDEATHSIG.

> +	pid = spawn_cpuid_child(_metadata, self->cgv2_fd, self->ctid, &ctx);
> +
> +	ASSERT_EQ(write_cpuid_override("\n"), 0);
> +	resume_cpuid_child(_metadata, pid);
> +	collect_cpuid_child(_metadata, pid, &ctx, &r);

If ASSERT_EQ(write_cpuid_override("\n"), 0) fails, or the kill(SIGCONT)
ASSERT in resume_cpuid_child() fails, or any ASSERT in
collect_cpuid_child() fails, the test body returns before the child is
resumed and waited for.

The child stays SIGSTOPped inside the VE cgroup and is reparented to init.
Then destroy_ve() in FIXTURE_TEARDOWN_PARENT fails with EBUSY because the
child is still in the cgroup.

Could the pid be stored in the fixture so teardown can kill and reap it
before destroy_ve()?  The later patches add more callers of
spawn_cpuid_child() but do not add any such cleanup.

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006-b4-prep-khorenko-vstor-148656-cpuid-v2-0-f6d1192c7d60@virtuozzo.com?part=1

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel